Avoid negative saturated-zone storage by limiting baseflow to available water - #325
Open
LINAMARIAOSORIO wants to merge 9 commits into
Open
Avoid negative saturated-zone storage by limiting baseflow to available water#325LINAMARIAOSORIO wants to merge 9 commits into
LINAMARIAOSORIO wants to merge 9 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix_320 #325 +/- ##
========================================
Coverage 92.47% 92.47%
========================================
Files 60 60
Lines 3906 3908 +2
Branches 487 487
========================================
+ Hits 3612 3614 +2
Misses 238 238
Partials 56 56 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
Soil.get_baseflowcomputed the recession baseflow without checking the water available in the saturated zone, so the balanceTU_S = TU_S(t-1) + REC - BFcould become negative. The baseflow is nowmin(BF_calc, TU_S(t-1) + REC), whereBF_calcis the recession equation (zero below the threshold).Follow-up commits on top of the fix:
tests/unit/hydrological_processes/test_module_soil.py: the limit binds and the balance ends at zero (fails onmain); the limit does not bind; below the threshold the baseflow stays zero although water is available.ruff formatof the changed module; theget_baseflowdocstring restored and extended with the limit.doc/source/overview.rst, baseflow section: the recession equation keeps its form under the labelbaseflow-recessionand the limitBF = min(BF_calc, TU_{S,T-1} + REC)is the newbaseflowequation, with the sameTU_{S,T-1}notation as the storage equationtus.fix_320and a changelog entry under Unreleased › Fixed.Stacked on #324 (base branch
fix_320): the diff of this pull request isrubem/hydrological_processes/_soil.py, the tests, the documentation and the changelog line.Related Issue
Motivation and context
Mass conservation: a negative saturated-zone storage means the baseflow delivered water that was not in the aquifer. Two points for the scientific review: the limited baseflow becomes
BF_{T-1}of the next step, so the recession sequence changes after a limited step; and the regression dataset never triggers the limit (theexactjob reports byte-identical outputs with and without it), so the behaviour is covered by the unit tests.How has this been tested
pytest --ignore=tests/integration/doc): 1001 passed, 1 skipped.test_baseflowCalc_capped_by_available_waterfails with_soil.pyfrommain(returns3.995instead of2.5) and passes here.docsjob (sphinx-build -W) was run locally withacton this branch: green. Golden fixtures are not affected: byte-exact reproduction verified in the golden environment container with this branch.ci-successgreen.Screenshots