Skip to content

SIP370 Add guard to check for leach+vol -> negative min N - #375

Merged
dlebauer merged 5 commits into
masterfrom
SIP370-Guard-Against-Negative-Min-N
Aug 19, 2026
Merged

SIP370 Add guard to check for leach+vol -> negative min N#375
dlebauer merged 5 commits into
masterfrom
SIP370-Guard-Against-Negative-Min-N

Conversation

@Alomir

@Alomir Alomir commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • What: Guard against leaching + volatilization causing negative mineral N
  • Motivation: This is an edge case, but still needs to be addressed

Details

  • Added guard in limitations.c
  • Guard function called right before general N limitation check
  • Unit test points added

How was this change tested?

  • Test points added for each case of excessive volatilization and leaching
  • All other unit tests pass unmodified
  • Smoke tests pass unmodified

Related issues

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 Alomir changed the title Interim commit SIP370 Add guard to check for leach+vol -> negative min N Aug 19, 2026
@Alomir
Alomir marked this pull request as ready for review August 19, 2026 18:04
@Alomir
Alomir requested a review from dlebauer as a code owner August 19, 2026 18:04
Copilot AI lite review requested due to automatic review settings August 19, 2026 18:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a defensive guard in SIPNET’s nitrogen-cycle limitation flow to prevent combined volatilization + leaching from over-drawing the mineral N pool (and cascading into negative growth-reduction factors), with regression tests to cover the edge case.

Changes:

  • Add a mineral-N loss guard that scales volatilization/leaching when their combined timestep loss would exceed available mineral N.
  • Add unit test coverage for “excess volatilization” and “excess leaching” scenarios.
  • Add a release-note entry for the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/sipnet/limitations.c Introduces and wires in a guard to bound volatilization + leaching against available mineral N before running the general N limitation logic.
tests/sipnet/test_modeling/testNitrogenCycle.c Adds regression tests validating the guard’s scaling behavior and end-of-step mineral N behavior.
docs/CHANGELOG.md Notes the new guard in the Unreleased changelog.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/sipnet/limitations.c
Comment thread tests/sipnet/test_modeling/testNitrogenCycle.c Outdated
Comment thread docs/CHANGELOG.md

@dlebauer dlebauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great, fixes #370.

@dlebauer
dlebauer merged commit bc96ae1 into master Aug 19, 2026
12 checks passed
@dlebauer
dlebauer deleted the SIP370-Guard-Against-Negative-Min-N branch August 19, 2026 19:39
Copilot AI mentioned this pull request Sep 3, 2026
6 tasks
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.

Guard nitrogen limitation against negative available mineral N

3 participants