NOAA-OWP / noah-owp-modular

Modularized version of the NOAH-MP land surface model.
Other
9 stars 19 forks source link

Refactor of module responsible for reading-in gridded inputs from NetCDF file #85

Closed GreyREvenson closed 1 year ago

GreyREvenson commented 1 year ago

PURPOSE

The purpose of this PR is to refactor the Noah-OM (gridded version) module that handles tasks associated with reading-in gridded model inputs from a NetCDF file. The previous iteration of this module assumed that each gridded input variable (e.g., soils, slope, etc.) would be provided in a separate input NetCDF file and had one subroutine to read each specific file. Instead, this PR assumes that all gridded input variables are provided in a single NetCDF file and implements a generic type-bound ReadVar subroutine that will be called for all gridded input variables.

ADDITIONS

The NetCDFVarsType module was added and contains the following type definitions:

The netcdfvars_type has one instance of netcdf2dvar_type (either netcdf2dvarINT_type or netcdf2dvarREAL_type) for each gridded input variable (e.g., soils, slope). Each instance of netcdf2dvar_type is given as an argument when calling netcdfvars_type%ReadVar. ReadVar is overloaded based on the argument instance of netcdf2dvar_type being either netcdf2dvarINT_type or netcdf2dvarREAL_type.

This PR adds a new dummy NetCDF input file (/data/netcdf_input.nc) to the repo. This PR also adds a py script (/test/analysis/create_dummy_netcdf_singlefile.py) to create the dummy NetCDF file. The dummy NetCDF file depicts cell (1,1) as having input values equivalent to the values within namelist.input of the main branch of this repo -- thus cell (1,1) should have the same model outputs as the column model (assuming use of same namelist.inpu file).

CHANGES

This PR assumes that all gridded inputs will be given to the model in a single NetCDF file and the path/name of this file must be provided via the namelist.input file. The names of all required variables, dimensions, and attributes within the single NetCDF file are planned to be read-into the model via namelist.input but they are currently hardcoded until I get feedback regarding this PR.

The netcdfvars_type replaces the now removed gridinfo_type and is therefore passed to the Init and/or the InitTransfer subroutines for the gridded derived type members of noahowpgrid_type (i.e. 'the model') -- either to provide access to n_x and n_y for array allocation purposes or to transfer read-in gridded variables.

TESTING

I executed test/analysis/compare_outputnc.py and got this output. As expected, grid cell (1,1) passed all tests while the other grid cells failed for one or more tests (inputs for cell (1,1) are the same as what's listed in namelist.input in the main branch of this repo).

TODO

Read-in variable/dimension/attribute names via namelist.input

GreyREvenson commented 1 year ago

@hellkite500, would you be able to provide a critique of this refactor of the NetCDF read-in functionality? Does this jive with what you were aiming for? Things to improve upon?

Note that I did not address your desire to be able to deallocate/reallocate the model's allocatable arrays, which I thought we'd address in a separate PR.

GreyREvenson commented 1 year ago

@hellkite500, I know you're a particularly busy person so I requested a review from @nmizukami too. Appreciate you having a look!

nmizukami commented 1 year ago

One another comment was about save statement in each F90 defining type. Wondering if there is need for this? These F90s are to define the type (nothing is instantiated, and nothing to save the values?)

GreyREvenson commented 1 year ago

@nmizukami I'll also test your suggestion on the save statements. I'll put more thought into that.

GreyREvenson commented 1 year ago

One another comment was about save statement in each F90 defining type. Wondering if there is need for this? These F90s are to define the type (nothing is instantiated, and nothing to save the values?)

You were right, @nmizukami : the save statement was not needed. I removed the save statement from the NetCDFVarsType module, recompiled, re-executed the model, and then re-executed /test/analysis/compare_outputnc.py with expected results. I pushed this change.

I'll also test removal off all other save statements in the type definition modules and put those in a separate PR for you to review.

nmizukami commented 1 year ago

One another comment was about save statement in each F90 defining type. Wondering if there is need for this? These F90s are to define the type (nothing is instantiated, and nothing to save the values?)

You were right, @nmizukami : the save statement was not needed. I removed the save statement from the NetCDFVarsType module, recompiled, re-executed the model, and then re-executed /test/analysis/compare_outputnc.py with expected results. I pushed this change.

I'll also test removal off all other save statements in the type definition modules and put those in a separate PR for you to review.

Hi @GreyEvenson-NOAA , Yes, I agree with separate PR for removing save! thanks for checking this.

GreyREvenson commented 1 year ago

Appreciate your time and input on this, @hellkite500 and @nmizukami. I'll merge this PR now.

@nmizukami: Depending on your availability, I may send one PR regarding the save statements in the type-definition modules and a second PR to read-in the NetCDF variable/dimension/attribute names from namelist.input.

@hellkite500: Sounds good regarding the decoupling. @SnowHydrology explained this as requirement for parallelization, though I admittedly may have misunderstood. Will be cool to talk more on this.

nmizukami commented 1 year ago

Hi @GreyEvenson-NOAA , yes, I am happy to review the next PRs too.