Skip to content

fix(expander): default output window starts at t_start, not min(0, t_start) (gh#143)#446

Open
vsbuffalo wants to merge 1 commit into
mainfrom
worktree-fix-output-window
Open

fix(expander): default output window starts at t_start, not min(0, t_start) (gh#143)#446
vsbuffalo wants to merge 1 commit into
mainfrom
worktree-fix-output-window

Conversation

@vsbuffalo

Copy link
Copy Markdown
Owner

What

The default trajectory output window started at min(0, t_start) instead
of t_start. For any model with t_start > 0 — an anchored from later
than origin, or an unanchored from > 0 — and **no explicit output
block, this forced the window start to 0, so the trajectory was emitted
(and recorded) over [0, t_end] instead of the requested [from, to].

Why it was wrong

min(0, t_start) was the gh#143 fix for the negative-t_start case (a
from before origin): starting at 0 leaves no snapshots in [t_start, 0),
so --obs-only / snap_at can't find a pre-origin snapshot and hard-exits.
But min(0, t_start) equals t_start for t_start < 0 and clamps to 0 for
t_start > 0 — it fixed the negative direction and silently broke the
positive one. start = t_start is correct in both directions (and
trivially at t_start = 0), subsuming the gh#143 negative-t_start fix
rather than replacing it.

Reproduction

Minimal model (origin = 2020-01-01, from = 2020-07-01,
to = 2020-07-12t_start = 182, t_end = 193), no output block:

  • before: camdl simulate m.camdl --dates --stdout emits 194 data rows
    with non-monotonic time (182, then 1, 2, …, 193).
  • after: 12 data rows over [182, 193], strictly increasing time.

Tests

TDD, red → green. New output_schedule unit test for the anchored
t_start > 0 case:

# RED (current code)
[FAIL] anchored t_start>0 → output.start = t_start
  Expected: 182
  Received: 0

# GREEN (after fix)
Test Successful. 12 tests run.

The former A.2 obs/output divergence guard is repurposed as an
agreement guard: both surfaces now lower start = t_start, so they line
up (and a future shared schedule_core lowering helper would be safe).

Golden impact

None. No committed golden has t_start > 0 (all are unanchored or use
at = [...]), verified by scanning all 108 committed *.ir.json. No IR
golden or trajectory baseline changes.

Spec §16 synced to the fixed reality ([t_start, t_end]; start defaults to
t_start).

Full make test green.

Follow-up (not in this PR)

The landed proposal docs/dev/proposals/2026-06-04-schedule-unification.md
(§A.2) still records the old divergence and instructs a future Option-B
implementer not to factor a shared lowering helper "because it would shift
observation times." That premise is now inverted — obs and output agree — so
that guidance is stale. Left untouched here to keep this PR to one logical
change; the live regression guard (the renamed agreement test) already pins
the new invariant.

…start) (gh#143)

`expand_output` defaulted the trajectory output schedule's start to
`min(0.0, t_start)`. For any model with `t_start > 0` (an anchored
`from` later than `origin`, or an unanchored `from > 0`) and no explicit
`output` block, this forced start = 0, so the trajectory was emitted —
and recorded — over [0, t_end] instead of the requested [from, to]. The
result was rows at times the dynamics never visited and, next to the
t_start prologue snapshot, non-monotonic time.

The gh#143 `min(0, t_start)` was aimed at the negative-t_start case
(a `from` before `origin`): starting at 0 leaves no snapshots in
[t_start, 0), so `--obs-only` / snap_at can't find a pre-origin
snapshot and hard-exits. But `min(0, t_start)` equals t_start for
t_start < 0 and clamps to 0 for t_start > 0 — it fixed the negative
direction and broke the positive one. `start = t_start` is correct in
both directions (and trivially at t_start = 0), so it subsumes the
negative-t_start fix rather than replacing it.

Reproduction (origin 2020-01-01, from 2020-07-01, to 2020-07-12 →
t_start=182, t_end=193; no output block):

  before: `camdl simulate m.camdl --dates --stdout` emits 194 data
          rows with non-monotonic t (182, then 1,2,…,193).
  after:  12 data rows over [182, 193], strictly increasing t.

Golden-neutral: no committed golden has t_start > 0 (all are unanchored
or use `at = [...]`), so no IR golden or trajectory baseline changes —
verified by scanning all 108 committed *.ir.json.

TDD (red-then-green), output_schedule test 4:

  # RED (current code):
  [FAIL] anchored t_start>0 → output.start = t_start
  ASSERT output.start = t_start (window is [from, to], not [0, to])
     Expected: `182'
     Received: `0'

  # GREEN (after fix):
  Test Successful in 0.003s. 12 tests run.

The A.2 obs/output "divergence" guard is repurposed as an *agreement*
guard: both surfaces now lower `start = t_start`, so they line up and a
future shared schedule_core lowering helper would be safe. Spec §16
synced to the fixed reality (`[t_start, t_end]`, start defaults to
`t_start`).

Full `make test` green.
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.

1 participant