SIP208, SIP209 Add carbon balance check by Alomir · Pull Request #248 · PecanProject/sipnet

Alomir · 2026-02-03T16:50:30Z

Summary

What: Add carbon and nitrogen balance checks, fix discovered issues

Buckle up...

General

balance.c|h:

added infrastructure to calculate total system C and N and system inputs and outputs for each. Also calculates deltas for each time step.

sipnet.c:

now writes a line to events.out when leaf-on or leaf-off is triggered
created an updatePoolsAndBalance function for all of updateXPools subfunctions plus new balance calcs-n-checks
added output columns for balance-check deltas for C and N
splits leafCreation flux into two parts to track creation from leaf-on event separately
leaf-on creation flux now subtracted from wood pool

state.h:

added new NPP storage pool
added explicit fluxes for tracking system input and output C and N caused by events

Carbon balance

fixes Check (and fix) mass balance of woodCreation #208 by formalizing the NPP storage concept in wood C, as described in our current model doc. Note that this shuffles some terms around , most notable root respiration terms. But, note below for further changes here

Nitrogen balance

Realized that having the "NPP storage" included in woodC was guaranteed to throw off N balance; split NPP storage into it's own pool.
NOTE: currently, we are outputting the same plantWoodC values as before (i.e., nppStorage + plantWoodC), but calculations involving plantWoodC are NOT including nppStorage (wood resp and wood litter)
Nitrogen balance is not finished, as there are two known issues:
leaf on/leaf off magically adds/removes N; this is a real issue [UPDATE: this should be fine now]
plant N uptake is not implemented yet (this should be fixed when it is)

How was this change tested?

I ran smoke tests, reviewed output for general logical sense, and verified that the balance checks worked (modulo known issues with N)

Steps to reproduce and verify the change locally:

make smoke

See sipnet.out in any smoke test directory

Smoke test check results:
smoke_check.txt

Related issues

Fixes Check (and fix) mass balance of woodCreation #208
Fixes Test C balance closure #209

Reproduction steps

If appropriate, list steps to reproduce the change locally

Related issues

Fixes # (or "Relates to #" if this is not a resolution of that ticket)

Checklist

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

For model structure changes:

Removed \fraktur font formatting from docs/model-structure.md for implemented features

…ool to envi

…ecks

Copilot

Pull request overview

This pull request implements carbon and nitrogen balance checking infrastructure to address issues #208 and #209. The changes separate the NPP storage concept into its own pool (distinct from plantWoodC) to enable proper nitrogen balance tracking, add comprehensive mass balance verification, and implement event tracking for leaf-on and leaf-off phenology.

Changes:

New balance checking infrastructure (balance.c/h) that calculates and verifies total system carbon and nitrogen at each timestep
Separation of nppStorage from plantWoodC pool to enable nitrogen balance calculations
Addition of explicit event flux tracking for mass balance (eventInputC, eventOutputC, eventInputN, eventOutputN)
Enhanced event output to include computed leaf-on and leaf-off events
New test suite (testBalance.c) and smoke test improvements

Reviewed changes

