Split initial density into to parameters with a switch on HLM side#1550
Split initial density into to parameters with a switch on HLM side#1550mvdebolskiy wants to merge 2 commits intoNGEET:mainfrom
Conversation
|
CTSM-side PR: https://github.com/ESCOMP/CTSM#3910 |
|
This is not bfb if I change the init_seed. But the switch is off by default, so I can revert that parameter change. |
|
This actually brings up a broader discussion on how we maintain (paradoxically) more than 1 default file. We have utilities to handle this: https://github.com/NGEET/fates/blob/main/tools/batch_patch_params.py Note that this json file that works in conjunction with the previous script, applies a patch that turns on sized based initialization: https://github.com/NGEET/fates/blob/main/parameter_files/patch_nocomp_noresm.json |
|
We discussed this more at the CTSM dev meeting. There were good arguments to support the idea to having dedicated parameters for DBH initialization and dedicated parameters for number initialization. One argument was that more than one user thought it was more logical to have two parameters, and that using negative values "flipped their brain". There was also an agument that the PPEs and parameter ranges are less confusing when they don't have two modes. It may also simplify how we connect compsets to different parmaeter configurations since it doesn't have to modify the parameter file, and would just modify the NL variable. It sounds like the consensus is to move ahead on this set of changes and get it in for the maintenance tag. Lets see if anyone else chimes in here. |
JessicaNeedham
left a comment
There was a problem hiding this comment.
Thanks @mvdebolskiy ! These changes look good to me and I agree that having two parameters is less confusing than having one with different meanings. Only a minor question about some of the checks.
| !----------------------------------------------------------------------------------- | ||
|
|
||
| if ( hlm_use_inventory_init == ifalse .and. & | ||
| abs( EDPftvarcon_inst%initd(ipft) ) < nearzero ) then |
There was a problem hiding this comment.
Should we remove the abs here since we don't want initd to be negative or a tiny number? Then we could also remove the check below?
| write(fates_log(),*) ' Aborting' | ||
| call endrun(msg=errMsg(sourcefile, __LINE__)) | ||
| endif | ||
| if (EDPftvarcon_inst%init_seed(ipft) < nearzero) then |
There was a problem hiding this comment.
Why do we need an extra check for init_seed being positive if we are in dbh initialisation mode? There's another check right below that should catch it.
There was a problem hiding this comment.
You can not initiate the model with 0 seed and dbh. It will crash within first few days because of the canopy. Basically you are starting with fat grown tries with no carbon?
There was a problem hiding this comment.
That makes sense. I guess I was wondering if this is specific to dbh initialisation? I'm assuming the same applies if you have 0 seeds and 0 number density? So is the check here not sufficient? https://github.com/mvdebolskiy/fates/blob/a78990cfcd80856c12ec9a25d91f9df8bb409490/main/EDPftvarcon.F90#L1302
There was a problem hiding this comment.
With 0 seeds non-zero density is fine.
Description:
Instead of using negative numbers in
fates_recruit_init_densityfor an option to coldstart nocomp with dbh instead of density. Addfates_recruit_init_dbhparameter that can only be used when a new switchuse_dbh_initis on. That one has to be set on the hlm side.This is needed to have NorESM calibrated parameters to work and be default while NOT braking any full-fates configurations.
Collaborators:
Expectation of Answer Changes:
None, since switch is off by default.
If the switch is on, the model will break anyway with the current values for
init_seed.Checklist
All checklist items must be checked to enable merging this pull request:
Contributor
Integrator
If satellite phenology regressions are not b4b, please hold merge and notify the FATES development team.
Documentation
Test Results:
CTSM (or) E3SM (specify which) test hash-tag:
CTSM (or) E3SM (specify which) baseline hash-tag:
FATES baseline hash-tag:
Test Output: