Introduce `TimeVaryingInput2D` #465

Sbozzolo · 2024-01-23T22:33:57Z

Okay, this is a massive commit (and I feel bad about it).

This commit splits PrescribedDataStatic and PrescibedDataTemporal in multiple independent modules.

Before, PrescribedData was:

reading files
regridding
discovering dates available
interpolating
taking care of edge cases (e.g., what to do when reading outside of where the date is outside of the definition range)

This commit splits all the different functions in different independent modules so that they can be more easily maintained and upgraded.

This will soon be needed to efficiently support reading and buffering reads for GPU runs.

In all of this, we are still using ClimaCoreTempestRemap and doing everything ahead of time. At the moment, we are not efficiently reusing any informtion across variables that share the same remapping weights, but this can be added in the future.

kmdeck · 2024-03-05T23:03:21Z

test/standalone/Bucket/albedo_types.jl

-# Delete testing directory and files
-rm(regrid_dir_static; recursive = true, force = true)
-rm(regrid_dir_temporal; recursive = true, force = true)
+# using Test


forgot to uncomment?

experiments/standalone/Bucket/global_bucket_staticmap.jl

kmdeck · 2024-03-05T23:04:49Z

experiments/standalone/Bucket/global_bucket_temporalmap.jl

@@ -87,17 +87,8 @@ tf = 50 * 86400;
 Δt = 3600.0;

 # Construct albedo parameter object using temporal map
-# Use separate regridding directory for CPU and GPU runs to avoid race condition
-device_suffix =


likewise here

kmdeck · 2024-03-05T23:11:24Z

src/shared_utilities/DataHandling.jl

+
+"""
+
+Note: It is best to have one file per variable with all the temporal data.


this may be a little inconvenient/ add a step - all the reanalysis data is in one file to start with.

kmdeck · 2024-03-05T23:11:52Z

src/shared_utilities/DataHandling.jl

+import ..FileReader: NCFileReader
+import ..Regridder
+import ..Regridder:
+    AbstractRegridder, TempestRegridder, regrid, AVAILABLE_REGRIDDERS


I dont see anything in the style guide for all caps in names - why is AVAILABLE_REGRIDDERS capitalized?

ah - I see some languages use that for constants. it doesnt seem like Julia follows that notation: https://docs.julialang.org/en/v1/base/base/#const

ALL_CAPS is mostly a way to tell the reader that the variable is a "global fixed parameter", with the emphasis being in "global". We don't have to do this, but I personally find it an useful additional qualifier.

(const in Julia doesn't even ensure that the value doesn't change.)

kmdeck · 2024-03-05T23:23:25Z

src/shared_utilities/DataHandling.jl

+Note: It is best to have one file per variable with all the temporal data.
+
+Assumptions:
+- There is only one file with all the data in time


this seems to contradict the above Note?

It's probably poorly worded: instead of having multiple files with ranges of dates, it is best to have one single file with alle the dates ("all the data (for a given variable) in time").

kmdeck · 2024-03-05T23:26:12Z

src/shared_utilities/DataHandling.jl

+    TSTART <: AbstractFloat,
+    DATES <: AbstractArray{<:Dates.DateTime},
+    DIMS,
+    TIMES <: AbstractArray{<:AbstractFloat},


We discussed this last time, butTIMES <: AbstractArray{<:AbstractFloat} is the same as AbstractArray{<:AbstractFloat} in this context, right?

If so, I dont see that we need to have these types as parametric types.

src/shared_utilities/DataHandling.jl

kmdeck · 2024-03-05T23:40:42Z

src/shared_utilities/DataHandling.jl

+end
+
+function next_time(data_handler::DataHandler, date::Dates.DateTime)
+    if date in data_handler.available_dates


maybe worth using the same code as is inside the functionnext_time(data_handler::DataHandler, time::AbstractFloat) for clarity (otherwise the reader wonders if next_date does something different than next_time)

I second this! I think readability is more important than being concise here

Addressed in the ClimaUtilities version.

kmdeck · 2024-03-05T23:51:12Z

src/shared_utilities/DataHandling.jl

+
+    snapshot = get!(data_handler._cached_regridded_fields, date) do
+        # Add here the arguments when we have more regridders
+        regrid_args = Dict(:TempestRegridder => (date,))


I am confused why the regridding needs the date.

I am a little confused by the notation also. we get the value of data_handler._cached_regridded_fields corresponding do the date, but then what is that called in the do block/where is it used in the do block? is that what date then refers to (instead of the actual date)?

TempestRegridder regrids everything ahead of time, saves everything into files, and then read the a filename that is constructed with the date. That's why we need the date.

This block does the following: if date is a key in data_handler._cached_regridded_fields, then return the associated value. If date is not yet a key, then execute the do-block and add the return value as key. So, we change data_handler._cached_regridded_fields to add a new date with the regridded value.

kmdeck · 2024-03-05T23:52:02Z

src/shared_utilities/DataHandling.jl

+    snapshot = get!(data_handler._cached_regridded_fields, date) do
+        # Add here the arguments when we have more regridders
+        regrid_args = Dict(:TempestRegridder => (date,))
+        regrid(data_handler.regridder, regrid_args[regridder_type]...)


Does this mean we need to regrid frame by frame, or is a future option to regrid batch by batch (many frames at once)? If regridding is matrix operations by a static weight matrix, we could process many snapshots at once?

src/shared_utilities/DataHandling.jl

kmdeck · 2024-03-05T23:57:22Z

src/shared_utilities/FileReader.jl

-    date_idx0 =
-        [argmin(abs.(Dates.value(date_start) .- Dates.value.(all_dates[:])))]
+    # NOTE: We are hardcoding a few things here!
+    dimensions = dataset["lon"][:], dataset["lat"][:]


is this not what dimensions::DIMS is for? if not, they could also pass in the dimension_names. then we dont need to hardcode

kmdeck · 2024-03-06T00:01:40Z

src/shared_utilities/FileReader.jl


-    args = (file_info, file_states, sim_info)
+    _cached_reads = Dict{Dates.DateTime, Array}()


in the future, this is where we would pre-allocate the space for the batch of data we would read in?

src/shared_utilities/SpaceVaryingInputs.jl

kmdeck · 2024-03-06T00:17:19Z

src/shared_utilities/TimeVaryingInputs.jl

@@ -81,19 +81,54 @@ When passing single-site data
 When a `times` and `vals` are passed, `times` have to be sorted and the two arrays have to
 have the same length.

+=======
+When the input is a function, the signature of the function can be `func(time, args...;


kmdeck · 2024-03-06T00:18:52Z

src/shared_utilities/TimeVaryingInputs.jl

+
+For example:
+```julia
+CO2fromp(time, Y, p) = p.atmos.co2


but maybe this isnt a good example? is this for the analytic time varying input case?

src/shared_utilities/analytic_time_varying_input.jl

kmdeck · 2024-03-06T00:29:33Z

src/shared_utilities/interpolating_time_varying_inputs.jl


 Check if the given `time` is in the range of definition for `itp`.
 """
-function Base.in(time, itp::InterpolatingTimeVaryingInput0D)
+function Base.in(time, itp::InterpolatingTimeVaryingInput)


is this still being used?

It is. It is defined at the beginning of this file:

InterpolatingTimeVaryingInput = Union{InterpolatingTimeVaryingInput0D, InterpolatingTimeVaryingInput2D}

juliasloan25

Thank you for all this work Gabriele! I left many comments, and I have a few more here:

We're opening Netcdf files and keeping them open throughout the simulation, then closing at the end. What will happen if the simulation crashes?
There are some things left to add in future PRs that we should open issues for:
- allow regridder-specific args: mono, regrid_dir, dates to regrid
- regrid only simulation dates (related to dates arg in previous point)
- add FileReaders cache
- add DataHandling cache cleanup function/implement LRU cache
please update the NEWS.md file too!

juliasloan25 · 2024-03-15T00:34:45Z

docs/Manifest.toml