Copilot reviewed 18 out of 23 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/sipnet/balance.c	New file implementing mass balance tracking and verification logic
src/sipnet/balance.h	New file defining balance tracker structure and interface
src/sipnet/state.h	Added nppStorage pool and event mass balance flux fields
src/sipnet/sipnet.c	Refactored NPP storage model, added balance checking to pool updates, enhanced output
src/sipnet/events.c	Added mass balance tracking for events, new writeComputedEventOut function
src/sipnet/events.h	Added writeComputedEventOut function declaration
src/sipnet/cli.c	Minor documentation clarification for do-single-outputs
tools/smoke_check.py	Added 'base' comparison option and updated column list for new outputs
tests/smoke/*/events.out	Updated with new leaf-on and leaf-off event outputs
tests/sipnet/test_modeling/testBalance.c	New comprehensive balance checking test
tests/sipnet/test_modeling/balance.param	Test parameter file
tests/sipnet/test_modeling/balance.clim	Test climate file
tests/sipnet/test_modeling/Makefile	Added testBalance to build
Makefile	Added balance.c to SIPNET_CFILES
CMakeLists.txt	Added balance.c and testBalance.c to builds

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

src/sipnet/balance.c

src/sipnet/events.c

dlebauer

Looks good. It is reassuring to have these checks (I think title should be C and N checks?).

I just discussed this with @Alomir online, and am noting down a few key decisions and rationale.

The N demand associated with leafOn can be met by the soil mineral N pool, constrained by the N limitation dynamics.
- If this causes the model to behave in unexpected ways (e.g. N limitation is too strong to meet demand for leafOn, and then plants are unable to grow realistically), we can revisit this assumption.
- A more physiologically realistic approach would be to have N translocated from leaves to a N storage pool prior to leafOff, then use that storage to support leafOn. We decided against that because plant biomass has fixed C:N in the current version of SIPNET.
Rename envi.nppStorage to better reflect that this is a carbon storage or non-structural carbohydrate delta (without N). We discussed using woodNSCDelta but I'm leaning toward 'storage' since it is an abstract bookkeeping term, while NSC is directly measurable. e.g. woodCStorageDelta.
Currently, plantWoodC implicitly includes the nppStorage storage delta. To explicitly track the storage delta and maintain existing behavior, any fluxes that use total wood C need to replace plantWoodC with (plantWoodC + nppStorage) where it is meant to represent total woodC e.g. calculations of flux and allocation. But only use plantWoodC in N accounting.

dlebauer · 2026-02-04T04:25:02Z

src/sipnet/state.h

  // litter (organic) nitrogen pool (g N m^-2 ground area)
  double litterN;
+
+  ///// New to SIPNET


maybe time to bump a version and say 'added to SIPNET in v2.1'

dlebauer · 2026-02-05T20:59:06Z

src/sipnet/balance.c

+    balanceTracker.deltaN = 0.0;
+  }
+
+  // TBD: warn if balance off?


Any reason not to error? Seems like this is a failure here.

Nope, error added!

src/sipnet/balance.c

Copilot · 2026-02-05T21:21:15Z

@dlebauer I've opened a new pull request, #252, to work on those changes. Once the pull request is ready, I'll request review from you.

dlebauer

Looks good to me. I fixed one typo, made one suggestion to comments for clarity (but I am not certain it is correct), and asked whether we should write out event fluxes that are 0.

@copilot please suggest updates to the documentation (model-structure.md, parameters.md) and CHANGELOG.md.

dlebauer · 2026-02-06T02:37:07Z

src/sipnet/balance.c

  }

  if (ctx.nitrogenCycle) {
+    // Note: this is the one place where we ust plantWoodC by itself; it's the


Suggested change

// Note: this is the one place where we ust plantWoodC by itself; it's the

// Note: this is the one place where we use plantWoodC by itself; it's the

dlebauer · 2026-02-06T02:45:09Z

tests/smoke/niwot/events.out

@@ -1,14 +1,14 @@
-1999  184  leafon   fluxes.leafCreation=0.00
+1999  184  leafon   fluxes.leafOnCreation=0.00


should these be 0? should they be written out if they are 0?

dlebauer · 2026-02-06T03:15:51Z

src/sipnet/state.h

+  // here that must be tracked. So, we split the wood pool into plantWoodC
+  // (above) and a new pool to track non-nitrogen-affecting changes over time.
+  // As this is a delta, it can be negative. Note that the actual "wood carbon"
+  // is the sum of these two pools.


I'm wondering is if there is a more clear way to define "non-nitrogen-affecting changes". My first thought is that this is separating out NPP + allocation as C-only processes? However, N affects both, and both of these eventually affect N (e.g. through demand and allocation).

Suggested change

// here that must be tracked. So, we split the wood pool into plantWoodC

// (above) and a new pool to track non-nitrogen-affecting changes over time.

// As this is a delta, it can be negative. Note that the actual "wood carbon"

// is the sum of these two pools.

// here that must be tracked. So, we split the wood pool into plantWoodC

// (above) and a new pool, plantWoodCStorage, to track non-nitrogen-affecting

// changes over time.

// As this is a delta, it can be negative. Note that the actual "wood carbon"

// is the sum of these two pools.

// plantWoodCStorage term was previously named nppStorage and tracked implicitly;

// tracking it explicitly allows SIPNET to handle NPP and allocation prior to

// accounting for N dynamics.

Copilot · 2026-02-06T03:28:02Z

@dlebauer I've opened a new pull request, #253, to work on those changes. Once the pull request is ready, I'll request review from you.

dlebauer · 2026-02-06T05:41:04Z

@copilot Can you provide a final review of this PR?

Copilot · 2026-02-06T05:41:13Z

@dlebauer I've opened a new pull request, #254, to work on those changes. Once the pull request is ready, I'll request review from you.

Alomir added 13 commits February 3, 2026 11:41

Add balance.c|h

9f91be1

Improve do-single-output help

7b182f7

Carbon and nitrogen balance checks

0ec16f7

Add event fluxes to track system inputs and outputs; add nppStorage p…

2bfd757

…ool to envi

Add external writeEvent function, refactor original

f6f89d8

Add event for leaf on/off; split woodC into two pools; add balance ch…

9aa91fb

…ecks

Updates for balance checks and leaf on/off events

0c454bf

Merge branch 'master' into add_carbon_balance_check

ec9b469

Add ability to compare to base

8ba00eb

Remove tolerance from balance tracker

b2e26c4

Update col list for niwot

67028b1

Avoid negative zero

13faa6d

Remove negative zeros

9211161

Alomir marked this pull request as ready for review February 3, 2026 21:18

Alomir requested review from dlebauer, infotroph, mdietze and mswilburn as code owners February 3, 2026 21:18

Alomir added 2 commits February 5, 2026 13:31

Add test for balance check

fee673b

Merge branch 'master' into add_carbon_balance_check

5b5c64a

dlebauer requested a review from Copilot February 5, 2026 19:48

Copilot started reviewing on behalf of dlebauer February 5, 2026 19:49 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

dlebauer reviewed Feb 5, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 5, 2026

Rename nppStorage to woodCStorageDelta for clarity #252

Closed

Alomir added 4 commits February 5, 2026 17:15

PR feedback

17da4b6

Turn off microbes, as it is now broken

bae820e

Test updates after balance check updates

83afb27

Split leaf-on carbon creation into own flux

1aacd4c

Alomir added 4 commits February 5, 2026 17:33

Fix leafOn flux

14f6d4a

Updates for leafOn split

2a8abae

Force skipping russell_4

b38d604

Update for new event fluxes

75a44c3

dlebauer approved these changes Feb 5, 2026

View reviewed changes

dlebauer reviewed Feb 6, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 6, 2026

Address PR #248 review feedback: document wood carbon storage pool #253

Open

8 tasks

Copilot AI mentioned this pull request Feb 6, 2026

Fix typo and incorrect variable name in balance.c comment #254

Merged

Alomir merged commit da427c9 into master Feb 6, 2026
12 checks passed

Alomir deleted the add_carbon_balance_check branch February 6, 2026 17:02

	// Note: this is the one place where we ust plantWoodC by itself; it's the
	// Note: this is the one place where we use plantWoodC by itself; it's the

		@@ -1,14 +1,14 @@
		1999 184 leafon fluxes.leafCreation=0.00
		1999 184 leafon fluxes.leafOnCreation=0.00

Conversation

Alomir commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

General

Carbon balance

Nitrogen balance

How was this change tested?

Related issues

Reproduction steps

Related issues

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlebauer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlebauer Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

dlebauer Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Alomir Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Feb 5, 2026

Uh oh!

dlebauer left a comment

Choose a reason for hiding this comment

Uh oh!

dlebauer Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

dlebauer Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

dlebauer Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

dlebauer commented Feb 6, 2026

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Alomir commented Feb 3, 2026 •

edited

Loading

dlebauer left a comment •

edited

Loading