diff --git a/README.md b/README.md index f2fee0f..244cf8b 100644 --- a/README.md +++ b/README.md @@ -131,23 +131,31 @@ need to find it. ## Current Phase -Stage 9 — Better Numerics -- not yet started: Stage 8 (Recording & -Playback, inserted ahead of this stage on 2026-09-07 -- `docs/planning/ -roadmap.md`'s own "Fourth divergence" entry) closed the same day it -opened, all three of its own tasks (TASK-045/046/047 -- headless -recording, deterministic windowed replay, interactive playback) landing -together. Its live status, generated from the roadmap rather than -restated here: -[Stage 9 in the status report](docs/planning/status.md#stage-9----better-numerics). +Stage 8 — Recording & Playback -- **reopened** 2026-09-09: an audit, +prompted by the maintainer's own suspicion that this stage "never +actually went through a design/planning session," found the suspicion +correct. Its original five completion criteria (TASK-045/046/047, +2026-09-07) are still met; four more were added the same day they were +found missing -- the Goal's own "scrubbed to any point" had shipped +with no seek mechanism at all, and two of the stage's own stated +deferrals (declared-field playback, partial-overlap cache reuse) plus +one gap nobody had named (checkpoint retention) were pulled forward +rather than left indefinitely deferred. TASK-049 closed the retention +gap the same day; TASK-048/050/051 will close the other three, one +branch each. Its live status, generated from the +roadmap rather than restated here: +[Stage 8 in the status report](docs/planning/status.md#stage-8----recording--playback). +Stage 9 (Better Numerics) still follows, once Stage 8 closes again. **Stage 8's own record, for anyone tracking how reliably this section -stays current**: opened and closed in one day, so the multi-day -staleness windows the two paragraphs below describe for Stages 7 and 8's -own *earlier* drafts of this section never had a chance to recur here -- -not because the failure mode was fixed, but because there was no gap -of real time for it to go unnoticed in. Don't read this as the pattern -solved; read Stage 9's own eventual entry here as the next real test of -it. +stays current**: opened and closed in one day (2026-09-07), then +reopened two days later by an audit this same session's own change is +keeping in sync -- the multi-day staleness windows the two paragraphs +below describe for Stages 7 and 8's own *earlier* drafts of this +section didn't recur here, because this edit landed in the same change +that reopened the stage rather than after. Don't read this as the +pattern solved; read whichever of Stage 8's closure or Stage 9's own +eventual entry comes next as the next real test of it. **This sentence said "not yet started" for Stage 8 itself, twice, while that stage was still open** -- once for the same reason a fourth time as @@ -179,8 +187,10 @@ golden demo renders a *solved* velocity field live. **Stage 6 is the proof that the engine underneath it is field-centric**: four named physical fields, added by configuration. -Stages 0 through 8 are complete, each closed against its own written -completion criteria (`docs/planning/roadmap.md`): +Stages 0 through 7 are complete, and Stage 8 is reopened (six of nine +criteria met, see below) -- each closed, or in Stage 8's case pending +re-closure, against its own written completion criteria +(`docs/planning/roadmap.md`): - Stage 0 — planning system, capability map, repository structure, development tooling, CI. Deliberately no CFD functionality. @@ -243,16 +253,23 @@ completion criteria (`docs/planning/roadmap.md`): added 93 step definitions, 28% of the repository's whole step vocabulary, which is evidence against its own claim rather than for it. -**Stage 8 (Recording & Playback) is complete.** `pyflow record`/ -`pyflow resume`/`pyflow play` (TASK-045/046/047, all 2026-09-07): record -a run headlessly, resume it from any checkpoint, or watch it back in a -real window with live pause and speed control -- no rendering window -ever needed for the first two, and no simulation code re-run for the -third. Its own Golden Demo is Lid-Driven Cavity (moved there from an -earlier Heat Diffusion choice once playback -- which renders a solved -velocity field -- turned out incompatible with a demo that has none; see -`docs/planning/roadmap.md`'s own Stage 8 Status section for the full -account). Try the whole pipeline: +**Stage 8 (Recording & Playback) is reopened -- six of nine criteria +met.** `pyflow record`/`pyflow resume`/`pyflow play` (TASK-045/046/047, +all 2026-09-07): record a run headlessly, resume it from any +checkpoint, or watch it back in a real window with live pause and speed +control -- no rendering window ever needed for the first two, and no +simulation code re-run for the third. Its own Golden Demo is Lid-Driven +Cavity (moved there from an earlier Heat Diffusion choice once playback +-- which renders a solved velocity field -- turned out incompatible +with a demo that has none; see `docs/planning/roadmap.md`'s own Stage 8 +Status section for the full account). **Reopened 2026-09-09** for four +more criteria an audit found the Goal itself already promised: live +scrub (keyboard and a mouse-draggable bar), combined solved-velocity + +declared-field playback (grounded in Smoke Transport), opt-in checkpoint +retention, and partial-overlap cache reuse. The retention piece is done +(TASK-049, 2026-09-09, `--max-checkpoints-retained` on `record`/ +`resume`); TASK-048/050/051 are not yet built. Try the whole pipeline as +it stands today: ```bash uv run python -m pyflow record --config examples/golden-demos/lid_driven_cavity.yaml --max-frames 500 --checkpoint-interval 100 diff --git a/docs/implementation/config-template.yaml b/docs/implementation/config-template.yaml index 203bafe..4ba1bdd 100644 --- a/docs/implementation/config-template.yaml +++ b/docs/implementation/config-template.yaml @@ -341,3 +341,8 @@ recording: # checkpoint is always written at frame 0 and at the run's final frame # too, regardless of this value). Invalid: zero or negative. checkpoint_interval: 100 + # Valid: null (default, unbounded -- every checkpoint is kept), or a + # positive integer -- the newest N non-zero checkpoints are kept, older + # ones deleted as new ones are written. Frame 0 is never deleted, whatever + # this value is. Invalid: zero or negative. + max_checkpoints_retained: null diff --git a/docs/planning/backlog.md b/docs/planning/backlog.md index bdd5eeb..29d4505 100644 --- a/docs/planning/backlog.md +++ b/docs/planning/backlog.md @@ -2411,6 +2411,22 @@ here.): here; open a new backlog item if a demo needs it before Stage 9 does anything else that would motivate it. + **Reopened 2026-09-09, not via a new backlog item.** An audit of + this stage, prompted by the maintainer's own suspicion that it + "never actually went through a design/planning session," found + the suspicion correct and one further gap beyond the two named + above: the Goal's own "scrubbed to any point" shipped with no + operationalised criterion and no seek mechanism at all, alongside + checkpoint retention (no gap anyone had named as a gap before this + audit). Rather than a fifth backlog item duplicating what + `docs/planning/roadmap.md`'s own Stage 8 now tracks directly (four + new Completion Criteria 6-9, TASK-048/049/050/051), this item's own + two named deferrals above are folded into that reopening instead. + Read Stage 8's own **Status as of 2026-09-09** section for the + current state; this item stays marked done for the halves it + actually built (recording, replay, playback's own first cut) and + does not track the reopening's own progress. + --- # Part III — Audit history diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index af582ff..61ffa67 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -306,7 +306,23 @@ This paragraph previously said `make install` and `make test` were still expected to fail, pending `uv.lock` and a test suite (B2/C1) -- stale since 2026-08-16 and corrected 2026-08-19. Both now succeed: `uv.lock` is committed (B2) and `make test` runs the suite with coverage -(C1a/C1b): **1171 tests as of 2026-09-08**, up from 1163 the same day (8 +(C1a/C1b): **1182 tests as of 2026-09-09**, up from 1172 the same day +(TASK-049, Checkpoint Retention Policy: 2 in `tests/unit/ +test_checkpoint.py` for the new `list_checkpoints` helper, 5 in +`tests/unit/test_recording.py` for the pruning behaviour itself +(no-cap-changes-nothing, pruning past the cap, frame 0 surviving a cap +that would otherwise drop it, `resume` pruning the whole directory not +only what it wrote, the config-file-only path), 1 in `tests/integration/ +test_record_cli.py` for a real `--max-checkpoints-retained` subprocess, +and 2 in `tests/unit/test_configuration.py` for the new schema field -- +`test_defaults_are_valid`'s own new assertion is not a new test), 1172 +itself up from 1171 the day before +(the Stage 8 reopening audit's own regression test, +`test_the_real_roadmap_reports_stage_0_as_complete` in +`tests/unit/test_check_stages.py` -- proving Stage 0's eleven tasks, +newly given the per-task `**Status: Done**` marker every later stage's +tasks already carry, are now read as `complete` rather than `opened`), +1171 itself up from 1163 the same day (8 new tests from the failure-mode audit: `test_hook_does_not_strip_an_ import_with_no_usage_yet`, two `check_manifest.py` tests for the new `claude-md-count-matches-live` rule, and five for the new @@ -1056,6 +1072,11 @@ isn't. ## TASK-000 — Create Engine Skeleton +**Status: Done, 2026-08-15** (date from this stage's own summary table +above; the inline marker itself added 2026-09-09, when a `check_stages.py` +audit found Stage 0's eleven tasks had never carried one -- see that +table's own row for what was actually verified). + ### Purpose Create the initial package structure and architectural skeleton for the PyFlow engine. @@ -1104,6 +1125,9 @@ No implementation beyond package initialisation is required. ## TASK-001 — Development Environment +**Status: Done, 2026-08-15** (see TASK-000's own note, above, for why +this marker was only added 2026-09-09). + ### Purpose Create a fully reproducible development environment. @@ -1158,6 +1182,8 @@ without manual configuration. ## TASK-002 — Build System +**Status: Done, 2026-08-15** (see TASK-000's own note, above). + ### Purpose Provide a consistent interface for common engineering tasks. @@ -1191,6 +1217,8 @@ Every documented command executes successfully. ## TASK-003 — Automated Testing +**Status: Done, 2026-08-16** (see TASK-000's own note, above). + ### Purpose Establish regression testing from the beginning of the project. @@ -1221,6 +1249,8 @@ Tests execute locally and produce coverage reports. ## TASK-004 — Continuous Integration +**Status: Done, 2026-08-19** (see TASK-000's own note, above). + ### Purpose Automatically validate every commit. @@ -1251,6 +1281,8 @@ Every pull request executes the validation pipeline automatically. ## TASK-005 — Configuration Framework +**Status: Done, 2026-08-16** (see TASK-000's own note, above). + ### Purpose Separate engine construction from engine execution. @@ -1283,6 +1315,8 @@ The application can be started entirely from configuration. ## TASK-006 — Logging Framework +**Status: Done, 2026-08-16** (see TASK-000's own note, above). + ### Purpose Provide consistent diagnostic output throughout the engine. @@ -1312,6 +1346,8 @@ Every subsystem logs through the common logging framework. ## TASK-007 — Rendering Framework +**Status: Done, 2026-08-16** (see TASK-000's own note, above). + ### Purpose Establish the rendering subsystem that will support all future visualisation. @@ -1348,6 +1384,12 @@ A rendering window opens, updates and closes cleanly. ## TASK-008 — Repository Documentation +**Status: Done** (see TASK-000's own note, above, for why this marker +was only added 2026-09-09; no completion date is recorded for this task +specifically -- its own summary-table row above states only that it was +*corrected* to say Done on 2026-08-19, not when the underlying work +actually finished, and this entry does not guess one). + ### Purpose Establish the repository as the authoritative source of project knowledge. @@ -1384,6 +1426,8 @@ Every core document exists and provides sufficient information for future develo ## TASK-009 — CLAUDE.md Hierarchy +**Status: Done, 2026-08-19** (see TASK-000's own note, above). + ### Purpose Provide concise contextual guidance to coding agents throughout the repository. @@ -1419,6 +1463,8 @@ Each file provides sufficient local context while remaining compact enough to mi ## TASK-010 — Engine Bootstraps +**Status: Done, 2026-08-16** (see TASK-000's own note, above). + ### Purpose Validate that the engineering infrastructure functions as a coherent system. @@ -11241,6 +11287,84 @@ established this project follows. and a check that the rendered pixels actually stop changing once paused, not only that `PlaybackState.paused` flips in isolation. +**Criteria 6-9 added 2026-09-09, when this stage was reopened -- see the +Status section below for why.** Drafted the same way 1-5 were meant to +be and, per that section's own honest accounting, mostly weren't: from +this stage's own Goal, independent of the four tasks that will discharge +them, which had not been drafted yet when these were written. + +6. **Playback can seek to any frame inside the materialized window, + live, not only play forward at a fixed speed from where it was + launched.** The Goal's own "scrubbed to any point" -- the one clause + of this stage's Goal that TASK-047 shipped without, and the clause no + criterion above ever named, which is exactly why it went unnoticed + while this stage was marked complete. + - **Both a keyboard and a mouse reach every frame in `[from_frame, + to_frame]`.** Left/Right step one frame; Home/End jump to the + window's own start/end; a draggable scrub bar reaches any frame in + between directly, not only by repeated stepping. + - **Checked against real rendered pixels, the same way Space's own + pause already is** -- not only against `PlaybackState` in + isolation. A keyboard seek and a mouse drag each change + `window.renderer.snapshot()`'s content, confirmed with a real + injected event against a genuinely running window. + - **Scoped to the window already requested at launch, not the whole + recording** -- a real, stated exclusion decided before code, not + discovered after: seeking past `from_frame`/`to_frame` still needs + a different `pyflow play` invocation, exactly as today. Extending + scrub to seek beyond the loaded window is real, deferred future + work, not built here. +7. **A config combining a solved velocity field with one or more + declared fields plays back correctly, not only a solved-velocity-only + config.** TASK-047's own stated scope boundary + (`UnsupportedPlaybackConfigError` for any config with declared + `fields`) closes here, against a real demo rather than a synthetic + fixture. + - **Grounded in Smoke Transport** + (`examples/golden-demos/smoke_transport.yaml`) -- solved velocity + plus a declared `smoke` field with two configured display panels, + run record-then-play through the real CLI end to end. + - **Both the arrows and every configured panel render from the same + materialized frame** -- checked by rebuilding a panel from a + `MaterializedWindow` frame and confirming it matches what a live + run's own `_add_declared_field_transport` would have coloured at + the same simulation state, not only that something is drawn. + - **No duplicate panel-rendering implementation** -- the + colour-mapping/legend logic this reuses is the same one + `bootstrap.py`'s live path calls, extracted into + `rendering/field_visualization.py` rather than copied, per this + project's own P-011 (single authoritative source). +8. **A recording's own checkpoint count can be bounded, opt-in, without + changing any existing config's behaviour.** Criterion 2's own "never + one file per frame" bounds the interval between checkpoints, not the + total count over a long run -- a real gap in what "bounded footprint" + means that this closes. + - **`RecordingConfig.max_checkpoints_retained`, unset by default** -- + every existing config and golden demo writes exactly the + checkpoints it always did; the cap only changes behaviour for a + config that sets it. + - **Frame 0 is never pruned**, whatever the cap -- a capped recording + still has a starting point to restart from. + - **Checked directly against a real multi-hundred-frame `pyflow + record` run**, not only against the pruning function in isolation: + the files actually on disk after the run match what the policy + predicts, not merely what a unit test of the deletion logic + asserts. +9. **Watching a sub-range of an already-cached wider window costs no + re-simulation.** TASK-046's own stated scope boundary + (`materialize_or_load_window`'s cache serving exact-range matches + only) narrows here for the one case with a concrete, avoidable cost: a + narrower request the cache could already answer. + - **Scoped to a requested range that is a full subset of an existing + cached range** -- checked by deleting every checkpoint before the + second call and confirming the narrower request still succeeds, the + same technique TASK-046's own exact-match test already established. + - **A request that only partially overlaps a cached range, or + extends past its edge, still falls back to full materialization** -- + a real, stated exclusion, not silently handled either way; full + stitching across a cached window's own edge is deferred future + work. + ### Discharge map | Criterion | Discharged by | @@ -11251,18 +11375,41 @@ established this project follows. | 4. A checkpoint file is self-contained | TASK-045 | | 5. Golden Demo runs end to end (record half) | TASK-045 | | 5. Golden Demo runs end to end (playback half) | TASK-046/047 | - -### Status as of 2026-09-07: Stage 8 complete, five of five criteria met - -**"Complete" here means both things at once, for the first time in this -stage's own history**: every `## TASK-NNN` entry under this heading is -Done (`stage-shape.yaml`'s own mechanical lifecycle meaning), *and* this -stage's own Goal ("recorded... and played back afterward") is actually -built, not only partially. The status line below was deliberately "in -progress" while only TASK-045 existed, precisely so `README.md`'s own -"Current Phase" cross-check would not advance past real, undrafted work --- see that entry's own note for the mechanism and the template this -line has to match exactly (`generate_status_report.py`'s `STATUS_LINE`). +| 6. Live scrub, keyboard and mouse, checked against rendered pixels | TASK-048 | +| 7. Combined solved-velocity + declared-field playback | TASK-051 | +| 8. Checkpoint retention, opt-in, frame 0 never pruned | TASK-049 | +| 9. Partial-overlap (subset) cache reuse | TASK-050 | + +### Status as of 2026-09-09: Stage 8 reopened, six of nine criteria met + +**This stage was audited 2026-09-09, at the maintainer's own request, +against the suspicion that it "never actually went through a +design/planning session" -- and the audit confirmed it.** Raised +2026-09-04 as a side comment while scoping unrelated work, opened and +fully built in a single day (2026-09-07), with Completion Criteria 1-5 +written the same day as TASK-045, its own first task, rather than +independently derived from the Goal beforehand the way this section's +own 2026-09-07 text originally claimed. That produced a real, +previously unrecorded gap: the Goal's own "paused, **scrubbed to any +point**, and watched at a different speed" was never operationalised by +any of the five criteria that shipped, so nothing caught `PlaybackState` +landing with pause and speed but no seek. Three further gaps -- named by +TASK-046/047 themselves as deliberate, stated deferrals, not +oversights -- were pulled forward into this stage rather than left +recorded-but-deferred indefinitely: declared-field/scalar-colormap +playback, partial-overlap cache reuse, and checkpoint retention (a gap +in what "bounded footprint" means that nobody had named as a gap at +all, until this audit). Criteria 6-9, above, are the four; TASK-048-051 +will discharge them, one branch each, in that numeric order except +049/050 (the two library-only changes) landing before 048/051 (the two +that touch rendering). + +**"Complete" reopened, not "complete" corrected** -- 2026-09-07's own +five criteria were genuinely met by what TASK-045/046/047 built; nothing +about the record above this line is being retracted. What changed is +that this stage's own Goal turned out to need four more checkable claims +than its first pass wrote down, found by re-reading the Goal against +what shipped rather than against the criteria that were meant to operationalise it. | Criterion | Verdict | |-----------|---------| @@ -11271,15 +11418,21 @@ line has to match exactly (`generate_status_report.py`'s `STATUS_LINE`). | 3. Resuming reproduces the same trajectory, bit-identically | **Met** -- TASK-045, mutation-tested | | 4. A checkpoint file is self-contained | **Met** -- TASK-045 | | 5. Golden Demo runs end to end, both halves | **Met** -- TASK-045 (record), TASK-046/047 (playback), against Lid-Driven Cavity | - -All five criteria are met. **One real course-correction happened along -the way, recorded rather than smoothed over**: TASK-045's own original -Golden Demo choice (Heat Diffusion) turned out incompatible with -TASK-047's own scope decision (playback renders a solved velocity field; -Heat Diffusion has none) -- found when TASK-047 was actually scoped, not -anticipated in advance, and resolved by reconciling the whole stage onto -one demo (Lid-Driven Cavity) rather than carrying two. See the stage's -own **Golden Demo** entry above for the full account. +| 6. Live scrub, keyboard and mouse | **Open** -- TASK-048, drafted, not started | +| 7. Combined solved-velocity + declared-field playback | **Open** -- TASK-051, drafted, not started | +| 8. Checkpoint retention, opt-in, frame 0 never pruned | **Met** -- TASK-049, mutation-tested | +| 9. Partial-overlap (subset) cache reuse | **Open** -- TASK-050, drafted, not started | + +Six of nine criteria are met; the stage is **in progress**, not +complete, until TASK-048/050/051 close the other three. **One real +course-correction happened during the original build, recorded rather +than smoothed over**: TASK-045's own original Golden Demo choice (Heat +Diffusion) turned out incompatible with TASK-047's own scope decision +(playback renders a solved velocity field; Heat Diffusion has none) -- +found when TASK-047 was actually scoped, not anticipated in advance, and +resolved by reconciling the whole stage onto one demo (Lid-Driven +Cavity) rather than carrying two. See the stage's own **Golden Demo** +entry above for the full account. --- @@ -11877,6 +12030,229 @@ updated in this same change. --- +## TASK-049 — Checkpoint Retention Policy + +**Status: Done, 2026-09-09.** Discharges Completion Criterion 8. + +### Purpose + +Bound a recording's total on-disk checkpoint count, opt-in -- the gap +between Criterion 2's own "never one file per frame" (bounds the +*interval* between checkpoints) and what "bounded footprint" actually +needs on a very long run (bounds the *total*), found by the audit that +reopened this stage. + +### Dependencies + +`checkpoint.py`/`recording.py` (TASK-045) only. + +### Design decisions, recorded here + +Settled directly with the maintainer when this stage was reopened, +before implementation, the same "ask directly, with a recommendation +and the trade-off named" discipline TASK-046/047 already used: + +1. **Opt-in, unbounded by default.** `RecordingConfig. + max_checkpoints_retained: int | None = None` -- every existing config + and golden demo keeps writing exactly the checkpoints it always did; + the cap only changes behaviour for a config that sets it. +2. **Frame 0 is never pruned**, whatever the cap -- a capped recording + still has a starting point to resume from. **Confirmed to have real + teeth, not just to pass**: a deliberate mutation removing frame 0's + exclusion from the prunable set (so pruning applied to every + checkpoint, oldest-first, overall) was run against + `test_record_retention_cap_never_prunes_frame_zero` and observed to + fail before being reverted -- this project's own mutation-testing + discipline (TASK-046's own precedent) applied here too. +3. **The frame-number-from-filename parsing `replay.py`'s own private + `_CHECKPOINT_FILENAME` regex already did is factored into a shared + `checkpoint.list_checkpoints` helper**, used by both the new pruning + logic and `replay.find_checkpoint_at_or_before` (which now calls it + instead of carrying its own copy) -- this project's own P-011 + (single authoritative source), the same reasoning that produced + `checkpoint.field_tensors`. + +### Artifacts Produced + +- `src/pyflow/configuration/schema.py` -- `RecordingConfig. + max_checkpoints_retained: int | None = None`, validated `> 0` if set. +- `src/pyflow/checkpoint.py` -- `CHECKPOINT_FILENAME` (the shared + filename regex), `list_checkpoints(directory) -> list[tuple[int, + Path]]`. +- `src/pyflow/replay.py` -- `find_checkpoint_at_or_before` now calls + `checkpoint.list_checkpoints` instead of its own private copy; no + behaviour change. +- `src/pyflow/recording.py` -- `_prune_checkpoints(output_dir, retain)`; + `_advance_and_checkpoint`/`record`/`resume` all gained a `retain`/ + `max_checkpoints_retained` parameter, threaded through the same + CLI-overrides-config shape `checkpoint_interval` already uses. +- `src/pyflow/__main__.py` -- `--max-checkpoints-retained` on both + `record_parser` and `resume_parser`. +- `tools/generators/generate_config_template.py` -- `FIELD_COMMENTS` + entry for the new field; `docs/implementation/config-template.yaml` + regenerated. +- Tests: 2 in `tests/unit/test_checkpoint.py` + (`list_checkpoints`), 5 in `tests/unit/test_recording.py` (no-cap + keeps everything, pruning beyond the cap, frame 0 surviving a cap + that would otherwise drop it, `resume` pruning across the whole + directory not only what it wrote, the config-file-only path), 1 in + `tests/integration/test_record_cli.py` (a real `pyflow record + --max-checkpoints-retained` subprocess), 3 in + `tests/unit/test_configuration.py` (default, load, rejection), 6 in + `tests/unit/test_main.py` (existing dispatch/`generate-config` tests + updated for the new parameter/field, not new cases). + +### Acceptance Criteria + +- `RecordingConfig.max_checkpoints_retained` defaults to `None`; every + existing config and golden demo written before this task produces the + same files on disk after it, unless it now sets the new field -- + checked by the full pre-existing suite passing unmodified. +- Given a cap, `record`/`resume` keep frame 0 plus the newest `cap` + non-zero checkpoints, deleting the rest, checked against the real + files present after a run -- not only against `RecordingResult. + checkpoint_frames`, which still reports every frame *written*, pruned + or not. +- The cap is checked against everything already in `output_dir`, not + only what one `record`/`resume` call itself wrote -- `resume` prunes a + directory `record` populated earlier in the same way `record` would. +- Verified by hand against the real CLI, not only the test suite (root + `CLAUDE.md`'s Feature Verification rule): `pyflow record --config + examples/golden-demos/heat_diffusion.yaml --max-frames 25 + --checkpoint-interval 5 --max-checkpoints-retained 2` reports 6 + checkpoints written but leaves exactly `{0, 20, 25}` on disk; both + `pyflow record --help` and `pyflow resume --help` show the new flag. + +### Discharges + +Completion Criterion 8 in full. + +--- + +## TASK-050 — Partial-Overlap Cache Reuse, Subset Only + +**Status: Not started, drafted 2026-09-09.** Discharges Completion +Criterion 9. + +### Purpose + +Let a `pyflow play --cache DIR` request that falls fully inside an +already-cached wider window reuse it directly, at zero re-simulation +cost -- narrowing TASK-046's own stated scope boundary (exact-range +cache matches only) for the one case with a concrete, avoidable cost: a +narrower request the cache could already answer. + +### Dependencies + +`replay.py` (TASK-046) only. + +### Design decisions, recorded here + +1. **Scoped to a requested range that is a full subset of an existing + cached range.** `materialize_or_load_window` also globs `cache_dir` + for any `window_{from:08d}_{to:08d}.pt` whose own range is a superset + of the request, and slices `frames[requested_from - cached_from : + requested_to - cached_from + 1]` -- no re-simulation, no new file + written for the sliced sub-range. +2. **A request that only partially overlaps a cached range, or extends + past its edge, still falls back to full `materialize_window`** -- a + real, stated exclusion, not silently handled either way, the + maintainer's own bounding of this task when the stage reopened. Full + stitching across a cached window's own edge is real, deferred future + work. + +Artifacts, Acceptance Criteria and Discharges are written when this task +is actually built, the same as every other entry in this file. + +--- + +## TASK-048 — Live Scrub + +**Status: Not started, drafted 2026-09-09.** Discharges Completion +Criterion 6. + +### Purpose + +Close the one clause of this stage's own Goal that TASK-047 shipped +without: seeking to any frame in the materialized window, live, by +keyboard and by mouse -- not only playing forward at a fixed speed from +wherever `pyflow play` was launched. + +### Dependencies + +`playback.py` (TASK-047), for `PlaybackState`/`play()`. `window.py`'s +existing pointer-drag pan machinery (`_begin_pan`/`_update_pan`/ +`_end_pan`) is the one real open risk this task starts from -- see +Design decisions below. + +### Design decisions, recorded here + +Settled directly with the maintainer when this stage was reopened: + +1. **Scoped to the window already requested at launch** + (`[from_frame, to_frame]`) -- seeking past either edge still needs a + different `pyflow play` invocation, exactly as today. Seeking beyond + the loaded window is real, deferred future work, not built here. +2. **Keyboard: Left/Right step one frame; Home/End jump to the window's + own start/end.** +3. **Mouse: a draggable scrub bar**, reaching any frame in the window + directly rather than only by repeated stepping. +4. **Open technical risk, to resolve empirically before building the + widget, not by assumption:** `RenderWindow.run()` already wires + generic camera-pan pointer handlers unconditionally on every + interactive run (`window.py`). A scrub-bar drag must not also pan the + camera underneath it. First step of this task: verify whether pygfx/ + rendercanvas's event dict supports stopping propagation to a + same-canvas handler registered afterward -- the same "verify sign + conventions and event behaviour before relying on them" discipline + `rendering/CLAUDE.md`'s own pan/zoom entries already establish. If it + does not, the fallback is disabling `RenderWindow`'s built-in pan for + playback windows specifically, the same shape `close_keys=None` + already gives a caller for the close-key default. + +Artifacts, Acceptance Criteria and Discharges are written when this task +is actually built, the same as every other entry in this file. + +--- + +## TASK-051 — Combined Solved-Velocity + Declared-Field Playback + +**Status: Not started, drafted 2026-09-09.** Discharges Completion +Criterion 7. + +### Purpose + +Let `pyflow play` render a config that combines a solved velocity field +with one or more declared fields -- Smoke Transport's own shape -- +rather than rejecting it outright with `UnsupportedPlaybackConfigError`. + +### Dependencies + +`playback.py` (TASK-047). `bootstrap.py`'s `_add_declared_field_transport` +(TASK-030/042), whose panel-rendering half (`_PanelRenderState`/ +`_panel_colors`/`_add_panel_legend`/`_panel_caption`) needs extracting +into `rendering/field_visualization.py` as public functions before +`playback.py` can reuse it, rather than reaching into another module's +private helpers -- the same "extract before reusing" precedent TASK-045 +already set for `simulation_run.py`. + +### Design decisions, recorded here + +1. **Grounded in Smoke Transport** + (`examples/golden-demos/smoke_transport.yaml`) -- solved velocity + plus a declared `smoke` field with two configured display panels -- + rather than a synthetic fixture, per the maintainer's own choice when + this stage reopened. +2. **The extraction is verified behaviour-preserving by the full + existing test suite passing unmodified**, before anything new is + added, the same way TASK-045's `simulation_run.py` extraction was + verified. + +Artifacts, Acceptance Criteria and Discharges are written when this task +is actually built, the same as every other entry in this file. + +--- + # Stage 9 — Better Numerics Goal diff --git a/docs/planning/stage-specification.md b/docs/planning/stage-specification.md index de4e16a..9e1dc0f 100644 --- a/docs/planning/stage-specification.md +++ b/docs/planning/stage-specification.md @@ -254,6 +254,23 @@ rendering stage done" was written about pixels; the same sentence holds for a document, and this is what it turned up the first time it was applied to one. +**A task entry's own `**Status:**` line has, until now, always read +`Done, ` -- every entry in this file's history was written the +moment it was built. `Not started, drafted ` is an equally valid +value, added 2026-09-09 when Stage 8's reopening needed to record four +tasks' Purpose, Dependencies and settled Design decisions ahead of +building any of them**, the same reason `docs/planning/stage-shape.yaml` +lets a stage stay `sketched` before its first task exists, applied one +level down to a single task inside a stage that is otherwise built. +`check_stages.py`'s own `TASK_DONE` pattern already matches only the +literal string `Done` -- it was never a closed enum, so a `Not started` +entry needs no change to the checker: the stage's lifecycle correctly +stays `opened` rather than advancing to `complete` while any task reads +anything else. Draft an entry this way when its design decisions are +genuinely settled and worth recording durably before implementation +starts, not as a place to park an idea that is still being shaped -- +that is what `intended-work`'s own bullet list is for, one section up. + --- ## What is deliberately not required diff --git a/docs/planning/status.md b/docs/planning/status.md index fc068b1..f5c2411 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -17,14 +17,14 @@ demand, not part of this file. ## Progress -**48/48 tasks complete (100%)** across 16 planned stages. For the full plan, including +**49/52 tasks complete (94%)** across 16 planned stages. For the full plan, including stages below not yet broken into tasks: [roadmap.md](roadmap.md). ```mermaid pie showData title "Tasks across the roadmap" - "Done" : 48 - "Not started" : 0 + "Done" : 49 + "Not started" : 3 ``` ### Milestones @@ -37,16 +37,15 @@ pie showData - **Stage 5 -- First Fluid Solver** complete (2026-08-29) - **Stage 6 -- Additional Physical Fields** complete (2026-08-31) - **Stage 7 -- Rendering Annotations** complete (2026-09-03) -- **Stage 8 -- Recording & Playback** complete (2026-09-07) ### Up next -**Stage 9 -- Better Numerics** is next, and has not been broken into tasks yet. +**Stage 8 -- Recording & Playback** is next, starting with TASK-050 (Partial-Overlap Cache Reuse, Subset Only), 2 more not yet started in this stage. ## Live repository facts - **49** `CLAUDE.md` files -- **1171** tests collected +- **1182** tests collected - **144** Gherkin scenarios (`tests/features/*.feature`) ## Stages @@ -57,17 +56,17 @@ pie showData | Task | Status | Date | Artifact | |------|--------|------|----------| -| TASK-000 -- Create Engine Skeleton | Done | 2026-08-15 | | -| TASK-001 -- Development Environment | Done | 2026-08-15 | | +| TASK-000 -- Create Engine Skeleton | Done | 2026-08-15 | `check_stages.py` | +| TASK-001 -- Development Environment | Done | 2026-08-15 | `docs/practices.md` | | TASK-002 -- Build System | Done | 2026-08-15 | | | TASK-003 -- Automated Testing | Done | 2026-08-16 | | | TASK-004 -- Continuous Integration | Done | 2026-08-19 | | | TASK-005 -- Configuration Framework | Done | 2026-08-16 | | | TASK-006 -- Logging Framework | Done | 2026-08-16 | | -| TASK-007 -- Rendering Framework | Done | 2026-08-16 | | +| TASK-007 -- Rendering Framework | Done | 2026-08-16 | `adr/ADR-005-compute-rendering-instances.md` | | TASK-008 -- Repository Documentation | Done | | | | TASK-009 -- CLAUDE.md Hierarchy | Done | 2026-08-19 | | -| TASK-010 -- Engine Bootstraps | Done | 2026-08-16 | | +| TASK-010 -- Engine Bootstraps | Done | 2026-08-16 | `knowledge-architecture.md` | ### Stage 1 -- Representing Space @@ -154,13 +153,17 @@ pie showData ### Stage 8 -- Recording & Playback -**complete, as of 2026-09-07** -- `██████████` 3/3 tasks; 5/5 criteria met +**no status recorded** -- `██████░░░░` 4/7 tasks; 9 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| | TASK-045 -- Periodic Checkpointing (Headless Recording) | Done | 2026-09-07 | `docs/planning/backlog.md` | | TASK-046 -- Deterministic Windowed Replay | Done | 2026-09-07 | `docs/planning/roadmap.md` | | TASK-047 -- Interactive Playback (`pyflow play`) | Done | 2026-09-07 | `replay.py` | +| TASK-049 -- Checkpoint Retention Policy | Done | 2026-09-09 | `checkpoint.py` | +| TASK-050 -- Partial-Overlap Cache Reuse, Subset Only | Not started | | | +| TASK-048 -- Live Scrub | Not started | | | +| TASK-051 -- Combined Solved-Velocity + Declared-Field Playback | Not started | | | ### Stage 9 -- Better Numerics diff --git a/planning/data/features.yaml b/planning/data/features.yaml index 2730757..f87e333 100644 --- a/planning/data/features.yaml +++ b/planning/data/features.yaml @@ -643,3 +643,43 @@ entities: to: stage-8 - type: depends_on to: task-046 + + - id: task-049 + name: "TASK-049 — Checkpoint Retention Policy" + documented_in: docs/planning/roadmap.md + must_appear_in: docs/planning/roadmap.md + edges: + - type: belongs_to + to: stage-8 + - type: depends_on + to: task-045 + + - id: task-050 + name: "TASK-050 — Partial-Overlap Cache Reuse, Subset Only" + documented_in: docs/planning/roadmap.md + must_appear_in: docs/planning/roadmap.md + edges: + - type: belongs_to + to: stage-8 + - type: depends_on + to: task-046 + + - id: task-048 + name: "TASK-048 — Live Scrub" + documented_in: docs/planning/roadmap.md + must_appear_in: docs/planning/roadmap.md + edges: + - type: belongs_to + to: stage-8 + - type: depends_on + to: task-047 + + - id: task-051 + name: "TASK-051 — Combined Solved-Velocity + Declared-Field Playback" + documented_in: docs/planning/roadmap.md + must_appear_in: docs/planning/roadmap.md + edges: + - type: belongs_to + to: stage-8 + - type: depends_on + to: task-047 diff --git a/src/pyflow/CLAUDE.md b/src/pyflow/CLAUDE.md index 0371372..c6530aa 100644 --- a/src/pyflow/CLAUDE.md +++ b/src/pyflow/CLAUDE.md @@ -152,6 +152,16 @@ only be a redundant record of `config.simulation.velocity_pattern`) -- it calls `build_simulation_state` again for the right structure, then overwrites `.fields` with the checkpoint's real values. +**`checkpoint.list_checkpoints(directory)` (TASK-049, Stage 8 +reopening, added 2026-09-09) is the one place "what checkpoints exist +here" is answered** -- `CHECKPOINT_FILENAME`, the filename regex, moved +here from being a private copy inside `replay.py` +(`find_checkpoint_at_or_before` now calls this instead), and +`recording.py`'s new retention pruning (below) is its second caller. +This project's own P-011: two callers reading one implementation of a +filename convention rather than two that could drift, the same +reasoning that produced `field_tensors` just above it. + `recording.py` holds `record`/`resume`/`RecordingResult`/ `NothingToRecordError`/`NothingToResumeError`, the functions `pyflow record`/`pyflow resume` dispatch to. **It never imports `rendering`, @@ -203,6 +213,29 @@ own mutually exclusive, required `argparse` group (`--checkpoint`/ `--config`) still makes structurally impossible -- this adds an alternate entry point, not a way to pass both at once. +**`RecordingConfig.max_checkpoints_retained` (TASK-049, Stage 8 +reopening, added 2026-09-09) is an opt-in cap on total checkpoint +count, not another interval.** Criterion 2's own "never one file per +frame" already bounds the gap *between* checkpoints; nothing bounded +the *total* over a very long recording until this. `_prune_checkpoints` +(`recording.py`) runs after every checkpoint `_advance_and_checkpoint` +writes -- keeping disk usage bounded continuously, not only once a run +finishes -- and deletes the oldest checkpoints beyond the newest `cap`, +**always excluding frame 0 from the count itself**, not merely because +it happens to be old enough to survive: a capped recording that lost +its own starting point would have nothing left to resume from at all. +Confirmed to have real teeth by a deliberate mutation (removing that +exclusion) observed to fail +`test_record_retention_cap_never_prunes_frame_zero` before being +reverted, the same mutation-testing discipline TASK-046's own +`materialize_window` test already established. Applies to *everything* +already on disk in `output_dir`, not only what one `record`/`resume` +call itself wrote -- `resume` prunes checkpoints `record` left behind +just as readily as its own new ones. `--max-checkpoints-retained` on +both `pyflow record` and `pyflow resume`, overriding `config.recording. +max_checkpoints_retained` the same way `--checkpoint-interval` already +overrides that field. + **`replay.py` (TASK-046) is the windowed-materialization library those two tasks needed** -- `MaterializedWindow`, `materialize_window`, `materialize_or_load_window`, `find_checkpoint_at_or_before`. No @@ -218,10 +251,12 @@ and writes one after materializing if not, so a caller opts into avoiding recomputation rather than getting a second, separate artifact by default. `find_checkpoint_at_or_before` ranks candidates by the frame number in the *filename* first (cheap, no I/O for a discarded -candidate), then reads only the winner and cross-checks its real -`frame_count` against that filename -- `checkpoint.py`'s own "the -filename is a convention, `frame_count` is authoritative" rule, applied -to a lookup that would otherwise trust the filename outright. Memory +candidate; `checkpoint.list_checkpoints`, factored out for exactly this +purpose by TASK-049, not this module's own glob any more), then reads +only the winner and cross-checks its real `frame_count` against that +filename -- `checkpoint.py`'s own "the filename is a convention, +`frame_count` is authoritative" rule, applied to a lookup that would +otherwise trust the filename outright. Memory footprint was measured directly before trusting it safe with no cap: the golden demo's own mesh (256 cells x 2 fields x 500 frames) is 2.05 MB; extrapolated to the largest mesh anywhere in this repository (128x128, diff --git a/src/pyflow/__main__.py b/src/pyflow/__main__.py index 4ae3bfc..221f312 100644 --- a/src/pyflow/__main__.py +++ b/src/pyflow/__main__.py @@ -290,6 +290,14 @@ def main(argv: list[str] | None = None) -> None: default=None, help="Frames between checkpoints (default: config.recording.checkpoint_interval).", ) + record_parser.add_argument( + "--max-checkpoints-retained", + type=int, + default=None, + help="Keep only the newest N non-zero checkpoints, deleting older ones as new " + "ones are written (frame 0 is never deleted). Default: " + "config.recording.max_checkpoints_retained, unbounded if that is also unset.", + ) resume_parser = subparsers.add_parser( "resume", @@ -342,6 +350,14 @@ def main(argv: list[str] | None = None) -> None: help="Frames between checkpoints (default: the checkpoint's own " "embedded config.recording.checkpoint_interval).", ) + resume_parser.add_argument( + "--max-checkpoints-retained", + type=int, + default=None, + help="Keep only the newest N non-zero checkpoints, deleting older ones as new " + "ones are written (frame 0 is never deleted). Default: the checkpoint's own " + "embedded config.recording.max_checkpoints_retained, unbounded if that is also unset.", + ) play_parser = subparsers.add_parser( "play", @@ -435,6 +451,7 @@ def main(argv: list[str] | None = None) -> None: max_frames=args.max_frames, output_dir=args.output_dir, checkpoint_interval=args.checkpoint_interval, + max_checkpoints_retained=args.max_checkpoints_retained, ) print(f"wrote {len(result.checkpoint_frames)} checkpoint(s) to {result.output_dir}") return @@ -446,6 +463,7 @@ def main(argv: list[str] | None = None) -> None: max_frames=args.max_frames, output_dir=args.output_dir, checkpoint_interval=args.checkpoint_interval, + max_checkpoints_retained=args.max_checkpoints_retained, ) print(f"wrote {len(result.checkpoint_frames)} checkpoint(s) to {result.output_dir}") return diff --git a/src/pyflow/checkpoint.py b/src/pyflow/checkpoint.py index 377854a..07bf7e6 100644 --- a/src/pyflow/checkpoint.py +++ b/src/pyflow/checkpoint.py @@ -37,6 +37,7 @@ from __future__ import annotations import dataclasses +import re from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path @@ -54,6 +55,18 @@ _SCHEMA_VERSION = 1 +# `checkpoint_00000010.pt` -- the one filename convention every +# checkpoint on disk follows. Factored out here (TASK-049, Stage 8 +# reopening, 2026-09-09) so `list_checkpoints` below and `replay.py`'s +# own `find_checkpoint_at_or_before` read one implementation of "what +# checkpoints exist in this directory", not two that could drift apart +# -- this project's own P-011. The filename is a convention; a +# checkpoint's real `frame_count` (read from the file itself) is +# authoritative, per `read_checkpoint`'s own docstring -- callers that +# need to trust a frame number still read the file, the same way +# `find_checkpoint_at_or_before` already does. +CHECKPOINT_FILENAME = re.compile(r"^checkpoint_(\d{8})\.pt$") + class UnsupportedCheckpointVersionError(ValueError): """Raised by `read_checkpoint` if a file's own `schema_version` isn't @@ -81,6 +94,22 @@ class Checkpoint: fields: dict[str, torch.Tensor] +def list_checkpoints(directory: str | Path) -> list[tuple[int, Path]]: + """Every checkpoint file in `directory`, as `(frame_number, path)` + pairs read from each filename -- unsorted, in whatever order + `Path.glob` yields them. A non-checkpoint file (no match against + `CHECKPOINT_FILENAME`) is silently skipped, the same tolerance + `find_checkpoint_at_or_before` already had before this was factored + out of it. Cheap: reads filenames only, opens no file. + """ + checkpoints: list[tuple[int, Path]] = [] + for path in Path(directory).glob("checkpoint_*.pt"): + match = CHECKPOINT_FILENAME.match(path.name) + if match is not None: + checkpoints.append((int(match.group(1)), path)) + return checkpoints + + def field_tensors(fields: Mapping[str, Field]) -> dict[str, torch.Tensor]: """A `dict[str, Field]` reduced to plain, cloned `(num_cells,)` tensors -- the shape `write_checkpoint` saves and `replay.py`'s own diff --git a/src/pyflow/configuration/CLAUDE.md b/src/pyflow/configuration/CLAUDE.md index b924820..787758a 100644 --- a/src/pyflow/configuration/CLAUDE.md +++ b/src/pyflow/configuration/CLAUDE.md @@ -801,6 +801,17 @@ record`'s own `--output-dir`/`--checkpoint-interval` CLI flags override this section's fields when given, the same override relationship `--backend` already has with `rendering.backend`. +**`max_checkpoints_retained: int | None = None` (TASK-049, Stage 8 +reopening, added 2026-09-09) is a third field, opt-in** -- `None` +prunes nothing, exactly the behaviour every config had before this +field existed; a positive integer bounds the total non-zero checkpoint +count `recording.py`'s pruning keeps (`validate()` rejects `<= 0`, the +same pattern `checkpoint_interval` already uses). Frame 0 is never +counted against it or pruned -- see `src/pyflow/CLAUDE.md`'s +`recording.py` entry for the mechanism. `--max-checkpoints-retained` +overrides it the same way `--checkpoint-interval` overrides +`checkpoint_interval`. + **`loader.py` split into `_config_from_raw(raw, *, source)` and a public `config_from_dict(raw)`, in the same change, for `checkpoint.py`'s benefit, not this section's.** `load_config(path)` used to read YAML and diff --git a/src/pyflow/configuration/schema.py b/src/pyflow/configuration/schema.py index 38386d9..de5dcec 100644 --- a/src/pyflow/configuration/schema.py +++ b/src/pyflow/configuration/schema.py @@ -868,10 +868,21 @@ class RecordingConfig: between checkpoints (a checkpoint is always written at frame 0 and at the run's own final frame too, regardless of this value -- see `recording.py`'s own `record` function). + + `max_checkpoints_retained: int | None = None` (Stage 8 reopening, + TASK-049, added 2026-09-09) bounds the *total* number of checkpoint + files a recording keeps on disk, opt-in: `None` (the default) prunes + nothing, exactly today's behaviour for every existing config and + golden demo. Frame 0 is never counted against the cap and never + pruned -- a capped recording still has a starting point to resume + from -- so the cap applies only to the non-zero checkpoints + `recording.py`'s own pruning keeps the newest of. See + `recording._prune_checkpoints` for the mechanism. """ output_dir: str = "checkpoints" checkpoint_interval: int = 100 + max_checkpoints_retained: int | None = None def validate(self) -> None: _require_str(self.output_dir, "recording.output_dir") @@ -881,6 +892,11 @@ def validate(self) -> None: raise ValueError( f"recording.checkpoint_interval must be > 0, got {self.checkpoint_interval!r}" ) + if self.max_checkpoints_retained is not None and self.max_checkpoints_retained <= 0: + raise ValueError( + "recording.max_checkpoints_retained must be > 0, got " + f"{self.max_checkpoints_retained!r}" + ) AdvectionSchemeName = Literal["first_order_upwind"] diff --git a/src/pyflow/recording.py b/src/pyflow/recording.py index 9359eb9..eb50426 100644 --- a/src/pyflow/recording.py +++ b/src/pyflow/recording.py @@ -33,7 +33,12 @@ from dataclasses import dataclass from pathlib import Path -from pyflow.checkpoint import read_checkpoint, restore_simulation_state, write_checkpoint +from pyflow.checkpoint import ( + list_checkpoints, + read_checkpoint, + restore_simulation_state, + write_checkpoint, +) from pyflow.configuration import load_config from pyflow.configuration.schema import PyFlowConfig from pyflow.engine.logging_setup import configure_logging, get_logger @@ -81,6 +86,27 @@ class RecordingResult: final_frame_count: int +def _prune_checkpoints(output_dir: Path, retain: int) -> None: + """Delete the oldest checkpoints in `output_dir` beyond the newest + `retain`, never touching frame 0. + + Frame 0 is excluded from the count itself, not merely old enough to + survive by coincidence: it is the recording's own starting point, + and a capped recording that lost it would have nothing left to + resume from at all. Applies to every checkpoint already on disk, + not only ones a particular call wrote -- `resume` must prune what + `record` left behind just as readily as its own new files + (`RecordingConfig.max_checkpoints_retained`'s own docstring). + """ + prunable = sorted(frame for frame, _path in list_checkpoints(output_dir) if frame != 0) + excess = len(prunable) - retain + if excess <= 0: + return + by_frame = dict(list_checkpoints(output_dir)) + for frame in prunable[:excess]: + by_frame[frame].unlink() + + def _advance_and_checkpoint( state: SimulationState, numerics: AssembledNumerics, @@ -90,6 +116,7 @@ def _advance_and_checkpoint( max_frames: int, output_dir: Path, interval: int, + retain: int | None, ) -> list[int]: """Advance `state` in place from `start_frame` to `max_frames`, writing a checkpoint every `interval` frames and at `max_frames` @@ -98,6 +125,11 @@ def _advance_and_checkpoint( this runs) and `resume` (`start_frame=checkpoint.frame_count`, already on disk as the file being resumed from), so the two can never drift apart on what "every `interval` frames" means. + + `retain`, given, prunes `output_dir` (`_prune_checkpoints`) after + every checkpoint this loop writes -- bounding disk usage as the + recording grows, not only once it finishes. `None` (the default) + prunes nothing, exactly today's behaviour. """ checkpoint_frames: list[int] = [] for frame_count in range(start_frame + 1, max_frames + 1): @@ -106,6 +138,8 @@ def _advance_and_checkpoint( path = output_dir / f"checkpoint_{frame_count:08d}.pt" write_checkpoint(path, frame_count=frame_count, config=config, fields=state.fields) checkpoint_frames.append(frame_count) + if retain is not None: + _prune_checkpoints(output_dir, retain) return checkpoint_frames @@ -115,6 +149,7 @@ def record( max_frames: int, output_dir: str | Path | None = None, checkpoint_interval: int | None = None, + max_checkpoints_retained: int | None = None, ) -> RecordingResult: """Load `config_path`, step it forward `max_frames` timesteps with no rendering at all, writing a checkpoint at frame 0, every @@ -122,9 +157,13 @@ def record( `max_frames` doesn't fall on the interval) -- the sparse seek index Stage 8's own Goal describes. - `output_dir`/`checkpoint_interval`, given, override `config. - recording`'s own fields, the same CLI-overrides-config shape - `bootstrap()`'s own `backend` parameter already establishes. + `output_dir`/`checkpoint_interval`/`max_checkpoints_retained`, given, + override `config.recording`'s own fields, the same CLI-overrides- + config shape `bootstrap()`'s own `backend` parameter already + establishes. `max_checkpoints_retained` bounds the total on-disk + checkpoint count, opt-in -- `None` (the default) prunes nothing; see + `_prune_checkpoints` for the mechanism and `RecordingConfig`'s own + docstring for why frame 0 is never pruned. `max_frames` is required, not optional -- unlike `bootstrap()`, there is no window and no user to stop this run any other way; an @@ -144,6 +183,11 @@ def record( if checkpoint_interval is not None else config.recording.checkpoint_interval ) + retain = ( + max_checkpoints_retained + if max_checkpoints_retained is not None + else config.recording.max_checkpoints_retained + ) mesh = StructuredCartesianMesh.from_config(config.mesh) numerics = assembled_numerics_for(config) @@ -170,6 +214,7 @@ def record( max_frames=max_frames, output_dir=resolved_output_dir, interval=interval, + retain=retain, ) checkpoint_frames = [0, *rest] @@ -193,6 +238,7 @@ def resume( max_frames: int, output_dir: str | Path | None = None, checkpoint_interval: int | None = None, + max_checkpoints_retained: int | None = None, ) -> RecordingResult: """Read the checkpoint at `checkpoint_path`, restore the `SimulationState` it holds, and continue stepping headlessly from its @@ -244,6 +290,7 @@ def resume( max_frames=max_frames, output_dir=output_dir, checkpoint_interval=checkpoint_interval, + max_checkpoints_retained=max_checkpoints_retained, ) assert checkpoint_path is not None # the exactly-one-of check above guarantees this @@ -264,6 +311,11 @@ def resume( if checkpoint_interval is not None else checkpoint.config.recording.checkpoint_interval ) + retain = ( + max_checkpoints_retained + if max_checkpoints_retained is not None + else checkpoint.config.recording.max_checkpoints_retained + ) _mesh, numerics, state = restore_simulation_state(checkpoint) resolved_output_dir.mkdir(parents=True, exist_ok=True) @@ -275,6 +327,7 @@ def resume( max_frames=max_frames, output_dir=resolved_output_dir, interval=interval, + retain=retain, ) logger.info( diff --git a/src/pyflow/replay.py b/src/pyflow/replay.py index 3199a57..f7cafd5 100644 --- a/src/pyflow/replay.py +++ b/src/pyflow/replay.py @@ -28,21 +28,23 @@ from __future__ import annotations import dataclasses -import re from dataclasses import dataclass from pathlib import Path import torch -from pyflow.checkpoint import field_tensors, read_checkpoint, restore_simulation_state +from pyflow.checkpoint import ( + field_tensors, + list_checkpoints, + read_checkpoint, + restore_simulation_state, +) from pyflow.configuration import config_from_dict from pyflow.configuration.schema import PyFlowConfig from pyflow.simulation_run import advance_simulation_state _WINDOW_SCHEMA_VERSION = 1 -_CHECKPOINT_FILENAME = re.compile(r"^checkpoint_(\d{8})\.pt$") - class NoCheckpointBeforeFrameError(ValueError): """Raised by `find_checkpoint_at_or_before` when no checkpoint in the @@ -72,13 +74,7 @@ def find_checkpoint_at_or_before(checkpoints_dir: str | Path, frame: int) -> Pat `NoCheckpointBeforeFrameError` if the filename claims a mismatched frame count (someone renamed the file) or if nothing qualifies. """ - candidates: list[tuple[int, Path]] = [] - for path in Path(checkpoints_dir).glob("checkpoint_*.pt"): - match = _CHECKPOINT_FILENAME.match(path.name) - if match is None: - continue - candidates.append((int(match.group(1)), path)) - + candidates = list_checkpoints(checkpoints_dir) qualifying = [candidate for candidate in candidates if candidate[0] <= frame] if not qualifying: raise NoCheckpointBeforeFrameError( diff --git a/tests/integration/test_record_cli.py b/tests/integration/test_record_cli.py index 6a59c66..ca25067 100644 --- a/tests/integration/test_record_cli.py +++ b/tests/integration/test_record_cli.py @@ -52,6 +52,38 @@ def test_record_writes_checkpoint_files_for_a_real_golden_demo_config(tmp_path: assert set(payload["fields"]) == {"tracer"} # heat_diffusion's own declared field name +def test_record_prunes_checkpoints_through_the_real_cli(tmp_path: Path) -> None: + output_dir = tmp_path / "checkpoints" + + result = subprocess.run( + [ + sys.executable, + "-m", + "pyflow", + "record", + "--config", + "examples/golden-demos/heat_diffusion.yaml", + "--max-frames", + "20", + "--output-dir", + str(output_dir), + "--checkpoint-interval", + "5", + "--max-checkpoints-retained", + "2", + ], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr + remaining = { + int(p.stem.removeprefix("checkpoint_")) for p in output_dir.glob("checkpoint_*.pt") + } + assert remaining == {0, 15, 20} + + def test_record_requires_config_and_max_frames() -> None: result = subprocess.run( [sys.executable, "-m", "pyflow", "record"], diff --git a/tests/unit/test_check_stages.py b/tests/unit/test_check_stages.py index 9c940e6..6a75ed7 100644 --- a/tests/unit/test_check_stages.py +++ b/tests/unit/test_check_stages.py @@ -337,6 +337,27 @@ def test_the_real_roadmap_has_the_shape_it_declares() -> None: assert problems == [] +def test_the_real_roadmap_reports_stage_0_as_complete() -> None: + """Stage 0's own eleven tasks predate the per-task inline + `**Status: Done**` marker every later stage's task entries carry -- + they used to record completion only in the stage's own summary + table, never in a form `TASK_DONE` matches, so `Stage.lifecycle` + read `opened` with `done_tasks=0` despite every task having long + been finished. Fixed by adding the marker to each of the eleven + task entries (2026-09-09), using the date already recorded in that + same summary table -- not by a stage-specific exemption in this + checker, so the general mechanism stays the only mechanism. Reads + the committed roadmap, so a failure here means the roadmap + regressed, not that this rule is broken. + """ + stages = parse_stages(ROADMAP_PATH.read_text(encoding="utf-8")) + stage_0 = next(s for s in stages if s.number == 0) + assert stage_0.lifecycle == "complete", ( + f"Stage 0: lifecycle is {stage_0.lifecycle!r} " + f"({stage_0.done_tasks}/{len(stage_0.task_lines)} tasks read as Done)" + ) + + def test_every_declared_rule_id_is_covered_by_a_test_in_this_module() -> None: """The guard against this module and the shape file drifting apart. diff --git a/tests/unit/test_checkpoint.py b/tests/unit/test_checkpoint.py index 4145568..06b561a 100644 --- a/tests/unit/test_checkpoint.py +++ b/tests/unit/test_checkpoint.py @@ -14,6 +14,7 @@ from pyflow.checkpoint import ( UnsupportedCheckpointVersionError, + list_checkpoints, read_checkpoint, write_checkpoint, ) @@ -77,6 +78,37 @@ def test_read_checkpoint_rejects_unknown_schema_version(tmp_path: Path) -> None: read_checkpoint(path) +def test_list_checkpoints_finds_every_checkpoint_file_by_frame_number(tmp_path: Path) -> None: + """Factored out of `replay.py`'s own private glob/regex (TASK-046) + so `recording.py`'s new retention pruning (TASK-049) and + `replay.find_checkpoint_at_or_before` read one implementation of + "what checkpoints exist here", not two that could drift apart. + """ + config = _non_default_config() + for frame in (0, 5, 10): + mesh = StructuredCartesianMesh.from_config(config.mesh) + field = ScalarField(mesh, "smoke", initial_value=lambda x, y: 1.0) + write_checkpoint( + tmp_path / f"checkpoint_{frame:08d}.pt", + frame_count=frame, + config=config, + fields={"smoke": field}, + ) + (tmp_path / "not_a_checkpoint.pt").write_text("ignore me") + + found = list_checkpoints(tmp_path) + + assert sorted(found) == [ + (0, tmp_path / "checkpoint_00000000.pt"), + (5, tmp_path / "checkpoint_00000005.pt"), + (10, tmp_path / "checkpoint_00000010.pt"), + ] + + +def test_list_checkpoints_is_empty_for_a_directory_with_none(tmp_path: Path) -> None: + assert list_checkpoints(tmp_path) == [] + + def test_restore_simulation_state_reconstructs_a_resumable_state_for_a_passive_config( tmp_path: Path, ) -> None: diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 8d667de..3adff19 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -59,6 +59,7 @@ def test_defaults_are_valid() -> None: assert config.units.time_scale == 1.0 assert config.recording.output_dir == "checkpoints" assert config.recording.checkpoint_interval == 100 + assert config.recording.max_checkpoints_retained is None for boundary_name in ("north", "south", "east", "west"): face = getattr(config.numerics.boundary_conditions, boundary_name) assert face.type == "dirichlet" @@ -978,6 +979,23 @@ def test_load_config_rejects_empty_output_dir(tmp_path: Path) -> None: load_config(config_file) +def test_load_config_reads_max_checkpoints_retained(tmp_path: Path) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text("recording:\n max_checkpoints_retained: 5\n") + + config = load_config(config_file) + + assert config.recording.max_checkpoints_retained == 5 + + +def test_load_config_rejects_non_positive_max_checkpoints_retained(tmp_path: Path) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text("recording:\n max_checkpoints_retained: 0\n") + + with pytest.raises(ValueError, match="recording.max_checkpoints_retained"): + load_config(config_file) + + def test_config_from_dict_round_trips_a_non_default_config(tmp_path: Path) -> None: """The read direction of `dataclasses.asdict(config)` -- the shape a checkpoint's own embedded config is stored as (`pyflow.checkpoint`, diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index e265272..f0e158e 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -146,11 +146,17 @@ def test_record_dispatches_to_record_with_parsed_args() -> None: "out", "--checkpoint-interval", "5", + "--max-checkpoints-retained", + "2", ] ) mock_record.assert_called_once_with( - Path("some-config.yaml"), max_frames=10, output_dir=Path("out"), checkpoint_interval=5 + Path("some-config.yaml"), + max_frames=10, + output_dir=Path("out"), + checkpoint_interval=5, + max_checkpoints_retained=2, ) @@ -162,7 +168,11 @@ def test_record_output_dir_and_checkpoint_interval_default_to_none( main(["record", "--config", "some-config.yaml", "--max-frames", "3"]) mock_record.assert_called_once_with( - Path("some-config.yaml"), max_frames=3, output_dir=None, checkpoint_interval=None + Path("some-config.yaml"), + max_frames=3, + output_dir=None, + checkpoint_interval=None, + max_checkpoints_retained=None, ) @@ -208,6 +218,8 @@ def test_resume_dispatches_to_resume_with_parsed_args() -> None: "out", "--checkpoint-interval", "3", + "--max-checkpoints-retained", + "2", ] ) @@ -217,6 +229,7 @@ def test_resume_dispatches_to_resume_with_parsed_args() -> None: max_frames=12, output_dir=Path("out"), checkpoint_interval=3, + max_checkpoints_retained=2, ) @@ -231,6 +244,7 @@ def test_resume_output_dir_and_checkpoint_interval_default_to_none() -> None: max_frames=9, output_dir=None, checkpoint_interval=None, + max_checkpoints_retained=None, ) @@ -249,6 +263,7 @@ def test_resume_dispatches_with_config_instead_of_checkpoint() -> None: max_frames=9, output_dir=Path("out"), checkpoint_interval=None, + max_checkpoints_retained=None, ) @@ -482,6 +497,7 @@ def test_generate_config_with_no_output_prints_to_stdout( "recording": { "output_dir": "checkpoints", "checkpoint_interval": 100, + "max_checkpoints_retained": None, }, } diff --git a/tests/unit/test_recording.py b/tests/unit/test_recording.py index 2695aff..23a2ee3 100644 --- a/tests/unit/test_recording.py +++ b/tests/unit/test_recording.py @@ -137,6 +137,102 @@ def test_record_falls_back_to_config_recording_section_when_not_overridden( assert result.checkpoint_frames == [0, 4, 8] +# -- retention (TASK-049, Stage 8 reopening, added 2026-09-09) ----------- + + +def _checkpoint_frames_on_disk(output_dir: Path) -> set[int]: + return {int(p.stem.removeprefix("checkpoint_")) for p in output_dir.glob("checkpoint_*.pt")} + + +def test_record_without_a_cap_keeps_every_checkpoint(tmp_path: Path) -> None: + """Unset (the default) changes nothing for an existing config -- no + pruning at all, exactly today's behaviour. + """ + config_file = tmp_path / "config.yaml" + config_file.write_text(_DECLARED_FIELD_CONFIG) + output_dir = tmp_path / "checkpoints" + + record(config_file, max_frames=20, output_dir=output_dir, checkpoint_interval=5) + + assert _checkpoint_frames_on_disk(output_dir) == {0, 5, 10, 15, 20} + + +def test_record_prunes_checkpoints_beyond_the_retention_cap(tmp_path: Path) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text(_DECLARED_FIELD_CONFIG) + output_dir = tmp_path / "checkpoints" + + record( + config_file, + max_frames=20, + output_dir=output_dir, + checkpoint_interval=5, + max_checkpoints_retained=2, + ) + + # Frame 0 plus the newest two non-zero checkpoints (15, 20) -- 5 and + # 10 pruned. + assert _checkpoint_frames_on_disk(output_dir) == {0, 15, 20} + + +def test_record_retention_cap_never_prunes_frame_zero(tmp_path: Path) -> None: + """Frame 0 is excluded from the retention *count* itself, not merely + old enough to survive by coincidence: with five non-zero checkpoints + and a cap of five, "keep the newest N files overall" (the wrong + reading) would drop frame 0 -- the oldest of six -- while excluding + it from the count keeps all six. Chosen so a broken implementation + and the correct one disagree, not just so frame 0 happens to survive + either way. + """ + config_file = tmp_path / "config.yaml" + config_file.write_text(_DECLARED_FIELD_CONFIG) + output_dir = tmp_path / "checkpoints" + + record( + config_file, + max_frames=25, + output_dir=output_dir, + checkpoint_interval=5, + max_checkpoints_retained=5, + ) + + assert _checkpoint_frames_on_disk(output_dir) == {0, 5, 10, 15, 20, 25} + + +def test_resume_prunes_across_the_whole_directory_not_only_what_it_wrote( + tmp_path: Path, +) -> None: + """The cap applies to everything on disk, including checkpoints an + earlier `record` call wrote -- not only the frames this particular + `resume` call writes. + """ + config_file = tmp_path / "config.yaml" + config_file.write_text(_DECLARED_FIELD_CONFIG) + output_dir = tmp_path / "checkpoints" + record(config_file, max_frames=10, output_dir=output_dir, checkpoint_interval=5) + + resume( + output_dir / "checkpoint_00000010.pt", + max_frames=20, + checkpoint_interval=5, + max_checkpoints_retained=2, + ) + + assert _checkpoint_frames_on_disk(output_dir) == {0, 15, 20} + + +def test_load_config_max_checkpoints_retained_is_used_when_not_overridden( + tmp_path: Path, +) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text(_DECLARED_FIELD_CONFIG + "\nrecording:\n max_checkpoints_retained: 1\n") + output_dir = tmp_path / "checkpoints" + + record(config_file, max_frames=15, output_dir=output_dir, checkpoint_interval=5) + + assert _checkpoint_frames_on_disk(output_dir) == {0, 15} + + # -- resume (extends TASK-045's own recording -- not replay or playback) -- diff --git a/tools/generators/generate_config_template.py b/tools/generators/generate_config_template.py index eac4186..dc453af 100644 --- a/tools/generators/generate_config_template.py +++ b/tools/generators/generate_config_template.py @@ -416,6 +416,13 @@ "the run's final frame too, regardless of this value). Invalid: " "zero or negative." ), + "recording.max_checkpoints_retained": ( + "Valid: null (default, unbounded -- every checkpoint is kept), " + "or a positive integer -- the newest N non-zero checkpoints are " + "kept, older ones deleted as new ones are written. Frame 0 is " + "never deleted, whatever this value is. Invalid: zero or " + "negative." + ), }