SIP273 Add waterDrainFrac to allow water > WHC

[Alomir]

Summary

• What: Adds a control that allows soil water to go above WHC, enabling modeling of flooding
• Motivation: Necessary for project

How was this change tested?

Added unit tests in tests/sipnet/test_modeling/testSoilMoisture.c covering:

• getClippedWaterFrac: boundary cases (within range, at WHC, above WHC, at zero, below zero)
• calcSoilWaterFluxes drainage with waterDrainFrac = 1.0, 0.5, 0.0, and no drainage when water ≤ WHC
• moisture(): transpiration is capped at soilWHC (not soilWater) when soil is flooded

make smoke
python tools/smoke_check.py run verbose

smoke_check.py results:
sc.txt
Minor differences in ET due to the added water frac cap

Related issues

• Fixes Add waterDrainFrac to allow flooding. #273

Checklist

• Related issues are listed above. PRs without an approved, related issue may not get reviewed.
• PR title has the issue number in it ("[#] <concise description of proposed change>")
• Tests added/updated for new features (if applicable)
• Documentation updated (if applicable)
• docs/CHANGELOG.md updated with noteworthy changes
• Code formatted with clang-format (run git clang-format if needed)

[Alomir]

@copilot please add a test that covers these changes. The test should be named testSoilMoisture.c in the directory tests/sipnet/test_modeling, and it should be written in the style of other tests in that directory.

[Copilot]

@copilot please add a test that covers these changes. The test should be named testSoilMoisture.c in the directory tests/sipnet/test_modeling, and it should be written in the style of other tests ...

Added testSoilMoisture.c in tests/sipnet/test_modeling/ (commit 552adf5). The test covers:

• testGetClippedWaterFrac: boundary cases (within range, at WHC, above WHC, at zero, below zero)
• testDrainageWithWaterDrainFrac: drainage behavior with waterDrainFrac = 1.0, 0.5, 0.0, and no drainage when water ≤ WHC
• testMoistureFloodedSoil: verifies transpiration uses soilWHC (not soilWater) as the cap when soil is flooded

[Alomir]

@copilot please remove all changes in the tests/sipnet/test_restart_infrastucture directory that were added in 552adf5

[Copilot]

@copilot please remove all changes in the tests/sipnet/test_restart_infrastucture directory that were added in [552adf5](552adf539ba4781617caaa1...

Removed those 4 files in commit 94699a4.

[Copilot]

Pull request overview

Adds a new moisture/drainage control (waterDrainFrac) so SIPNET can retain soil water above WHC (flooding/saturation) rather than instantaneously draining all excess, with corresponding model, test, and documentation updates.

Changes:

• Introduces waterDrainFrac as a new model parameter and applies it to excess-water drainage (per time step), while allowing initial soil water to exceed WHC.
• Caps transpiration-available water at WHC (so pooled/flood water doesn’t increase transpiration) and clips soil-water fraction inputs to moisture response terms.
• Updates parameter docs/changelog and extends unit + smoke test fixtures to include the new parameter.

Reviewed changes

Copilot reviewed 20 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/smoke/russell_4/sipnet.param	Adds waterDrainFrac to smoke-test parameter set.
tests/smoke/russell_3/sipnet.param	Adds waterDrainFrac to smoke-test parameter set.
tests/smoke/russell_2/sipnet.param	Adds waterDrainFrac to smoke-test parameter set.
tests/smoke/russell_1/sipnet.param	Adds waterDrainFrac to smoke-test parameter set.
tests/smoke/niwot/sipnet.param	Adds waterDrainFrac to Niwot smoke-test parameter set.
tests/sipnet/test_sipnet_infrastructure/with_est.param	Adds waterDrainFrac to infra test parameter file.
tests/sipnet/test_sipnet_infrastructure/with_est.exp	Updates expected parameter dump to include waterDrainFrac.
tests/sipnet/test_sipnet_infrastructure/testParamInput.c	Prints waterDrainFrac in parameter output for infra tests.
tests/sipnet/test_sipnet_infrastructure/standard.param	Adds waterDrainFrac to standard infra parameter file.
tests/sipnet/test_sipnet_infrastructure/standard.exp	Updates expected parameter dump to include waterDrainFrac.
tests/sipnet/test_sipnet_infrastructure/spatial_val.param	Adds waterDrainFrac to spatial validation parameter file.
tests/sipnet/test_restart_infrastructure/restart.param	Adds waterDrainFrac to restart infra parameter file.
tests/sipnet/test_modeling/testSoilMoisture.c	Adds unit tests for clipped water fraction, drainage behavior vs waterDrainFrac, and transpiration cap under flooding.
tests/sipnet/test_modeling/Makefile	Registers the new soil moisture test for the modeling test suite.
tests/sipnet/test_modeling/balance.param	Adds waterDrainFrac to balance test parameter file.
src/sipnet/state.h	Adds waterDrainFrac to Parameters struct and clarifies flooded initialization semantics.
src/sipnet/sipnet.c	Reads waterDrainFrac, applies it to drainage, clips water fraction in evaporation terms, caps transpiration-available water at WHC, and removes initial WHC clamp.
docs/parameters.md	Documents waterDrainFrac in moisture-related parameter table.
docs/model-structure.md	Updates drainage narrative to define f_drain per time step and discuss variable time step implications.
docs/CHANGELOG.md	Adds changelog entry for the new parameter.

Comments suppressed due to low confidence (1)

docs/model-structure.md:752

• The drainage equation omits the time-step scaling needed to match the implementation and the surrounding text. In code, drainage is a flux (cm/day) computed as (W_soil - W_WHC) / Δt * f_drain, where Δt = climate->length (days). As written, the equation yields units of cm (per step), not cm/day. Please update the equation (or explicitly redefine flux units here) so it’s consistent with Eq. (A4) and the model’s variable time step support.

Under well-drained conditions, drainage occurs when soil water content $(W_{\text{soil}})$ exceeds the soil water
holding capacity $(W_{\text{WHC}})$. Beyond this point, additional water drains off at a rate controlled by the
drainage parameter $f_{\text{drain}}$, defined as the fraction of excess soil water that can be removed per time step.
For well drained soils, set $f_{\text{drain}} = 1$ to ensure full drainage with each step.

Setting $f_{\text{drain}} < 1$ reduces the rate of drainage. Flooding can be simulated by using a combination of low 
$f_{\text{drain}}$ and sufficient $F^W_\text{irrig|precip,soil}$ to maintain flooded conditions. Note, though, that 
if the model time steps are not constant, the actual per-day drainage rate will differ in the different steps.

\begin{equation}
F^W_{\text{drainage}} = f_\text{drain} \cdot \max(W_{\text{soil}} - W_{\text{WHC}}, 0)
\label{eq:drainage}
\end{equation}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dlebauer]

