Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update GetNutrientTargetCNP for Deciduous PFT #1348

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sharma-bharat
Copy link

@sharma-bharat sharma-bharat commented Feb 28, 2025

Title: Preventing Declines in Carbon Flux During Leaf-Off Period in Deciduous PFTs

Description:

During the leaf-off period in deciduous plant functional types (PFTs), the target leaf biomass was being reset to zero which meant the target fine root biomass also goes to zero that casuses fine-root mass to decline. This led to a decline in carbon fluxes over time (Fig. 1).

Solution:
We modified the elongf_leaf, elongf_fnrt, and elongf_stem parameters to a value of 1.0 in the parteh/PRTAllometricCNPmod.F90 module.

Key Changes:

call bleaf(dbh,ipft,crown_damage,canopy_trim, 1.0_r8, leaf_c_target)
call bfineroot(dbh,ipft,canopy_trim,l2fr, 1.0_r8, fnrt_c_target)
call bsap_allom(dbh,ipft,crown_damage,canopy_trim, 1.0_r8, sapw_area,sapw_c_target)
call bagw_allom(dbh,ipft,crown_damage, 1.0_r8, agw_c_target)
call bbgw_allom(dbh,ipft, 1.0_r8, bgw_c_target)

This modification stabilizes carbon fluxes (Fig. 2).

image
Figure 1: Carbon fluxes using default ELM-FATES-CNP under transient run

image
Figure 2: Carbon fluxes with modification described in this PR under transient run

Please note: Sudden decline in Fig. 1 and 2 towards the end is due to logging event.

Collaborators:

Anthony Walker, ORNL (@walkeranthonyp)

Expectation of Answer Changes:

This update is expected to impact model outputs related to seasonal carbon fluxes and biomass allocation in deciduous PFTs.

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

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided

Documentation

Test Results:

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag: v2.1.0-13087-ge9afb7cb13

FATES baseline hash-tag: sci.1.68.2_api.31.0.0

Test Output:

@sharma-bharat sharma-bharat added science: bug Bugs that are specific to the implementation of a scientific model software: bug Bug that is specific to software labels Feb 28, 2025
@sharma-bharat sharma-bharat reopened this Feb 28, 2025
@rgknox rgknox requested review from mpaiao and rgknox March 3, 2025 20:10
Copy link
Contributor

@mpaiao mpaiao left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @sharma-bharat! I went through it and it looks good if the intent is to use the target biomass independently on the phenological state (I have not used the CNP code).

My only really minor suggestion is to remove the commented chunk of code.

Comment on lines 2090 to 2107
! bug fix: Preventing decline in Carbon Flux During Leaf-Off Period in Deciduous PFTs
! This call to organs' target biomass is independent of phenological stage 
! more details: https://github.com/NGEET/fates/pull/1348

call bleaf(dbh,ipft,crown_damage,canopy_trim, 1.0_r8, leaf_c_target)
call bfineroot(dbh,ipft,canopy_trim,l2fr, 1.0_r8, fnrt_c_target)
call bsap_allom(dbh,ipft,crown_damage,canopy_trim, 1.0_r8, sapw_area,sapw_c_target)
call bagw_allom(dbh,ipft,crown_damage, 1.0_r8, agw_c_target)
call bbgw_allom(dbh,ipft, 1.0_r8, bgw_c_target)


!call bleaf(dbh,ipft,crown_damage,canopy_trim, elongf_leaf, leaf_c_target)
!call bfineroot(dbh,ipft,canopy_trim,l2fr, elongf_fnrt, fnrt_c_target)
!call bsap_allom(dbh,ipft,crown_damage,canopy_trim, elongf_stem, sapw_area,sapw_c_target)
!call bagw_allom(dbh,ipft,crown_damage, elongf_stem, agw_c_target)
!call bbgw_allom(dbh,ipft, elongf_stem, bgw_c_target)
!call bdead_allom(agw_c_target,bgw_c_target, sapw_c_target, ipft, struct_c_target)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for spotting this bug @sharma-bharat! If the goal is to be independent on phenological state, then your changes are correct. My only minor suggestion is to actually delete the commented out lines because they are incorrect.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks Marcus. Yes that is our goal! I have removed the commented code.

Copy link
Contributor

@rgknox rgknox left a comment

Choose a reason for hiding this comment

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

@sharma-bharat, these changes make sense. We don't want to make the nutrient storage target a proportional to leaf flushing status.

@sharma-bharat
Copy link
Author

This is great news! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
science: bug Bugs that are specific to the implementation of a scientific model science: nutrients software: bug Bug that is specific to software
Projects
Status: Final Testing
Development

Successfully merging this pull request may close these issues.

4 participants