Remove regridding funcs that are in ClimaUtilities #1109
Conversation
imreddyTeja
force-pushed
the
tr/use-ClimaUtilities.Regridders
branch
7 times, most recently
from
9432748 to
266da3f
Compare
| ## FieldExchanger Internal Functions | ||
|
|
||
| ```@docs | ||
| ClimaCoupler.FieldExchanger.combine_surfaces! |
There was a problem hiding this comment.
nans_to_zero is also an internal function here now
src/FieldExchanger.jl
Outdated
| - `target`: [CC.Fields.Field] destination of remapping. | ||
| - `source`: [CC.Fields.Field] source of remapping. | ||
| """ | ||
| function dummmy_remap!(target, source) |
There was a problem hiding this comment.
This is just a small style choice, but it's good practice to put internal functions at the end of a file, so if a user looks into the file to find a function they're using they can find it more easily.
test/TestHelper.jl
Outdated
| @@ -118,13 +119,41 @@ function gen_ncdata_time(FT, path, varname, val) | |||
| # Define variables | |||
| lon = NCDatasets.defVar(nc, "lon", FT, ("lon",)) | |||
| lat = NCDatasets.defVar(nc, "lat", FT, ("lat",)) | |||
| time = NCDatasets.defVar(nc, "time", Float64, ("time",)) | |||
| time = NCDatasets.defVar(nc, "time", Dates.DateTime.([2020.0, 2021.0]), ("time",)) | |||
There was a problem hiding this comment.
why was this change needed?
There was a problem hiding this comment.
it looks like gen_ncdata_time and gen_ncdata_date were only used in the regridder tests so now they're unused and can be removed
There was a problem hiding this comment.
That change was made because the read_available_dates in ClimaUtilities incorrectly interprets the time dimension when it is a Number. The unused gen_ncdata_time and gen_ncdata_date are now removed.
test/field_exchanger_tests.jl
Outdated
| Interfacer.get_field(::TestSurfaceSimulationA, ::Val{:random}) = 1.0 | ||
| Interfacer.get_field(::TestSurfaceSimulationB, ::Val{:random}) = 1.0 | ||
| Interfacer.get_field(::TestSurfaceSimulationC, ::Val{:random}) = 1.0 | ||
| Interfacer.get_field(::TestSurfaceSimulationD, ::Val{:random}) = 1.0 |
There was a problem hiding this comment.
it would be good for some of these values to be not 1 (like you have for the area_fraction functions below)
|
It will be good to make the ClimaUtilities TempestRegridder changes we talked about too, but I'm okay with those going in after this is merged :) |
imreddyTeja
force-pushed
the
tr/use-ClimaUtilities.Regridders
branch
from
34c7381 to
7b76d0c
Compare
Remove all unused functions from Regridder module. Used functions relocated to Utilities and FieldExchanger modules. Docs and tests appropriately updated. Re-add ClimaUtilities as dependancy after rebase ClimaUtilities needs to be re-added after rebasing, because it is now used for the ouput paths Make suggested changes
The strdate_to_datetime and datetime_to_strdate are unused after removing the Regridders module. Additionally, their funcitonality is contained within the Dates package with Dates.format and DateFormats.
imreddyTeja
force-pushed
the
tr/use-ClimaUtilities.Regridders
branch
from
6f61bb7 to
f5e7431
Compare
Closes #1107 and Closes #425
Regridder module removed. The hdwrite_regridfile_rll_to_cgll only works for 2d spaces in ClimaUtilities, and the function in Regridder.jl also worked for 3d spaced, so this functionality is lost.
The functions read_available_dates and yyyymmmddd_to_datetime are copied from ClimaUtilities. This is necessary because land_fraction needs the first date in the dataset. TempestRegridder calls hdwrite_regridfile_rll_to_cgll, which extracts the dates and returns them, but TempestRegridder then discards those extracted dates.The regridder functions in ClimaUtilities do not work properly when the time dim is made up of numbers, and it errors when the time dim is made up of large floats. To function properly, the time time must be datetimes. The functions in Regridder.jl worked with the time dim as numbers.
Many of the regridding functions in Regridder.jl allowed the outfile_root to be set. When using the TempestRegridder, it is autmatically set to the varname. This results in file with names structured like: "varname_varname_time.hdf5"
Purpose
To-do
move Regridder module docs to news.mdsee here
changes to the time reading probably require a small discussion, and making TempestRegridder support multiple varnames requires enough changes to be a separate issue (implementing would change interface, or would need to continue supporting old interface)
Docs differentiate internal vs external functionstest for binary maskContent