Update neg growth handling to be an actual limitation check

[Alomir]

Summary

• What: Bring negative growth handling in line with SIPNET methodology
• Motivation: Better model design

Details

• The wood/leaf and root/root checks for inappropriate allocation of negative growth have been moved to limitations.c, in a proper limitation check
• nResorptionFlux handling moved into nitrogen.c, which is more appropriate than sipnet.c (better modularity)
  • Note this also removes the signed weirdness required in the old updatedNResorptionFlux function
• Unit tests updated as appropriate for the new code locations/flow

How was this change tested?

This should be a zero-functional-change PR, with one minor exception - leaf-off events no longer report resorbed N (but this can be calculated, so no real loss). Some unit tests needed to be updated (and were), but the smoke tests pass without modification.

Related issues

• Fixes #N/A

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) (not applicable)
• docs/CHANGELOG.md updated with noteworthy changes (None)
• Code formatted with clang-format (run git clang-format if needed)

[Copilot]

Pull request overview

This PR refactors negative-growth handling by moving “can’t drive pools negative” allocation adjustments into the limitations layer, and consolidates reduction-driven nitrogen resorption accounting into the nitrogen module. This fits the SIPNET execution model by separating flux calculation, limitation checks, and pool updates more cleanly.

Changes:

• Moved negative-growth allocation correction (leaf↔wood and fine↔coarse root) out of sipnet.c into limitations.c via a new checkCarbonLimitations() step.
• Replaced incremental updateNResorptionFlux() updates with a centralized calcReductionResorptionFlux() calculation in nitrogen.c.
• Updated unit tests to reflect the new call order/locations for carbon limitation checks and reduction resorption.

Reviewed changes

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

Show a summary per file

File	Description
tests/sipnet/test_modeling/testNitrogenCycle.c	Removes the unit test for the deleted updateNResorptionFlux() helper.
tests/sipnet/test_modeling/testFluxCalculations.c	Adjusts flux tests to call checkCarbonLimitations() and calcReductionResorptionFlux() explicitly and updates expected resorption values.
src/sipnet/sipnet.c	Removes in-function negative-creation adjustments and adds a centralized checkCarbonLimitations() call before nitrogen flux calculations.
src/sipnet/nitrogen.h	Removes the old resorption update API and exposes calcReductionResorptionFlux() (used by tests).
src/sipnet/nitrogen.c	Implements centralized reduction-driven resorption and changes nitrogen flux wrapper behavior.
src/sipnet/limitations.h	Adds checkCarbonLimitations() to the limitations interface.
src/sipnet/limitations.c	Implements carbon limitation logic previously embedded in wood/leaf and root flux calculators.

Suppressed comments (1)

src/sipnet/nitrogen.h:64

• The calcNitrogenFluxes() header comment says it computes all nitrogen fluxes "except N resorption", but calcNitrogenFluxes() now calls calcReductionResorptionFlux(). The comment should be updated so it matches the current behavior.

/*!
 * Calculate all nitrogen fluxes (except N resorption)
 *
 * This is the general flux calculation wrapper for sipnet.c
 */

---

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

[Alomir]

I agree that this is a weak check, but:

• if mean NPP < 0, ALL creation terms are <= 0; likewise, if mean NPP >=0, all creation terms are >= 0
• this only fails if the wood allocation fraction parameter is zero, which I think is nonsense?

However...

• better commenting would be good, and
• given that they are all non-positive-or-not, checking the sum is stronger

Will update.

[Alomir]

Done

[Alomir]

Good catch! In the long run, I think it's better to consider the leaf-off litter too, but let's stick with "no functional change" for this PR. I'll make a ticket to consider leaf-off - and events - in this limitation check.

[Alomir]

Ticket #372 has been created

[Alomir]

Note - a discovered TODO: N resorption from leaf-off is still in sipnet.c, and should be moved to nitrogen.c with the rest of it.

Done.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/sipnet/nitrogen.c:193

• This introduces an additional model-state change beyond the event-output change described in the PR. fluxes.leafLitter includes calculated phenology leaf-off litter regardless of ctx.events, whereas the previous resorption update for that litter was inside if (leafOff > TINY && ctx.events). Consequently, nitrogen-enabled runs with events disabled now resorb phenology leaf N when they previously did not. Either preserve the old guard/flux split for this zero-functional-change PR, or explicitly document and regression-test this behavior change.

  // Leaf litter resorption; at this point, fluxes.leafLitter counts both normal
  // turnover and leaf-off calcs. Note that event leaf off is handled in
  // events.c
  double nResorp =
      params.leafNResorptionFrac * fluxes.leafLitter / params.leafCN;
  fluxes.leafOffNResorption += nResorp;

src/sipnet/limitations.c:161

• The relocated below-ground limitation has no test exercising either redistribution branch. The existing negative-root test leaves both root pools solvent and does not call checkCarbonLimitations(), so a regression in this core moved logic would pass. Add cases where only the fine-root pool and only the coarse-root pool would go negative, and verify that creation is transferred to the other root pool.

  if ((fineRootDeficit < 0.0) != (coarseRootDeficit < 0.0)) {
    // If neither are negative, nothing to do
    // If both are negative, the plant will die in checkForMortality()
    if (fineRootDeficit < 0.0) {
      fluxes.coarseRootCreation += fineRootDeficit;
      fluxes.fineRootCreation -= fineRootDeficit;
    }
    if (coarseRootDeficit < 0.0) {
      fluxes.fineRootCreation += coarseRootDeficit;
      fluxes.coarseRootCreation -= coarseRootDeficit;

[dlebauer]

LGTM
Please update changelog

[Alomir]

linter check appears to be hung; close and reopen to restart.