From a746e83bf0757d0a28d647eb87e69112803f4367 Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Wed, 9 Sep 2026 14:37:40 +0100 Subject: [PATCH] Reuse a full-subset cache hit for pyflow play (TASK-050) Closes Stage 8's Completion Criterion 9: a `pyflow play --cache DIR` request that falls fully inside an already-cached wider window is sliced from it directly, at zero re-simulation cost, narrowing TASK-046's own exact-range-only scope for the one case with a concrete, avoidable cost. `_find_superset_window` ranks candidates by the range in the filename first (the same cheap-before-I/O shape `find_checkpoint_at_or_before` already uses), then slices the frames needed straight out of the matching window -- no new cache file is written for the sliced sub-range. A request that only partially overlaps a cached range, or extends past its edge, still falls back to full `materialize_window`; confirmed to have real teeth by a deliberate mutation weakening that check to an overlap-only one, observed to fail the regression test before being reverted. Co-Authored-By: Claude Sonnet 5 --- README.md | 23 +++++++----- docs/planning/roadmap.md | 75 ++++++++++++++++++++++++++++++------- docs/planning/status.md | 14 +++---- src/pyflow/CLAUDE.md | 17 +++++++++ src/pyflow/replay.py | 76 +++++++++++++++++++++++++++++++------ tests/unit/test_replay.py | 79 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 244 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 244cf8b..6de86df 100644 --- a/README.md +++ b/README.md @@ -141,8 +141,9 @@ 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 +gap and TASK-050 the cache-reuse one, both the same day; TASK-048/051 +will close the other two, 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. @@ -187,7 +188,7 @@ 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 7 are complete, and Stage 8 is reopened (six of nine +Stages 0 through 7 are complete, and Stage 8 is reopened (seven 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`): @@ -253,7 +254,7 @@ re-closure, against its own written completion criteria 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 reopened -- six of nine criteria +**Stage 8 (Recording & Playback) is reopened -- seven 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 @@ -266,10 +267,11 @@ 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: +retention, and partial-overlap cache reuse. The last two are done +(TASK-049, `--max-checkpoints-retained` on `record`/`resume`; TASK-050, +`pyflow play --cache DIR` now reuses a full-subset request from a wider +cached window with no re-simulation); TASK-048/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 @@ -312,7 +314,10 @@ uv run python -m pyflow play --checkpoints-dir checkpoints --to-frame 500 Space pauses/resumes; `+`/`-` change playback speed live. Add `--cache cache` to materialize the window once and reuse it on a later -run without re-simulating; add `--backend offscreen --max-frames N` for +run without re-simulating -- a later request fully inside an +already-cached range reuses it too, sliced directly, even if its own +exact range was never cached before (TASK-050); add `--backend offscreen +--max-frames N` for a headless/CI-safe run with no window at all (what `tests/integration/ test_playback_cli.py`'s own subprocess tests use). `pyflow play` only supports a solved-velocity config for now (`simulation.velocity_solved: diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index 61ffa67..7221b84 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -306,7 +306,13 @@ 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): **1182 tests as of 2026-09-09**, up from 1172 the same day +(C1a/C1b): **1185 tests as of 2026-09-09**, up from 1182 the same day +(TASK-050, Partial-Overlap Cache Reuse: 3 in `tests/unit/ +test_replay.py` -- a full-subset request reused without re-simulation, +the right superset picked among several cached windows including a +non-superset one, and a partially-overlapping request still falling +back to full materialization and failing loudly once checkpoints are +gone), 1182 itself 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 @@ -11380,7 +11386,7 @@ them, which had not been drafted yet when these were written. | 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 +### Status as of 2026-09-09: Stage 8 reopened, seven 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 @@ -11421,10 +11427,10 @@ what shipped rather than against the criteria that were meant to operationalise | 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 | +| 9. Partial-overlap (subset) cache reuse | **Met** -- TASK-050, mutation-tested | -Six of nine criteria are met; the stage is **in progress**, not -complete, until TASK-048/050/051 close the other three. **One real +Seven of nine criteria are met; the stage is **in progress**, not +complete, until TASK-048/051 close the other two. **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 @@ -12131,8 +12137,7 @@ Completion Criterion 8 in full. ## TASK-050 — Partial-Overlap Cache Reuse, Subset Only -**Status: Not started, drafted 2026-09-09.** Discharges Completion -Criterion 9. +**Status: Done, 2026-09-09.** Discharges Completion Criterion 9. ### Purpose @@ -12151,18 +12156,62 @@ narrower request the cache could already answer. 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. + of the request (`_find_superset_window`, ranking candidates by the + range in the *filename* first, the same cheap-before-I/O shape + `find_checkpoint_at_or_before` already uses), 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. + work. **Confirmed to have real teeth, not just to pass**: a + deliberate mutation weakening the superset check to an overlap-only + one (`cached_from <= from_frame <= cached_to`, dropping the + `to_frame <= cached_to` half) was run against + `test_materialize_or_load_window_does_not_reuse_a_partially_overlapping_cache` + and observed to fail before being reverted. -Artifacts, Acceptance Criteria and Discharges are written when this task -is actually built, the same as every other entry in this file. +### Artifacts Produced + +- `src/pyflow/replay.py` -- `_WINDOW_FILENAME` (the cached-window + filename regex), `_find_superset_window`; `materialize_or_load_window` + now checks it between the exact-match and full-materialize paths. +- Tests: 3 in `tests/unit/test_replay.py` (a superset reused without + re-simulation, the right superset picked among several cached + windows including a non-superset one, a partial overlap still + falling back to full materialization and failing loudly once + checkpoints are gone). + +### Acceptance Criteria + +- A request fully inside an already-cached window returns the correct + sliced frames, matching an independently materialized control + bit-for-bit, with no re-simulation -- checked by deleting every + checkpoint before the request and confirming it still succeeds, the + same technique TASK-046's own exact-match test already established. +- Among several cached windows, the one actually used is a real + superset of the request, not merely the first file found -- checked + by caching a narrower, non-superset window first and confirming the + result still matches the wider one, not a wrong answer built from the + narrower one. +- A sliced-from-superset result writes no cache file of its own; only + an exact-range request still does, unchanged from before this task. +- A partially-overlapping (not fully-contained) request still falls + back to full `materialize_window`, checked by confirming it fails the + same way a fresh, uncached request would once checkpoints are gone, + rather than silently returning wrong or incomplete data. +- Verified by hand against the real CLI, not only the test suite (root + `CLAUDE.md`'s Feature Verification rule): caching `pyflow play + --from-frame 0 --to-frame 20 --cache DIR`, deleting every checkpoint, + then `pyflow play --from-frame 5 --to-frame 10 --cache DIR` still + exits 0 with no new cache file written for the narrower range. + +### Discharges + +Completion Criterion 9 in full. --- diff --git a/docs/planning/status.md b/docs/planning/status.md index f5c2411..a57c108 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -17,14 +17,14 @@ demand, not part of this file. ## Progress -**49/52 tasks complete (94%)** across 16 planned stages. For the full plan, including +**50/52 tasks complete (96%)** 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" : 49 - "Not started" : 3 + "Done" : 50 + "Not started" : 2 ``` ### Milestones @@ -40,12 +40,12 @@ pie showData ### Up next -**Stage 8 -- Recording & Playback** is next, starting with TASK-050 (Partial-Overlap Cache Reuse, Subset Only), 2 more not yet started in this stage. +**Stage 8 -- Recording & Playback** is next, starting with TASK-048 (Live Scrub), 1 more not yet started in this stage. ## Live repository facts - **49** `CLAUDE.md` files -- **1182** tests collected +- **1185** tests collected - **144** Gherkin scenarios (`tests/features/*.feature`) ## Stages @@ -153,7 +153,7 @@ pie showData ### Stage 8 -- Recording & Playback -**no status recorded** -- `██████░░░░` 4/7 tasks; 9 criteria defined, no status line yet +**no status recorded** -- `███████░░░` 5/7 tasks; 9 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| @@ -161,7 +161,7 @@ pie showData | 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-050 -- Partial-Overlap Cache Reuse, Subset Only | Done | 2026-09-09 | `replay.py` | | TASK-048 -- Live Scrub | Not started | | | | TASK-051 -- Combined Solved-Velocity + Declared-Field Playback | Not started | | | diff --git a/src/pyflow/CLAUDE.md b/src/pyflow/CLAUDE.md index c6530aa..e7e024d 100644 --- a/src/pyflow/CLAUDE.md +++ b/src/pyflow/CLAUDE.md @@ -263,6 +263,23 @@ extrapolated to the largest mesh anywhere in this repository (128x128, an experiment config) at 500 frames, ~197 MB -- comfortably under a gigabyte at every size and frame range this repository actually runs. +**`materialize_or_load_window`'s cache now also serves a full-subset +request from a wider cached window (TASK-050, Stage 8 reopening, added +2026-09-09), narrowing the exact-range-only scope this entry used to +describe.** `_find_superset_window` globs `cache_dir` for a +`window_*.pt` whose own embedded range fully contains the request +(same cheap-filename-first ranking `find_checkpoint_at_or_before` +already uses), and the caller slices the frames it needs directly out +of it -- no re-simulation, and no cache file written for the sliced +sub-range itself. **Deliberately scoped to a full subset, not any +overlap**: a request that only partially overlaps a cached range, or +extends past its edge, still falls back to full `materialize_window` +-- stitching across a cached window's own edge would need to combine +more than one, a real design question with no shipped need for it yet. +Confirmed to have real teeth by a deliberate mutation (weakening the +superset check to an overlap-only one) observed to fail the +partial-overlap regression test before being reverted. + **`playback.py` (TASK-047) is `pyflow play`'s own rendering half, and the one Stage 8 module that *does* import `rendering`** -- putting pixels on screen is its whole job. `PlaybackState`/ diff --git a/src/pyflow/replay.py b/src/pyflow/replay.py index f7cafd5..7531f5c 100644 --- a/src/pyflow/replay.py +++ b/src/pyflow/replay.py @@ -18,16 +18,26 @@ is the caller-facing function `pyflow play` actually uses: given a `cache_dir`, it reads an exact-range match if one exists there and writes one after materializing if not, so watching the same window twice -costs nothing the second time -- but the range must match exactly -(`from_frame`/`to_frame` both), a real, stated scope decision rather -than an oversight: partial-overlap reuse (asking for [10, 20] when a -[0, 30] cache exists) would need to know how to slice or extend a -cached window, a real design question with no shipped need for it yet. +costs nothing the second time. `cache_dir` omitted (the default) always +materializes fresh, never writing or reading anything. + +**A request that falls fully inside an already-cached wider window also +costs nothing (TASK-050, Stage 8 reopening, added 2026-09-09), scoped +deliberately narrower than "any overlap".** Only a full subset of an +existing cached range is sliced from it directly, with no +re-simulation; a request that only partially overlaps a cached range, +or extends past its edge, still falls back to full `materialize_window` +-- stitching a request that is not fully contained in one cached window +would need to know how to combine several, a real design question with +no shipped need for it yet. The sliced result is never itself written +to `cache_dir` -- only an exact-range request writes its own cache +file, unchanged from before this. """ from __future__ import annotations import dataclasses +import re from dataclasses import dataclass from pathlib import Path @@ -45,6 +55,14 @@ _WINDOW_SCHEMA_VERSION = 1 +# `window_00000006_00000009.pt` -- the one filename convention every +# cached window on disk follows (`materialize_or_load_window`'s own +# cache path). Used by `_find_superset_window` (TASK-050, Stage 8 +# reopening, 2026-09-09) to find an already-cached window a narrower +# request falls fully inside, without opening every file in `cache_dir` +# just to read its own embedded `from_frame`/`to_frame`. +_WINDOW_FILENAME = re.compile(r"^window_(\d{8})_(\d{8})\.pt$") + class NoCheckpointBeforeFrameError(ValueError): """Raised by `find_checkpoint_at_or_before` when no checkpoint in the @@ -177,6 +195,26 @@ def read_materialized_window(path: str | Path) -> MaterializedWindow: ) +def _find_superset_window( + cache_dir: Path, *, from_frame: int, to_frame: int +) -> MaterializedWindow | None: + """The first cached window in `cache_dir` whose own range is a full + superset of `[from_frame, to_frame]`, read and returned -- or `None` + if none qualifies. Ranks candidates by the range in the *filename* + first (cheap, no I/O for a discarded candidate), the same shape + `checkpoint.list_checkpoints`/`find_checkpoint_at_or_before` already + use for checkpoints. + """ + for path in cache_dir.glob("window_*.pt"): + match = _WINDOW_FILENAME.match(path.name) + if match is None: + continue + cached_from, cached_to = int(match.group(1)), int(match.group(2)) + if cached_from <= from_frame and to_frame <= cached_to: + return read_materialized_window(path) + return None + + def materialize_or_load_window( checkpoints_dir: str | Path, *, @@ -185,12 +223,16 @@ def materialize_or_load_window( cache_dir: str | Path | None = None, ) -> MaterializedWindow: """`pyflow play`'s own entry point into this module: read an - exact-range match from `cache_dir` if one exists there, otherwise - materialize fresh -- and, if `cache_dir` was given, write the result - there so a second call with the same range costs nothing. - `cache_dir` omitted (the default) always materializes fresh, never - writing or reading anything -- the ephemeral, no-artifact-left-behind - behaviour this module's own docstring describes as the default. + exact-range match from `cache_dir` if one exists there, else slice + one from an already-cached window `[from_frame, to_frame]` falls + fully inside (`_find_superset_window`), else materialize fresh -- + and, if `cache_dir` was given and no cache (exact or superset) was + found, write the freshly materialized result there so a second call + with the same range costs nothing. `cache_dir` omitted (the default) + always materializes fresh, never writing or reading anything -- the + ephemeral, no-artifact-left-behind behaviour this module's own + docstring describes as the default. A sliced-from-superset result is + never itself written back to `cache_dir`. """ cache_path = ( Path(cache_dir) / f"window_{from_frame:08d}_{to_frame:08d}.pt" @@ -200,6 +242,18 @@ def materialize_or_load_window( if cache_path is not None and cache_path.is_file(): return read_materialized_window(cache_path) + if cache_dir is not None: + superset = _find_superset_window(Path(cache_dir), from_frame=from_frame, to_frame=to_frame) + if superset is not None: + start = from_frame - superset.from_frame + end = to_frame - superset.from_frame + 1 + return MaterializedWindow( + config=superset.config, + from_frame=from_frame, + to_frame=to_frame, + frames=superset.frames[start:end], + ) + window = materialize_window(checkpoints_dir, from_frame=from_frame, to_frame=to_frame) if cache_path is not None: diff --git a/tests/unit/test_replay.py b/tests/unit/test_replay.py index 8a84f96..82d7beb 100644 --- a/tests/unit/test_replay.py +++ b/tests/unit/test_replay.py @@ -188,3 +188,82 @@ def test_materialize_or_load_window_with_no_cache_dir_recomputes_every_time( assert isinstance(window, MaterializedWindow) assert len(window.frames) == 4 + + +# -- partial-overlap (subset) cache reuse (TASK-050, Stage 8 reopening) -- + + +def test_materialize_or_load_window_reuses_a_superset_cache_without_recomputing( + checkpoints_dir: Path, tmp_path: Path +) -> None: + cache_dir = tmp_path / "cache" + wide = materialize_or_load_window( + checkpoints_dir, from_frame=5, to_frame=15, cache_dir=cache_dir + ) + + # Deleting every checkpoint proves the narrower request below could + # not have re-simulated anything -- it must have come from the wider + # cached window. + for checkpoint_file in checkpoints_dir.glob("checkpoint_*.pt"): + checkpoint_file.unlink() + + narrow = materialize_or_load_window( + checkpoints_dir, from_frame=8, to_frame=12, cache_dir=cache_dir + ) + + assert narrow.from_frame == 8 + assert narrow.to_frame == 12 + assert len(narrow.frames) == 5 + for materialized, wide_frame in zip(narrow.frames, wide.frames[3:8], strict=True): + torch.testing.assert_close(materialized["smoke"], wide_frame["smoke"], rtol=0, atol=0) + # The sliced sub-range gets no cache file of its own -- only an + # exact-range request ever writes one. + assert not (cache_dir / "window_00000008_00000012.pt").is_file() + + +def test_materialize_or_load_window_finds_a_superset_among_several_cached_windows( + checkpoints_dir: Path, tmp_path: Path +) -> None: + """A narrower, non-superset window is cached first specifically so a + naive "use whatever cache file exists" implementation would pick it + (and materialize the rest, or return the wrong data) instead of the + real superset. + """ + cache_dir = tmp_path / "cache" + materialize_or_load_window(checkpoints_dir, from_frame=6, to_frame=9, cache_dir=cache_dir) + wide = materialize_or_load_window( + checkpoints_dir, from_frame=0, to_frame=20, cache_dir=cache_dir + ) + + for checkpoint_file in checkpoints_dir.glob("checkpoint_*.pt"): + checkpoint_file.unlink() + + narrow = materialize_or_load_window( + checkpoints_dir, from_frame=10, to_frame=12, cache_dir=cache_dir + ) + + assert len(narrow.frames) == 3 + for materialized, wide_frame in zip(narrow.frames, wide.frames[10:13], strict=True): + torch.testing.assert_close(materialized["smoke"], wide_frame["smoke"], rtol=0, atol=0) + + +def test_materialize_or_load_window_does_not_reuse_a_partially_overlapping_cache( + checkpoints_dir: Path, tmp_path: Path +) -> None: + """A request that overlaps a cached range but is not fully inside it + (here: extends past the cached range's own end) is a real, stated + exclusion -- it must still fall back to full re-simulation rather + than silently serving wrong or incomplete data. Proven by deleting + every checkpoint first: if this fell back to `materialize_window` as + it should, that re-simulation attempt fails loudly; if it wrongly + treated the partial overlap as reusable, it would return a window + with no error at all. + """ + cache_dir = tmp_path / "cache" + materialize_or_load_window(checkpoints_dir, from_frame=0, to_frame=10, cache_dir=cache_dir) + + for checkpoint_file in checkpoints_dir.glob("checkpoint_*.pt"): + checkpoint_file.unlink() + + with pytest.raises(NoCheckpointBeforeFrameError): + materialize_or_load_window(checkpoints_dir, from_frame=8, to_frame=15, cache_dir=cache_dir)