diff --git a/docs/camdl-language-spec.md b/docs/camdl-language-spec.md index bd5da562..b8f40a7c 100644 --- a/docs/camdl-language-spec.md +++ b/docs/camdl-language-spec.md @@ -3313,7 +3313,7 @@ sampled on a schedule. With no `output {}` block the default schedule applies; declare one to set the cadence or give explicit output times. > **Default schedule.** Snapshots every `1` in the model's `time_unit`, covering -> `[min(0, t_start), t_end]` — where the window is taken from the `simulate {}` +> `[t_start, t_end]` — where the window is taken from the `simulate {}` > block (or `(0, 100)` if `simulate {}` is omitted). The simulate command writes > the trajectory to `--output` (or stdout) and writes observation files only when > `--obs` / `--obs-dir` / `--obs-only` is passed. @@ -3346,9 +3346,10 @@ metadata.json # run provenance (see §19) ### 16.2 IR Mapping The trajectories block compiles to the IR `output` schedule: `every = E` → -`OutRegular { start, step }` (start defaults to `min(0, t_start)` so the -schedule covers anchored models with a negative `t_start`); `at = [...]` → -`OutAtTimes`. The runtime writes the trajectory directly during simulation. +`OutRegular { start, step }` (start defaults to `t_start` so the schedule covers +exactly the requested window `[from, to]`, including anchored models with a +negative `t_start`); `at = [...]` → `OutAtTimes`. The runtime writes the +trajectory directly during simulation. Output emission is confined to `[start, simulation.t_end]`: `simulation.t_end` is the sole horizon authority, and output times are derived from it at emission diff --git a/docs/dev/proposals/2026-06-04-schedule-unification.md b/docs/dev/proposals/2026-06-04-schedule-unification.md index e112b54f..b26509c2 100644 --- a/docs/dev/proposals/2026-06-04-schedule-unification.md +++ b/docs/dev/proposals/2026-06-04-schedule-unification.md @@ -121,16 +121,27 @@ Each construct reuses it and keeps its own extension rule; the expander lowers - **observations** use the full core (`SchedEvery`→`ObsRegular`, `SchedAt`→`ObsAtTimes`). (`ObsFromData` has no frontend producer today — it is an IR-only stub, not a live obs `from_data` rule — so the obs surface is - exactly the two-arm core.) **Critical — the obs and output lowerings are NOT - identical:** obs `every` lowers `start = t_start` (expander.ml:3922); output - lowers `start = min 0.0 t_start` (expander.ml:3217). They agree at - `t_start = 0` (the common case) and for negative `t_start` (where - `min(0, t_start) = t_start`), but **diverge for `t_start > 0`** — e.g. - `simulate { from = 10 }` or an anchored `from` after `origin`: obs.start = 10 - while output.start = 0. Share only the grammar rule + AST type; keep the two - expander lowering sites distinct with their existing `start` expressions. Do - **not** factor a shared `lower_schedule_core` helper — that would silently - shift observation times (which PGAS conditions on). + exactly the two-arm core.) + + > **Superseded (gh#143):** the obs/output lowering **divergence** described + > below was a bug — output lowered `start = min 0.0 t_start`, which clamped + > `t_start > 0` to 0 and emitted the trajectory over `[0, t_end]` instead of + > `[from, to]`. Output now lowers `start = t_start`, so **both surfaces + > agree** in all regimes, and a shared `lower_schedule_core` helper would be + > safe (indeed correct) for a future Option B. The historical reasoning is + > kept below as the decision record; do not act on the "keep the sites + > distinct" instruction. + + **Critical — the obs and output lowerings are NOT identical:** obs `every` + lowers `start = t_start` (expander.ml:3922); output lowers + `start = min 0.0 t_start` (expander.ml:3217). They agree at `t_start = 0` (the + common case) and for negative `t_start` (where `min(0, t_start) = t_start`), + but **diverge for `t_start > 0`** — e.g. `simulate { from = 10 }` or an + anchored `from` after `origin`: obs.start = 10 while output.start = 0. Share + only the grammar rule + AST type; keep the two expander lowering sites + distinct with their existing `start` expressions. Do **not** factor a shared + `lower_schedule_core` helper — that would silently shift observation times + (which PGAS conditions on). - **output** use the full core (`→ OutRegular` / `OutAtTimes`); `format` stays its own field. - **interventions / events** reuse only the `SchedAt` arm (explicit times). diff --git a/ocaml/lib/compiler/expander.ml b/ocaml/lib/compiler/expander.ml index 4d41b23a..7e6157db 100644 --- a/ocaml/lib/compiler/expander.ml +++ b/ocaml/lib/compiler/expander.ml @@ -5283,7 +5283,7 @@ let expand_output ctx = (* The output window's upper bound is no longer stored on the schedule (gh#143): `simulation.t_end` is the sole horizon authority, and the runtime derives output times from `[start, t_end]` at emission. Only - `start` is set here — a deliberate widening to `min(0, t_start)`. *) + `start` is set here — defaulting to `t_start` (see below). *) let t_start = match ctx.simulate with | None -> 0.0 | Some sd -> resolve_float_expr ctx sd.sim_from @@ -5292,17 +5292,21 @@ let expand_output ctx = | Some { out_trajectories = Some ot; _ } -> ot.otformat | _ -> "tsv" in - (* Default the output schedule's start to cover the full integration - window. With anchored models that use `from = date(...)` before - `origin`, t_start is negative; an output schedule starting at 0 - leaves no snapshots in [t_start, 0), and `--obs-only` / any - state-at-obs-time path (snap_at) can't find a snapshot for the - pre-origin observations and hard-exits. Defaulting to - min(0.0, t_start) preserves the existing start=0 behaviour for - unanchored models (t_start ≥ 0) and extends it to cover negative - t_start without changing the step or output cadence. (start applies - only to the regular schedule; `at = [...]` lists explicit times.) *) - let start = min 0.0 t_start in + (* Default the output schedule's start to the requested window origin, + t_start, so the trajectory covers exactly [t_start, t_end] = [from, to]. + Two directions matter: + - t_start < 0 (anchored `from = date(...)` before `origin`): starting + at 0 would leave no snapshots in [t_start, 0), so `--obs-only` / any + state-at-obs-time path (snap_at) can't find a snapshot for the + pre-origin observations and hard-exits. start = t_start covers them. + - t_start > 0 (anchored `from` after `origin`, or unanchored from > 0): + starting at 0 would emit — and record — over [0, t_end] instead of + [from, to], yielding rows at times the dynamics never visited (and + non-monotonic time next to the t_start prologue snapshot). + start = t_start is correct in both directions (and trivially at t_start = 0). + (start applies only to the regular schedule; `at = [...]` lists explicit + times.) *) + let start = t_start in let times = match ctx.output_decl with | Some { out_trajectories = Some ot; _ } -> (match ot.otschedule with diff --git a/ocaml/test/test_compiler.ml b/ocaml/test/test_compiler.ml index 7f3c25ba..39f81c22 100644 --- a/ocaml/test/test_compiler.ml +++ b/ocaml/test/test_compiler.ml @@ -800,16 +800,16 @@ let test_output_step_default () = Alcotest.(check (float 0.01)) "default output step" 1.0 r.Ir.step | _ -> Alcotest.fail "expected OutRegular schedule") -(* Regression: the default output schedule must cover the full - integration window. With anchored models that resolve `from = - date(...)` to a negative t_start, the default `start = 0.0` would - leave [t_start, 0) without snapshots and the `--obs-only` writer - (and any state-at-obs-time consumer) would hard-exit with - "no snapshot at or before t=…" for pre-origin observations. - The fix: default `start = min(0.0, t_start)`. *) +(* Regression: the default output schedule must cover exactly the requested + window [t_start, t_end]. With anchored models that resolve `from = + date(...)` to a negative t_start, a `start = 0.0` would leave [t_start, 0) + without snapshots and the `--obs-only` writer (and any state-at-obs-time + consumer) would hard-exit with "no snapshot at or before t=…" for pre-origin + observations. The default is `start = t_start`. *) let test_output_default_start_unanchored_stays_zero () = - (* Unanchored (no origin), positive t_start → output.start = 0.0 (no regression). *) + (* Unanchored (no origin), from = 0 → t_start = 0 → output.start = 0.0 + (the common case; start = t_start is trivially 0 here). *) let src = {| compartments { S, I, R } parameters { beta : rate gamma : rate N0 : count I0 : count } @@ -857,6 +857,38 @@ let test_output_default_start_anchored_negative_t_start () = "output.start covers negative t_start" (-34.0) r.Ir.start | _ -> Alcotest.fail "expected OutRegular schedule") +let test_output_default_start_anchored_positive_t_start () = + (* Anchored with `from = date("2020-07-01")` *after* `origin = + date("2020-01-01")` → t_start = 182. The default output schedule must + start at t_start (182), not 0: otherwise the trajectory is emitted over + [0, t_end] instead of the requested [from, to], producing rows at times + the dynamics never visited (and, alongside the t_start prologue snapshot, + non-monotonic time). This is the anchored-positive counterpart of the + negative-t_start case above. *) + let src = {| + time_unit = 'days + origin = date("2020-01-01") + compartments { S, I, R } + parameters { beta : rate gamma : rate N0 : count I0 : count } + let N = S + I + R + transitions { + infection : S --> I @ beta * S * I / N + recovery : I --> R @ gamma * I + } + init { S = N0 - I0 I = I0 } + simulate { from = date("2020-07-01") to = date("2020-07-12") } + |} in + match Compiler.compile ~name:"test_output_anchored_pos_start" src with + | Error e -> Alcotest.failf "compile failed: %s" e + | Ok m -> + Alcotest.(check (float 1e-9)) "t_start" 182.0 m.Ir.simulation.Ir.t_start; + (match m.Ir.output.Ir.times with + | Ir.OutRegular r -> + Alcotest.(check (float 1e-9)) + "output.start = t_start (window is [from, to], not [0, to])" + 182.0 r.Ir.start + | _ -> Alcotest.fail "expected OutRegular schedule") + (* Output trajectory customization (Phase 1): the trajectories block accepts `every = E` (regular cadence) and `at = [...]` (explicit times), mirroring the observation schedule surface, plus `format = NAME`. *) @@ -919,13 +951,14 @@ let test_output_every_and_at_conflict () = | Error _ -> () (* expected: conflicting schedule rejected *) | Ok _ -> Alcotest.fail "expected error: every and at are mutually exclusive" -(* A.2 guard: observations and output share the `schedule_core` grammar/AST - but their expander lowerings are NOT identical — obs `every` lowers - start = t_start, output `every` lowers start = min(0, t_start). For - t_start > 0 they diverge (obs.start = t_start, output.start = 0). Pin it - so a future shared schedule_core lowering helper can't silently shift - observation times (which PGAS conditions on). *) -let test_obs_output_start_divergence () = +(* A.2 guard: observations and output share the `schedule_core` grammar/AST, + and their expander lowerings now agree — both lower `every` to + start = t_start. For t_start > 0 (here from = 10 'days) obs.start and + output.start both equal 10, so the trajectory covers [from, to] and the + observation times (which PGAS conditions on) line up with it. Pin the + agreement so a future shared schedule_core lowering helper stays honest and + the min(0, t_start) window bug can't silently return. *) +let test_obs_output_start_agrees () = let src = {| compartments { S, I, R } parameters { beta : rate gamma : rate rho : rate N0 : count I0 : count } @@ -951,14 +984,14 @@ let test_obs_output_start_divergence () = | Ok m -> (match m.Ir.output.Ir.times with | Ir.OutRegular r -> - Alcotest.(check (float 1e-9)) "output.start = min(0,t_start) = 0" 0.0 r.Ir.start + Alcotest.(check (float 1e-9)) "output.start = t_start = 10" 10.0 r.Ir.start | _ -> Alcotest.fail "expected OutRegular output schedule"); (match m.Ir.observations with | om :: _ -> (match om.Ir.emit_schedule with | Some (Ir.ObsRegular r) -> Alcotest.(check (float 1e-9)) - "obs.start = t_start = 10 (NOT min(0,t_start))" 10.0 r.Ir.start + "obs.start = t_start = 10 (agrees with output.start)" 10.0 r.Ir.start | _ -> Alcotest.fail "expected ObsRegular emit_schedule") | [] -> Alcotest.fail "expected an observation model") @@ -9421,13 +9454,15 @@ let () = `Quick test_output_default_start_unanchored_stays_zero; Alcotest.test_case "anchored t_start<0 → output.start covers full integration window" `Quick test_output_default_start_anchored_negative_t_start; + Alcotest.test_case "anchored t_start>0 → output.start = t_start (window is [from, to])" + `Quick test_output_default_start_anchored_positive_t_start; Alcotest.test_case "every = E → OutRegular step" `Quick test_output_every_explicit; Alcotest.test_case "every = 0.5 → sub-unit cadence" `Quick test_output_every_subunit; Alcotest.test_case "at = [...] → OutAtTimes" `Quick test_output_at_times; Alcotest.test_case "format = parquet" `Quick test_output_format_parquet; Alcotest.test_case "every and at conflict → error" `Quick test_output_every_and_at_conflict; - Alcotest.test_case "obs.start=t_start vs output.start=0 (A.2 lowering divergence guard)" - `Quick test_obs_output_start_divergence; + Alcotest.test_case "obs.start = output.start = t_start (A.2 lowering agreement guard)" + `Quick test_obs_output_start_agrees; Alcotest.test_case "stratified obs header emits (dim,level) stratum + serde round-trip" `Quick test_stratified_observation_emits_stratum; ];