Add snow-vegetation interactions, NGEE Arctic IM3 #6607

thorntonpe · 2024-09-11T20:45:51Z

Change handling for how snow burial of shrubs is calculated, with parameterization for bending of stems.
Change the treatment of surface weather downscaling on topounits, so that the downscaling happens when using the coupler bypass code branch.
Introduce four new parameters controlling the burial of shrubs by snow into vegetation physiology file.
If new parameters are not included on the physiology file the code replaces with default values, which then produce BFB results with code before this PR.
The four new parameters are: taper, stocking, bendresist, and vegshape.
[BFB]
Development Lead: Rich Fiorella rfiorella@lanl.gov
Development Team: Katrina Bennett kbennett@lnl.gov, Cade Trotter ctrotter@lanl.gov, Claire Bachand cbachand@lanl.gov

Enables two major changes to the parameterization that calculates LAI burial by snow: a) pulls stocking density and taper parameters that influence vegetation height from hard-wired values in biogeochem/VegStructUpdateMod.F90 to allow to be specified on the surface file (per main/pftvarcon) and b) updates the LAI burial by snow rate from Wang and Zeng, 2007 (linear burial) to form proposed by Liston and Hiemstra, 2011 (linear or non-linear) burial. The Liston and Heimstra (2011) formulation introduces two new parameters: 1) bendresist, which specifies how resistant branches are to bending under snow loading (1 = no bending, lim->0 branches bend very rapidly) and 2) vegshape, which specifies how the shape of the vegetation crown affects how rapidly it is buried by snow (1 = paraboloid; 2 = hemispheroid). These new parameters can also be specified on surface file.

Temporary change, pending merge of IM4 updates. IM4 removed a lot of the hard-coded tests that looked for particular pfts by their assigned number. this required modifying the helper function "woody" to be a non-binary (i.e., now 0 = herbaceous, 1 = woody tree, 2 = woody shrub), but this updated function isn't available to IM3 currently.

The taper array was being initialized incorrectly, leading to developer test failures. This moves the definition down further to be able to initialize correctly.

Adds a new exact restart test that uses new parameters in the param file for snow/vegetation interactions.

fixes an issue where using less than the maximum pfts set new parameters with non-physical values

thorntonpe · 2024-09-11T21:12:09Z

@peterdschwartz and @bishtgautam My branch is rebased to the current master. I think it should pass the land_developer test suite BFB.

bishtgautam

@thorntonpe, the PR looks good to me.

I believe you inadvertently deleted a lot of whitespaces in this PR (e.g., https://github.com/E3SM-Project/E3SM/pull/6607/files#diff-f9276b0713e2af1bcd0495bdbd37de680e27b15dcf180b1d8da1ec0243ba17dfL39). Maybe the setting of your editor is such that it removes whitespaces automatically at the end of the line of the current cursor location and you can try avoiding whitespace deletion in the future.

thorntonpe · 2024-09-12T13:28:44Z

@bishtgautam yes, the whitespace deletion was due to an editor setting from one of the LANL developers. I'll ask that they change that setting for future development.

thorntonpe · 2024-09-12T15:46:11Z

Based on discussion yesterday at ORNL I looked into the CPP flags that might have some influence on the use of shared executables for land developer tests. I'm not seeing any obvious issues. Could we do a test integration of this PR?

bishtgautam · 2024-09-12T15:50:50Z

We are waiting for the last two open PRs for the v3.0.1 tag to be merged. Then, we should be able to begin integrating this PR.

peterdschwartz · 2024-09-12T20:47:14Z

submitted test suite on pm with this on next, so should know more tomorrow

thorntonpe · 2024-09-13T12:19:28Z

I'm seeing the pm-cpu results show most tests passing, but I was expecting to see a fail (no baseline) for the new test in this PR:
ERS.ELM_USRDAT.I1850CNPRDCTCBC.elm-snowveg_arctic
That test is not showing up in either the PASS or FAIL lists.

thorntonpe · 2024-09-13T12:34:08Z

@peterdschwartz Realizing now that your test suite was probably submitted outside the test dashboard. Ignore previous comment.

peterdschwartz · 2024-09-13T16:24:40Z

@thorntonpe I did get several DIFFs while running the test suite:

ERS.ELM_USRDAT.I1850CNPRDCTCBC.pm-cpu_gnu.elm-snowveg_arctic (Overall: DIFF) details:
  ERS.ELM_USRDAT.I1850CNPRDCTCBC.pm-cpu_gnu.elm-usrpft_codetest_I1850CNPRDCTCBC (Overall: DIFF) details:
  ERS.ELM_USRDAT.I1850CNPRDCTCBC.pm-cpu_gnu.elm-usrpft_default_I1850CNPRDCTCBC (Overall: DIFF) details:
  ERS.ELM_USRDAT.IELM.pm-cpu_gnu.elm-surface_water_dynamics (Overall: DIFF) details:
  ERS.f09_g16.I1850ELMCN.pm-cpu_gnu.elm-bgcinterface (Overall: DIFF) details:
  ERS.f19_f19.I1850ELMCN.pm-cpu_gnu (Overall: DIFF) details:
  ERS.f19_f19.I20TRELMCN.pm-cpu_gnu (Overall: DIFF) details:
  ERS.f19_g16.I1850ELM.pm-cpu_gnu.elm-betr (Overall: DIFF) details:
  ERS.f19_g16.I1850ELM.pm-cpu_gnu.elm-vst (Overall: DIFF) details:
  ERS_Ln9.ne4pg2_ne4pg2.F2010-MMF1.pm-cpu_gnu.eam-mmf_crmout (Overall: DIFF) details:
  ERS.r05_r05.IELM.pm-cpu_gnu.elm-lnd_rof_2way (Overall: DIFF) details:
  ERS.r05_r05.IELM.pm-cpu_gnu.elm-V2_ELM_MOSART_features (Overall: DIFF) details:
  SMS_Ld1.hcru_hcru.I1850CRUELMCN.pm-cpu_gnu (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.pm-cpu_gnu.elm-fan (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.pm-cpu_gnu.elm-force_netcdf_pio (Overall: DIFF) details:
  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.pm-cpu_gnu.elm-per_crop (Overall: DIFF) details:
  SMS.r05_r05.I1850ELMCN.pm-cpu_gnu.elm-qian_1948 (Overall: DIFF) details:
  SMS.r05_r05.IELM.pm-cpu_gnu.elm-topounit (Overall: DIFF) details:

Took one and submitted it by itself and it passes, so I can look into this further

ERS.ELM_USRDAT.IELM.pm-cpu_gnu.elm-surface_water_dynamics (Overall: PASS) details:

thorntonpe · 2024-09-13T17:04:30Z

This is the same behavior I was observing. Thanks for looking into it.

peterdschwartz · 2024-09-13T17:19:54Z

Ok, I can confirm that error is due to the new test ERS.ELM_USRDAT.I1850CNPRDCTCBC.pm-cpu_gnu.elm-snowveg_arctic using MODAL_AER in ELM_CONFIG_OPTS.

When run as a part of the test suite, the EXEROOT for a DIFF'ing test will be set to the new test:

EXEROOT: /pscratch/sd/p/pschwar3/IM3_testing/output/ERS.ELM_USRDAT.I1850CNPRDCTCBC.pm-cpu_gnu.elm-snowveg_arctic.C.20240912_133219_khssny/bld

The new test's ELM_CONFIG_OPTS are:

pschwar3@login35: ERS.ELM_USRDAT.I1850CNPRDCTCBC.pm-cpu_gnu.elm-snowveg_arctic.C.20240912_133219_khssny $
 --> ./xmlquery ELM_CONFIG_OPTS
        ELM_CONFIG_OPTS: -phys elm -phys elm -cppdefs -DMODAL_AER

But the DIFF'ing tests originally had:

pschwar3@login16: SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.pm-cpu_gnu.elm-fan.C.20240912_133219_khssny $
 --> ./xmlquery ELM_CONFIG_OPTS
        ELM_CONFIG_OPTS: -phys elm

I looked at the tests within the e3sm_land_exeshare and those tests all have modal aerosols enabled. So moving the test there should solve the issue.

pschwar3@login35: ERS.r05_r05.ICNPRDCTCBC.pm-cpu_gnu.elm-cbudget.C.20240912_133219_khssny $
 --> ./xmlquery ELM_CONFIG_OPTS
        ELM_CONFIG_OPTS: -phys elm -phys elm -cppdefs -DMODAL_AER

peterdschwartz · 2024-09-16T17:13:38Z

@thorntonpe pushed a commit moving the tests to a different suite, which should solve testing related issues for this branch.

rljacob · 2024-09-16T17:34:26Z

The discussion above is confusing me especially this part "When run as a part of the test suite, the EXEROOT for a DIFF'ing test will be set to the new test".

peterdschwartz · 2024-09-16T17:54:24Z

@rljacob In this case, for pm-cpu_gnu, the EXEROOT for e3sm_land_developer tests was being set to the test introduced by this PR (new test: ERS.ELM_USRDAT.I1850CNPRDCTCBC.elm-snowveg_arctic), but that test uses a compset that has different CPPDEFS than the other tests. So the other tests will DIFF.

The EXEROOT seems to be set (or influenced) depending on machine/compiler name so if the EXEROOT is set to a pre-existing test, the tests (except the new test) will share an executable built with the correct CPPDEFs. That's why testing with this PR and the previous related land PR would give diffs on certain machine/compiler configs but not on others.

thorntonpe · 2024-09-17T17:06:26Z

@peterdschwartz do you have an estimate for when this will be merged to next? I'm giving an update to the NGEE Arctic team later today

rljacob · 2024-09-17T17:19:42Z

We are prioritizing PRs for the v3.0.1 tag this week. This could go in next week.

thorntonpe · 2024-10-04T13:45:26Z

@peterdschwartz and @bishtgautam I'd like to get this integrated in advance of the 3.1beta bug fixes. Can we move it to the top of the list?

peterdschwartz · 2024-10-04T14:31:41Z

Sure, I don't see why we couldn't.

Note: The PR is still marked BFB, but it may be possible any tests diffs that were blessed from the arctic vegetation test may DIFF as result of this PR moving that test to the correct test suite.

thorntonpe · 2024-10-04T14:36:39Z

@peterdschwartz Thanks, and understood on the movement of tests in the test suite. We'll be watching for those.

Change handling for how snow burial of shrubs is calculated, with parameterization for bending of stems. Change the treatment of surface weather downscaling on topounits, so that the downscaling happens when using the coupler bypass code branch. Introduce four new parameters controlling the burial of shrubs by snow into vegetation physiology file. If new parameters are not included on the physiology file the code replaces with default values, which then produce BFB results with code before this PR. The four new parameters are: taper, stocking, bendresist, and vegshape. [BFB] Development Lead: Rich Fiorella rfiorella@lanl.gov Development Team: Katrina Bennett kbennett@lnl.gov, Cade Trotter ctrotter@lanl.gov, Claire Bachand cbachand@lanl.gov

peterdschwartz · 2024-10-07T21:23:01Z

merged to next

peterdschwartz · 2024-10-08T16:08:28Z

No unexpected diffs due to this PR as far as I can tell -- A couple of land tests had either a licensing issue or a time out issue.

So I will merge to master and submit bless requests for the newly added test and the moved tests.

thorntonpe and others added 15 commits September 11, 2024 15:51

Added downscaling code for coupler bypass

2faa927

Bug fixes for coupler bypass downscaling

4188433

initial commit of adding shrub bending parameter

db23793

Update ci-compile-test.yml

f9799c6

add bounds checking on bendresist and vegshape parameters

702f73c

Rearrange taper assignment in pftvarcon

20bb484

The taper array was being initialized incorrectly, leading to developer test failures. This moves the definition down further to be able to initialize correctly.

Add IM3 ERS developer test to e3sm_land_developer

e979a2c

Adds a new exact restart test that uses new parameters in the param file for snow/vegetation interactions.

Fix new IM3 paramter values in pftvarcon

f0c93cf

fixes an issue where using less than the maximum pfts set new parameters with non-physical values

remove local ci test

0f02df7

Update comment to correctly reflect references in VegStructUpdateMod

01ac9bd

use woody instead of hardcoding to pft number

3734c0f

update woody comment description to reflect IM4 changes

18fa418

Fix syntax error, missing comment marker

f5ee59a

thorntonpe added Land BFB PR leaves answers BFB in progress labels Sep 11, 2024

thorntonpe assigned peterdschwartz Sep 11, 2024

thorntonpe requested a review from bishtgautam September 11, 2024 20:48

bishtgautam approved these changes Sep 11, 2024

View reviewed changes

Moved tests to e3sm_land_exeshare test suite

3cb0e38

thorntonpe removed the in progress label Sep 16, 2024

peterdschwartz merged commit 4b85dbe into E3SM-Project:master Oct 8, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add snow-vegetation interactions, NGEE Arctic IM3 #6607

Add snow-vegetation interactions, NGEE Arctic IM3 #6607

thorntonpe commented Sep 11, 2024

thorntonpe commented Sep 11, 2024

bishtgautam left a comment

thorntonpe commented Sep 12, 2024

thorntonpe commented Sep 12, 2024

bishtgautam commented Sep 12, 2024

peterdschwartz commented Sep 12, 2024

thorntonpe commented Sep 13, 2024

thorntonpe commented Sep 13, 2024

peterdschwartz commented Sep 13, 2024

thorntonpe commented Sep 13, 2024

peterdschwartz commented Sep 13, 2024 •

edited

Loading

peterdschwartz commented Sep 16, 2024

rljacob commented Sep 16, 2024

peterdschwartz commented Sep 16, 2024

thorntonpe commented Sep 17, 2024

rljacob commented Sep 17, 2024

thorntonpe commented Oct 4, 2024

peterdschwartz commented Oct 4, 2024

thorntonpe commented Oct 4, 2024

peterdschwartz commented Oct 7, 2024

peterdschwartz commented Oct 8, 2024

Add snow-vegetation interactions, NGEE Arctic IM3 #6607

Add snow-vegetation interactions, NGEE Arctic IM3 #6607

Conversation

thorntonpe commented Sep 11, 2024

thorntonpe commented Sep 11, 2024

bishtgautam left a comment

Choose a reason for hiding this comment

thorntonpe commented Sep 12, 2024

thorntonpe commented Sep 12, 2024

bishtgautam commented Sep 12, 2024

peterdschwartz commented Sep 12, 2024

thorntonpe commented Sep 13, 2024

thorntonpe commented Sep 13, 2024

peterdschwartz commented Sep 13, 2024

thorntonpe commented Sep 13, 2024

peterdschwartz commented Sep 13, 2024 • edited Loading

peterdschwartz commented Sep 16, 2024

rljacob commented Sep 16, 2024

peterdschwartz commented Sep 16, 2024

thorntonpe commented Sep 17, 2024

rljacob commented Sep 17, 2024

thorntonpe commented Oct 4, 2024

peterdschwartz commented Oct 4, 2024

thorntonpe commented Oct 4, 2024

peterdschwartz commented Oct 7, 2024

peterdschwartz commented Oct 8, 2024

peterdschwartz commented Sep 13, 2024 •

edited

Loading