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

calculate sapwood cross-sectional area for grass PFT #1268

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

XiulinGao
Copy link
Contributor

@XiulinGao XiulinGao commented Oct 23, 2024

Description:

A subroutine, sapwood_area_grass, is added to calculate sapwood cross-sectional area specifically for grass PFT.
This is added to fix issue #1254

Collaborators:

Expectation of Answer Changes:

Answers will change is FATES Hydro is turned on and using sapwood allometry mod 2

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:

FATES baseline hash-tag:

Test Output:

Copy link
Contributor

@glemieux glemieux 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 fix @XiulinGao. I'm going to make some minor formatting adjustments, but otherwise I think this looks good.

@glemieux
Copy link
Contributor

glemieux commented Nov 13, 2024

Reminder to self:

@glemieux
Copy link
Contributor

I conducted some initial regression testing of this by manually modifying the FatesColdHydro test mod to remove the override of fates_allom_smode in the current default parameter file. Both hydro tests pass now using the default param file. Difference are isolated to the relevant hydro related variables and are round off.

Copy link
Contributor

@ZacharyRobbins ZacharyRobbins left a comment

Choose a reason for hiding this comment

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

This all seems to be good to me, apologies for the delay.

@glemieux
Copy link
Contributor

Final regression testing underway. Note that I'm using ESCOMP/CTSM#2882 to test as it captures commits that revert the temporary workaround to #1254 which this PR fixes.

@glemieux
Copy link
Contributor

glemieux commented Nov 18, 2024

Regression testing is complete. The FatesColdHydro tests are the only differences with FATES_ROOTUPTAKE variables and FATES_SAPFLOW_SZPF showing the only differences on the first timestep. I confirmed that the hydro tests used the default parameter file as intended. All other expected tests pass b4b.

@glemieux glemieux merged commit 08f2410 into NGEET:main Nov 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready to Integrate
3 participants