Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a modified infiltration scheme in ELM to improve surface water dynamics #5974

Conversation

donghuix
Copy link
Contributor

@donghuix donghuix commented Oct 9, 2023

In default, the infiltration rate is the same for the non-water surface and surface water storage within the grid cell. This assumption results in a significant underestimation of surface water because the infiltration in the surface water storage is not constrained.

In this PR, we introduced a modified infiltration scheme to constrain the infiltration in the surface water storage. The infiltration rate in the surface water storage is smaller than the infiltration rate in the non-water surface. More details about this new scheme and corresponding evaluations can be found in the preprint: https://doi.org/10.21203/rs.3.rs-2733749/v1 The manuscript is still under review.

The new infiltration scheme can be turned by adding use_modified_infil = .true. in user_nl_elm.

We passed all the tests in e3sm_land_developer test suite.

We also created a test for the simulation of surface water dynamics with the new infiltration scheme and calibrated parameter for NLDAS domain: surface_water_dynamics. This test was added into e3sm_land_developer test suite.

[BFB]

@rljacob rljacob added the Land label Oct 9, 2023
@bishtgautam bishtgautam added the BFB PR leaves answers BFB label Oct 9, 2023
Copy link
Contributor

@bishtgautam bishtgautam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@donghuix I have a few minor comments for you to address.

call ncd_pio_openfile (ncid, locfn, 0)
call ncd_io(ncid=ncid, varname='pc', flag='read', data=this%pc, dim1name=grlnd, readvar=readvar)
if (.not. readvar) then
this%pc(:) = 0.4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change 0.4 to 0.4_r8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using 0.4_r8 will result in non BFB to the master branch. That is because pc is set to 0.4 in the master (Please see next reply).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, leave it as is. Thanks.

components/elm/src/main/elm_varcon.F90 Outdated Show resolved Hide resolved
components/elm/src/biogeophys/SoilHydrologyMod.F90 Outdated Show resolved Hide resolved
@peterdschwartz
Copy link
Contributor

Are all the input files for the test on the input data server? Will use_modified_infil = .true. work without needing calibrated parameters? If not, I would suggest making a compset or use case to take care of all the dependencies.

Also, urls can't go into the git log, so do you have a citation that could be used instead?

@donghuix
Copy link
Contributor Author

@bishtgautam I addressed your comments in the new commit. However, I cannot add ._r8 for the assignment of pc, which will result in differences compared to baseline simulation.

@peterdschwartz Yes, @bishtgautam has uploaded a calibrated ELM surface dataset to the input data server for the test case. @bishtgautam Can you confirm it?

use_modified_infil = .true. doesn't need calibrated parameter, and it works with any ELM surface dataset as default parameter values will be used.

I changed the urls to citation in the log. Please let me know if additional modifications are needed.

Thanks!

@peterdschwartz
Copy link
Contributor

I changed the urls to citation in the log. Please let me know if additional modifications are needed.

Thanks!

@donghuix I should have been more precise. When I merge your PR, I'll copy/paste your PR description at the top of this page as the merge commit message. So, I just need this researchgate url replaced with the citation

In this PR, we introduced a modified infiltration scheme to constrain the infiltration in the surface water storage. The infiltration rate in the surface water storage is smaller than the infiltration rate in the non-water surface. More details about this new scheme and corresponding evaluations can be found in the preprint: https://www.researchsquare.com/article/rs-2733749/v1. The manuscript is still under review.

Thanks

@rljacob rljacob added the Stealth PR has feature which, if turned on, could change climate. fka FCC label Oct 16, 2023
@rljacob
Copy link
Member

rljacob commented Oct 16, 2023

You can use the DOI https://doi.org/10.21203/rs.3.rs-2733749/v1

@peterdschwartz
Copy link
Contributor

@peterdschwartz Yes, @bishtgautam has uploaded a calibrated ELM surface dataset to the input data server for the test case. @bishtgautam Can you confirm it?

Tried to run the new test on chrysalis and the input file is not available for download..... @donghuix @bishtgautam

@bishtgautam
Copy link
Contributor

The surfdata_nldas2_simyr2000_c181207_surfacewater.nc is now available on https://web.lcrc.anl.gov/public/e3sm/inputdata/lnd/clm2/surfdata_map/

@peterdschwartz
Copy link
Contributor

Tests show DIFFs in these tests:

 SMS_Ln9.ne4_oQU240.F2010.chrysalis_intel.eam-outfrq9s (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-force_netcdf_pio (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-per_crop (Overall: DIFF) details:
  SMS_Ly2_P1x1_D.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-lulcc_sville (Overall: DIFF) details

Appear to be only due to the new history field added. Could put an if statement around the new history field addition with the new namelist flag ?

@donghuix
Copy link
Contributor Author

donghuix commented Nov 8, 2023

Tests show DIFFs in these tests:

 SMS_Ln9.ne4_oQU240.F2010.chrysalis_intel.eam-outfrq9s (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-force_netcdf_pio (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-per_crop (Overall: DIFF) details:
  SMS_Ly2_P1x1_D.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-lulcc_sville (Overall: DIFF) details

Appear to be only due to the new history field added. Could put an if statement around the new history field addition with the new namelist flag ?

I removed the new variable in the history field, as it is not needed. Let me know if it passes the test now.

@peterdschwartz
Copy link
Contributor

Yes it looks like the DIFFs are resolved.

peterdschwartz added a commit that referenced this pull request Nov 14, 2023
…n-scheme' into next (PR #5974)

In default, the infiltration rate is the same for the non-water surface and surface water storage within the grid cell.
This assumption results in a significant underestimation of surface water because the infiltration in the surface water storage is not constrained.

In this PR, we introduced a modified infiltration scheme to constrain the infiltration in the surface water storage.
The infiltration rate in the surface water storage is smaller than the infiltration rate in the non-water surface.
More details about this new scheme and corresponding evaluations can be found in the preprint:
https://doi.org/10.21203/rs.3.rs-2733749/v1 The manuscript is still under review.

The new infiltration scheme can be turned by adding use_modified_infil = .true. in user_nl_elm.

We passed all the tests in e3sm_land_developer test suite.

We also created a test for the simulation of surface water dynamics with the new infiltration scheme
 and calibrated parameter for NLDAS domain: surface_water_dynamics. This test was added into e3sm_land_developer test suite.

[BFB] [STEALTH]
@peterdschwartz
Copy link
Contributor

merged to next

@peterdschwartz peterdschwartz merged commit a3a53ba into E3SM-Project:master Nov 16, 2023
2 checks passed
@peterdschwartz
Copy link
Contributor

merged to master. Will wait till tomorrow to bless pm test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB Land Stealth PR has feature which, if turned on, could change climate. fka FCC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants