Feature/ldt mmf groundwater

[smahanam]

This PR is related to the LDT updates for Issue Issue #1037.

DESCRIPTION
LDT was updated to produce 5 MMF parameters FDEPTH, RECHLIM, RIVERBED, EQWTD, and HGT_M on LIS tiles using global 1k GEOGRID data files at:
/discover/nobackup/projects/lis/LS_PARAMETERS/noahmp401_parms/groundwater/

SOURCE CODE UPDATES
All updates are in the below directory.
ldt/params/Noah/
i) Noah_parmsMod.F90 was updated
ii) module_MMF_groundwater.F90 is a new module which serves as the LDT GEOGRID data reader.
iii) read_geogrid.c and write_geogrid.c came with geogrid data that are good to read/write data in GEOGRID format. module_MMF_groundwater.F90 calls read_geogrid.c to read GEOGRID tiles.
iv) core/LDT_paramMaskCheckMod.F90: MMF scheme is active in lake/water grid cells. Thus, when MMF parameters are being processed, new "leave_good_data" optional argument in SUBROUTINE LDT_contIndivParam_Fill is used to leave acceptable parameter values in lake/water grid cells intact.

LDT Config file
Below text must be added to the LDT config file in order to process MMF parameters.

#######################################################
# MMF Groundwater parameters:
# NOTE: Currently, Noah-MP.4.0.1 is the only model that
# supports the MMF groundwater scheme.
#######################################################

Process Noah-MP-4.0.1 MMF groundwater parameters: .true.

MMF transmissivity dir: ./LS_PARAMETERS/noahmp401_parms/groundwater/f/
MMF climatological recharge dir: ./LS_PARAMETERS/noahmp401_parms/groundwater/recharge/
MMF riverbed elevation dir: ./LS_PARAMETERS/noahmp401_parms/groundwater/riverbed/
MMF equilibrium water table depth dir: ./LS_PARAMETERS/noahmp401_parms/groundwater/wtd/
MMF HGT_M dir: ./LS_PARAMETERS/noahmp401_parms/groundwater/topo_gmted2010_30s/

MMF raw data are on the30-arcsec latlon grid and LDT uses UpscaleByAveraging to interpolate to the model grid.
MMF map projection: latlon
MMF spatial transform: average

FDEPTH fill option: neighbor
FDEPTH fill radius: 0 # Number of pixels to search for neighbor
FDEPTH fill value: 100.
FDEPTH water value: 100. # suitable value for lakes/rivers in case GEOGRID didn't have valid data at the location.

RECHCLIM fill option: neighbor
RECHCLIM fill radius: 0 # Number of pixels to search for neighbor
RECHCLIM fill value: -1.
RECHCLIM water value: -1 # suitable value for lakes/rivers in case GEOGRID didn't have valid data at the location.

EQWTD fill option: neighbor
EQWTD fill radius: 0 # Number of pixels to search for neighbor
EQWTD fill value: 0.
EQWTD water value: 0. # suitable value for lakes/rivers in case GEOGRID didn't have valid data at the location.

HGT_M fill option: neighbor
HGT_M fill radius: 0 # Number of pixels to search for neighbor
HGT_M fill value: 0.

[bmcandr]

Hi @smahanam, can you please share a full ldt.config file along with the LIS parameter file that you generated? I need these to be able to test your changes. You can point to a location on Discover rather than uploading them here.

[dmocko]

Hi @smahanam

I have reviewed the code, and overall it looks really good.

The code in /LISF/ldt/params/Noah processes parameters for both Noah-3.X and for Noah-MP-3.6 and Noah-MP-4.0.1.

From my review, it appears that the MMF parameters will be required in the ldt.config file for any version of Noah or Noah-MP. I believe that we should only look for these parameters if the LSM is Noah-MP-4.0.1. Also, it might be good to have a flag in the ldt.config file (e.g., "Process Noah-MP-4.0.1 MMF groundwater parameters: .true."). This flag can be assumed to be .false. in the code unless the config entry sets it to .true.. That way only users who plan to run the MMF groundwater will need to have/process the MMF parameters in the LDT step.

Please let me know if I have missed something in the code that already handles this.

[smahanam]

Hi @smahanam, can you please share a full ldt.config file along with the LIS parameter file that you generated? I need these to be able to test your changes. You can point to a location on Discover rather than uploading them here.

Hi @bmcandr,
Lambert Conical Grid example:
(one that I just used)
/gpfsm/dnb32/smahanam/LIS/MMF-GroundWater/ldt_tests/WPS_validate/ldt.config.3Rivers_MMF_lambert_Ridges
Lat/Lon case (GLDAS global)
/gpfsm/dnb32/smahanam/LIS/MMF-GroundWater/ldt_tests/ldt.config_noahmp401_gldas20_025d_hymap2.mmf

[smahanam]

Hi @dmocko,

I understood and agreed.
Just wondering, do we also need to put a protection in Noah_parmsMod.F90 to ensure LDT does not enter the MMF block unless the land surface model version is:
if (LDT_rc%lsm == "Noah-MP.4.0.1")

[dmocko]

Hi @smahanam

Yes - please put this protection into Noah_parmsMod.F90.

For the flag that I am suggesting for the ldt.config file, you can add this bit of code to the call ESMF_ConfigGetAttribute for this config entry: ,default=.false.

One other request, can you please add documentation for all of these config entries into LISF/ldt/configs/ldt.config.adoc file?

[smahanam]

One other request, can you please add documentation for all of these config entries into LISF/ldt/configs/ldt.config.adoc file?

Hi @dmocko,
LISF/ldt/configs/ldt.config.adoc
Is line 2378 a good place for new entries?

[dmocko]

Hi @emkemp - thanks for trying this pull request. The MMF team actually has a meeting at the top of the hour, which will include a discussion of this LDT pull request.

I believe that if @smahanam runs the testcase again with the latest code you won't see differences in those fields.

That said, we still have a concern with the RIVERBED parameter. This parameter is the only one where the source data is not continuous, so we are seeing some unrealistic fields with certain config settings. We have reached out to the MMF developers and will try their suggestions to come up with a solution. But more (likely minor) code changes on this PR are still probable before it should be merged.

[smahanam]

@emkemp We made some changes on June 24th and reran the testcase as part of the update as well. I just forgot to update tar-gzipped file.

/discover/nobackup/smahanam/mmfldttest.tar.gz
I just created the above file using the June 24th testcase run.

[emkemp]

Thanks @smahanam! I ran your updated test case and reproduced the results. I also recompiled with strict checks and did not encounter any runtime problems.

I will await further word on resolving the RIVERBED parameter that @dmocko mentioned above.

[emkemp]

PING

Hello @smahanam, @dmocko, and @TimLahmers:

Is there any news on resolving the RIVERBED parameter? That seems to be the remaining sticking point for this pull request.

[dmocko]

PONG

Hi @emkemp - thanks for checking in.

The RIVERBED parameter is used in computing the river conductivity. Further investigation by @TimLahmers has revealed that this calculation may not be applicable at fine spatial resolutions. We are exploring in LIS a physically-based river conductivity solution only at river points. This solution may require additional river channel parameters to be added to LDT.

An argument could be made that the RIVERBED implementation in LDT for this pull request is correct and ready-to-go (maybe with one additional tweak), and that the user should be aware of the above issue when running fine spatial resolutions until resolved in LIS. We have our MMF internal meeting later this week, and will discuss and report back.

[emkemp]

Hi @dmocko: Any update on resolving the RIVERBED parameter question?

[dmocko]

Yes - @smahanam will be making a commit or two to finalize this parameter, and then we can do one last review and, if it looks good, finally merge this PR.

[smahanam]

Hi @TimLahmers ,
You said "! I still need to reach out to Gonzalo to confirm that this RIVERBED configuration produces realistic output at 4-km resolution, so we might need to hold off on this for a couple more days"

So are we still waiting for Gonzalo's feedback?

[TimLahmers]

@smahanam I am sorry for the delay on this. Gonzalo has been slow to respond, so I will send him another reminder today. Thanks for bringing this up!

[smahanam]

I incorporated Tim's suggestion: RIVERBED will always <= HGT_M

[dmocko]

Many thanks to @smahanam for these latest changes. They look great!

Based on our MMF group meeting this afternoon, @smahanam @TimLahmers and I agree that this PR is complete.

@smahanam - Can you please re-run and share the testcase?

@emkemp - If you are satisfied with the code after running the testcase, you can go ahead and merge this PR.

[smahanam]

As I have said before, Github does not notify me these comments.
@dmocko I just saw your request to rerun the testcase.

I reran the testcase:
/gpfsm/dnb32/smahanam/mmfldttest/

[cmclaug2]

Hi @dmocko @emkemp @TimLahmers,

I ran @smahanam older testcase (generated on July 21) at /discover/nobackup/smahanam/mmfldttest.tar.gz using the Intel 21 compiler. I was able to reproduce the LDT parameter file, expect for small differences in the MMF_RIVERBED field.

I also ran @smahanam's latest test case (generated October 28th, 2022) at /discover/nobackup/smahanam/mmfldttest using the Intel 21 compiler. Again, I was able to reproduce the LDT parameter file, expect for larger differences in the MMF_RIVERBED field. If my understanding of the ongoing changes to the PR are correct, this difference is expected due to @smahanam's commit to ldt/params/Noah/Noah_parmsMod.F90 on October 11th.

I was also able to compile and run LDT with the Intel compiler with strict checks and with the gnu compiler.

My interpretation of this conversation is that my successful tests (excluding differences in MMF_RIVERBED) grants approval for this PR. I would like to double check with @emkemp to see what he thinks.

[TimLahmers]

@cmclaug2 Thanks for the updates, and those results are expected. The most recent code changes were updates to the MMF_RIVERBED scheme to ensure MMF_RIVERBED <= MMF_HGTM, and are consistent with guidance from Gonzalo Miguez-Macho on setting the parameters.

[dmocko]

Yes, thanks @cmclaug2 for testing.

However, I will check with @smahanam to see if he's run the testcase with the latest code.

I believe we are expecting the code to be merged to be identical for all parameters, including MMF_RIVERBED.

[dmocko]

One more thing @cmclaug2 - can you please share a figure showing the differences in MMF_RIVERBED between your test and the testcase?

[cmclaug2]

Hi @dmocko,

The following image depicts the target field for MMF_RIVERBED in the latest testcase (updated October 28nd):

The following image depicts the field for MMF_RIVERBED when I ran the October 28th testcase using the feature branch I checked out this morning:

You can find these files in '/discover/nobackup/cmclaug2/github/PR_1038/work_smahanam/mmfldttest_intel'

[TimLahmers]

@dmocko @cmclaug2 The second image looks good! The first image looks like something we were trying earlier in the project, when we considered setting RIVERBED to an unphysical value outside of channels. The second image should be what users are able to derive with normal settings.

[smahanam]

My bad @dmocko @TimLahmers @cmclaug2

I reran my testcase.

[cmclaug2]

Thank you for the update @smahanam! @dmocko, I am now able to reproduce the results of the testcase.

I would still like to check with @emkemp before confirming this code is ready to be merged.

[emkemp]

Good morning @cmclaug2

Can you tar up and store the test case input and output in /discover/nobackup/projects/lis/PR_TESTCASES/1038?

Once that's done, we can merge the pull request.

[emkemp]

@cmclaug2 will make a last minute change to the Noah_parmsMod.F90 to make sure the directory variables have length LIS_CONST_PATH_LEN.

[cmclaug2]

Hi @dmocko @smahanam,

I was able to reproduce the testcase and the proper documentation is included. This PR is ready to be accepted. I will be making the changes Eric mentioned above in a separate PR.