NOAA-OWP / noah-owp-modular

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

Add capacity to read-in gridded soils data from NetCDF #80

Closed GreyREvenson closed 1 year ago

GreyREvenson commented 1 year ago

PURPOSE

This PR adds capacity to read-in gridded soils data from an input NetCDF file. The path/name of the NetCDF file must be provided by the model-user in namelist.input.

ADDITIONS

Added the private type-bound subroutine gridinfo_type%ReadSoils. The ReadSoils subroutine takes one argument (filename), which is the path/name of the NetCDF input file containing soils data as specified by the model-user in namelist.input. The purpose of this subroutine is to read soils data from the NetCDF input file to populate/set gridinfo_type%isltyp, which will be transferred to domaingrid_type%isltyp with a subsequent call to domaingrid_type%InitTransfer.

data/netcdf_soils.nc was added as an example soil NetCDF file and to facilitate testing. This file has the same spatial information/dimensions as data/netcdf_landuse.nc and has a soil type of 1 for grid cell (1,1), which is consistent with the soil type specified in run/namelist.input in this repo's main branch corresponding to the unmodified column model. All other grid cells have randomly assigned soil types.

CHANGES

The subroutines gridinfo_type%ReadLandUse (renamed from gridinfo_type%ReadVegtyp) and gridinfo_type%ReadSpatial were revised such that the NetCDF files are opened and closed within the scope of only those subroutines. My thought was that this approach would better satisfy our desire for 'separation of concerns' though there is an extra computation cost of opening/closing a NetCDF file more than what is absolutely necessary.

The public type bound subroutine gridinfo_type%ReadGridInfo was revised to include additional actions (i.e., allocating the gridinfo_type arrays and calling ReadSpatial).

data/netcdf_landuse.nc was renamed from data/netcdf_vegtyp.

TESTING

The PR-modified model was successfully compiled and executed in stand-alone on MacOS using GNU Fortran (Homebrew GCC 13.1.0) 13.1.0.

The test script (i.e., test/analysis/compare_outputnc.py which compares gridded model output against output from the unmodified column model) was executed. As the testing input NetCDF files depicts grid cell (1,1) as having a soil type (isltyp) of 1 and a land use (vegtyp) of 1 -- which is consistent with run/namelist.input in this repo's main branch -- the expected behavior of the script is that grid cell (1,1) will pass all tests while all other grid cells should fail at least one test. The output from test/analysis/compare_outputnc.py is: output.txt

The unit test program was re-executed and its output was observed normal. The output is: output.txt

NOTES

The gridinfo_type%ReadLandUse and gridinfo_type%ReadSoils subroutines are nearly identical. Hence, I considered removing these subroutines and replacing them with a single generalizable subroutine (e.g., something like gridinfo_type%ReadVariable). However, it seems like we'd need to create a separate ReadVariable subroutine for each variable data type (i.e., integer, real) so we'd end up having, for example, ReadVariable_int and ReadVariable_real. I'm fine going that way but I am currently expecting to simply create a single subroutine for each variable that will be read into the model (soils, land use, slope, etc). The adopted approach seemed like it might afford more flexibility if variable-specific logic checks need to be executed (e.g., check that land use values are within expected bounds).

GreyREvenson commented 1 year ago

Okay, @nmizukami, I think we're ready for you to have another look. I'll review another review

nmizukami commented 1 year ago

I just approved (just in case i needed it?)