Skip to content

fix(hydraulics): fix SumBetweenDepths layer integration logic and add pFUnit tests - #1603

Open
johnpaulalex wants to merge 3 commits into
NGEET:mainfrom
johnpaulalex:fix/sum-btwn-depths+hydr-plant
Open

fix(hydraulics): fix SumBetweenDepths layer integration logic and add pFUnit tests#1603
johnpaulalex wants to merge 3 commits into
NGEET:mainfrom
johnpaulalex:fix/sum-btwn-depths+hydr-plant

Conversation

@johnpaulalex

@johnpaulalex johnpaulalex commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Parent PR: #1608 (Base branch: test/hydr-plant+hydr-base)

Description:

Fixes partial-layer depth integration calculations in SumBetweenDepths within biogeophys/FatesPlantHydraulicsMod.F90. Specifically:

  1. Single-layer integration fix: Handles cases where both the top depth (depth_t) and bottom depth (depth_b) fall within the exact same soil rhizosphere layer (i_rhiz_t - 1 == i_rhiz_b + 1). Previously, partial layer fractions were miscalculated or double-counted across layer boundaries.
  2. Layer 0 boundary fix: Adds explicit boundary handling for i_rhiz_b == 0 when integrating from the surface down through the top layer (frac = depth_b / dz_rhiz(1)).
  3. pFUnit Test Suite: Exposes SumBetweenDepths for unit testing and adds comprehensive test coverage (test_SumBetweenDepths in FatesPlantHydraulicsMod_geom_test.pf) verifying integration density across non-uniform soil layer interfaces, partial overlaps, and out-of-bound depth limits.

Specific notes

Collaborators:

Linked issues addressed, if any:

Expectation of Answer Changes:

  • Expected Answer Changes (ECA)
    • Details: Answers change for simulations with active plant hydraulics when depth integration intervals fall within a single soil layer or at the topmost layer boundary (i_rhiz_b == 0). Simulations without plant hydraulics remain Bit-for-Bit (B4B).

Description of generative AI usage (as necessary)

Google Antigravity was used to write the code and tests, followed by human-guided verification.

Checklist

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

Test Results:

CTSM (or) E3SM (specify which) test hash-tag: N/A
CTSM (or) E3SM (specify which) baseline hash-tag: N/A
FATES baseline hash-tag: de7afd15

Test Output:
[x] Executed pFUnit / CIME regression tests

… pFUnit test

Corrects partial-layer depth integration calculations in SumBetweenDepths when
top and bottom integration depths fall within a single soil layer or at the
topmost layer boundary (i_rhiz_b == 0). Adds test_SumBetweenDepths to
FatesPlantHydraulicsMod_geom_test.pf.
@johnpaulalex
johnpaulalex force-pushed the fix/sum-btwn-depths+hydr-plant branch from de7afd1 to 44d99c1 Compare August 12, 2026 01:45
@glemieux

Copy link
Copy Markdown
Contributor

@johnpaulalex this is a meta-question: how come the draft doesn't use the FATES PR template? I think it looks nicely formatted and we could update our standard FATES PR template. That said, it looks like some details might have been inspired by the CTSM PR which may not apply here.

@johnpaulalex

Copy link
Copy Markdown
Contributor Author

ah TIL; it's because I've been using AI to build my PR's and I didn't tell it to dynamically choose the right template for the repo. I've fixed that.

@johnpaulalex johnpaulalex changed the title fix(hydraulics): fix SumBetweenDepths layer integration logic and add pFUnit test fix(hydraulics): fix SumBetweenDepths layer integration logic and add pFUnit tests Aug 19, 2026
@johnpaulalex
johnpaulalex marked this pull request as ready for review August 19, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants