Sipnet v2 support by infotroph · Pull Request #3813 · PecanProject/pecan

infotroph · 2026-02-14T01:15:23Z

Description

Pushing some long-stashed changes so we can iterate on them. ~~NOT YET TESTED.~~

Implemented so far:

write clim files in the 12-column vs format by default
switch on version when choosing a template for sipnet.in and sipnet.param

Before merging, need to at least:

add CH4 and N2O to model2netcdf
update and extend unit tests

And probably also:

check for simplifications made possible by the param file being name/value pairs instead of multicolumn
update write_restart/read_restart
Implement changing command-line switches from settings (esp. turning off GDD)

Motivation and Context

Review Time Estimate

Immediately
Within one week
When possible

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation.
My name is in the list of CITATION.cff
I agree that PEcAn Project may distribute my contribution under any or all of
- the same license as the existing code,
- and/or the BSD 3-clause license.
I have updated the CHANGELOG.md.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

…e parameters for SIPNET v2

…RESP, WATER_HRESP, LEAF_WATER)

…ater, add N/CH4 NetCDF outputs

…ers from v2 template

…g litterWater handling

divine7022 · 2026-03-04T04:52:09Z

picking up from Chris's initial v2 scaffolding , the main gaps were that the v2 templates were incomplete and model2netcdf couldn't parse v2 output. Here's what this round adds:

template.param_v2 -- stripped the 22 params that no longer exist in v2 (microbeInit, qualityLeaf, cn_ prefixed ones, etc.) and added the 16 N-cycle/anaerobic/CH4 params that initializeOneModelParam() now expects. All 73 params verified against sipnet.c

sipnet.in_v2 -- added the six runtime flags (NITROGEN_CYCLE, LITTER_POOL, ANAEROBIC, GROWTH_RESP, WATER_HRESP, LEAF_WATER) that replaced the old compile time #ifdefs in v2

write.config.SIPNET -- flag injection block so users can toggle N-cycle/litter/anaerobic from the XML settings. Also added guards so v1 only params (litterWHC, litWaterDrainRate, litterWFracInit, microbeInit, m_ballBerry) don't get written into v2 configs where they would be unknown

model2netcdf.SIPNET -- v2 dropped the "Notes:" line before the header row, so the old skip=1 broke on v2 output. Now auto detects the format. Also handles missing litterWater column (LITTER_WATER flag removed in v2) and converts the 8 new N/CH4 output columns to NetCDF (mineral_N, soil_organic_N, litter_N, N2O, N_leaching, N_fixation, N_uptake, CH4)

added tests for v1 and v2 format coverage, including N2O/CH4 extraction and the missing litterWater edge case.

v1 configs still work exactly as before when <revision> is unset or 1.0; and set 2.0 for v2 support

dlebauer

Thanks for getting this started @infotroph and for bringing this home @divine7022

I left a few questions & comments, none critical; most are thoughts to consider in future iterations.

dlebauer · 2026-03-05T03:43:36Z

 write.config.SIPNET <- function(defaults, trait.values, settings, run.id, inputs = NULL, IC = NULL,
                                restart = NULL, spinup = NULL) {
+
+  sipnet_version <- package_version(settings$model$revision, strict = FALSE)


Isn't there an output file that already provides version information? Seems like that would be a more reliable source of version information - at least for >= v2

good idea in principle, sipnet v2 now has a version.h that has version "2.0.0" (and optionally a git hash) into the binary);
but major limitation would be write.config runs pre execution so there's no output to inspect yet, but that querying the binary's version string could be a future enhancement. Worth noting that sipnet v2 binary does support --version output, so this could be added as a validation step

There are no outputs at this stage -- we have a binary path but haven't tried calling it yet, and conceptually Sipnet might not even exist on the machine writing the configs. Seems simpler to me to have write.configs do what the settings file says rather than try to infer it from any other source.

haven’t pushed my local changes for this yet, I am commenting here since the context flows -- 2d7559c

