SIP182b Add litter and soil org n fluxes #199

Alomir · 2025-11-25T17:12:55Z

Second (of three) PRs for #182

This PR:

Adds CN ratio params for leaf, wood, and fine root (wood is used for coarse root)
Adds fluxes for nOrgLitter and nOrgSoil, and updates those pools with the flux results
Event handling ignores nitrogen when nitrogen-cycle is off, and warns if fertilization events have non-zero N amounts
Makes litter-pool required for nitrogen-cycle to be on
Tweaks sipnet.out format to be more readable (should parse the same way)
Updates tests

Here's the smoke_check output for russell_2 with these changes:

**********************
Running test russell_2
**********************
     soilOrgN           litterOrgN         
          old       new        old      new
0       135.0  134.9993       14.0  13.9994
1       135.0  134.9985       14.0  13.9987
2       135.0  134.9976       14.0  13.9980
3       135.0  134.9967       14.0  13.9973
4       135.0  134.9958       14.0  13.9966
...       ...       ...        ...      ...
5843    135.0   73.5237       44.0  10.6355
5844    135.0   73.5223       44.0  10.6343
5845    135.0   73.5209       44.0  10.6332
5846    135.0   73.5196       44.0  10.6320
5847    135.0   73.5182       44.0  10.6309

[5848 rows x 4 columns]
Difference Summary:
           first diff total diffs   old mean    new mean
soilOrgN            0        5848      135.0  103.039779
litterOrgN          0        5848  40.060192   18.561273

That is, a slow decline in organic N for both soil and litter. Please comment below if that does not make sense.

…nd-soil-org-N-fluxes

Copilot

Pull request overview

This PR adds organic nitrogen pool dynamics to SIPNET as part of implementing a nitrogen cycle model. It introduces CN ratio parameters for plant biomass pools, calculates nitrogen fluxes for litter and soil organic matter, and ensures proper handling of nitrogen when the nitrogen-cycle feature is disabled.

Key changes:

Adds three CN ratio parameters (leafCNRatio, woodCNRatio, rootCNRatio) for converting carbon fluxes to nitrogen fluxes
Implements calcOrgNFluxes() to calculate nitrogen dynamics for litter and soil organic matter pools
Updates nitrogen pools in a new updateNitrogenPools() function separated from carbon pool updates

Reviewed changes

Copilot reviewed 12 out of 17 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`tests/smoke/russell_1/sipnet.param`	Adds CN ratio parameters to test configuration
`tests/smoke/russell_2/sipnet.param`	Adds CN ratio parameters to test configuration
`tests/smoke/russell_2/sipnet.config`	Updates timestamp in auto-generated config
`tests/sipnet/test_modeling/testNitrogenCycle.c`	Refactors test structure and adds organic N pool testing
`tests/sipnet/test_events_types/testEventFertilization.c`	Updates tests to handle nitrogen-cycle toggle correctly
`src/sipnet/state.h`	Adds CN ratio parameters and organic N flux variables, fixes typo
`src/sipnet/sipnet.c`	Implements organic N flux calculations and pool updates, reformats output
`src/sipnet/events.c`	Adds warning and handling for nitrogen when cycle is off
`src/common/context.c`	Validates that litter-pool is required for nitrogen-cycle
`docs/user-guide/model-inputs.md`	Adds reference to parameters documentation
`docs/model-structure.md`	Updates formatting and clarifies nitrogen dynamics equations
`docs/CHANGELOG.md`	Documents new nitrogen cycle features

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

src/sipnet/events.c

src/sipnet/sipnet.c

docs/model-structure.md

Copilot · 2025-12-17T00:24:21Z

src/sipnet/sipnet.c

+  // The litter org N flux is determined by the carbon fluxes from wood and leaf
+  // litter, and N loss due to mineralization. N added via fertilization
+  // is handled elsewhere.
+  litterCN = envi.litter / envi.litterOrgN;