@@ -1,6 +1,6 @@
 # This file is machine-generated - editing it directly is not advised

-julia_version = "1.10.1"
+julia_version = "1.10.0"


we can just update to 1.10.2 and use that across land/atmos/coupler now

juliasloan25 · 2024-03-15T00:35:05Z

docs/Manifest.toml

 uuid = "d414da3d-4745-48bb-8d80-42e94e092884"
-version = "0.13.2"
+version = "0.13.1"


why is this downgraded?

juliasloan25 · 2024-03-15T00:35:29Z

experiments/Manifest.toml

 uuid = "d414da3d-4745-48bb-8d80-42e94e092884"
-version = "0.13.2"
+version = "0.13.1"


same here, packages are downgraded. I just updated everything today so you probably don't need to modify the manifests

juliasloan25 · 2024-03-15T00:40:23Z

src/shared_utilities/DataHandling.jl

+- CPU/GPU communication can be a bottleneck
+
+The `DataHandling` takes the divide and conquer approach: the various core tasks and
+features and split into other independent modules (chiefly `FileReaders`, and `Regridders`).


typo: "and" -> "are"

Addressed in the ClimaUtilities version

juliasloan25 · 2024-03-15T00:43:15Z

src/shared_utilities/DataHandling.jl

+- `available_times` (`available_dates`): to list all the `times` (`dates`) over which the
+                    data is defined.
+- `previous_time(time/date)` (`next_time(time/date)`): to obtain the time of the snapshot
+                         before the given `time` or `date`. This can be used to compute the


could change "before" to "before/after" to explain both cases of previous and next times

Addressed in the ClimaUtilities version

juliasloan25 · 2024-03-15T20:00:26Z

test/shared_utilities/data_handling.jl

+        PATH = temporal_data_path
+        varname = "sp"