heads up:
https://github.com/PecanProject/pecan/actions/runs/22737237619/job/65941705561?pr=3813

fixed version parsing to handle non standard DB revisions like "136", "unk", "git", now only recognizes major versions 1 and 2 as valid sipnet versions; everything else defaults to v1

f99032d

dlebauer · 2026-03-05T03:53:28Z

+    lp <- as.integer(settings$model$litter_pool %||% 1)
+    an <- as.integer(settings$model$anaerobic %||% 1)
+    # NITROGEN_CYCLE requires both LITTER_POOL and ANAEROBIC
+    if (nc == 1 && !(lp == 1 && an == 1)) {


This is enforced by sipnet. In one hand, catching it early makes sense. On the other, it simplifies the pecan code if we delegate consistency checks to sipnet

agreed on the tradeoff.; keeping it for now because it gives clearer error messages before job submission, but noting it's duplicated from sipnet's validateContext()

👍 To delegating to Sipnet here.

And while we're looking at this section:

How do we feel about the current named options (<model><nitrogen_cycle>0</nitrogen_cycle>...) vs the approach PEcAn.ed2 takes of passing a literal string of command flags (<model><binary_args>--no-nitrogen-cycle</binary_args>...)? I think I prefer the named options, but seemed worth asking.

If using named options, would it make sense to accept them as a block and pass them all without needing to enumerate? something like

<model> <options> <NITROGEN_CYCLE>1</NITROGEN_CYCLE> <GDD>0</GDD> <NEW_FLAG_SIPNET_HAS_ADDED_SINCE_LAST_REVISION_OF_WRITE.CONFIGS_CODE>1</NEW_FLAG_...> ... </options> ...

...which could then be processed as flag_lines <- paste(names(settings$model$options), "=", settings$model$options, collapse = "\n")?

If this looks good I'll implement it.

this is a good design decision, since new SIPNET flags don't require code changes to write.config;
and for me it seems like could be deferred to follow up PR but shouldn't be mixed into the current one (it changes the XML schema and requires documentation)
make sense ?

deferred to follow up PR

Fine with me

changes the XML schema

No more than this PR does already -- we're already going from not expecting any options in the XML to expecting them directly inside model. But it's probably worth a review of how other models do this so that we can keep some uniformity.

infotroph · 2026-03-05T16:11:54Z

 write.config.SIPNET <- function(defaults, trait.values, settings, run.id, inputs = NULL, IC = NULL,
                                restart = NULL, spinup = NULL) {
+
+  sipnet_version <- package_version(settings$model$revision, strict = FALSE)


There are no outputs at this stage -- we have a binary path but haven't tried calling it yet, and conceptually Sipnet might not even exist on the machine writing the configs. Seems simpler to me to have write.configs do what the settings file says rather than try to infer it from any other source.

infotroph · 2026-03-05T16:22:10Z

+    lp <- as.integer(settings$model$litter_pool %||% 1)
+    an <- as.integer(settings$model$anaerobic %||% 1)
+    # NITROGEN_CYCLE requires both LITTER_POOL and ANAEROBIC
+    if (nc == 1 && !(lp == 1 && an == 1)) {


👍 To delegating to Sipnet here.

infotroph · 2026-03-05T16:44:56Z

+    lp <- as.integer(settings$model$litter_pool %||% 1)
+    an <- as.integer(settings$model$anaerobic %||% 1)
+    # NITROGEN_CYCLE requires both LITTER_POOL and ANAEROBIC
+    if (nc == 1 && !(lp == 1 && an == 1)) {


And while we're looking at this section:

How do we feel about the current named options (<model><nitrogen_cycle>0</nitrogen_cycle>...) vs the approach PEcAn.ed2 takes of passing a literal string of command flags (<model><binary_args>--no-nitrogen-cycle</binary_args>...)? I think I prefer the named options, but seemed worth asking.

If using named options, would it make sense to accept them as a block and pass them all without needing to enumerate? something like

<model> <options> <NITROGEN_CYCLE>1</NITROGEN_CYCLE> <GDD>0</GDD> <NEW_FLAG_SIPNET_HAS_ADDED_SINCE_LAST_REVISION_OF_WRITE.CONFIGS_CODE>1</NEW_FLAG_...> ... </options> ...

...which could then be processed as flag_lines <- paste(names(settings$model$options), "=", settings$model$options, collapse = "\n")?

If this looks good I'll implement it.

Co-authored-by: David LeBauer <dlebauer@arizona.edu>

…nsistently

infotroph · 2026-03-07T06:35:29Z

+  # determine which SIPNET major version we're configuring for.
+  # settings$model$revision may be a SIPNET version ("1", "2", "2.0")
+  # or a internal model identifier ("136", "unk", "git", NULL).
+  # Only "1" and "2" (and their dotted forms) are valid SIPNET versions;
+  # everything else defaults to v1 for backward compatibility
+  rev_str <- "v1"
+  if (!is.null(rev_raw) && nzchar(rev_raw)) {
+    if (grepl("^[0-9]+$", rev_raw)) {
+      rev_raw <- paste0(rev_raw, ".0")
+    }


This feels like a lot of complication to me, especially since it lets us accept a bare integer but not "v<integer>"; IMO the latter is quite a bit more widely used.

I wonder if we should instead require a numeric version for anything but an enumerated set of legacy versions (maybe just 136 and git, dropping unk?)

I think the right approach would be to simplify to accept "v1", "v2", "1", "2" and default everything else to v1

may be

rev_raw <- settings$model$revision %||% "1" # accept "v1", "v2", "1", "2", "1.0", "2.0"; legacy DB identifiers (136, git) default to v1 rev_str <- if (grepl("^v?2(\\.0)?$", rev_raw, ignore.case = TRUE)) "v2" else "v1"

make sense ?

right approach would be to simplify to accept "v1", "v2", "1", "2"

If you're saying accept those but reject x.y.z numeric versions, I disagree. settings$model$revision is declaring the version of Sipnet that will actually run when you call settings$model$binary, and needs to be interpretable by any function that needs it and not just run.write.configs.

Say hypothetically we do a couple releases with restart support and then decide we want to change its API without breaking support for the previous ones; we'd want to be able to add conditions like if (settings$model$revision >= "2.0.3") {... into write.restart without breaking the write.configs v1/v2 logic.

enumerated known legacy identifiers ("136", "git") and map them to v1 explicitly;
sipnet_version variable is also available in function scope for any future >= comparisons (e.g. >= "2.0.3" check for api changes)

3f36714

…t conversion

divine7022 · 2026-03-09T06:43:34Z

https://github.com/PecanProject/pecan/actions/runs/22837731305/job/66245148269?pr=3813

is this an upstream issue with how the sipnet v1.3.0 binary interacts with the current gha runner ?

infotroph · 2026-03-09T23:41:27Z

@divine7022 Took me a while to track this down (had to run the Github action locally), but: Read unexpected input item: GROWTH_RESP, because the GHA Bety returns revision "102319", which compares >= "2.0", so PEcAn.SIPNET uses the v2 parameter template.

infotroph added 5 commits February 13, 2026 16:06

WIP: first steps toward supporting Sipnet v2.0

715a2db

met2model.SIPNET: do not write grid index by default

029fd61

remove soilwetness too

abe5e0c

NEWS

e92cbe9

changelog

48daa06

github-actions Bot added the models label Feb 14, 2026

infotroph requested a review from divine7022 February 14, 2026 03:06

divine7022 reviewed Feb 17, 2026

View reviewed changes

Comment thread models/sipnet/inst/template.param_v2

Comment thread models/sipnet/R/met2model.SIPNET.R Outdated

divine7022 added 9 commits March 3, 2026 21:11

Merge remote-tracking branch 'origin/develop' into sipnet-v2

266462c

remove 22 obsolete v1 parameters and add 16 nitrogen/anaerobic/methan…

a278722

…e parameters for SIPNET v2

add v2 runtime flags (NITROGEN_CYCLE, LITTER_POOL, ANAEROBIC, GROWTH_…

f1161ca

…RESP, WATER_HRESP, LEAF_WATER)

fix v2 output parsing (auto detect Notes line), guard missing litterW…

4311c27

…ater, add N/CH4 NetCDF outputs

add v2 runtime flag injection from settings and guard v1 only paramet…

815e5b0

…ers from v2 template

add tests for v1 and v2 output formats including N2O, CH4, and missin…

5c7e22e

…g litterWater handling

update NEWS.md

552386f

update comment

5b6cdbf

update CHANGELOG.md

340a27f

github-actions Bot added the tests label Mar 4, 2026

divine7022 requested a review from dlebauer March 4, 2026 04:53

dlebauer approved these changes Mar 5, 2026

View reviewed changes

infotroph commented Mar 5, 2026

View reviewed changes

infotroph and others added 7 commits March 5, 2026 09:00

Apply suggestions from code review

74cc3ec

Co-authored-by: David LeBauer <dlebauer@arizona.edu>

replaced boolean param with version based, and use seconds_per_day co…

c1cc1c6

…nsistently

normalize package version number

2d7559c

update NEWS.md

ed0379c

Merge branch 'sipnet-v2' of github.com:infotroph/pecan into sipnet-v2

e3365fa

update met2model.SIPNET.Rd

1e853aa

update read_restart.SIPNET.Rd

74438a2

infotroph commented Mar 5, 2026

View reviewed changes

Comment thread models/sipnet/NEWS.md Outdated

divine7022 added 3 commits March 5, 2026 15:51

drop unit conversion statement from news

cb027c0

update .Rd

e5bf9d6

fix version parsing for NULL and non standard DB revisions

f99032d

infotroph changed the title ~~WIP: Sipnet v2 support~~ Sipnet v2 support Mar 7, 2026

infotroph commented Mar 7, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

infotroph commented Mar 7, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

infotroph commented Mar 7, 2026

View reviewed changes

infotroph added 6 commits March 6, 2026 22:42

Merge branch 'develop' into sipnet-v2

0422871

remove sipnet.unk support, fix Transp time units, more consistent uni…

014d5c0

…t conversion

typos

b5c173f

accept only v1 and v2 for clim format versions

f1e51aa

comment fixes

8ec5186

met2model.SIPNET version flag test

9c48e4e

infotroph commented Mar 8, 2026

View reviewed changes

Comment thread models/sipnet/NEWS.md

changelog tweaks

5bd8d28

infotroph mentioned this pull request Mar 8, 2026

Sipnet: Separate model-version logic from input-availability logic #3854

Open

infotroph and others added 3 commits March 8, 2026 00:47

roxygen

ca0b67a

correct parameter name

7f6983b

correct version logic

3f36714

infotroph commented Mar 10, 2026

View reviewed changes

Comment thread models/sipnet/R/write.configs.SIPNET.R Outdated

add all sipnet v1 revisions listed in bety-0

95ef212

dlebauer added this pull request to the merge queue Mar 10, 2026

Merged via the queue into PecanProject:develop with commit b15916b Mar 10, 2026
19 of 25 checks passed

infotroph deleted the sipnet-v2 branch March 28, 2026 08:01

Conversation

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

Uh oh!

Description

Motivation and Context

Review Time Estimate

Types of changes

Checklist:

Uh oh!

Uh oh!

Uh oh!

divine7022 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlebauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

infotroph Mar 8, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

divine7022 commented Mar 9, 2026

Uh oh!

infotroph commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

infotroph commented Feb 14, 2026 •

edited

Loading

divine7022 commented Mar 4, 2026 •

edited

Loading

infotroph Mar 8, 2026 •

edited

Loading

infotroph commented Mar 9, 2026 •

edited

Loading