Event checks ignore every events.in record after the first

Open
  • Is something wrong?
#429 3 comments 0 reactions 1 assignee View on GitHub

Maintainers usually reply within 2 days

@ANAMASGARD is already working on this.

Since Oct 30, 2026.

  • #431 by @ANAMASGARD — open

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
70/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c
Domain
backend, testing

Research direction

Start by locating readEventData() and validateEvent() in the C source, then inspect the reader tests and event validation cases mentioned in the issue. Reproduce the later-record validation failures and the valid day-transition sequence from the issue. Done means the listed invalid event sequences fail with the expected exits, valid sequences succeed, and later malformed, unknown-type, and out-of-order records remain rejected.

Written by the indexing model from the issue text.

Description

Description

readEventData() validates the first event repeatedly instead of validating each subsequent record. This allows:

  • Input leafon/leafoff events alongside calculated phenology, contrary to the documented incompatibility.
  • Planting and harvest on the same day, despite the existing validator’s prohibition.

Affected runs exit successfully and apply the conflicting events. Syntax, event-type, parameter, and chronological checks still inspect later records.

Reproduction

Verified on master at c3479bd681ad092e6be33fcf14272e23e0b3fcbf, Linux x86_64, GCC 16.2.1.

Run from the repository root:

make
repo="$(pwd)"
run="$(mktemp -d)"
cp tests/smoke/niwot/sipnet.param "$run/run.param"
: > "$run/sipnet.in"

for day in $(seq 1 30); do
  printf '2022 %d 0.00 1.000000 20.00 20.00 20.0000 0.0 1000.0 500.0 1000.0 2.0\n' "$day"
done > "$run/run.clim"

check_events() {
  printf '%s\n' "$@" > "$run/ev.in"
  rc=0
  (cd "$run" && "$repo/sipnet" -f run -e ev --no-do-main-output) || rc=$?
  printf 'exit: %s\n' "$rc"
}

check_events '2022 5 leafon'
check_events '2022 1 till 0.2' '2022 5 leafon'
check_events '2022 5 plant 10 3 2 5' '2022 5 harv 0.1 0 0.9 0'
Input Actual exit Expected exit
Leaf-on first 3 3
Tillage, then leaf-on 0 3
Same-day planting and harvest 0 5

Expected errors are the existing calculated-leaf incompatibility message and planting and harvest events cannot occur on the same day. Reversing planting/harvest order also bypasses validation.

User impact

Adding explicit leaf timing after a management event can silently produce unintended additional leaf growth.

With leafGrowth=50, the reproduction produces day-5 plantLeafC=1181.69 with tillage plus input leaf-on, versus 1131.69 with tillage alone. Both runs still apply calculated leaf-on of 50 on day 25.

This is erroneous carbon allocation within the plant, not evidence of total ecosystem carbon creation.

Cause and proposed fix

The reader creates next but validates newEvent, which still points to the first record:

next = createEventNode(year, day, eventType, eventParamsStr);
validateEvent(newEvent, currYear, currDay);

The complete correction requires:

  1. Pass next to validateEvent(), retaining the previous record’s currYear/currDay until validation completes.
  2. Move the existing daily planting/harvest flag reset before the leaf-event early return in validateEvent().

The second change prevents a false rejection exposed by the pointer-only fix. With calculated phenology disabled (gdd=0, soil-phenol=0, leafOnDay=leafOffDay=0), this valid sequence must succeed:

2022 1 plant 10 3 2 5
2022 2 leafon
2022 2 harv 0.1 0 0.9 0

Otherwise, the leaf event advances the reader’s date without clearing the previous day’s planting flag.

Regression tests and verification

Add reader tests covering:

  • Both leaf types in first, second, and third positions, with each calculated-phenology source enabled independently: exit 3.
  • Same-day planting/harvest in either order, including intervening tillage: exit 5.
  • Valid explicit leaf timing, planting/harvest on different dates, and the day-transition sequence above: success.
  • Continued rejection of later malformed, unknown-type, and out-of-order records.

An isolated 36-case CLI matrix passed 16/36 on the original code, 35/36 with the pointer-only correction, and 36/36 with both corrections.

make test passed in all three states: 28 unit executables and four executed smoke scenarios, plus one existing forced smoke pass.

This read-time validation defect is separate from runtime year-boundary handling in #421/#428 and event-processing redesign in #423.

Dominant language
C
Stars
42
Forks
38
Avg merge
3d 21h
Merged PRs (30d)
10

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PecanProject/sipnet

All issues in PecanProject/sipnet

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.