these could be moved outside of the for loop for consistency with the previous test (but it doesn't really matter)

Fixed in ClimaUtilities

juliasloan25 · 2024-03-15T20:04:56Z

test/shared_utilities/file_readers.jl

+        @test ncreader.dimensions[1] == nc["lon"][:]
+        @test ncreader.dimensions[2] == nc["lat"][:]
+
+        close(ncreader)


we could check that OPEN_NCFILES is updated correctly in the case without time too

It wouldn't test anything that we are not testing in the case with time

juliasloan25 · 2024-03-15T20:05:20Z

test/shared_utilities/time_varying_inputs.jl

 end

-@testset "Temporal TimeVaryingInput 1D" begin
+@testset "InteprolatingTimeVaryingInput0D" begin


typo: Inteprolating -> Interpolating

Fixed in ClimaUtilities

juliasloan25 · 2024-03-15T20:05:49Z

test/shared_utilities/time_varying_inputs.jl

@@ -102,3 +112,106 @@ end
        end
    end
 end
+
+@testset "InteprolatingTimeVaryingInput2D" begin


typo: Inteprolating -> Interpolating

Fixed in ClimaUtilities

juliasloan25 · 2024-03-15T20:06:38Z

test/shared_utilities/time_varying_inputs.jl

+        TimeVaryingInputs.evaluate!(dest, input_nearest, target_time)
+
+        @test isequal(
+            Array(parent(dest)),


why do we need to use Array here but not in the "on node" case?

I added the Array there too

Okay, this is a massive commit (and I feel bad about it). This commit splits PrescribedDataStatic and PrescibedDataTemporal in multiple independent modules. Before, PrescribedData was: - reading files - regridding - discovering dates available - interpolating - taking care of edge cases (e.g., what to do when reading outside of where the date is outside of the definition range) This commit splits all the different functions in different independent modules so that they can be more easily maintained and upgraded. This commit also introduces InterpolationsRemapper, which uses Interpolations.jl to do online/ and distributed remapping from files. This will soon be needed to efficiently support reading and buffering reads for GPU runs.

Sbozzolo · 2024-03-26T17:26:56Z

We're opening Netcdf files and keeping them open throughout the simulation, then closing at the end. What will happen if the simulation crashes?

Since all the files are opened in read-only mode, it shouldn't be too much of a problem if a simulation crashes, we should start doing error handling and have a cleanup function

allow regridder-specific args: mono, regrid_dir, dates to regrid

regrid only simulation dates (related to dates arg in previous point)

Opened an issue in ClimaUtiltiees

add FileReaders cache

This is implemented.

add DataHandling cache cleanup function/implement LRU cache

Opened an issue in ClimaUtilities

Sbozzolo · 2024-03-28T17:35:13Z

Implemented in #560

Sbozzolo force-pushed the gb/gpu_albedo branch 2 times, most recently from 70d645e to e6232a9 Compare January 25, 2024 21:48

Sbozzolo force-pushed the gb/gpu_albedo branch from e6232a9 to 9f25053 Compare March 1, 2024 23:44

Sbozzolo mentioned this pull request Mar 4, 2024

O? Distributed unstructured sparse CPU & GPU matrix-vector multiply, supporting mixed precision CliMA/ClimaCoupler.jl#188

Open

10 tasks

kmdeck reviewed Mar 5, 2024

View reviewed changes

experiments/standalone/Bucket/global_bucket_staticmap.jl Show resolved Hide resolved

kmdeck reviewed Mar 5, 2024

View reviewed changes

src/shared_utilities/DataHandling.jl Show resolved Hide resolved

kmdeck reviewed Mar 5, 2024

View reviewed changes

src/shared_utilities/DataHandling.jl Outdated Show resolved Hide resolved

kmdeck reviewed Mar 5, 2024

View reviewed changes

kmdeck reviewed Mar 6, 2024

View reviewed changes

src/shared_utilities/SpaceVaryingInputs.jl Outdated Show resolved Hide resolved

kmdeck reviewed Mar 6, 2024

View reviewed changes

src/shared_utilities/analytic_time_varying_input.jl Outdated Show resolved Hide resolved

kmdeck reviewed Mar 6, 2024

View reviewed changes

Sbozzolo force-pushed the gb/gpu_albedo branch 7 times, most recently from 49698ca to f9bdd2e Compare March 13, 2024 22:51

Sbozzolo force-pushed the gb/gpu_albedo branch from f9bdd2e to 44d1a3f Compare March 14, 2024 18:26

Use SpaceVaryingInput for runoff

308dbfc

Sbozzolo force-pushed the gb/gpu_albedo branch from 44d1a3f to 48459f1 Compare March 14, 2024 18:30

Sbozzolo marked this pull request as ready for review March 14, 2024 18:31

Sbozzolo self-assigned this Mar 14, 2024

Sbozzolo requested a review from juliasloan25 March 14, 2024 18:33

juliasloan25 reviewed Mar 15, 2024

View reviewed changes

juliasloan25 added enhancement New feature or request breaking change and removed enhancement New feature or request labels Mar 15, 2024

Sbozzolo force-pushed the gb/gpu_albedo branch from 48459f1 to 1f020ff Compare March 19, 2024 22:26

Sbozzolo mentioned this pull request Mar 20, 2024

Add prescribed aerosols CliMA/ClimaAtmos.jl#2815

Merged

6 tasks

Sbozzolo mentioned this pull request Mar 27, 2024

Use ClimaUtilities FileReading/Inputs/Regridding #560

Merged

Sbozzolo closed this Mar 28, 2024


		"""

		Note: It is best to have one file per variable with all the temporal data.


		args = (file_info, file_states, sim_info)
		_cached_reads = Dict{Dates.DateTime, Array}()

Introduce TimeVaryingInput2D #465

Introduce TimeVaryingInput2D #465

Conversation

Sbozzolo commented Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmdeck Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmdeck Mar 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliasloan25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sbozzolo commented Mar 26, 2024

Sbozzolo commented Mar 28, 2024

Introduce `TimeVaryingInput2D` #465

Introduce `TimeVaryingInput2D` #465

Sbozzolo commented Jan 23, 2024 •

edited

Loading

kmdeck Mar 5, 2024 •

edited

Loading

kmdeck Mar 6, 2024 •

edited

Loading