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

Fix water balance error for grass when use FATES-Hydro #910

Merged
merged 6 commits into from
Mar 8, 2023

Conversation

JunyanDing
Copy link
Contributor

Description:

Changed the FatesPlantHydraulicsMod.F90, when the PFT is grass, pool the storage of leaf and stem

Collaborators:

Expectation of Answer Changes:

Checklist:

  • My change requires a change to the documentation.
  • I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
  • [ x] I have read the CONTRIBUTING document.
  • FATES PASS/FAIL regression tests were run
  • If answers were expected to change, evaluation was performed and provided

Test Results:

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

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

FATES baseline hash-tag:

Test Output:

else
v_leaf_donate(1:n_hypool_leaf) = ccohort_hydr%v_ag(1:n_hypool_leaf) * l2sap_vol_donate_frac
ccohort_hydr%v_ag(1:n_hypool_leaf) = ccohort_hydr%v_ag(1:n_hypool_leaf) - v_leaf_donate(1:n_hypool_leaf)
ccohort_hydr%v_ag(n_hypool_leaf+1:n_hypool_ag) = (v_sapwood + sum(v_leaf_donate(1:n_hypool_leaf))) / n_hypool_stem
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, this makes sense

@@ -29,7 +29,7 @@ module FatesHydraulicsMemMod
integer, parameter, public :: n_hypool_stem = 1
integer, parameter, public :: n_hypool_troot = 1 ! CANNOT BE CHANGED
integer, parameter, public :: n_hypool_aroot = 1 ! THIS IS "PER-SOIL-LAYER"
integer, parameter, public :: nshell = 5
integer, parameter, public :: nshell = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this change too, sensitivity analysis I've done have shown that we get small/subtle differences when we use multiple versus a single shell.

Copy link
Contributor

Choose a reason for hiding this comment

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

checked in via email with @xuchongang and @bchristo and they both blessed this change as well

@glemieux glemieux self-assigned this Jan 30, 2023
@glemieux
Copy link
Contributor

glemieux commented Mar 7, 2023

Regression testing on cheyenne underway.

@glemieux
Copy link
Contributor

glemieux commented Mar 8, 2023

Regression testing on cheyenne returns B4B results for all non-hydro tests as expected. Both hydro tests DIFFs look reasonable given that the initial differences are in rootuptake output variables. Issue #701 appears to have been corrected here, at least temporarily, in that the exact restart hydro test is passing now COMPARE_BASE_REST now.

Folder location: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr910

One note: for some reason FATES_LAI was not included in the previous baseline, fates-sci.1.64.0_api.25.1.0-ctsm5.1.dev118, for some of the testmods resulting in FIELDLIST diffs. I'm currently looking into why that is and will regenerate

@glemieux
Copy link
Contributor

glemieux commented Mar 8, 2023

... Issue #701 appears to have been corrected here, at least temporarily, in that the exact restart hydro test is passing now COMPARE_BASE_REST now.

This looks like its a tempermental issue and not necessarily fixed by this PR. In re-running the previous baseline, this issue wasn't present suggesting that there is something else going on to cause #701.

@glemieux
Copy link
Contributor

glemieux commented Mar 8, 2023

One note: for some reason FATES_LAI was not included in the previous baseline, fates-sci.1.64.0_api.25.1.0-ctsm5.1.dev118, for some of the testmods resulting in FIELDLIST diffs. I'm currently looking into why that is and will regenerate

This baseline has been regenerated and tested against the old version (which I've saved with the suffix -nolai). Everything was B4B except that the regenerated baseline has FATES_LAI reinstated. Still not sure why that happened.

@glemieux glemieux merged commit 8661255 into NGEET:main Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants