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

prevent FATES-Hydro from failing upon restart with a drought deciduous PFT #1156

Merged
merged 4 commits into from
Mar 19, 2024

Conversation

jennykowalcz
Copy link

@jennykowalcz jennykowalcz commented Feb 2, 2024

Description:

As discussed in issue 1151 , FATES-Hydro was failing upon restart for simulations including a drought-deciduous PFT, with error messages saying is recruitment = F, LAI = 0, stem_water = 0, w_tot_* = NaN, and LWP = NaN. This PR implements a fix suggested by @mpaiao to prevent the failure by preventing the leaf elongation factor from being zero when the Fates-Hydro code is calculating water storage volume.

Collaborators:

@mpaiao @ckoven

Expectation of Answer Changes:

I tested this PR using a small region in the central Amazon with a control case from the main branch and a test case from this branch, for sets of simulations with a single evergreen PFT and a single drought-deciduous PFT. The control cases comprised of 20 year runs (no restart) while the test cases reached 20 years with restarts every 2 years. Changes are near-zero for the evergreen PFT for the quantities I looked at. For the drought-deciduous PFT quantities change by up to a couple percent.
Hydro restart test.pdf

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:

@jennykowalcz jennykowalcz linked an issue Feb 2, 2024 that may be closed by this pull request
@rgknox
Copy link
Contributor

rgknox commented Feb 2, 2024

My understanding is that this change will ensure that all plants have at least some nominal volume to the leaf compartment at all times. That sounds good, it looks like this was not being applied in all scenarios.

The only thing that is throwing me off a little bit, is that we are saying that minimum allowed value is 0.1 of the target amount of leaf at the actual leaf elongation, which is also protected with a minimum value of 0.1. So this minimum has 0.1*0.1=0.01 of the target biomass (volume). I don't know if that is too low or too high, but I think a larger value may be more stable and will have little impact on results.

Essentially, this compartment volume "v_ag" dictates the capacity for holding water. And we are accepting that when leaves are dropped, we are maintaining kind of a ghost volume that stores water there even when the leaves are gone. We do this for numerical stability. The conductance should still be low or zero and more representative of any losses through the petiole connection points (if we even represent that). In light of this, I think we might do better by making this relationship a little simpler, and maybe allow a larger ghost volume? Maybe have the minimum volume be keyed off the leaf target at full extension and then apply min_leaf_frac to that?

  call bleaf(ccohort%dbh,ccohort%pft,ccohort%crowndamage, &
         max(ccohort%canopy_trim,min_trim),ccohort%efleaf_coh, leaf_c_target)
         max(ccohort%canopy_trim,min_trim),1.0, leaf_c_target)

    ccohort_hydr%v_ag(1:n_hypool_leaf) = max(leaf_c,min_leaf_frac*leaf_c_target) * &
         prt_params%c2b(ft) / denleaf/ real(n_hypool_leaf,r8)

@mpaiao
Copy link
Contributor

mpaiao commented Feb 2, 2024

Good point, @rgknox I think your suggestion would be a good implementation. This is likely a bug I introduced when I made efleaf_coh an argument in bleaf, before that change the code probably used to implicitly assume fully flushed leaves for this calculation.

@jennykowalcz
Copy link
Author

jennykowalcz commented Feb 2, 2024

Thanks @rgknox and @mpaiao , this makes sense and sounds like a good idea. I gave it a quick test and the differences vs control are the same as what I posted earlier.

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 submitting this fix @jennykowalcz. I checked it and it looks good with me.

@rgknox rgknox self-requested a review February 27, 2024 18:52
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.

looks good to me too

@glemieux
Copy link
Contributor

glemieux commented Mar 19, 2024

Regression testing on derecho results against fates-sci.1.72.4_api.34.0.0-ctsm5.1.dev174 baseline shows b4b results for all expected tests. Note that the current hydro regression tests are failing due to a fix waiting to come in via #1164.

@glemieux
Copy link
Contributor

In an effort to test hydro b4b-ness of this, I tested this against fates-sci.1.72.0_api.33.0.0-ctsm5.1.dev170 using ERS_D_Ld5.1x1_brazil.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesColdHydro. While the the exact restart issue recorded in #701 is still true, it is consistent and the comparison against the baseline was b4b.

This is ready to integrate.

@glemieux glemieux merged commit b054cd0 into NGEET:main Mar 19, 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
None yet
Development

Successfully merging this pull request may close these issues.

FATES-Hydro restarts fail with drought-deciduous PFT
4 participants