Skip to content

Change events.out to pool amounts instead of fluxes#349

Open
Alomir wants to merge 4 commits into
masterfrom
SIP348-Pools-instead-of-fluxes-in-events.out
Open

Change events.out to pool amounts instead of fluxes#349
Alomir wants to merge 4 commits into
masterfrom
SIP348-Pools-instead-of-fluxes-in-events.out

Conversation

@Alomir

@Alomir Alomir commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • What: Change values in events.out to pool amounts instead of flux values
  • Motivation: Reduce confusion when reading events.out

How was this change tested?

Other than values in events.out, there should be no functional changes in this PR.

  • All existing unit tests pass (after update for testEventOutputFile canned output)
  • smoke tests pass for sipnet.out

Updated events.out for relevant smoke tests are included. Since there were no changes in sipnet.out for the smoke tests, there is no output capture for smoke-check.py.

Related issues

Checklist

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

Note: See CONTRIBUTING.md for additional guidance. This repository uses automated formatting checks; if the pre-commit hook blocks your commit, run git clang-format to format staged changes.

@Alomir Alomir marked this pull request as ready for review July 2, 2026 21:15
Copilot AI review requested due to automatic review settings July 2, 2026 21:15

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

Updates SIPNET’s events.out output to report per-event pool deltas (amounts applied over the timestep) instead of per-day flux rates, to reduce confusion when interpreting event diagnostics.

Changes:

  • Switch event writing in processEvents() (and leaf-on computed output) from fluxes.* rate-style values to timestep deltas and remove the fluxes. prefix from parameter names.
  • Update unit-test canned outputs and smoke-test events.out baselines to match the new event output semantics.
  • Refresh user-facing documentation and changelog to describe the new events.out format.

Reviewed changes

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

Show a summary per file
File Description
tests/smoke/russell_3/events.out Updates smoke baseline to new events.out pool-delta values and parameter names.
tests/smoke/russell_2/events.out Updates smoke baseline to new events.out pool-delta values and parameter names (includes fert events).
tests/smoke/russell_1/events.out Updates smoke baseline to new events.out pool-delta values and parameter names.
tests/sipnet/test_events_infrastructure/events_output_no_header.out Updates unit-test expected output for no-header event output format/values.
tests/sipnet/test_events_infrastructure/events_output_header.out Updates unit-test expected output for header-included event output format/values.
src/sipnet/sipnet.c Writes leaf-on computed event values as timestep deltas and drops fluxes. prefix for those fields.
src/sipnet/events.c Writes scheduled/instant event outputs as timestep deltas with event* parameter names (while still accumulating per-day event fluxes internally).
docs/user-guide/model-outputs.md Updates the events.out example and description to reflect pool-delta outputs and new parameter names.
docs/CHANGELOG.md Notes the user-visible change in events.out semantics.

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

Comment thread src/sipnet/sipnet.c
Comment on lines 1251 to +1258
void writeLeafOnEventIfNeeded(void) {
const char *type = eventTypeToString(LEAFON);
const double len = climate->length;
if (fluxes.leafOnCreation > TINY && ctx.events) {
writeComputedEventOut(climate->year, climate->day, type, 2,
"fluxes.leafOnCreation", fluxes.leafOnCreation,
"fluxes.leafOnCreationFromWood",
fluxes.leafOnCreationFromWood);
"leafOnCreation", fluxes.leafOnCreation * len,
"leafOnCreationFromWood",
fluxes.leafOnCreationFromWood * len);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

When you're right, you're right. Will fix.

Comment thread docs/CHANGELOG.md
### Changed

- Renamed the CLI option `--file-name` to `--file-prefix` for clarity while keeping `--file-name` as a backward-compatible alias (#320)
- Values in `events.out` changed to pool deltas rather than flux amounts (#349)

@dlebauer dlebauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, though holding off on approval until the copilot feedback is addressed

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.

Change flux values to pool amounts in events.out

3 participants