LGTM, looking forward to some flooded 🌾 & 🍚!

[Copilot]

Pull request overview

Copilot reviewed 30 out of 33 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/sipnet/restart.c:95

• A new serialized restart key (flags.flooding) is being added, but RESTART_SCHEMA_VERSION is still "1.0". With the current parser, older checkpoint files will fail with “missing required key flags.flooding” while still claiming schema 1.0; please bump the restart schema version and update/extend restart compatibility tests accordingly (the file header comment and _Static_assert message both indicate this is required when the contract changes).

// NUM_CONTEXT_MODEL_FLAGS is defined in context.h, as that is the authoritative
// source
typedef struct RestartContextModelFlags {
  // Context flags that alter model runtime behavior
  int events;
  int gdd;
  int growthResp;
  int leafWater;
  int litterPool;
  int snow;
  int soilPhenol;
  int waterHResp;
  int nitrogenCycle;
  int anaerobic;
  int flooding;
} RestartContextModelFlags;
static RestartContextModelFlags modelFlags;

_Static_assert(sizeof(RestartContextModelFlags) == NUM_CONTEXT_MODEL_FLAGS * 4,
               "Restart schema 1.0 drift: Model flags changed; bump restart "
               "schema version and update schema_layout.* checks");

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dlebauer]

Documentation suggestions added

[dlebauer]

do we need to bump schema version? should the need to bump schema version after adding a term be caught by a test?

[Alomir]

That's a really good question; TBH, I've been ignoring schema version for now, thinking of this as still "in flux". I think once we release a new SIPNET version, we should probably start bumping the schema version.

[dlebauer]

Suggested change

	Flooding can be simulated by using a combination of low $f_{\text{drain}}$ and sufficient
	The parameter $f_{\text{drain}}$ is a per-day rate ($\text{day}^{-1}$) and is internally converted to a per-timestep flux using the model timestep length $\Delta t$.
	Flooding can be simulated by using a combination of low $f_{\text{drain}}$ and sufficient

[Alomir]

All fluxes are per-day - which is converted to a per-time-step amount when the pools are updated. As f_drain is already a per-day rate, there's no need to apply the climate length here.

[dlebauer]

Should this also be scaled to per-tilmestep?

Suggested change

	*drainage = excessWater * params.waterDrainFrac;
	*drainage = excessWater * params.waterDrainFrac / climate->length;

[Alomir]

Nope, waterDrainFrac is already a per-day rate.

[dlebauer]

I made suggestions assuming that the goal is to use a daily time step and convert it internally; trying to keep code and docs consistent with this and with each other, and clear. I also unresolved a few suggestions that I think shouldn't have been resolved, but perhaps were through the many code changes.

[dlebauer]

Note: added the alternate flooding scheme ideas to .alternate-model-structure.md #313