Sync SIPNET user/model docs with current implementation - #380
Conversation
dlebauer
left a comment
There was a problem hiding this comment.
@copilot you have done a nice job. I have left some comments, questions, and requests for changes. Please submit all requests for changes as a separate PR, and respond to comments/questions inline. Accept any of my code suggestions that you agree with. if you don't agree, make a counter-suggestion or comment.
| Leaf on and leaf off events define the timing of leaf emergence and senescence, respectively. These events do not carry | ||
| their own magnitude parameters; instead, the transferred carbon and nitrogen are computed from model state and the |
There was a problem hiding this comment.
revising for clarity
| Leaf on and leaf off events define the timing of leaf emergence and senescence, respectively. These events do not carry | |
| their own magnitude parameters; instead, the transferred carbon and nitrogen are computed from model state and the | |
| Leaf on and leaf off events define the timing of leaf emergence and senescence, respectively. These events only specify the date, not the amount, of leaf biomass C and N added or subtracted; instead, the transferred carbon and nitrogen are computed from model state and the |
| the leaf carbon pool on the leaf off date. | ||
| Leaf on and leaf off events define the timing of leaf emergence and senescence, respectively. These events do not carry | ||
| their own magnitude parameters; instead, the transferred carbon and nitrogen are computed from model state and the | ||
| parameters `leafGrowth`, `leafOnReallocFrac`, `fracLeafFall`, and `leafNResorptionFrac`. |
There was a problem hiding this comment.
@copilot Please use mathematical symbols consistently in this section. Before replacing the source code names here, add leafOnReallocFrac and leafNResorptionFrac, with symbols that follow conventions in that file, to docs/parameters.md - they are currently absent. Then use those symbols throughout lines 1214–1235, not only in this paragraph.
Please inform me if there are other places where source code names are used in this file and should be replaced.
There was a problem hiding this comment.
Updated in d4566ee. I added leafOnReallocFrac and leafNResorptionFrac to /home/runner/work/sipnet/sipnet/docs/parameters.md and rewrote the leaf-on/leaf-off section to use symbols throughout. The other remaining source-facing names I noticed in this file are the literal event codes (leafon, leafoff, plantdeath) and option names (FLOODING, ANAEROBIC), where the text is referring to file or CLI values rather than equation terms.
There was a problem hiding this comment.
| - Nitrogen (when enabled): leaf-on reallocation changes plant N storage by $\Delta N = C_{\text{leafon}}(1/CN_{\text{leaf}} - 1/CN_{\text{wood}})$. Positive $\Delta N$ is drawn from storage and can limit leaf-on; negative $\Delta N$ is returned to storage. | |
| The carbon constraint, and the nitrogen constraint when applicable, scale the transfer down. The scaled amount is applied to the pools and is not subsequently reduced by the general nitrogen limitation (Sec. [Nitrogen Limitation](#nitrogen-limitation)). |
There was a problem hiding this comment.
| from the leaf carbon pool to the litter pool (or soil pool, if the litter pool is not being used). When `nitrogen-cycle` is enabled, the nitrogen corresponding to the transferred leaf carbon follows it, except for the fraction resorbed to the plant nitrogen storage pool. |
|
@copilot are you still planning to respond to my comments in this PR? |
it fails too often for reasons unrelated to a PR. It is sufficient to be notified of broken links.
Co-authored-by: Alomir <1099935+Alomir@users.noreply.github.com>
Co-authored-by: dlebauer <464871+dlebauer@users.noreply.github.com>
d4566ee to
752c115
Compare
Co-authored-by: David LeBauer <dlebauer@gmail.com>
Co-authored-by: David LeBauer <dlebauer@gmail.com>
Co-authored-by: David LeBauer <dlebauer@gmail.com>
Co-authored-by: David LeBauer <dlebauer@gmail.com>
Co-authored-by: David LeBauer <dlebauer@gmail.com>
Co-authored-by: David LeBauer <dlebauer@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
A few updated doc statements/labels remain inconsistent with the implementation (e.g., config syntax, missing INPUT_FILE in the key table, SOIL_TEMP vs SOIL_PHENOL, and the rSoil symbol).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates SIPNET’s user and model documentation to match the current implementation in src/, focusing on configuration/CLI behavior, input formats (params/climate/events), and the emitted outputs (including computed/internal events).
Changes:
- Documented current runtime flags/keys (including
flooding) and clarified how config values are normalized/merged and reported via--dump-config. - Updated climate, parameter, and events input format docs to reflect current parsers and event handling (including
leafon/leafoff). - Updated output field documentation and examples to match the current
sipnet.outheader andevents.outsemantics (including internally computed events).
File summaries
| File | Description |
|---|---|
| docs/user-guide/running-sipnet.md | Updates config/CLI docs, config key tables, and troubleshooting guidance. |
| docs/user-guide/model-outputs.md | Aligns sipnet.out/events.out field descriptions and examples with current outputs. |
| docs/user-guide/model-inputs.md | Updates parameter/climate/events input format docs and runtime option notes. |
| docs/user-guide/getting-started.md | Refreshes smoke-test walkthrough to include merged config output. |
| docs/parameters.md | Adds/clarifies parameters related to leaf-on reallocation and N resorption. |
| docs/model-structure.md | Updates model-structure narrative/equations for leaf events, plant death, and N handling. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| Note that it is possible for $\overline{\text{NPP}}$ to be negative, which can occur when autotrophic respiration | ||
| exceeds GPP. In this case, the plant pools will shrink as carbon is lost to respiration and litter. Also, as this is not | ||
| an instantaneous calculation of NPP, it is possible that a negative allocation may by applied to a pool even if the |
|
|
||
| - the growing season start and end dates as specified by the parameters `leafOnDay` and `leafOffDay`, or | ||
| - growing degree days when `GDD = 1`, or | ||
| - soil temperature when `SOIL_TEMP = 1`. |
|
|
||
| Both fine and coarse root carbon change in the same way as leaf carbon. Change in carbon for these pools is determined | ||
| as follows, applied separately to fine and coarse roots: | ||
| Change in carbon for the fine root and coarse root pools is determined as follows:: |
| | 17 | $GPP$ | gpp | Gross primary production | g C m$^{-2}$ | | ||
| | 18 | | rAboveground | Aboveground autotrophic respiration (leaves + wood) | g C m$^{-2}$ | | ||
| | 19 | $R_H$ | rSoil | Heterotrophic respiration | g C m$^{-2}$ | | ||
| | 19 | $R_H$ | rSoil | Total belowground respiration (rRoot + rh). | g C m$^{-2}$ | |
| ### Configuration File Format | ||
|
|
||
| Each line in the configuration file contains a key-value pair separated by whitespace: | ||
| Each line in the configuration file uses `KEY = VALUE` syntax; `!` starts a comment line: |
| | Key | Value Type | Description | | ||
| |--------------------|------------|-------------------------------------------------------------------------------------------------------------------| | ||
| | `INPUT_FILE` | string | Name of configuration file to read | | ||
| | `FILE_PREFIX` | string | Prefix for climate and parameter input files | | ||
| | `PARAM_FILE` | string | Path to model parameters file (optional; defaults to `<FILE_PREFIX>.param`) | | ||
| | `CLIM_FILE` | string | Path to climate file (optional; defaults to `<FILE_PREFIX>.clim`) | | ||
| | `OUT_FILE` | string | Path for main output file (optional; defaults to `<FILE_PREFIX>.out`) | | ||
| | `OUT_CONFIG_FILE` | string | Path for config dump file (optional; defaults to `<FILE_PREFIX>.config`) | | ||
| | `EVENTS_PREFIX` | string | Prefix used to derive events input and output filenames | |
|
|
||
| ! Output | ||
| DO_MAIN_OUTPUT = 1 | ||
| DO_SINGLE_OUTPUTS = 0 |
The documentation had drifted from the current SIPNET behavior, especially around runtime configuration, input formats, event handling, and emitted outputs. This updates the affected user and model docs to reflect the implementation in
src/.Runtime configuration and CLI
flooding=, or:FILE_PREFIXversus directly configuredDO_SINGLE_OUTPUTInput file formats
sipnet.paramdocs to match the current parser:name valueper non-comment line, with extra columns tolerated for backward compatibilityOutputs and event records
plantStorageNandnppStoragesoilWetnessFracbehavior under floodingevents.outmay include internally computed events such asleafon,leafoff, andplantdeath, not only user-specified agronomic eventsModel structure
Example of the corrected configuration syntax and key naming: