Skip to content

Deactivate history fields in SP mode#1568

Open
mvdebolskiy wants to merge 4 commits into
NGEET:mainfrom
mvdebolskiy:inaktiv-hist-sp
Open

Deactivate history fields in SP mode#1568
mvdebolskiy wants to merge 4 commits into
NGEET:mainfrom
mvdebolskiy:inaktiv-hist-sp

Conversation

@mvdebolskiy

@mvdebolskiy mvdebolskiy commented May 11, 2026

Copy link
Copy Markdown
Contributor

Use a bool switch to deactivate default fields in SP-mode, so that on the HLM-side long hist_fexl* are not used.

Description:

See above

Collaborators:

@rgknox @glemieux

Expectation of Answer Changes:

For SP-test a few extra fields will be deactivated, since f.e. in CTSM tests, the FATES-SP usermod has not been updated in a while.

Description of generative AI usage (as necessary)

None.

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

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
  • Describe use of generative AI (if 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:

Only field list will change for SP tests with:

 Could not find match for file1 variable FATES_SEED_BANK_LUPF in file2
 Could not find match for file1 variable FATES_SEED_BANK_PF in file2

Tested with ctsm's FatesColdSatPhen testmod. with commenting out exclude in the fates-sp usermod vs default tag. Test with FatesCold and removing hist_fincl and empty_tapes showed no diffs.

*CTSM (or) E3SM (specify which) test hash-tag: ctsm5.4.038

*CTSM (or) E3SM (specify which) baseline hash-tag: ctsm5.4.038

*FATES baseline hash-tag: sci.1.92.4_api.45.0.0

Test Output:

@rgknox

rgknox commented May 14, 2026

Copy link
Copy Markdown
Contributor

Love this change

call this%set_history_var(vname='FATES_FROOTC', units='kg m-2', &
long='total biomass in live plant fine roots in kg carbon per m2', &
use_default='active', avgflag='A', vtype=site_r8, hlms='CLM:ALM', &
use_default=trim(drop_in_sp), avgflag='A', vtype=site_r8, hlms='CLM:ALM', &

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.

These allometric C structure variables are, I think, relevant/used in SP mode. The cohort is defined based on the height and the other pools are set accordingly. So there should be valid values for VEGC, SAPWOODC, FROOTC, STRUCTC, NONSTRUCTC

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.

See comment below.

call this%set_history_var(vname='FATES_VEGC_PF', units='kg m-2', &
long='total PFT-level biomass in kg of carbon per land area', &
use_default='active', avgflag='A', vtype=site_pft_r8, hlms='CLM:ALM', &
use_default=trim(drop_in_sp), avgflag='A', vtype=site_pft_r8, hlms='CLM:ALM', &

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.

This should exist in SP mode.

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.

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.

I don't think we calculate it though.

WHen in SP or ST3 mode, we perform a bypass of the whole allocation algorithm:

https://github.com/NGEET/fates/blob/main/main/EDMainMod.F90#L1133

Without calling it, there is no VEGC. We could certainly improve the bypass though to set VEGC to allometric targets though....

call this%set_history_var(vname='FATES_LAI_CANOPY_SZ', units = 'm2 m-2', &
long='leaf area index (LAI) of canopy plants by size class', &
use_default='active', avgflag='A', vtype=site_size_r8, &
use_default=trim(drop_in_sp), avgflag='A', vtype=site_size_r8, &

@rosiealice rosiealice May 14, 2026

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.

In principle the plants in SP mode do have a height and and LAi and so in theory this has some meaning...

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.

But there is only one size ("SZ") right? I see this diagnostic as not wrong, just unneeded with SP.

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.

I actually think this might be useful, even in SP. since the height bins aren't the height of the plant, but the height of the leaf area distributions of the plants, so there could in principle be some information here


call this%set_history_var(vname='FATES_NPLANT_SZ', units = 'm-2', &
long='number of plants per m2 by size class', use_default='active', &
long='number of plants per m2 by size class', use_default=trim(drop_in_sp), &

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.

This might also be useful as a diagnostic to check that the plants are coming out in the right size class given HTOP...

@rosiealice rosiealice left a comment

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.

Looks great @mvdebolskiy ... Thanks for pushing this back.

@mvdebolskiy

Copy link
Copy Markdown
Contributor Author

@rosiealice I don't quite get the comments. If the variables there "should be active" why are they exluded in the usermod that is getting picked up by FATES-SP compsets? Do you need this variables ouputted all the time you have an SP run, or can they just be requested by user whenever user needs them?

@rosiealice

Copy link
Copy Markdown
Contributor

@rosiealice I don't quite get the comments. If the variables there "should be active" why are they exluded in the usermod that is getting picked up by FATES-SP compsets? Do you need this variables ouputted all the time you have an SP run, or can they just be requested by user whenever user needs them?

Fair enough. It's not critical either way. I guess i just didn't think about these reasons for keeping them last time I went through the variable list (I realize I'm critiquing my own earlier classification effort here, so should have been clearer about that :) ) .

Comment thread main/FatesHistoryInterfaceMod.F90 Outdated
@rosiealice

Copy link
Copy Markdown
Contributor

Just for now, none of this is super critical and we can always add these variables back in later if necessary. I think we can pull this PR as it is...

@rosiealice rosiealice left a comment

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.

I think this is really OK as it is, for the sake of time. We can always modify the details later, but this is necessary for the whole FATES compset workflow and so it's probably best to include it sooner rather than later (thus retracting my earlier nitpicking over the inclusion of certain fields :) )

@ckoven

ckoven commented May 22, 2026

Copy link
Copy Markdown
Contributor

I think this is really OK as it is, for the sake of time. We can always modify the details later, but this is necessary for the whole FATES compset workflow and so it's probably best to include it sooner rather than later (thus retracting my earlier nitpicking over the inclusion of certain fields :) )

agreed

@glemieux glemieux moved this from Under Review to Final Testing in FATES Pull Request Planning and Status May 26, 2026
Refactor initial seed parameter into two new parameters

This creates a new parameter `fates_recruit_init_dbh` to set the initial
seedling DBH.  Prior to this tag, `fates_recruit_init_density` could
enact this functionality by setting a negative value.
@glemieux

glemieux commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Regression testing underway on derecho.

@mvdebolskiy

Copy link
Copy Markdown
Contributor Author

Btw, this needs an acompaning PR that removes the hist_fexl statement from fates-sp usermod or a PR with refactor for ctsm compsets NorESMhub/CTSM#224 the answers won't be b4b, since there are more fields removed than in the usermod.

@wwieder

wwieder commented Jun 2, 2026

Copy link
Copy Markdown

Can you remind me, Is this something we want to integrate for the CLM-FATES freeze? If so, is there a CLM side issue or PR that we can use for tracking?

@glemieux

glemieux commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Can you remind me, Is this something we want to integrate for the CLM-FATES freeze? If so, is there a CLM side issue or PR that we can use for tracking?

@wwieder I don't think there is an ctsm-side issue yet. Has there been discussion with the CTSM SE team about adopting the FATES SP mode build namelist BGC flag option that NorESM implemented in the PR NorESMhub/CTSM#224 @mvdebolskiy mentioned above (i.e. moving away from the user_mod setup for FATES-SP complist).
https://github.com/mvdebolskiy/CTSM/blob/fb55d3470691726315d986a63fd9f950e85f59c9/bld/CLMBuildNamelist.pm#L88-L108

I've got a branch just update the user_mod per Matvey's reminder if that's the preference instead.

@glemieux glemieux moved this from Final Testing to Hold in FATES Pull Request Planning and Status Jun 2, 2026
@mvdebolskiy

Copy link
Copy Markdown
Contributor Author

@wwieder I've tested my noresm-compsets PR that also reduces some fates usermod burden on derecho and it did not affect clm-test's namelists in aux_clm. The only issue with it is that I need a PR to put drydep and bvoc's into ngeet from NorESM fork, in order to have CTSM-side working. And the last question is how many fates_parameter files we need to maintain. Plus, the initial datasets/landuse forcing is not finalized yet, since we are fixing some bugs with tiny numbers causing balance errors.

@glemieux

glemieux commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Btw, this needs an acompaning PR that removes the hist_fexl statement from fates-sp usermod

This is waiting on approval of ESCOMP/CTSM#4079.

@glemieux glemieux moved this from Hold to Final Testing in FATES Pull Request Planning and Status Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Final Testing

Development

Successfully merging this pull request may close these issues.

6 participants