Add snow-vegetation interactions, NGEE Arctic IM3 #6607
Conversation
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
|
@peterdschwartz and @bishtgautam My branch is rebased to the current master. I think it should pass the land_developer test suite BFB. |
There was a problem hiding this comment.
@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.
|
@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. |
|
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? |
|
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. |
|
submitted test suite on pm with this on next, so should know more tomorrow |
|
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: |
|
@peterdschwartz Realizing now that your test suite was probably submitted outside the test dashboard. Ignore previous comment. |
|
@thorntonpe I did get several DIFFs while running the test suite: Took one and submitted it by itself and it passes, so I can look into this further |
|
This is the same behavior I was observing. Thanks for looking into it. |
|
Ok, I can confirm that error is due to the new test When run as a part of the test suite, the EXEROOT for a DIFF'ing test will be set to the new test: The new test's But the DIFF'ing tests originally had: I looked at the tests within the |
|
@thorntonpe pushed a commit moving the tests to a different suite, which should solve testing related issues for this branch. |
|
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". |
|
@rljacob In this case, for pm-cpu_gnu, the EXEROOT for 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. |
|
@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 |
|
We are prioritizing PRs for the v3.0.1 tag this week. This could go in next week. |
|
@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? |
|
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. |
|
@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 [email protected]
Development Team: Katrina Bennett [email protected], Cade Trotter [email protected], Claire Bachand [email protected]