Potential division by zero: If envi.litterOrgN is zero or very close to zero, the calculation litterCN = envi.litter / envi.litterOrgN on line 1263 will result in division by zero or numerical instability. Similarly for envi.soilOrgN on line 1272. Consider adding validation to ensure these values are positive before performing the division.

for consistent naming, would it make more sense to name these litter variables envi.litterC and envi.litterN?

Yes it would!

Copilot · 2025-12-17T00:24:21Z

src/sipnet/sipnet.c

+  litterCN = envi.litter / envi.litterOrgN;
+  fluxes.nOrgLitter = fluxes.leafLitter / params.leafCNRatio +
+                      fluxes.woodLitter / params.woodCNRatio -
+                      fluxes.rLitter / litterCN;
+
+  // soil
+  // The soil org N flux is determined by the carbon flux from the litter pool,
+  // carbon fluxes from roots, and N loss due to mineralization
+  // (Note: woodCNRatio is used for coarse roots)
+  soilCN = envi.soil / envi.soilOrgN;
+  fluxes.nOrgSoil = (fluxes.litterToSoil - fluxes.rSoil) / soilCN +
+                    fluxes.fineRootLoss / params.rootCNRatio +
+                    fluxes.coarseRootLoss / params.woodCNRatio;


Potential division by zero: If envi.soilOrgN is zero or very close to zero, the calculation soilCN = envi.soil / envi.soilOrgN will result in division by zero or numerical instability. Consider adding validation to ensure this value is positive before performing the division.

Suggested change

litterCN = envi.litter / envi.litterOrgN;

fluxes.nOrgLitter = fluxes.leafLitter / params.leafCNRatio +

fluxes.woodLitter / params.woodCNRatio -

fluxes.rLitter / litterCN;

// soil

// The soil org N flux is determined by the carbon flux from the litter pool,

// carbon fluxes from roots, and N loss due to mineralization

// (Note: woodCNRatio is used for coarse roots)

soilCN = envi.soil / envi.soilOrgN;

fluxes.nOrgSoil = (fluxes.litterToSoil - fluxes.rSoil) / soilCN +

fluxes.fineRootLoss / params.rootCNRatio +

fluxes.coarseRootLoss / params.woodCNRatio;

if (fabs(envi.litterOrgN) >= TINY) {

litterCN = envi.litter / envi.litterOrgN;

fluxes.nOrgLitter = fluxes.leafLitter / params.leafCNRatio +

fluxes.woodLitter / params.woodCNRatio -

fluxes.rLitter / litterCN;

} else {

// Avoid division by zero or near-zero litterOrgN; omit mineralization term

fluxes.nOrgLitter = fluxes.leafLitter / params.leafCNRatio +

fluxes.woodLitter / params.woodCNRatio;

}

// soil

// The soil org N flux is determined by the carbon flux from the litter pool,

// carbon fluxes from roots, and N loss due to mineralization

// (Note: woodCNRatio is used for coarse roots)

if (fabs(envi.soilOrgN) >= TINY) {

soilCN = envi.soil / envi.soilOrgN;

fluxes.nOrgSoil = (fluxes.litterToSoil - fluxes.rSoil) / soilCN +

fluxes.fineRootLoss / params.rootCNRatio +

fluxes.coarseRootLoss / params.woodCNRatio;

} else {

// Avoid division by zero or near-zero soilOrgN; omit mineralization term

fluxes.nOrgSoil = fluxes.fineRootLoss / params.rootCNRatio +

fluxes.coarseRootLoss / params.woodCNRatio;

}

Copilot · 2025-12-17T00:24:21Z

src/common/context.c

  }

