Skip to content

Split initial density into to parameters with a switch on HLM side#1550

Open
mvdebolskiy wants to merge 2 commits intoNGEET:mainfrom
mvdebolskiy:add-init-dbh-nocomp
Open

Split initial density into to parameters with a switch on HLM side#1550
mvdebolskiy wants to merge 2 commits intoNGEET:mainfrom
mvdebolskiy:add-init-dbh-nocomp

Conversation

@mvdebolskiy
Copy link
Copy Markdown
Contributor

Description:

Instead of using negative numbers in fates_recruit_init_density for an option to coldstart nocomp with dbh instead of density. Add fates_recruit_init_dbh parameter that can only be used when a new switch use_dbh_init is 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

  • The in-code documentation has been updated with descriptive comments
  • The documentation has been assessed to determine if updates are necessary

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided
  • FATES-CLM6 Code Freeze: satellite phenology regression tests are b4b

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:

@mvdebolskiy
Copy link
Copy Markdown
Contributor Author

CTSM-side PR: https://github.com/ESCOMP/CTSM#3910

@mvdebolskiy
Copy link
Copy Markdown
Contributor Author

This is not bfb if I change the init_seed. But the switch is off by default, so I can revert that parameter change.

@rgknox
Copy link
Copy Markdown
Contributor

rgknox commented Apr 10, 2026

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

@rgknox
Copy link
Copy Markdown
Contributor

rgknox commented Apr 16, 2026

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.

Copy link
Copy Markdown
Contributor

@JessicaNeedham JessicaNeedham left a comment

Choose a reason for hiding this comment

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

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.

Comment thread main/EDPftvarcon.F90
!-----------------------------------------------------------------------------------

if ( hlm_use_inventory_init == ifalse .and. &
abs( EDPftvarcon_inst%initd(ipft) ) < nearzero ) then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

Comment thread main/EDPftvarcon.F90
write(fates_log(),*) ' Aborting'
call endrun(msg=errMsg(sourcefile, __LINE__))
endif
if (EDPftvarcon_inst%init_seed(ipft) < nearzero) then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With 0 seeds non-zero density is fine.

@JessicaNeedham
Copy link
Copy Markdown
Contributor

This will also need an update to FATES user guide. @rgknox and @glemieux if you ping me when this gets merged I can make those changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Under Review

Development

Successfully merging this pull request may close these issues.

4 participants