Skip to content

SIP350 Update carbon accounting + plant mortality - #360

Merged
Alomir merged 4 commits into
SIP350-Redux-Revamp-C-N-Handlingfrom
copilot/sip350-update-carbon-accounting-plant-mortality
Aug 3, 2026
Merged

SIP350 Update carbon accounting + plant mortality#360
Alomir merged 4 commits into
SIP350-Redux-Revamp-C-N-Handlingfrom
copilot/sip350-update-carbon-accounting-plant-mortality

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • What: Revamp handling of the C pseudo-storage pool; add unit tests covering the changes
  • Motivation: Current handling is broken.

NOTE: THIS REPLACES #353

How was this change tested?

New unit tests in tests/sipnet/test_modeling/:

  • testPlantMortality.c — tests for initPlantSurvivalTracker and checkForMortality (plant stays alive, dies with/without litter pool, dies with nitrogen cycle, dead stays dead, plant re-emerges)
  • testFluxCalculations.c — tests for calcWoodAndLeafFluxes (positive/negative NPP, leaf deficit case, accounting delta, additivity) and calcRootFluxes (positive/negative NPP with N resorption, additivity)
  • testNitrogenCycle.c — added testUpdateNResorptionFlux for the new updateNResorptionFlux helper

All existing unit and smoke tests continue to pass.

Checklist

  • ] Related issues are listed above. [PRs without an approved, related issue may not get reviewed.
  • PR title has the issue number in it ("[#

Copilot AI changed the title [WIP] [#350] Update carbon accounting and plant mortality SIP350 Update carbon accounting + plant mortality Aug 3, 2026
Copilot AI requested a review from Alomir August 3, 2026 15:29

@Alomir Alomir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good

@Alomir
Alomir marked this pull request as ready for review August 3, 2026 16:10
Copilot AI review requested due to automatic review settings August 3, 2026 16:10

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

This PR adjusts plant mortality event output behavior to respect the ctx.events flag (preventing computed-event writes when event output isn’t enabled), and adds new unit tests to validate plant mortality behavior and key flux calculations (including N resorption flux handling).

Changes:

  • Guard PLANTDEATH computed event output behind ctx.events in checkForMortality().
  • Add new unit tests for plant mortality transitions and for wood/leaf/root flux calculations (including accounting delta behavior).
  • Extend nitrogen-cycle unit tests with coverage for updateNResorptionFlux() and register the new test binaries in the modeling test Makefile.

Reviewed changes

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

Show a summary per file
File Description
src/sipnet/sipnet.c Prevents computed plant-death event output when events are disabled (ctx.events == 0).
tests/sipnet/test_modeling/testPlantMortality.c New unit tests covering initPlantSurvivalTracker() and checkForMortality() scenarios (alive/dead/re-emergence, litter/no-litter, N-cycle).
tests/sipnet/test_modeling/testFluxCalculations.c New unit tests for calcWoodAndLeafFluxes() and calcRootFluxes(), including accounting-delta and negative-NPP cases.
tests/sipnet/test_modeling/testNitrogenCycle.c Adds a focused unit test for updateNResorptionFlux() behavior (basic/additive/negative cases).
tests/sipnet/test_modeling/Makefile Adds the new modeling test sources to the build/run list.
Suppressed comments (3)

tests/sipnet/test_modeling/testPlantMortality.c:243

  • These context flags aren’t fully reset after the nitrogen-cycle test (which sets ctx.anaerobic/waterHResp=1), so this test can inherit unrelated context and become order-dependent. Reset the coupled flags here to keep the suite isolated.
int testMortalityReemergence(void) {
  int status = 0;
  logTest("Running testMortalityReemergence\n");

tests/sipnet/test_modeling/testPlantMortality.c:265

  • This test only resets litterPool/nitrogenCycle, but earlier tests enable ctx.anaerobic/waterHResp. That makes the suite order-dependent and can cause surprises if mortality logic later consults those flags. Reset them here to keep tests independent.

int testMortalityWithAccountingDelta(void) {
  int status = 0;

tests/sipnet/test_modeling/testFluxCalculations.c:283

  • After the negative-NPP root test enables litterPool/anaerobic/waterHResp, this test resets only ctx.nitrogenCycle. Reset the other coupled context flags too so this test remains independent of execution order.
  status |= checkFlux(fluxes.leafOffNResorption, 0.024,
                      "leafOffNResorption (negative NPP roots)");

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

Comment thread tests/sipnet/test_modeling/testPlantMortality.c Outdated
Comment thread tests/sipnet/test_modeling/testFluxCalculations.c Outdated
@Alomir
Alomir merged commit b3da5de into SIP350-Redux-Revamp-C-N-Handling Aug 3, 2026
12 checks passed
@Alomir
Alomir deleted the copilot/sip350-update-carbon-accounting-plant-mortality branch August 3, 2026 16:47
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.

3 participants