+  if (ctx.nitrogenCycle && !ctx.litterPool) {
+    logError("nitrogen-cycle require litter-pool to be turned on\n");


Grammar error: The error message should be "nitrogen-cycle requires litter-pool to be turned on" (with 's' on 'requires').

Suggested change

logError("nitrogen-cycle require litter-pool to be turned on\n");

logError("nitrogen-cycle requires litter-pool to be turned on\n");

Copilot · 2025-12-17T00:24:22Z

src/sipnet/events.c

+          fluxes.eventOrgN += 0;
+          fluxes.eventMinN += 0;


Redundant code: Lines 457-458 explicitly add 0 to the fluxes, which has no effect. These lines can be removed as the fluxes would remain unchanged (either already zero or not modified). This else block can either be empty or contain just a comment explaining the behavior.

Suggested change

fluxes.eventOrgN += 0;

fluxes.eventMinN += 0;

I was also confused - is this useful?

Nope, not at all. I wonder if I had something different there at first? A comment here really is better.

Copilot · 2025-12-17T00:24:22Z

src/sipnet/sipnet.c

+  // carbon fluxes from roots, and N loss due to mineralization
+  // (Note: woodCNRatio is used for coarse roots)
+  soilCN = envi.soil / envi.soilOrgN;
+  fluxes.nOrgSoil = (fluxes.litterToSoil - fluxes.rSoil) / soilCN +


The calculation appears to have an error in the nitrogen flux accounting. The term (fluxes.litterToSoil - fluxes.rSoil) / soilCN incorrectly divides the litter-to-soil carbon flux by the soil C:N ratio. The nitrogen flux from litter to soil should use the litter C:N ratio (litterCN), while only the soil mineralization term (fluxes.rSoil) should use the soil C:N ratio. The expression should be: fluxes.litterToSoil / litterCN - fluxes.rSoil / soilCN.

Suggested change

fluxes.nOrgSoil = (fluxes.litterToSoil - fluxes.rSoil) / soilCN +

fluxes.nOrgSoil = fluxes.litterToSoil / litterCN - fluxes.rSoil / soilCN +

I understand why I wrote it that way, but I think Copilot is correct here. The N transferred to soil is dependent on litterCN, not soilCN.

dlebauer

Looks great! A few minor suggestions, and one critical check / change: please make sure documentation and implementation of root C and N transfer to soil docs are consistent.

dlebauer · 2026-01-03T04:08:17Z

src/sipnet/events.c

+          fluxes.eventOrgN += 0;
+          fluxes.eventMinN += 0;


I was also confused - is this useful?

dlebauer · 2026-01-03T04:12:17Z

src/sipnet/sipnet.c

+  // The litter org N flux is determined by the carbon fluxes from wood and leaf
+  // litter, and N loss due to mineralization. N added via fertilization
+  // is handled elsewhere.
+  litterCN = envi.litter / envi.litterOrgN;


for consistent naming, would it make more sense to name these litter variables envi.litterC and envi.litterN?

dlebauer · 2026-01-03T04:13:42Z

src/sipnet/sipnet.c

+  litterCN = envi.litter / envi.litterOrgN;
+  fluxes.nOrgLitter = fluxes.leafLitter / params.leafCNRatio +
+                      fluxes.woodLitter / params.woodCNRatio -
+                      fluxes.rLitter / litterCN;


also for naming consistency, I don't think it is necessary to include Ratio in the names of these params. But if desired, consider adding it to litterCN

Oh, right, good call

docs/model-structure.md

-$$\small i \in \{\text{leaf, wood, fine root, coarse root}\}$$
+$$\small i \in \{\text{leaf, wood}\}$$

 The flux of nitrogen from living biomass to the litter pool is proportional to the carbon content of the biomass, based on the C:N ratio of the biomass pool \eqref{eq:cn_stoich}. Similarly, nitrogen from organic matter amendments is calculated from the carbon content and the C:N ratio of the inputs.


docs/model-structure.md

 <!-- 
 _existing equation + harvest transfer and organic matter inputs
 -->
 $$\small i \in \{\text{leaf, wood, fine root, coarse root}\}$$


dlebauer

Update - I've tried to clean up the documentation of C and N routing in #216

Please review / revise / merge and make sure that implementation and docs are consistent.

* Clarify C routing from plant to litter and soil Separate Aboveground biomass --> litter Belowground biomass --> soil For both C and N. * Apply suggestions from code review * Refine text in model-structure documentation Removed unnecessary comments and fixed minor wording issues for clarity. * Update terminology and clarify carbon/nitrogen fluxes Corrected terminology and clarified definitions related to carbon and nitrogen fluxes in the model structure documentation.

dlebauer

Looks great!

When implementing plant N it will make sense to use the same convention to separate C and N components.

Alomir added 16 commits November 20, 2025 12:56

Add org N handling for fert

2380796

Adds fields for org N handling

640bb2e

Adds organic N pools and params, with output

5ac872d

Updates for fert org N

37663f3

Updates cols list for niwot

7137e15

Updates for initial org N implementation

f8fb01a

Updates to eqs 13 and 14

4c96eb7

Adds litter pool req for nitrogen cycle

896f99d

Initial implementation for organic N pools plus fert updates

6f77a31

Updates to fert for org N pools

cc2ef42

Updates for org N pools

789f143

Merge branch 'master' into SIP182a-org-N-pools-and-fert

d58dbad

Merge branch 'SIP182a-org-N-pools-and-fert' into SIP182b-add-litter-a…

ddfa75e

…nd-soil-org-N-fluxes

Reorg and updates for org N pools

73fa60c

Adds CN ration params

a6a08ef

Tweak output spacing for readability

48a547f

Base automatically changed from SIP182a-org-N-pools-and-fert to master December 3, 2025 20:36

Alomir added 8 commits December 3, 2025 12:44

Merge branch 'master' into SIP182b-add-litter-and-soil-org-N-fluxes

0fd073d

Fix broken link

07dd270

Fix merge errors

309ea45

Allow russell ranch link

2bf2ea1

Brings russell_1 params up to date

71b240b

Merge branch 'master' into SIP182b-add-litter-and-soil-org-N-fluxes

fb91e8d

Updates for organic N calcs

d540a76

Removes 'frak' font from some implemented sections

d2fae26

Alomir marked this pull request as ready for review December 10, 2025 17:28

Alomir requested review from dlebauer, infotroph, mdietze and mswilburn as code owners December 10, 2025 17:28

Alomir added 2 commits December 10, 2025 12:32

Updates for nitrogen cycle additions to date

068021e

Updates CN text

0d09b86

dlebauer requested a review from Copilot December 17, 2025 00:17

Copilot started reviewing on behalf of dlebauer December 17, 2025 00:18 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

dlebauer requested changes Jan 5, 2026

View reviewed changes

Alomir and others added 4 commits January 6, 2026 10:56

PR feedback; mostly renames and doc updates

097e221

Updates after soilCN->litterCN change

819af58

Merge branch 'master' into SIP182b-add-litter-and-soil-org-N-fluxes

f683cf6

Alomir requested a review from dlebauer January 6, 2026 16:20

dlebauer approved these changes Jan 6, 2026

View reviewed changes

dlebauer merged commit b6a56eb into master Jan 6, 2026
12 checks passed

dlebauer deleted the SIP182b-add-litter-and-soil-org-N-fluxes branch January 6, 2026 20:17

	logError("nitrogen-cycle require litter-pool to be turned on\n");
	logError("nitrogen-cycle requires litter-pool to be turned on\n");

	fluxes.nOrgSoil = (fluxes.litterToSoil - fluxes.rSoil) / soilCN +
	fluxes.nOrgSoil = fluxes.litterToSoil / litterCN - fluxes.rSoil / soilCN +

SIP182b Add litter and soil org n fluxes #199

SIP182b Add litter and soil org n fluxes #199

Uh oh!

Conversation

Alomir commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alomir Jan 6, 2026 • 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!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

dlebauer left a comment

Choose a reason for hiding this comment

Uh oh!

dlebauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Alomir commented Nov 25, 2025 •

edited

Loading

Alomir Jan 6, 2026 •

edited

Loading

dlebauer left a comment •

edited

Loading