From 3070e5220e8405522bc1738f591bd8249fcbc084 Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Fri, 28 Aug 2026 21:00:39 +0100 Subject: [PATCH 1/6] Open Stage 5 with real completion criteria, two decisions, four open questions Stage 5 (First Fluid Solver) completion criteria, written before its first task per docs/practices.md's own rule -- the sixth stage in a row. Eleven criteria covering the stage's goal rather than the union of its tasks': velocity transported by the same mechanism as every other field (checked structurally, not only behaviourally), pressure solved from the constraint rather than transported (with explicit null-space removal), divergence decreasing monotonically per corrector iteration, a timestep solving momentum and continuity together with a null test the profile comparisons cannot substitute for, physical correctness per case, and the stage-level exit obligations TASK-034 owns. This stage defines the MVP, so its criteria are reconciled item by item against docs/implementation/mvp.md's own Definition of Done (Criterion 11) -- a condition the Stage 5 section has carried since 2026-08-22. Doing it rather than assuming the documents agreed found two conflicts, both decided by the maintainer: - mvp.md requires the MVP to reproduce heat diffusion, while implementation-plan.md and demos.yaml both placed the Heat Diffusion demo at Capability Level 3 -- one stage after the MVP. Decided: Stage 5 owes it, as a transported scalar (no Temperature field needed). Stage 6's TASK-035 keeps the named-Temperature version, which is a different claim. mvp.md, implementation-plan.md (Levels 2 and 3 and the Golden Demos table), demos.yaml, the roadmap's own Stage 6 demo list and the backlog were all amended in the same change. - ADR-007's illustrative "matches Ghia et al. within 2%" is not this stage's bar. Reynolds number 100 is adopted; the tolerance is not. The MVP's advection scheme is first-order upwind, whose numerical diffusion dominates at MVP mesh resolutions, so a fixed 2% would be a bar the MVP's own numerics are not built to clear -- and a criterion that can only be met by loosening its own number later is not a criterion. Criterion 5 requires convergence instead: error against Ghia's tabulated profiles decreasing monotonically across at least three mesh resolutions, plus the qualitative structure at the finest. Four design questions are recorded open, following Stage 4's precedent of flagging them the day the criteria are drafted. Two are gaps the code makes concrete rather than speculation: - A VectorField cannot go through the Stage 4 transport path at all. Both concrete schemes read a cell value with float(field.value_at(c)); handing either a VectorField raises "TypeError: float() argument must be a string or a real number, not 'tuple'". Verified by running it, not by reading the code. Criterion 1 cannot be satisfied until this is answered, and the three candidate answers differ by a whole Stage 3 interface change. - BoundaryFaceConfig.velocity is the boundary-normal component only, and both of this stage's validation cases need tangential wall values (a moving lid, a moving plate, no-slip walls). That field's docstring deferred this to "whichever task builds a concrete condition against a real consumer"; Stage 5 is that consumer. Two smaller things this drafting found and fixed at source: the backlog's conservation-checks item still read as though TASK-027 would discharge the divergence-free claim (it landed deliberately weaker; the strong claim is now Stage 5 Criterion 3, owned by TASK-033), and docs/references/papers.md had no entry for Ghia et al. at all -- three documents named the paper by author and year, none gave volume, issue or pages, and a completion criterion now depends on reading its tables. That file's scope was extended to cover papers a completion criterion is checked against, with docs/references/CLAUDE.md amended rather than the rule quietly broken. make ci clean: 614 tests, 99% coverage, all twelve gates, including the four Stage 5 feature files now listed in check_references.py's PLANNED table as checked promises. Co-Authored-By: Claude Opus 5 --- README.md | 10 +- docs/CHANGELOG-DESIGN.md | 97 ++++ docs/implementation/mvp.md | 15 + docs/planning/backlog.md | 56 +- docs/planning/implementation-plan.md | 51 +- docs/planning/roadmap.md | 764 ++++++++++++++++++++++++++- docs/planning/status.md | 2 +- docs/references/CLAUDE.md | 14 +- docs/references/papers.md | 30 ++ planning/data/demos.yaml | 17 +- tools/validators/check_references.py | 16 +- 11 files changed, 1033 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 233e0c0..84a9cb1 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,15 @@ completion criteria (`docs/planning/roadmap.md`): velocity/pressure solve -- that's Stage 5. Stage 5 will solve incompressible flow: a coupled velocity/pressure -system built from Stage 4's now-real numerical schemes. +system built from Stage 4's now-real numerical schemes. **Its completion +criteria were written on 2026-08-28, before its first task started** +(`docs/planning/status.md` has the live count), per the standing rule +every stage since Stage 2 has followed +-- including the reconciliation against `docs/implementation/mvp.md`'s +own Definition of Done that Stage 5 has owed since 2026-08-22, since +this is the stage that defines the MVP. Four design questions are +recorded there as open, two of them gaps the current code makes +concrete rather than speculation. Try the most recent demonstration -- a scalar blob advected and diffused across a periodic domain, stepped live: diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index 7d820db..8fc27d5 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -5833,3 +5833,100 @@ whole point of the scenario making the call. read off the actual failure output rather than predicted; the conftest-scoping claim confirmed by a throwaway probe module that was run and then deleted, not by reading documentation. + +### Stage 5 opened: eleven completion criteria, two maintainer decisions, four open design questions + +Written before TASK-031 starts, per `docs/practices.md`'s "A stage gets +completion criteria before its first task" -- the same point in the +cycle Stage 4's own criteria were written at, and the sixth stage in a +row the rule has been applied to. + +**The criteria are in `docs/planning/roadmap.md`, not restated here.** +What belongs in this log is the decisions taken to write them, and there +were two the maintainer made and four nobody can make yet. + +**Decision one: Stage 5 owes a Heat Diffusion demo, as a scalar.** +`docs/implementation/mvp.md`'s Validation section requires the MVP to +reproduce three cases -- passive scalar transport, heat diffusion, +lid-driven cavity -- while `docs/planning/implementation-plan.md` and +`planning/data/demos.yaml` both placed Heat Diffusion at Capability +Level 3, which is Stage 6, one stage after the MVP. Nobody had ever put +the two side by side; the divergence surfaced only because Stage 5's own +section has carried an instruction since 2026-08-22 that its criteria +and `mvp.md`'s Definition of Done "must be reconciled explicitly when +they are written, not assumed to agree." Decided in favour of `mvp.md`: +heat diffusion is the diffusion equation on a transported scalar, which +needs no Temperature field at all, so Stage 5 can run it. Stage 6's +TASK-035 then adds the named Temperature field with buoyancy coupling -- +a different claim, not this one repeated. `mvp.md`, +`implementation-plan.md` (Levels 2 and 3, and the Golden Demos table), +`planning/data/demos.yaml` and `docs/planning/backlog.md` were all +amended in the same change. + +**Decision two: the lid-driven cavity's criterion is convergence, not +Ghia et al.'s illustrative 2%.** Three documents point at Ghia, Ghia & +Shin (1982) as the obvious quantitative target, and +`implementation-plan.md` Level 2 asked whoever drafted these criteria to +reach for it rather than invent a fresh reference. It does -- Reynolds +number 100 is adopted. The tolerance is not: `adr/ADR-007`'s "within 2%" +appears in a worked example of what an executable physics criterion +*looks like*, and PyFlow's MVP advection scheme is first-order upwind, +whose numerical diffusion at MVP mesh resolutions is the dominant error +term. A fixed 2% would have been a bar the MVP's own documented numerics +are not built to clear, and a criterion that can only be met by quietly +loosening its own number later is not a criterion. The criterion instead +requires the error against Ghia's tabulated profiles to *decrease +monotonically across at least three mesh resolutions*, plus the +qualitative structure (primary vortex centre, both secondary corner +vortices) at the finest, with the absolute tolerance stated and defended +in the feature file against the mesh actually used. + +**Four design questions, recorded open rather than answered.** Stage 4 +set the precedent of flagging these on the day the criteria are drafted +and resolving each before the task that needs it starts; two of Stage +5's four are gaps the code makes concrete, not speculation: + +- **A vector field cannot go through the Stage 4 transport path at + all.** `AdvectionScheme.flux`/`DiffusionScheme.flux` return one value + per face, and both concrete schemes read a cell value with + `float(field.value_at(cell))`. Verified directly rather than reasoned + about: handing either a `VectorField` raises `TypeError: float() + argument must be a string or a real number, not 'tuple'`. Criterion 1 + ("velocity is transported by the same mechanism every other field is") + cannot be satisfied until this is answered, and the three candidate + answers differ by a whole Stage 3 interface change. +- **`BoundaryFaceConfig.velocity` is the boundary-normal component + only**, and both of this stage's validation cases need a *tangential* + wall value -- a moving lid, a moving plate, and no-slip walls + everywhere else. That field's own docstring deferred this "to whichever + task builds a concrete condition against a real consumer"; Stage 5 is + that consumer. +- **What carries the momentum-equation coefficients TASK-033 needs**, the + question TASK-027 hit experimentally and deferred by name. Flagged as + the one most likely to move this stage's scope, and the one to resolve + with numerical prototyping *before* Criterion 3 becomes a feature file. +- **How a Reynolds number is configured at all**, given + `NumericsConfig.diffusion_coefficient` is one global constant serving + the single scalar Stage 4 had, and viscosity is not a scalar's + diffusivity. + +**Two smaller things this drafting found and fixed at source.** +`docs/planning/backlog.md`'s conservation-checks item still read as +though TASK-027 would discharge the divergence-free claim; it landed +with a deliberately weaker one, and the strong claim is now Stage 5 +Criterion 3 owned by TASK-033 -- corrected where it was stated, not only +where it was moved to. And `docs/references/papers.md` had no entry for +Ghia et al. at all: three documents named the paper by author and year, +none gave volume, issue or pages, and a completion criterion now depends +on reading its tables. That file's scope ("what `docs/handbook/` cites") +was extended to cover papers a completion criterion is checked against, +with `docs/references/CLAUDE.md` amended in the same change rather than +the rule quietly broken. + +- *Verified by:* `make ci` clean on this branch; `make check-references` + reporting the four planned Stage 5 feature files as promises rather + than errors; `make check-graph` clean after the `demos.yaml` edit; + `make status-report` regenerated, showing Stage 5 with 11 criteria + defined and no status line yet. The `TypeError` above was produced by + running both schemes against a real `VectorField` on a real mesh, not + inferred from reading the code. diff --git a/docs/implementation/mvp.md b/docs/implementation/mvp.md index e5c9b3b..3c91000 100644 --- a/docs/implementation/mvp.md +++ b/docs/implementation/mvp.md @@ -61,6 +61,21 @@ The MVP shall successfully reproduce: - Heat diffusion. - Lid-driven cavity flow. +**"Heat diffusion" here means the diffusion equation on a transported +scalar, not a named Temperature field** (recorded 2026-08-28, +maintainer's call). The distinction was never stated, and the project +had drifted on it: `docs/planning/implementation-plan.md` and +`planning/data/demos.yaml` both placed the Heat Diffusion demo at +Capability Level 3 -- Stage 6, one stage *after* the MVP -- while this +document required the MVP to reproduce it. Found by the reconciliation +`docs/planning/roadmap.md`'s Stage 5 Completion Criterion 11 requires +between this document and that stage's criteria, and decided in favour +of this document: heat diffusion is the diffusion equation with a +different name on the field, so PyFlow can run it with no Temperature +field at all. Stage 6's TASK-035 then adds the named field with +buoyancy coupling, which is a different claim rather than this one +repeated. Both other documents were amended in the same change. + ## Definition of Done - Simulation runs end-to-end. diff --git a/docs/planning/backlog.md b/docs/planning/backlog.md index 1f27562..6a82109 100644 --- a/docs/planning/backlog.md +++ b/docs/planning/backlog.md @@ -1961,6 +1961,20 @@ here.): prescribed-velocity boundaries must satisfy global mass conservation as a precondition for the system to be solvable at all -- currently a documented requirement, not yet a test. + + **Half-closed 2026-08-27, and the other half reassigned + 2026-08-28.** TASK-027 landed a single correction pass whose + criterion is a *measured, bounded reduction* in divergence, + checked in isolation -- not divergence-free "after every + correction step", which that task verified numerically it could + not deliver on a collocated mesh without momentum-equation + coefficients no Stage 4 interface could supply (its own Design + decision Two). The strong claim this bullet states is now **Stage + 5 Completion Criterion 3, owned by TASK-033**, where a real + velocity and pressure field finally exist to iterate against. + The boundary-condition precondition named above is separately + closed (the next bullet); what remains open here is the loop's + own convergence, and it is scheduled rather than unassigned. - **Boundary conditions** -- **done, not Stage 4 -- closed early by TASK-019 (Stage 3, 2026-08-23)**, found while drafting Stage 4's own Completion Criteria (2026-08-25): the global mass- @@ -2066,6 +2080,23 @@ here.): exist yet and won't until Stage 5 opens. `implementation-plan.md` Level 2 now says so explicitly, so whoever drafts them reaches for Ghia et al. rather than inventing a fresh reference. + + **Closed 2026-08-28, when Stage 5's Completion Criteria were + drafted.** They did reach for Ghia et al., and the two halves of + this gap were answered differently, which is why this bullet + records both rather than "done": Reynolds number 100 was + adopted; **the illustrative 2% was rejected** (maintainer's call). + The MVP's advection scheme is first-order upwind, whose numerical + diffusion at MVP mesh resolutions is the dominant error term, so + a fixed 2% would have been a bar the MVP's own documented + numerics are not built to clear -- and a criterion that can only + be met by quietly loosening its own number later is not a + criterion. Stage 5 Completion Criterion 5 requires convergence + instead: error against Ghia's tabulated profiles decreasing + monotonically across at least three mesh resolutions, plus the + qualitative structure at the finest, with the absolute tolerance + stated and defended in the feature file. `implementation-plan.md` + Level 2 was amended in the same change. - **Flow Around Cylinder's von Kármán/Strouhal correlation was already flagged as unclaimed** in `implementation-plan.md` before this pass (2026-08-20) -- confirmed still accurate, @@ -2099,11 +2130,26 @@ here.): a future task that genuinely has no natural closed-form case. *Unblock condition:* the two closed gaps need no further action. - Ghia et al.'s tolerance activates when Stage 5's own Completion - Criteria are drafted; the cylinder correlation when Stage 8 gets - real task numbers; MMS itself stays open as a general-purpose - technique for whichever future task turns out to need it, TASK-024 - no longer being that task. + Ghia et al.'s tolerance activated on 2026-08-28 and is now Stage 5 + Completion Criterion 5, closed above -- as a convergence + requirement rather than the illustrative 2%. The cylinder + correlation activates when Stage 8 gets real task numbers; MMS + itself stays open as a general-purpose technique for whichever + future task turns out to need it, TASK-024 no longer being that + task. **This item's only remaining open thread is the cylinder + correlation**, which is why it is still unticked. + + **A third divergence, found 2026-08-28 by the same Stage 5 + drafting and closed in the same change:** `docs/implementation/ + mvp.md`'s Validation section requires the MVP to reproduce heat + diffusion, while `implementation-plan.md` and + `planning/data/demos.yaml` both placed the Heat Diffusion demo at + Capability Level 3 -- one stage after the MVP. Decided + (maintainer's call): Level 2 owes it, as a transported scalar + rather than a named Temperature field, and it is Stage 5 + Completion Criterion 8. Recorded here because this item is the + project's own catalog of which benchmark is testable where, and it + had the demo in the wrong place. - [x] **`capability-map.md`'s "Analysis" capability's fate: decided 2026-08-20, maintainer's call -- threaded like Rendering, no diff --git a/docs/planning/implementation-plan.md b/docs/planning/implementation-plan.md index c70a03b..d3128af 100644 --- a/docs/planning/implementation-plan.md +++ b/docs/planning/implementation-plan.md @@ -212,20 +212,42 @@ Unlocks Golden Demo Lid-driven cavity. **The quantitative target has a name and a number, -recorded here 2026-08-27 though not yet a drafted acceptance -criterion:** Ghia, Ghia & Shin (1982) publish tabulated centerline +recorded here 2026-08-27 and drafted into a real acceptance criterion on +2026-08-28:** Ghia, Ghia & Shin (1982) publish tabulated centerline velocity profiles for this exact case at several Reynolds numbers; `adr/ADR-007-executable-acceptance-criteria.md`'s own worked example already uses "Reynolds number 100... matches Ghia et al. within 2%" to illustrate what an executable physics criterion looks like, and `docs/glossary.md`'s definition of "Validation" cites the same paper. -Neither commits Stage 5 to that Reynolds number or that tolerance -- -Stage 5's own Completion Criteria are, per `docs/practices.md`'s own -rule ("a stage gets completion criteria before its first task"), -drafted when that Stage opens, not now -- but whoever drafts -them should reach for Ghia et al. rather than inventing a fresh -reference, since two other documents already point at it as the obvious -answer. +This note previously said neither committed Stage 5 to that Reynolds +number or that tolerance, and asked whoever drafted that Stage's +criteria to reach for Ghia et al. rather than invent a fresh reference. +**They did, and the answer to the tolerance half was no** (`docs/ +planning/roadmap.md`, Stage 5 Completion Criterion 5, 2026-08-28, +maintainer's call): Reynolds number 100 is adopted, the 2% is not. +The MVP's advection scheme is first-order upwind, whose numerical +diffusion at MVP mesh resolutions dominates the error, so a fixed 2% +would be a bar the MVP's own documented numerics are not built to clear. +The criterion is convergence instead -- error against Ghia's profiles +decreasing monotonically across at least three mesh resolutions, plus +the qualitative structure (primary vortex centre, both secondary corner +vortices) at the finest -- with the absolute tolerance stated and +defended in the feature file against the mesh actually used. + +**Heat diffusion, added 2026-08-28** -- maintainer's call, taken while +drafting Stage 5's Completion Criteria and reconciling them against +`docs/implementation/mvp.md`'s own Definition of Done, which is a +condition Stage 5 has carried since 2026-08-22. `mvp.md` requires the +MVP to reproduce three validation cases -- passive scalar transport, +heat diffusion, lid-driven cavity -- while this document and +`planning/data/demos.yaml` both placed Heat Diffusion at Level 3, which +is Stage 6. **Decided: Level 2 owes it, as a scalar.** Heat diffusion +*is* the diffusion equation on a transported scalar; only the field's +name differs, and PyFlow can run it with no Temperature field at all. +Level 3's "Heat transport" (below) is then the named-Temperature +version with buoyancy coupling -- a genuinely different claim, not this +one repeated -- the same reuse-at-a-later-Level pattern Taylor-Green +vortex already follows between Levels 2 and 4. **Couette flow, added 2026-08-27** (`docs/planning/backlog.md`, "physical correctness validation") -- plane shear flow between two @@ -286,6 +308,15 @@ Heat transport. Smoke transport. +**"Heat transport" here is the named-Temperature version, not the first +heat demo** (clarified 2026-08-28, when Level 2 took ownership of Heat +Diffusion -- see that Level's own note). The distinction is the point of +this Level: Level 2's Heat Diffusion demo runs the diffusion equation on +an anonymous transported scalar, which needs no Temperature field at +all. This Level's claim is that a *named* field with its own physical +coupling (buoyancy, TASK-035) adds nothing to the engine -- which is +only demonstrated if the anonymous version already worked. + **Rayleigh-Bénard convection** (added 2026-08-20, same reference as above) -- a fluid layer heated from below, once Temperature and Density are both unlocked at this Level. Extends the "Thermal buoyancy" demo @@ -554,7 +585,7 @@ Each Golden Demo permanently validates one or more major capabilities. | Field Display | Variables (added 2026-08-22, TASK-017) | | Numerics Assembly | Numerical architecture (added 2026-08-23, TASK-021 -- proves the six-component assembly mechanism, not a physical capability; "no CFD yet") | | Scalar Transport | Advection | -| Heat Diffusion | Diffusion | +| Heat Diffusion | Diffusion (Level 2 as a transported scalar, decided 2026-08-28; reused at Multiple Transported Fields as named-Temperature heat transport) | | Couette Flow | Incompressible Navier-Stokes (added 2026-08-27, physical correctness validation) | | Poiseuille Flow | Incompressible Navier-Stokes (added 2026-08-20, physical correctness validation) | | Lid-Driven Cavity | Pressure-Velocity Coupling | diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index b030765..e859316 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -5737,22 +5737,497 @@ Goal Solve incompressible flow. -### Completion Criteria — due when this Stage opens, not now +### Completion Criteria -Same reasoning as Stage 4's, above. This stage defines the MVP -(`docs/implementation/mvp.md`), so its criteria and `mvp.md`'s own -Definition of Done must be reconciled explicitly when they are written, -not assumed to agree. +Written 2026-08-28, before TASK-031 starts, per `docs/practices.md`'s "A +stage gets completion criteria before its first task" -- now that Stage +4 has actually delivered the concrete schemes these tasks must couple +(closed 2026-08-28), rather than committing to their shape while the +components they compose were still `_Null*` registrations. + +Criteria are about the stage's goal -- *solve incompressible flow* -- +not the union of TASK-031..034's own Acceptance Criteria. Every +qualifying clause is its own bullet (`docs/practices.md`, "The intent +lives in the qualifier") and every criterion names the task(s) that +discharge it (the discharge map below), following the two rules Stage 3 +and Stage 4 both carried. + +**Neither of Stage 3's two exemptions extends here**, same as Stage 4: +the physical-correctness extension applies in full, and executable +Gherkin criteria apply in full +(`adr/ADR-007-executable-acceptance-criteria.md`) -- every task below is +simulation work. TASK-034's own criteria are the sharpest test of that +form: "the right instability emerges under the right configuration, and +does not emerge under a configuration where it should not" is a pair of +scenarios, and reads as one. + +**This stage defines the MVP, so `docs/implementation/mvp.md`'s own +Definition of Done is reconciled against these criteria explicitly, in +Criterion 11 below** -- the condition this section carried as a +placeholder from 2026-08-22 until it was drafted. Two conflicts came out +of doing that rather than assuming the two documents agreed, both +decided by the maintainer on 2026-08-28 and both recorded where they +were found: the fate of `mvp.md`'s "heat diffusion" validation case +(Criterion 8), and whether Ghia et al.'s illustrative 2% is this stage's +actual bar (Criterion 5). + +1. **Velocity is transported by the same mechanism every other field + is.** The stage's field-centric claim is only worth anything if + nothing here special-cases velocity -- Stage 6 adds four more + transported fields (TASK-035..038) on exactly that claim, and it is + testable only if velocity went through the path they will. + - The orchestrator advances velocity through the same + `src/pyflow/engine/simulation.py` `step` path a scalar takes: the + same `AdvectionScheme`/`DiffusionScheme`, the same + `accumulate_flux_to_cells`, the same `TimeIntegrator`. Checked + structurally as well as behaviourally -- no branch anywhere in + `simulation.py` keyed on a field's name, or on a field being a + `VectorField` rather than a `ScalarField`. + - **A scalar transported alongside velocity in the same run is + advanced by the same call, with no engine change.** Stated as a + Stage 5 criterion rather than left for Stage 6 to discover: the + "no new machinery" claim Stage 6's own criteria already make is far + cheaper to check here, with two fields, than there, with five. + - **Momentum diffusion is viscosity, and is not the same number as a + scalar's diffusivity.** `NumericsConfig.diffusion_coefficient` + (`src/pyflow/configuration/schema.py`) is currently one global + constant serving the one transported scalar Stage 4 had. A + configuration that sets a fluid's viscosity must not silently set + an unrelated scalar's diffusivity, and a Reynolds number cannot be + configured at all until the two are distinguishable -- see design + question four below. + - Whatever resolves design question one applies to *any* vector + field, not to a field named `"velocity"`. A name-keyed branch + anywhere in the engine fails this criterion however well it works. +2. **Pressure is solved from the incompressibility constraint, not + transported.** A criterion that treats pressure as another advected + scalar has misunderstood the task + (`docs/handbook/numerical-methods/pressure-velocity-coupling.md`: + pressure "is best understood as a constraint-enforcing field, not a + transported one"). + - Pressure never appears among the fields `step` advances, and a + configuration that tries to transport it is rejected with a named + error rather than quietly advecting it. + - Pressure is determined by the velocity field it is solved against, + checked as a *pair* of scenarios, not either alone: a provisional + velocity that is already divergence-free yields a pressure field + that is constant to solver tolerance, and one with a known nonzero + divergence yields one that is not. A solver stuck at zero passes + the first scenario and fails the second, which is the point of + requiring both. + - **The null space is removed explicitly, and which remedy was + chosen is visible and tested.** Every boundary of the MVP's own + lid-driven cavity prescribes velocity, so the discrete pressure + system is singular and positive *semi*-definite with a + one-dimensional null space of constant vectors + (`pressure-velocity-coupling.md`, "When the Pressure Equation Has + No Unique Solution"). Either remedy that document names -- a + pinned reference cell, or a mean-subtracting projection -- is + acceptable; leaving it implicit is not, and the criterion is + checked by adding a constant to the pressure field and confirming + the corrected velocity is unchanged to floating-point tolerance. + - **The compatibility condition is not re-implemented here.** Zero + net boundary flux is already rejected at configuration-load time + by Stage 3 TASK-019's `_validate_boundary_conditions_jointly`. + This stage's own share is showing the solve *depends* on it: a + configuration violating it fails to load, rather than failing deep + inside the linear solver with a confusing message. +3. **Divergence decreases monotonically with each corrector iteration, + and reaches the configured tolerance -- measured across iterations, + not asserted at the end.** A loop that reaches tolerance by luck on + iteration one and diverges thereafter passes an end-state check; the + recorded sequence of per-iteration maximum divergence magnitudes is + non-increasing at every element, and its last element is at or below + the configured tolerance. + - **This is where Stage 4 Completion Criterion 4's Pressure-Velocity + Coupling bullet's stronger claim is discharged**, the deferral + TASK-027 recorded explicitly rather than silently narrowing its own + criterion. When this task closes, that bullet, TASK-027's own entry + and `docs/practices.md`'s "A criterion whose strong reading depends + on a later task must say so when drafted" are all re-read against + what actually landed -- `docs/practices.md`'s "A deferral gated on + a task must be revisited when that task closes", applied + deliberately rather than remembered. + - **`piso` must become the algorithm it is named after, or the name + must be corrected.** `PISO` is registered under that name today but + is honestly documented as a single `dt`-scaled correction pass, not + Issa's multi-pass algorithm + (`src/pyflow/engine/numerics/pressure_coupling.py`'s own class + docstring). `mvp.md` names PISO as an MVP component and + `pressure-velocity-coupling.md` describes it as "two or more + pressure-correction passes within a single timestep". Either this + stage makes the registered scheme genuinely multi-pass, or + `mvp.md`, `docs/architecture/icds.md` and `PressureCouplingName` + are corrected in the same change. The MVP must not ship claiming an + algorithm by name only. + - Non-convergence is reported, never returned as a plausible field -- + already true of `PressureSolveDidNotConvergeError` for a single + solve, extended here to the loop's own iteration limit: exhausting + the corrector passes without reaching tolerance is an error, not a + quietly-returned best effort. +4. **One timestep solves momentum and continuity together, and the + sequence is checked, not only its endpoint.** Predictor, corrector + loop, corrected state -- the assembled per-step sequence + `pressure-velocity-coupling.md` describes, with each part observable. + - **A null test the rest cannot substitute for:** a divergence-free + initial field on a closed domain, with no forcing and zero + viscosity, stays divergence-free and stays put over many steps, to + a stated tolerance. An implementation that manufactures velocity + out of its own correction term fails this and passes almost + everything else in this list. + - Deterministic: the same configuration run twice produces identical + state, checked directly. Already required of every golden demo + (`docs/implementation/golden-demos.md`'s Definition of Done), + restated here because Criterion 8's demo and any future + checkpoint/replay both rest on it. + - **Checkpoint/pause/rewind is explicitly not a criterion of this + stage**, stated so its absence is not later mistaken for a gap: it + is recorded as future scope under TASK-034 below, and + `docs/architecture/sequences.md` Section 3 carries it as a + "Planned: checkpointing" placeholder anchored to that task. If it + is built here, that placeholder is replaced with the real sequence + in the same change (that document's own obligation). If it is not, + the placeholder stays accurate and nothing is owed. +5. **Physical correctness against a known answer, per case** -- per + `docs/practices.md`'s testable-physics extension, and stated per case + rather than left generic, the same shape Stage 4's own Criterion 4 + took. + - **Couette flow, first.** Plane shear flow between two plates, one + stationary and one moving, with no imposed pressure gradient: the + steady velocity profile is exactly linear, and is checked against + that analytic profile cell by cell. The simplest incompressible + Navier-Stokes case there is + (`docs/planning/implementation-plan.md` Level 2), and the one that + isolates viscous diffusion plus the wall treatment from any + pressure-gradient effect -- which is exactly why it comes before + the cavity rather than after it. + - **Lid-driven cavity against Ghia, Ghia & Shin (1982)**, whose + tabulated centreline velocity profiles at Reynolds number 100 are + the published reference three other documents already point at + (`adr/ADR-007-executable-acceptance-criteria.md`'s worked example, + `docs/glossary.md`'s "Validation" definition, + `docs/planning/implementation-plan.md` Level 2, which asked + whoever drafted these criteria to reach for it rather than invent a + fresh reference). + - **The criterion is convergence, not a fixed percentage** + (decided 2026-08-28, maintainer's call, recorded because the + obvious reading was the other one): the error against Ghia's + tabulated profiles must *decrease monotonically across at least + three mesh resolutions*, and the qualitative structure must be + right at the finest -- primary vortex centre within a stated + distance of Ghia's, and both downstream secondary corner vortices + present. + - **ADR-007's illustrative "within 2%" does not bind this stage.** + That number appears in a worked example showing what an + executable physics criterion *looks like*, and in the glossary's + definition of validation; neither was ever a commitment, and + `implementation-plan.md` Level 2 says so directly. The MVP's + advection scheme is first-order upwind, whose numerical diffusion + at MVP mesh resolutions is the dominant error term + (`docs/handbook/numerical-methods/advection.md`; + `docs/implementation/upgrade-paths.md` is where a better scheme + lands). A criterion that could only be met by quietly loosening + its own number later is not a criterion, which is why the + convergence claim above is the one that gates and the absolute + tolerance is stated and defended in the feature file against the + mesh actually used. + - **The emergent phenomenon, and its negative control.** The + qualifier this stage has carried since 2026-08-20 (this document's + own "Stages and Capability Levels" note; `docs/planning/backlog.md`, + "physical correctness validation"): the right phenomenon under the + right configuration, which means a configuration under which it + should *not* emerge is tested too. An instability that appears + regardless of parameters is not the instability. Named candidates + rather than left generic -- Kelvin-Helmholtz roll-up present above + a shear layer's instability threshold and absent below it, or + Taylor-Green's closed-form decay rate matched at one viscosity and + demonstrably not at another; both are Level 2 cases already + (`docs/planning/implementation-plan.md`), and either pair satisfies + this bullet. + - **Conservation, a claim none of the three above makes.** On a + closed domain with no forcing, an inviscid flow's total kinetic + energy must not *grow*. A pressure correction that injects energy + can still match a steady-state profile comparison and still be + wrong; this is the bullet that catches it, and it is the velocity + counterpart of the conservation checks Stage 4 already required of + Advection and Diffusion separately (`docs/planning/backlog.md`, + conservation checks -- whose Pressure-Velocity Coupling bullet + names this stage's own TASK-033 as where mass conservation proper + gets its check). +6. **Every real implementation's own error and rejection conditions are + exercised against actual bad input**, not inherited untested from an + interface's shared helper -- Stage 4's own Criterion 5, which applies + here unchanged and is restated rather than assumed to carry over. The + specific new surfaces this stage creates: a velocity field whose + component count disagrees with the mesh's dimensionality, a + configuration that names a boundary treatment velocity has no meaning + for, a corrector loop that exhausts its iteration limit, and whatever + rejection path design question one's resolution introduces. +7. **Every task's acceptance criteria are a Gherkin `.feature` file + under `tests/features/`, and `make check-scenarios` gates that every + scenario it contains actually runs.** The mechanism + `adr/ADR-007-executable-acceptance-criteria.md` commits this stage to; + restated as a checkable exit condition, not left only as the drafting + instruction it also is. + - Steps are built from the shared building blocks + (`tests/unit/_numerics.py` for unit-level scenarios, + `tests/golden/_demo.py` for demo-level ones), not re-derived -- and + the venue is named correctly here rather than assumed, since Stage + 4's own version of this bullet named an unreachable one for three + days (`tests/golden/conftest.py`, corrected at that stage's exit + audit: a `conftest.py` applies only to its own directory subtree). + - **Every scenario's fixture avoids a degenerate case that could let + a wrong implementation agree with a right one by coincidence** -- + non-square mesh, non-trivial origin, spacing that isn't 1, values + that aren't 0 or 1 everywhere, and, specific to this stage, a + viscosity that isn't 1, a lid velocity that isn't 1, and a timestep + that isn't the mesh spacing. Stated once here rather than repeated + under each task. +8. **Stage 5 has working, visible demonstrations: Lid-Driven Cavity and + Heat Diffusion.** + - Each demo *is* a config file under `examples/golden-demos/`, run + via `pyflow run --config `, per the public-API rule every + golden demo already follows + (`docs/implementation/golden-demos.md`), with at least one + regression test per demo invoking it through the real CLI as a + subprocess. + - **Lid-Driven Cavity is the MVP's own golden demo** -- the "Initial + Golden Demo" `docs/implementation/golden-demos.md` and `mvp.md`'s + "golden demo exists" both refer to. It renders a *solved* velocity + field live, through the existing field rendering (TASK-017), which + is what makes `mvp.md`'s "visualisation shows the result" true for + the first time: every velocity PyFlow has rendered until now was + prescribed or seeded, never computed. **If rendering a solved field + turns out to need work the existing renderer cannot do, that is a + finding to report and schedule, not a silent addition to this + stage** -- no rendering task is planned here, and the criterion is + stated this way so that its absence is a decision rather than an + oversight. + - **Heat Diffusion is Stage 5's, resolved 2026-08-28 (maintainer's + call) rather than assumed either way.** `mvp.md`'s Validation + section requires the MVP to reproduce three cases -- passive scalar + transport (closed by Stage 4's TASK-030), heat diffusion, and + lid-driven cavity -- while `docs/planning/implementation-plan.md` + and `planning/data/demos.yaml` both placed Heat Diffusion at + Capability Level 3 alone, which is Stage 6. That was a real + divergence between the document defining this stage's own exit bar + and the two describing the long-range plan, and it was found by + doing the reconciliation Criterion 11 requires rather than assuming + the two agreed. **Decided: Stage 5 owes it, as a scalar.** Heat diffusion + *is* the diffusion equation on a transported scalar; only the + field's name differs, and PyFlow can run it with no Temperature + field at all (`docs/handbook/numerical-methods/diffusion.md`). + Stage 6's own TASK-035 then adds the named Temperature field with + buoyancy coupling, which is a genuinely different claim, not this + one repeated. `implementation-plan.md` and + `planning/data/demos.yaml` are amended in the same change as this + criterion, not left to be rediscovered. + - **Poiseuille flow, Taylor-Green vortex and Kelvin-Helmholtz + instability are not required as demos here**, stated so their + absence is not read as a gap: all three are Level 2 catalog entries + (`docs/planning/implementation-plan.md`), and + `docs/implementation/golden-demos.md`'s own rule is not to write a + demo entry for a capability that does not exist yet. Taylor-Green + and Kelvin-Helmholtz appear in Criterion 5 as *scenarios*, which is + a weaker and sufficient obligation; Poiseuille appears in neither + and stays scheduled where it is. +9. **`make ci` passes on both CI platforms, on a real runner** -- not + only locally, matching every prior stage's standard of evidence, read + from the actual run rather than inferred from a merged PR. +10. **Documentation describes what now exists**, and the sweep is a grep + against what this stage *invalidated*, not a review of what it + touched (`docs/practices.md`, "A stage's documentation sweep is a + grep, not a diff review" -- the rule Stage 4's own exit audit + produced after finding seven stale claims in files no Stage 4 task + had opened). + - **`docs/architecture/icds.md`'s Pressure-Velocity Coupling entry + is the one this stage is most likely to falsify, and it is + specific rather than general.** That entry currently records, in + detail, that `piso` "is not, and does not claim to be, the full + multi-pass Issa algorithm", and names TASK-033 as where the + stronger claim belongs -- written by TASK-027 on 2026-08-27 and + accurate today. Criterion 3 is what makes it stop being accurate. + Re-read it, its `Choices:` line and its `Expected behaviour` + prose against what actually landed. + - `docs/architecture/engine.md`'s `Implementation:` lines for the + Variables, Flux and Pressure-Velocity Coupling layers name the + concrete modules, and that document's own maintenance rule + (don't rewrite the tense, per its closing section) is followed + rather than worked around. + - `docs/implementation/golden-demos.md` gains a real entry for each + demo Criterion 8 lands, written when the demo exists rather than + ahead of it. + - Every touched `CLAUDE.md` and both inventories + (`docs/repository-manifest.md`, `docs/repository-inventory.md`) + are checked against the tree directly. + - **The capability map is part of this criterion, not an + afterthought** -- `mvp.md`'s Definition of Done names "capability + map is updated" as its own bullet, and it is the one bullet no + previous stage has ever had cause to touch: + `docs/planning/capability-map.md`, + `planning/data/capabilities.yaml` and + `docs/planning/implementation-plan.md`'s Level 2 all describe a + capability this stage is what realises. +11. **`docs/implementation/mvp.md`'s own Definition of Done is + discharged item by item, or its divergence is recorded at source.** + This stage defines the MVP, so its exit *is* the MVP's exit; the + reconciliation is written here, when the criteria are drafted, + rather than reconstructed at the exit audit. + + | `mvp.md` Definition of Done | Where it is discharged | + |------|------| + | Simulation runs end-to-end | Criteria 4 and 8 | + | Physical fields evolve | Criteria 1 and 2 (velocity and pressure; a scalar already evolved in Stage 4) | + | Boundary conditions operate | Criterion 5's Couette and cavity bullets -- a moving wall and a no-slip wall, which is velocity's own boundary treatment, distinct from the scalar boundaries Stage 4 closed | + | Pressure/velocity coupling works | Criterion 3, in the strong sense TASK-027 explicitly deferred | + | Numerical solution is measurable | Criterion 5 | + | Visualisation shows the result | Criterion 8's Lid-Driven Cavity bullet | + | Golden demo exists | Criterion 8 | + | Documentation describes the implemented functionality | Criterion 10 | + | Tests verify the core behaviour | Criterion 7 | + | Capability map is updated | Criterion 10's own capability-map bullet | + + **`mvp.md`'s Components and Validation sections are reconciled too, + not only its Definition of Done** -- three findings, recorded here + rather than left for the exit audit: + - *Already true, nothing owed:* 2D, structured Cartesian, uniform + spacing, single incompressible fluid, FVM, collocated arrangement, + first-order upwind, central-difference diffusion, RK4, Conjugate + Gradient, Dirichlet/Neumann/Periodic boundaries, and + configuration-selected components. All landed across Stages 1-4; + this stage adds none of them. + - *Owed by Criterion 3:* PISO. Named as an MVP component, currently + a single-pass approximation registered under that name. + - *Owed by Criterion 8:* the Validation section's three cases. One is + closed (passive scalar transport, TASK-030); the other two are + this stage's, per the Heat Diffusion decision recorded above. + +### Four design questions, all open + +Flagged here at stage open rather than left to surface mid-implementation +(`docs/practices.md`, "When intent is ambiguous, hold a design session +before implementing"), following Stage 4's own precedent of recording its +two on the day its criteria were drafted and resolving each before the +task that needed it started. **None of these is resolved yet**, and each +names the task that cannot start drafting without the answer. + +**One: how does a vector field go through an interface that returns one +value per face? (TASK-031.)** `AdvectionScheme.flux(field, velocity)` and +`DiffusionScheme.flux(field)` both return a tensor of shape +`(mesh.num_faces,)` -- one scalar per face -- and +`FirstOrderUpwindAdvection`/`CentralDifferenceDiffusion` both read a cell +value with `float(field.value_at(cell))`. **Verified directly, not +reasoned about: handing either scheme a `VectorField` raises `TypeError: +float() argument must be a string or a real number, not 'tuple'`** -- so +velocity cannot go through the Stage 4 transport path today at all, and +Criterion 1 cannot be satisfied without answering this. Three readings, +none yet chosen: +- Momentum is transported as one `ScalarField` per component, with the + `VectorField` assembled from them for the consumers that need one + (`AdvectionScheme`'s own `velocity` argument, `GreenGaussDivergence`, + rendering). Cheapest, and makes "velocity is transported like a scalar" + literally true rather than analogously true. +- `flux` widens to return `(num_faces, num_components)`, with a scalar + field being the one-component case. The most uniform, and the most + expensive: it is a change to two Stage 3 interfaces and their contract + suites, in the category `adr/ADR-008-time-integrator-derivative-callable.md` + and `adr/ADR-009-pressure-coupling-dt.md` were. +- `step` adapts per component internally, leaving both interfaces + untouched. Cheapest to write and the worst fit for Criterion 1's + structural clause, since the adapter is exactly the special-casing that + criterion forbids. + +**Two: `BoundaryFaceConfig.velocity` is the boundary-normal component +only, and a lid-driven cavity's lid is tangential. (TASK-031/034.)** That +field's own docstring already flagged this and deferred it "to whichever +task builds a concrete condition against a real consumer (P-016)" -- this +stage is that consumer, for two of its own cases at once: the cavity's +moving lid and Couette's moving plate are both tangential, and every wall +in both is no-slip, which is a *tangential* zero. A normal-component-only +boundary value cannot express either. Whatever is decided has a blast +radius that must be worked out in the same change: +`_validate_boundary_conditions_jointly`'s zero-net-flux check reads this +field and is about normal flux specifically, +`docs/implementation/config-template.yaml` is generated from the schema +and its own `FIELD_COMMENTS`, and `pyflow generate-config` (TASK-039) +emits it. + +**Three: what does the corrector loop need that TASK-027's interface +could not supply? (TASK-033.)** Criterion 3's monotonic-convergence claim +is the one TASK-027 measured and could not make: on PyFlow's collocated +mesh, suppressing pressure-velocity decoupling under repeated correction +needs Rhie-Chow interpolation, which needs momentum-equation +coefficients, and TASK-027 verified numerically that three correction +strategies without them all leave most of the original divergence in +place (that task's own Design decision Two). TASK-031/032 are what +finally produce a momentum system to draw those coefficients from, so the +question is what carries them: a widened `PressureCoupling.correct` (the +`adr/ADR-009-pressure-coupling-dt.md` precedent, which widened it once +already), a momentum operator handed to the strategy at construction (the +`LinearSolver` precedent), or outer-loop state the strategy owns. **This +is the question most likely to move the stage's own scope, and the one to +resolve with numerical prototyping before Criterion 3 is turned into a +feature file** -- a criterion that cannot be met is worse than one +drafted late. + +**Four: how is a Reynolds number configured at all? (TASK-031.)** +Criterion 5's cavity bullet compares against Ghia et al. at Re = 100, and +Re = UL/nu needs a kinematic viscosity that is currently +indistinguishable from `NumericsConfig.diffusion_coefficient`, the single +global constant Stage 4's one transported scalar used. Related, and worth +resolving together rather than twice: TASK-040's own Design decision +recorded that PyFlow has *one* global set of boundary conditions shared +across every transported field, "correct for a single transported +scalar... but does not yet express 'field A is 300K at this wall, field B +is 0 at the same wall'". Stage 5 is the first stage with two genuinely +different transported fields in one run -- velocity at a moving lid and a +scalar at the same wall -- so the case that note deferred to Stage 6's +"no new machinery" criterion may bite here first. -**Executable acceptance criteria apply here in full** -(`adr/ADR-007-executable-acceptance-criteria.md`): every task below is -simulation work. TASK-034's own criteria are the sharpest test of the -form -- "the right instability emerges under the right configuration, -and does not emerge under a configuration where it should not" is a -pair of scenarios, and reads as one. +### Discharge map + +Every criterion has an owning task, assigned now rather than +reconstructed at the exit audit, following Stage 3 and Stage 4's own +precedent. A task's own **Discharges** section is authoritative; this +table is the index. + +**Build order is TASK-031, 032, 033, 034 -- numerical order, for once**, +and structurally rather than conveniently: TASK-032's pressure field has +nothing to be solved against until velocity is transported, TASK-033's +corrector loop has nothing to correct until both exist, and TASK-034 +assembles what the first three build. The only thing that could reorder +this is design question three's answer, if it turns out the momentum +coefficients TASK-033 needs must be produced by TASK-031's own work +rather than read from it -- which would make part of TASK-033 a TASK-031 +obligation, not a reordering. + +| Criterion | Discharged by | +|-----------|---------------| +| 1. Velocity transported by the same mechanism as every other field | TASK-031 | +| 2. Pressure solved from the constraint, not transported | TASK-032 | +| 3. Divergence decreases monotonically to the configured tolerance | TASK-033 | +| 4. One timestep solves momentum and continuity together | TASK-034 | +| 5. Physical correctness against a known answer, per case | TASK-034, each for its own bullet; Couette flow jointly with TASK-033, the first task able to run it | +| 6. Rejection paths exercised against real bad input | TASK-031..034, each for its own error conditions | +| 7. Executable Gherkin criteria, `make check-scenarios` gates | TASK-031..034, each for its own `.feature` file | +| 8. Demonstrations: Lid-Driven Cavity and Heat Diffusion | TASK-034 (this stage's last task) | +| 9. `make ci` green on a real runner | TASK-034 | +| 10. Documentation matches the tree, capability map included | TASK-034 | +| 11. `mvp.md`'s Definition of Done discharged item by item | TASK-034, reading Criterion 11's own table back against what landed | + +**TASK-034 is this stage's last task in build order and therefore owns +the stage-level criteria** -- the demonstrations, the CI evidence, the +documentation sweep and the MVP reconciliation -- the same assignment +Stage 4 made to TASK-030 for the same reason. ### Intent, recorded now +Recorded 2026-08-22, ahead of the criteria above, because it is the +durable half and the half this repository keeps losing. Each line states +what the task must not merely *nominally* satisfy (`docs/practices.md`, +"The intent lives in the qualifier"). The Completion Criteria above were +drafted against these on 2026-08-28, not in place of them. + ## TASK-031 Velocity Field Support @@ -5764,6 +6239,69 @@ fields (TASK-035..038) on the claim that the architecture is field-centric, and that claim is only testable if velocity went through the same path they will. +### Purpose + +Make velocity a transported field: something `src/pyflow/engine/ +simulation.py`'s `step` advances, using the same Advection, Diffusion +and Time Integrator components a scalar already goes through, rather +than something supplied fixed from configuration as Stage 4's own +Passive Scalar Transport demo does +(`src/pyflow/configuration/schema.py`'s `SimulationConfig.velocity`, "a +prescribed (not solved) constant vector"). + +### Dependencies + +TASK-012 (`Mesh`), TASK-014..016 (`Field`/`ScalarField`/`VectorField`), +TASK-023 (`FirstOrderUpwindAdvection`), TASK-024 +(`CentralDifferenceDiffusion`), TASK-025 (`RK4TimeIntegrator`), +TASK-028/029 (the real Dirichlet/Neumann conditions a velocity boundary +resolves to), TASK-040 (`step`, `accumulate_flux_to_cells`). + +### Open design questions + +**One and four above, both open, both blocking this task's own +drafting.** One decides how a vector field goes through an interface +that returns one value per face -- verified as a real `TypeError`, not +a hypothetical. Four decides how a viscosity is configured separately +from a scalar's diffusivity, without which Criterion 5's Reynolds-number +comparison has nothing to read. Two (tangential boundary values) is +listed against TASK-034 as well as here, because whichever of the two +first needs a no-slip wall is where it has to be answered. + +### Artifacts Produced + +- `tests/features/velocity_field_support.feature` -- this task's + Acceptance Criteria, per + `adr/ADR-007-executable-acceptance-criteria.md`. +- Source artifacts are deliberately not named yet: which modules change, + and whether any new one appears at all, is exactly what design + question one decides. Naming them now would be the speculation P-016 + refuses, and a wrong name here is a `make check-references` failure + rather than a harmless guess. + +### Acceptance Criteria + +`tests/features/velocity_field_support.feature` is the criteria, not +restated here as prose. Written to cover, at minimum: + +- Velocity is advanced by the same `step` call, over the same timestep, + that advances a scalar -- and a run carrying both advances both, with + no engine change between the one-field and two-field cases. +- A momentum diffusion coefficient (viscosity) distinct from a scalar's + diffusivity: setting one leaves the other's computed flux unchanged. +- Velocity advected by itself reproduces a hand-derived result on a + small non-square mesh with non-unit spacing -- self-advection is the + one nonlinearity in the momentum equation, and the case a scalar + transported by a *prescribed* velocity never exercises. +- A velocity field whose component count disagrees with the mesh's + dimensionality is rejected with the existing named error + (`IncompatibleVelocityFieldError`), exercised against this task's own + new path rather than assumed inherited (Criterion 6). + +### Discharges + +Criterion 1, entirely. Criterion 6 and Criterion 7, its own share. + --- ## TASK-032 @@ -5775,6 +6313,55 @@ incompressibility constraint. A criterion that treats it as another advected scalar has misunderstood the task. See `docs/handbook/numerical-methods/pressure-velocity-coupling.md`. +### Purpose + +Give the engine a real pressure field: one produced by a solve against +the current velocity field, with its null space removed explicitly, and +one that no part of the transport path advances. + +### Dependencies + +TASK-031 (a transported velocity field to solve against), TASK-026 +(`ConjugateGradientSolver`), TASK-027 (`PISO`, `GreenGaussGradient`, +`GreenGaussDivergence`, and the compact symmetric Laplacian that task +already builds the Poisson matrix from). + +### Artifacts Produced + +- `tests/features/pressure_field.feature` -- this task's Acceptance + Criteria. +- Source artifacts named when this task is drafted, after TASK-031's own + design questions are answered: whether pressure needs a type of its + own, or is a `ScalarField` the coupling owns, is a real question this + task decides rather than one to guess now. + +### Acceptance Criteria + +`tests/features/pressure_field.feature` is the criteria. Written to +cover, at minimum: + +- The divergence-free/divergent pair Criterion 2 requires: constant + pressure to solver tolerance for an already-divergence-free velocity + field, non-constant for one with a known nonzero divergence. Both + scenarios, since either alone is passed by an implementation that + always returns the same answer. +- Adding a constant to the pressure field leaves the corrected velocity + unchanged to floating-point tolerance -- the null-space property made + observable, and the check that says which remedy was chosen was + actually applied. +- Pressure is not among the fields `step` advances, and a configuration + that tries to transport it is rejected with a named error + (Criterion 2's first bullet, and this task's share of Criterion 6). +- A boundary configuration violating the zero-net-flux compatibility + condition fails at load time, not inside the solver -- checked against + Stage 3's existing `_validate_boundary_conditions_jointly`, which is + where that rejection already lives and is deliberately not + reimplemented here. + +### Discharges + +Criterion 2, entirely. Criterion 6 and Criterion 7, its own share. + --- ## TASK-033 @@ -5799,6 +6386,69 @@ engine. This task is where a real, wired velocity/pressure field finally exists to iterate against, and where the monotonic-convergence claim TASK-027 explicitly deferred is meant to be checked. +### Purpose + +Turn one correction pass into a corrector *loop* that converges: the +"two or more pressure-correction passes within a single timestep" PISO +is defined by (`docs/handbook/numerical-methods/pressure-velocity- +coupling.md`), with the momentum coupling that makes repeated passes +actually reduce divergence rather than stall. + +### Dependencies + +TASK-031, TASK-032, and TASK-027's own finding -- which is a dependency +in the literal sense: this task starts from a measured negative result +about what does not work, not from a blank page. + +### Open design question + +**Three above, open, and the one most likely to move this stage's +scope.** What carries the momentum-equation coefficients Rhie-Chow +needs: a widened `PressureCoupling.correct`, a momentum operator handed +in at construction, or outer-loop state. **Resolve it with numerical +prototyping before writing this task's feature file**, the same +sequence TASK-027 used (five prototype scripts, then a scoping +decision, then the tests) -- writing a criterion first and discovering +it is unreachable second is precisely the failure `docs/practices.md`'s +"A criterion whose strong reading depends on a later task must say so +when drafted" was added to prevent, and this is the later task. + +### Artifacts Produced + +- `tests/features/pressure_correction_loop.feature` -- this task's + Acceptance Criteria. +- An ADR if the resolution to design question three widens a Stage 3 + interface, matching the precedent + `adr/ADR-009-pressure-coupling-dt.md` set for the last such widening. + +### Acceptance Criteria + +`tests/features/pressure_correction_loop.feature` is the criteria. +Written to cover, at minimum: + +- The recorded sequence of per-iteration maximum divergence magnitudes + within one timestep is non-increasing at every element, and its last + element is at or below the configured tolerance -- the sequence + asserted, not just its last value. +- Exhausting the corrector iteration limit without reaching tolerance + raises rather than returning a best effort, the same honesty + `PressureSolveDidNotConvergeError` already applies to a single solve + (Criterion 3's last bullet, and this task's share of Criterion 6). +- Couette flow's exact linear profile, jointly with TASK-034: this is + the first task with enough machinery to run it, and it is the cheapest + quantitative check that the coupled solve is right rather than merely + convergent. +- Whether `piso` is now genuinely multi-pass, or has been renamed -- one + or the other, per Criterion 3, and whichever it is must be visible in + the scenario rather than only in a docstring. + +### Discharges + +Criterion 3, entirely. Criterion 5's Couette bullet, jointly with +TASK-034. Criterion 6 and Criterion 7, its own share. **Stage 4 +Completion Criterion 4's Pressure-Velocity Coupling deferral**, which is +re-read at this task's close rather than assumed discharged. + --- ## TASK-034 @@ -5830,13 +6480,77 @@ open design question either" once it is built. **Intent:** this task's acceptance criteria already include emergent phenomena -- "does the right instability emerge under the right -configuration" (`docs/planning/roadmap.md`'s own "Stages and Capability -Levels" note, 2026-08-20; `docs/planning/backlog.md`, "physical -correctness validation"). The qualifier to hold on to: **the right -phenomenon under the right configuration**, which means a -configuration under which it should *not* emerge must be tested too. An -instability that appears regardless of parameters is not the -instability. +configuration" (this document's own "Stages and Capability Levels" note, +2026-08-20; `docs/planning/backlog.md`, "physical correctness +validation"). The qualifier to hold on to: **the right phenomenon under +the right configuration**, which means a configuration under which it +should *not* emerge must be tested too. An instability that appears +regardless of parameters is not the instability. + +### Purpose + +Assemble the first three tasks into one incompressible Navier-Stokes +timestep -- predictor, corrector loop, corrected state -- and then use +it: the demonstrations, the quantitative validation against published +and analytic answers, and the stage-level exit obligations this stage's +last task owns. + +### Dependencies + +TASK-031, TASK-032, TASK-033; TASK-017 (field rendering, for the demos' +own visual output); TASK-030's own demo wiring +(`src/pyflow/bootstrap.py`'s live per-frame stepping), which is the +mechanism a live demo already uses and this one extends rather than +replaces. + +### Open design question + +**Two above** -- tangential boundary values -- if TASK-031 has not +already had to answer it. A no-slip wall and a moving lid are both +tangential, and both of this task's own validation cases need them. + +### Artifacts Produced + +- `tests/features/navier_stokes_timestep.feature` -- this task's own + Acceptance Criteria. +- One `.feature` file per demo Criterion 8 requires, named for its demo, + the same pairing every golden demo already has + (`tests/features/passive_scalar_transport.feature` being the most + recent). +- A config file per demo under `examples/golden-demos/`, and a + regression test per demo under `tests/golden/` invoking it through the + real CLI as a subprocess. +- Entries in `docs/implementation/golden-demos.md` for each demo built, + written when it exists rather than ahead of it, per that document's own + rule. + +### Acceptance Criteria + +`tests/features/navier_stokes_timestep.feature`, plus each demo's own +feature file, are the criteria. Written to cover, at minimum: + +- The predictor/corrector/corrected sequence, each part observable -- + not only the end state (Criterion 4). +- The null test: a divergence-free field on a closed domain, no forcing, + zero viscosity, stays divergence-free and stays put over many steps. +- Determinism: the same configuration run twice produces identical + state. +- Couette flow against its exact linear profile (jointly with TASK-033). +- Lid-driven cavity against Ghia, Ghia & Shin (1982) at Re = 100: + monotonically decreasing error across at least three mesh resolutions, + plus the qualitative structure at the finest -- **not** a fixed + percentage, per Criterion 5's own bullet and the 2026-08-28 decision + recorded there. +- The emergent-phenomenon pair: the instability under a configuration + that should produce it, and its absence under one that should not. +- Kinetic energy does not grow for an inviscid, unforced, closed-domain + flow. + +### Discharges + +Criteria 4, 8, 9, 10 and 11, entirely. Criterion 5, all bullets (its +Couette bullet jointly with TASK-033). Criterion 6 and Criterion 7, its +own share. Golden Demo @@ -5920,10 +6634,22 @@ exactly -- the only check that can fail. Golden Demos -- Heat diffusion +- Heat transport (a named Temperature field, with buoyancy coupling) - Smoke transport - Thermal buoyancy +**This list read "Heat diffusion" until 2026-08-28**, when Stage 5's own +Completion Criteria took ownership of that demo (maintainer's call, Stage +5 Criterion 8): `docs/implementation/mvp.md` requires the MVP to +reproduce heat diffusion, the MVP is Stage 5, and heat diffusion is the +diffusion equation on a transported scalar -- no Temperature field +needed. What is left here is the genuinely different claim, and the one +this stage's own goal is about: a *named* field with its own physical +coupling adds no new machinery. Rewriting the bullet rather than deleting +it, because the demo is not gone -- it is reused, the same way +`docs/planning/implementation-plan.md` reuses Taylor-Green vortex between +Levels 2 and 4. + --- # Stage 7 — Better Numerics diff --git a/docs/planning/status.md b/docs/planning/status.md index 1147508..52ec673 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -115,7 +115,7 @@ pie showData ### Stage 5 -- First Fluid Solver -**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 0 criteria defined, no status line yet +**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 11 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| diff --git a/docs/references/CLAUDE.md b/docs/references/CLAUDE.md index dc8eb79..22969dd 100644 --- a/docs/references/CLAUDE.md +++ b/docs/references/CLAUDE.md @@ -4,7 +4,19 @@ Three reference lists (`books.md`, `papers.md`, `websites.md`), per `docs/planning/knowledge-architecture.md`'s reference section and `docs/planning/backlog.md` E6. Each is populated from what `docs/handbook/` actually cites -- not a general bibliography, and not -somewhere to add a reference that no Handbook entry cites yet. +somewhere to add a reference that nothing in the repository cites yet. + +**Amended 2026-08-28: "what the Handbook cites" is no longer the only +qualifying source.** A paper a *completion criterion* is checked against +also belongs here, even with no Handbook entry citing it -- Ghia, Ghia & +Shin (1982), `papers.md`'s new "Validation Benchmarks" section, is the +first. Stage 5 Completion Criterion 5 requires comparing the lid-driven +cavity against that paper's tabulated profiles, which makes finding the +paper a prerequisite for closing a stage; three documents named it by +author and year, and none gave volume, issue or pages. The rest of the +rule is unchanged, and the "not a general bibliography" half is the part +that matters: a citation earns a place here by something in this +repository depending on it, not by being relevant. Written 2026-08-17 alongside the sixteen Handbook entries (E3/E4) that first gave these files something real to list. `websites.md` currently diff --git a/docs/references/papers.md b/docs/references/papers.md index 3e43e26..e51fc05 100644 --- a/docs/references/papers.md +++ b/docs/references/papers.md @@ -5,6 +5,18 @@ article/paper referenced from `docs/handbook/`. Populated from what the Handbook entries actually cite (`docs/planning/backlog.md` E3/E4) -- see `books.md` for the same convention applied to books/monographs. +**Scope extended 2026-08-28: also every paper a *completion criterion* +depends on.** Ghia, Ghia & Shin (1982) below is the first -- no Handbook +entry cites it, but `docs/planning/roadmap.md`'s Stage 5 Completion +Criterion 5 requires the lid-driven cavity's centreline profiles to be +compared against that paper's tabulated data, which makes finding the +paper a prerequisite for closing a stage rather than a nicety. Three +documents named it by author and year and none gave the volume, issue or +pages. The narrow rule this file was written under ("not somewhere to +add a reference that no Handbook entry cites yet", +`docs/references/CLAUDE.md`) is amended in the same change rather than +quietly broken. + --- ## Numerical Methods @@ -56,6 +68,24 @@ Handbook entries actually cite (`docs/planning/backlog.md` E3/E4) -- see `docs/implementation/upgrade-paths.md`'s Linear Solvers path naming GMRES where the handbook entry did not cover it. +## Validation Benchmarks + +Papers whose published results a PyFlow acceptance or completion +criterion is checked against. Section added 2026-08-28 with its first +entry; see the scope note at the top of this file for why these belong +here despite not being Handbook citations. + +- Ghia, U., Ghia, K.N., and Shin, C.T. (1982). "High-Re solutions for + incompressible flow using the Navier-Stokes equations and a multigrid + method." *Journal of Computational Physics*, 48(3), 387-411. Cited by: + `docs/planning/roadmap.md` (Stage 5 Completion Criterion 5), + `docs/planning/implementation-plan.md` (Level 2), + `adr/ADR-007-executable-acceptance-criteria.md` (worked example), + `docs/glossary.md` ("Validation"). The tabulated lid-driven-cavity + centreline velocity profiles PyFlow's MVP golden demo is validated + against. **Stage 5 adopts its Reynolds number 100 case but not + ADR-007's illustrative 2% tolerance** -- see that criterion for why. + ## Physics None yet -- every physics-entry citation so far (`docs/planning/ diff --git a/planning/data/demos.yaml b/planning/data/demos.yaml index 759fe2a..6e0acdd 100644 --- a/planning/data/demos.yaml +++ b/planning/data/demos.yaml @@ -87,9 +87,24 @@ entities: description: Heat spreading through a fluid by conduction. documented_in: docs/planning/implementation-plan.md edges: + - type: validates + to: capability-level-2 + note: >- + added 2026-08-28, maintainer's call: `docs/implementation/mvp.md` + requires the MVP to reproduce heat diffusion, and the MVP is + Level 2 -- while this file and `implementation-plan.md` both + placed the demo at Level 3 only. Heat diffusion is the + diffusion equation on a transported scalar, which needs no + Temperature field, so Level 2 can and must run it. Found by the + mvp.md reconciliation Stage 5's Completion Criteria required, + not by a task. - type: validates to: capability-level-3 - note: Level 3's Golden Demo, "Heat transport" + note: >- + Level 3's Golden Demo, "Heat transport" -- the + named-Temperature version with buoyancy coupling, reused rather + than re-specified, the same two-edge pattern + demo-taylor-green-vortex uses across Levels 2 and 4 - id: demo-couette-flow name: Couette Flow diff --git a/tools/validators/check_references.py b/tools/validators/check_references.py index b9674fe..18e6060 100644 --- a/tools/validators/check_references.py +++ b/tools/validators/check_references.py @@ -100,7 +100,21 @@ # exactly the planned name (`src/pyflow/engine/simulation.py`) -- its # entry is deleted, per this table's own instruction, rather than left # to prove the path resolves twice. -PLANNED: dict[str, str] = {} +# +# Stage 5's four feature files were added 2026-08-28, when that Stage's +# Completion Criteria were drafted: each task's own Acceptance Criteria +# *are* its feature file (`adr/ADR-007-executable-acceptance- +# criteria.md`), so naming one is naming a real artifact, and the name +# follows the task title the same way every Stage 4 feature file's did. +# No Stage 5 *source* module is listed, deliberately: which modules +# change is what that Stage's own design question one decides, and a +# guess here would be the speculation P-016 refuses. +PLANNED: dict[str, str] = { + "tests/features/velocity_field_support.feature": "TASK-031", + "tests/features/pressure_field.feature": "TASK-032", + "tests/features/pressure_correction_loop.feature": "TASK-033", + "tests/features/navier_stokes_timestep.feature": "TASK-034", +} EXTS = (".md", ".py", ".yaml", ".yml", ".toml", ".cfg", ".txt", ".lock", ".json", ".ini") SPAN = re.compile(r"`([^`\n]+)`") From c580398fde39f82c6e8af90b48e65acd8b6934bf Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Fri, 28 Aug 2026 22:12:24 +0100 Subject: [PATCH 2/6] Audit the Stage 5 draft: six criteria weakened by their own wording, one missing An auditor pass over yesterday's draft, run under prompts/common/AUDITOR.md's stance ("what would make this false?") rather than as a re-read. Six criteria changed, one added, two design questions found. Every correction is the same shape: a criterion that sounded strong and could be passed by an implementation that did nothing of the kind -- the failure Stage 4's exit audit found three times over, caught here for a paragraph instead. - Criterion 3's monotonicity was satisfiable by a no-op. "The sequence of per-iteration divergence magnitudes is non-increasing" is true of a constant sequence, which is exactly what a corrector that does nothing produces. Now: the fixture starts orders of magnitude above tolerance, the decrease is strict across the first passes, and the scenario is mutation-checked against a no-op corrector. - Criterion 4's null test was not physically coherent. A divergence-free field is not automatically steady (a vortex advects itself), and a uniform flow cannot exist inside no-penetration walls. Split into two that are: uniform flow on a periodic domain, and rest in a closed one. - Criterion 2 asked for "a configuration that tries to transport pressure" to be rejected. No configuration surface names which fields are transported, so nothing could have discharged it. Moved to step()'s own signature, where the guard is real. - Criterion 5's kinetic-energy check would have passed regardless: net energy cannot grow while first-order upwind is dissipating. Now no single step may increase it. - Criterion 1's structural clause had no mechanism. Stage 4 already has one -- test_simulation.py's "is_boundary_face" not in inspect.getsource(simulation) -- so it is named rather than reinvented, and the sibling "no engine change" clause got an executable form: a scalar's result must be identical whether its velocity was solved or prescribed. - Criterion 5's cavity bullet gained what makes a convergence study mean something: reference values in a committed fixture citing the paper's table (read off the paper, not from memory), "steady" as a measured residual rather than a step count, and the runtime three resolutions imply named up front. Couette gained the opposite note -- its tolerance should be tight, because the nonlinear term vanishes identically there, so a loose one indicates a defect rather than discretisation error. Criterion 12 is new and was nearly missed: the first draft filed mvp.md's Configuration component under "already true, nothing owed", because ADR-003's six components have been configuration-selected since Stage 3. Checked field by field against schema.py, PyFlow cannot express a viscosity distinct from a scalar's diffusivity, a corrector-loop tolerance distinct from the CG solve's, a tangential wall velocity, a solved-rather-than-prescribed velocity, or any run-length control -- Criterion 3 was already leaning on the second when it said "the configured tolerance". A component being selectable is not a fluid simulation being specifiable. Two design questions neither previously surfaced anywhere: - Where the pressure correction sits relative to RK4's four derivative evaluations -- once per timestep outside the integrator (classical fractional step; splitting error caps the temporal order) or inside every stage (four solves, more accurate). It decides what Criterion 3's "within a single timestep" measures, so it precedes that criterion becoming a feature file. - Whether the pressure gradient reaches momentum through SourceTerm, the only Stage 3 interface with no implementation, no registry entry and no consumer. Either answer is fine; the answer is owed in writing, because an unimplemented interface reads as an oversight the moment the physics it was built for arrives without it. Design question four was broadened from "how is a Reynolds number configured" to the shape of the whole configuration surface, settled once at TASK-031 rather than five times, and absorbed a new half: a single fixed numerics.timestep cannot serve Criterion 5's three mesh resolutions, since explicit RK4's stability limit tightens with dx and dx squared. make ci clean: 614 tests, 99%, all twelve gates; status.md regenerated (Stage 5 now 12 criteria defined, no status line yet). Co-Authored-By: Claude Opus 5 --- README.md | 6 +- docs/CHANGELOG-DESIGN.md | 117 ++++++++++- docs/planning/roadmap.md | 405 +++++++++++++++++++++++++++++++++------ docs/planning/status.md | 2 +- 4 files changed, 460 insertions(+), 70 deletions(-) diff --git a/README.md b/README.md index 84a9cb1..abbd93f 100644 --- a/README.md +++ b/README.md @@ -164,9 +164,9 @@ criteria were written on 2026-08-28, before its first task started** every stage since Stage 2 has followed -- including the reconciliation against `docs/implementation/mvp.md`'s own Definition of Done that Stage 5 has owed since 2026-08-22, since -this is the stage that defines the MVP. Four design questions are -recorded there as open, two of them gaps the current code makes -concrete rather than speculation. +this is the stage that defines the MVP. Six design questions are +recorded there as open, four of them gaps in the current code -- +verified against it rather than anticipated. Try the most recent demonstration -- a scalar blob advected and diffused across a periodic domain, stepped live: diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index 8fc27d5..8a9ca2d 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -5834,7 +5834,7 @@ whole point of the scenario making the call. conftest-scoping claim confirmed by a throwaway probe module that was run and then deleted, not by reading documentation. -### Stage 5 opened: eleven completion criteria, two maintainer decisions, four open design questions +### Stage 5 opened: twelve completion criteria, two maintainer decisions, six open design questions Written before TASK-031 starts, per `docs/practices.md`'s "A stage gets completion criteria before its first task" -- the same point in the @@ -5881,10 +5881,11 @@ qualitative structure (primary vortex centre, both secondary corner vortices) at the finest, with the absolute tolerance stated and defended in the feature file against the mesh actually used. -**Four design questions, recorded open rather than answered.** Stage 4 +**Six design questions, recorded open rather than answered.** Stage 4 set the precedent of flagging these on the day the criteria are drafted -and resolving each before the task that needs it starts; two of Stage -5's four are gaps the code makes concrete, not speculation: +and resolving each before the task that needs it starts; four of Stage +5's six are gaps in the code, verified against it rather than +anticipated: - **A vector field cannot go through the Stage 4 transport path at all.** `AdvectionScheme.flux`/`DiffusionScheme.flux` return one value @@ -5910,6 +5911,105 @@ and resolving each before the task that needs it starts; two of Stage the single scalar Stage 4 had, and viscosity is not a scalar's diffusivity. +**The draft was then audited under `prompts/common/AUDITOR.md`'s stance +-- "what would make this false?" rather than "does this read well?" -- +and that pass changed six criteria, added a twelfth, and found two more +design questions.** Recorded because the corrections are all one shape: +a criterion that sounded strong and could be passed by an +implementation that did nothing of the kind. This is what Stage 4's own +exit audit found three times over, and the point of catching it at +drafting time is that it costs a paragraph now rather than a stage +audit later. + +- **Criterion 3's monotonicity had a hole big enough to drive a no-op + through.** "The sequence of per-iteration divergence magnitudes is + non-increasing" is satisfied by a *constant* sequence -- which is + exactly what a corrector that does nothing produces. Now: the + fixture's starting divergence is stated and orders of magnitude above + tolerance, the decrease is strict across the first passes, and the + scenario must be mutation-checked against a no-op corrector. +- **Criterion 4's null test was not physically coherent.** It asked for + a divergence-free field on a closed domain to "stay put" -- but a + divergence-free field is not automatically steady (a vortex advects + itself), and a uniform flow cannot exist inside no-penetration walls + at all. Split into two that are: uniform flow on a fully periodic + domain, and fluid at rest in a closed no-slip domain. +- **Criterion 2's rejection clause was unbuildable as written.** It + asked for "a configuration that tries to transport pressure" to be + rejected; no configuration surface names which fields are transported, + so nothing could have discharged it. Moved to `step`'s own signature, + where the guard is real. +- **Criterion 5's kinetic-energy check would have passed regardless.** + Net energy over a run cannot grow when first-order upwind is doing the + dissipating, so the check as drafted said nothing about the pressure + correction it was aimed at. Now: no *single step* increases kinetic + energy. +- **Criterion 1's structural clause had no mechanism.** "No branch keyed + on a field's name" is a code-review opinion until someone names how it + is checked -- and Stage 4 already established how, in + `tests/unit/test_simulation.py`'s `"is_boundary_face" not in + inspect.getsource(simulation)`. Named, rather than reinvented. Its + sibling clause ("no engine change" for a second field) got an + executable form too: a scalar's result must be identical whether the + velocity carrying it was solved or prescribed. +- **Criterion 5's cavity bullet gained the things that make a + convergence study mean something**: the reference values live in a + committed fixture citing the paper's own table rather than as literals + in an assertion (and are read off the paper, not from memory); + "steady" is a measured residual, not a step count, which matters most + at the finest mesh where a coarse-mesh step count is guaranteed to be + short; and the runtime three resolutions to steady state implies is + named as part of the criterion rather than discovered in CI. + Couette's bullet gained the opposite note -- its tolerance should be + *tight*, because the nonlinear term vanishes identically for plane + Couette, so upwind's diffusion has no gradient to act on and a loose + tolerance there indicates a defect rather than discretisation error. + +**Criterion 12 is new, and it was nearly missed in the way that matters: +the first draft filed `mvp.md`'s Configuration component under "already +true, nothing owed".** ADR-003's six components have been +configuration-selected since Stage 3, so the sentence looked discharged. +It is not: checked field by field against +`src/pyflow/configuration/schema.py`, PyFlow cannot currently express a +viscosity distinct from a scalar's diffusivity, a corrector-loop +tolerance distinct from the inner CG solve's, a tangential wall +velocity, a "velocity is solved rather than prescribed" mode, or any +run-length control. Criterion 3 was already leaning on the second of +those when it said "the configured tolerance". A component being +selectable is not the same claim as a fluid simulation being +specifiable, and `docs/implementation/golden-demos.md`'s public-API rule +means every one of those has to reach the schema rather than a fixture. + +**Two further design questions, neither previously surfaced anywhere:** + +- **Where the pressure correction sits relative to RK4's stages.** + `RK4Integrator.advance` evaluates its derivative callable at four + states within one timestep; the projection either happens once per + timestep outside it (classical fractional step -- one pressure solve, + splitting error caps the temporal order, and the intermediate stages + see a non-divergence-free field) or inside every stage (four solves, + more accurate). It decides what Criterion 3's "within a single + timestep" is even measuring, so it has to be answered before that + criterion becomes a feature file. +- **Whether the pressure gradient reaches momentum through + `SourceTerm`.** That interface has existed since TASK-018 with no + implementation, no registry entry and no consumer -- the only Stage 3 + interface nothing has ever used. A momentum equation's pressure + gradient is its obvious first candidate; a projection method equally + obviously does not need it. Either answer is fine and the answer is + owed in writing, because an unimplemented interface reads as an + oversight the moment the physics it was built for arrives without it. + +**Design question four was broadened rather than left as one field.** +It was "how is a Reynolds number configured"; it is now the shape of the +whole configuration surface, decided once at TASK-031 rather than five +times as each task hits its own piece -- and it absorbed a new half +nobody had raised: a single fixed `numerics.timestep` cannot serve +Criterion 5's three mesh resolutions, because explicit RK4's stability +limit tightens with the mesh (with `dx` for advection, `dx` squared for +diffusion). A hand-tuned timestep per resolution would make the +convergence study a measurement of the tuning. + **Two smaller things this drafting found and fixed at source.** `docs/planning/backlog.md`'s conservation-checks item still read as though TASK-027 would discharge the divergence-free claim; it landed @@ -5926,7 +6026,12 @@ the rule quietly broken. - *Verified by:* `make ci` clean on this branch; `make check-references` reporting the four planned Stage 5 feature files as promises rather than errors; `make check-graph` clean after the `demos.yaml` edit; - `make status-report` regenerated, showing Stage 5 with 11 criteria + `make status-report` regenerated, showing Stage 5 with 12 criteria defined and no status line yet. The `TypeError` above was produced by running both schemes against a real `VectorField` on a real mesh, not - inferred from reading the code. + inferred from reading the code. Criterion 12's own list of missing + configuration fields was checked field by field against + `src/pyflow/configuration/schema.py`, and the `inspect.getsource` + mechanism Criterion 1 now names was read out of + `tests/unit/test_simulation.py` rather than proposed -- both are + things this repository already does, not things this stage invents. diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index e859316..8af3013 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -5779,15 +5779,29 @@ actual bar (Criterion 5). - The orchestrator advances velocity through the same `src/pyflow/engine/simulation.py` `step` path a scalar takes: the same `AdvectionScheme`/`DiffusionScheme`, the same - `accumulate_flux_to_cells`, the same `TimeIntegrator`. Checked - structurally as well as behaviourally -- no branch anywhere in - `simulation.py` keyed on a field's name, or on a field being a - `VectorField` rather than a `ScalarField`. + `accumulate_flux_to_cells`, the same `TimeIntegrator`. **Checked + structurally as well as behaviourally, by the mechanism Stage 4 + already established for exactly this shape of claim** -- + `tests/unit/test_simulation.py` asserts `"is_boundary_face" not in + inspect.getsource(simulation)` to hold TASK-040 to Stage 4 + Criterion 1's uniformity clause. The same assertion, for the names + this criterion forbids, is what makes "nothing special-cases + velocity" a test rather than a hope: no `"velocity"` string + literal, and no `isinstance(..., VectorField)`, in the + orchestrator's own module. - **A scalar transported alongside velocity in the same run is - advanced by the same call, with no engine change.** Stated as a - Stage 5 criterion rather than left for Stage 6 to discover: the - "no new machinery" claim Stage 6's own criteria already make is far - cheaper to check here, with two fields, than there, with five. + advanced by the same call, and its result does not depend on + velocity being solved rather than prescribed.** The executable form + of "no engine change", which a scenario can actually check and a + diff review cannot: run a scalar with a *solved* velocity field, + capture that velocity, then run the same scalar with that same + velocity *prescribed* as Stage 4's demo does -- the scalar's field + must agree to floating-point tolerance. A transport path that + quietly treats a solved velocity differently fails this; one that + does not, cannot. Stated as a Stage 5 criterion rather than left + for Stage 6 to discover: the "no new machinery" claim Stage 6's own + criteria already make is far cheaper to check here, with two + fields, than there, with five. - **Momentum diffusion is viscosity, and is not the same number as a scalar's diffusivity.** `NumericsConfig.diffusion_coefficient` (`src/pyflow/configuration/schema.py`) is currently one global @@ -5805,9 +5819,17 @@ actual bar (Criterion 5). (`docs/handbook/numerical-methods/pressure-velocity-coupling.md`: pressure "is best understood as a constraint-enforcing field, not a transported one"). - - Pressure never appears among the fields `step` advances, and a - configuration that tries to transport it is rejected with a named - error rather than quietly advecting it. + - Pressure never appears among the fields `step` advances, and + handing `step` a `fields` mapping that contains the pressure field + raises a named error rather than quietly advecting it. **Stated at + the API level deliberately, not the configuration level** -- there + is no configuration surface today that names which fields are + transported (`SimulationConfig` seeds one scalar pattern and one + velocity pattern, nothing more), so "a configuration that tries to + transport pressure" is not currently expressible and a criterion + phrased that way could not be discharged. `step`'s own signature is + where the guard is real, and it is the same place + `MismatchedMeshError` already guards. - Pressure is determined by the velocity field it is solved against, checked as a *pair* of scenarios, not either alone: a provisional velocity that is already divergence-free yields a pressure field @@ -5839,6 +5861,25 @@ actual bar (Criterion 5). recorded sequence of per-iteration maximum divergence magnitudes is non-increasing at every element, and its last element is at or below the configured tolerance. + - **Non-increasing is not sufficient on its own, and the scenario + must close that hole explicitly.** A corrector that does nothing at + all produces a *constant* sequence, which is non-increasing, and if + the fixture's starting field happens to be near-divergence-free the + final element passes too -- the exact shape of "passes for reasons + unrelated to what it claims" that Stage 4's own exit audit found in + the Advection conservation scenario, and found by mutation rather + than by reading. So: the fixture's initial maximum divergence is + stated and is orders of magnitude above the configured tolerance, + the decrease is *strict* across at least the first two passes, and + the whole scenario is mutation-checked by making the corrector a + no-op and confirming it fails. + - **The tolerance must be genuinely configured, not a constant in the + test.** `NumericsConfig` has no divergence tolerance today -- + `linear_solver_tolerance` is the inner CG solve's, a different + number for a different convergence question -- so this criterion + depends on Criterion 12's config surface existing, and says so + rather than letting an implementer read "configured" as "chosen in + the fixture". - **This is where Stage 4 Completion Criterion 4's Pressure-Velocity Coupling bullet's stronger claim is discharged**, the deferral TASK-027 recorded explicitly rather than silently narrowing its own @@ -5869,12 +5910,21 @@ actual bar (Criterion 5). sequence is checked, not only its endpoint.** Predictor, corrector loop, corrected state -- the assembled per-step sequence `pressure-velocity-coupling.md` describes, with each part observable. - - **A null test the rest cannot substitute for:** a divergence-free - initial field on a closed domain, with no forcing and zero - viscosity, stays divergence-free and stays put over many steps, to - a stated tolerance. An implementation that manufactures velocity - out of its own correction term fails this and passes almost - everything else in this list. + - **Two null tests the rest cannot substitute for**, stated as a + pair because the obvious single version of this does not hold: a + divergence-free field is not automatically a *steady* solution (a + vortex is divergence-free and advects itself), and a uniform flow + cannot exist inside a closed domain at all, since no-penetration + walls forbid it. So: + - **Uniform flow on a fully periodic domain, zero viscosity, no + forcing:** stays divergence-free and translates at exactly the + prescribed speed, unchanged in shape, over many steps. Genuinely + steady, so any spurious pressure correction shows up immediately. + - **Fluid at rest in a closed domain, every wall no-slip:** stays + at rest, to floating-point tolerance, over many steps. Trivial to + state and the sharpest guard in this list -- an implementation + that manufactures velocity out of its own correction term fails + it and passes almost everything else here. - Deterministic: the same configuration run twice produces identical state, checked directly. Already required of every golden demo (`docs/implementation/golden-demos.md`'s Definition of Done), @@ -5892,7 +5942,8 @@ actual bar (Criterion 5). `docs/practices.md`'s testable-physics extension, and stated per case rather than left generic, the same shape Stage 4's own Criterion 4 took. - - **Couette flow, first.** Plane shear flow between two plates, one + - **Couette flow, first, and to a tight tolerance rather than a + generous one.** Plane shear flow between two plates, one stationary and one moving, with no imposed pressure gradient: the steady velocity profile is exactly linear, and is checked against that analytic profile cell by cell. The simplest incompressible @@ -5901,6 +5952,18 @@ actual bar (Criterion 5). isolates viscous diffusion plus the wall treatment from any pressure-gradient effect -- which is exactly why it comes before the cavity rather than after it. + - **The tolerance is tight because the nonlinear term vanishes + identically here, not because the scheme is accurate.** With + `v = 0` everywhere and `u` a function of `y` alone, the + convective term `u du/dx + v du/dy` is exactly zero, so + first-order upwind's numerical diffusion -- the error term that + makes the cavity's own tolerance a negotiation below -- has no + gradient to act on and contributes nothing. Agreement should be + at solver tolerance, and a scenario that passes only at a loose + tolerance is evidence of a defect rather than of discretisation + error. Recorded here so that nobody later reads a loosened + Couette tolerance as the same kind of honest concession the + cavity's is. - **Lid-driven cavity against Ghia, Ghia & Shin (1982)**, whose tabulated centreline velocity profiles at Reynolds number 100 are the published reference three other documents already point at @@ -5917,6 +5980,40 @@ actual bar (Criterion 5). right at the finest -- primary vortex centre within a stated distance of Ghia's, and both downstream secondary corner vortices present. + - **The reference values are committed data with their citation + attached, not literals typed into an assertion.** Ghia et al.'s + tabulated `u` along the vertical centreline and `v` along the + horizontal one, for Re = 100, plus the paper's own primary-vortex + centre coordinates, belong in a small committed fixture naming + the paper, table and column each number came from. **The values + are read off the paper itself, not from memory or a secondary + source** -- the widely-quoted primary-vortex centre for Re = 100 + is approximately `(0.617, 0.734)` in unit-cavity coordinates, and + that approximation is written here as a sanity check on the + fixture, not as the number to assert against. This repository has + no test-data directory yet, so wherever it lands is a new + convention: `docs/repository-manifest.md` and the nearest + `CLAUDE.md` are updated in the same change, per the Blast Radius + rule. + - **"Steady" is a measured residual, not a step count.** The + comparison is against Ghia's *steady-state* solution, so the run + has to reach steady state to be comparable at all -- and a fixed + number of timesteps is not evidence that it did, at any + resolution, let alone at three. The scenario states the + steadiness measure it uses (e.g. the maximum per-cell velocity + change per step falling below a stated threshold) and fails on + *not reaching it* rather than silently comparing an unconverged + field. This matters more under mesh refinement than anywhere + else: a finer mesh needs both more steps and a smaller timestep + (design question four), so a step count that sufficed at the + coarsest resolution is guaranteed not to suffice at the finest. + - **The runtime this implies is part of the criterion, not a + surprise to discover in CI.** Three resolutions run to steady + state is the most expensive check this project will have + attempted; the whole suite runs in under two minutes today. If + the honest version does not fit that budget, the resolution to + state is where the check runs and how it stays a real gate -- + not a quietly reduced number of resolutions. - **ADR-007's illustrative "within 2%" does not bind this stage.** That number appears in a worked example showing what an executable physics criterion *looks like*, and in the glossary's @@ -5944,11 +6041,31 @@ actual bar (Criterion 5). demonstrably not at another; both are Level 2 cases already (`docs/planning/implementation-plan.md`), and either pair satisfies this bullet. + - **Which of the two to use is decided by measurement, and the + risk that neither survives is stated now rather than met as a + surprise.** Both phenomena are sensitive to numerical diffusion, + and the MVP's advection scheme is the most diffusive one there + is: upwind can suppress Kelvin-Helmholtz roll-up entirely at + coarse resolution, and can dominate Taylor-Green's physical decay + rate so that the measured rate reflects the scheme rather than + the viscosity. Measure before committing a scenario to either. + **If neither survives at a resolution this stage can afford, + that is a real finding about the MVP's numerics -- report it and + rescope with the maintainer** (Stage 7 is where a less diffusive + scheme lands, `docs/implementation/upgrade-paths.md`). It is not + licence to quietly drop the negative control, which is the half + of this bullet that does the work. - **Conservation, a claim none of the three above makes.** On a closed domain with no forcing, an inviscid flow's total kinetic - energy must not *grow*. A pressure correction that injects energy - can still match a steady-state profile comparison and still be - wrong; this is the bullet that catches it, and it is the velocity + energy must not *grow* -- **checked step by step, not as a net + change over the run.** The distinction is the whole value of the + bullet: first-order upwind is strongly dissipative, so the net + trend is downward whatever the pressure correction does, and a net + check therefore passes trivially while a correction that injects + energy on individual steps goes unseen. No single step increases + total kinetic energy. A pressure correction that injects energy can + still match a steady-state profile comparison and still be wrong; + this is the bullet that catches it, and it is the velocity counterpart of the conservation checks Stage 4 already required of Advection and Diffusion separately (`docs/planning/backlog.md`, conservation checks -- whose Pressure-Velocity Coupling bullet @@ -6088,21 +6205,74 @@ actual bar (Criterion 5). | Capability map is updated | Criterion 10's own capability-map bullet | **`mvp.md`'s Components and Validation sections are reconciled too, - not only its Definition of Done** -- three findings, recorded here + not only its Definition of Done** -- four findings, recorded here rather than left for the exit audit: - *Already true, nothing owed:* 2D, structured Cartesian, uniform spacing, single incompressible fluid, FVM, collocated arrangement, first-order upwind, central-difference diffusion, RK4, Conjugate - Gradient, Dirichlet/Neumann/Periodic boundaries, and - configuration-selected components. All landed across Stages 1-4; - this stage adds none of them. + Gradient, and Dirichlet/Neumann/Periodic boundaries. All landed + across Stages 1-4; this stage adds none of them. **Rendering is + the one entry here that is true but untested in the way that + matters** -- `mvp.md` requires "real-time visualisation of scalar + and vector fields", which exists (TASK-017), but no vector field + PyFlow has rendered was ever *solved*; Criterion 8 is where that + stops being an assumption. - *Owed by Criterion 3:* PISO. Named as an MVP component, currently a single-pass approximation registered under that name. - *Owed by Criterion 8:* the Validation section's three cases. One is closed (passive scalar transport, TASK-030); the other two are this stage's, per the Heat Diffusion decision recorded above. - -### Four design questions, all open + - *Owed by Criterion 12, and nearly missed:* `mvp.md`'s + **Configuration** component -- "simulation components are selected + through configuration rather than by modifying engine code". The + first draft of this reconciliation filed that under "already true, + nothing owed", because `adr/ADR-003`'s six components have been + configuration-selected since Stage 3. That reading is too + generous by exactly the physics this stage adds: a *component* is + selectable, but not one of the physical parameters a fluid + simulation is actually specified by. See Criterion 12. +12. **Everything this stage adds is configuration-driven, validated, and + documented -- not reachable only from a test fixture.** + `docs/implementation/golden-demos.md`'s standing rule is that "if a + demo needs a capability configuration doesn't yet expose, that + capability gets added to the public configuration schema... the fix + is never demo-specific code working around the gap." Stage 5 needs + more of that than any stage so far, and the gap is concrete rather + than anticipated -- checked field by field against + `src/pyflow/configuration/schema.py` while drafting these criteria, + not assumed: + - **A viscosity.** `NumericsConfig.diffusion_coefficient` is one + global constant serving the one transported scalar Stage 4 had. + Criterion 5's Reynolds-number comparison cannot be configured + without separating the two (design question four). + - **A corrector-loop tolerance, and an iteration limit.** Neither + exists; `linear_solver_tolerance`/`linear_solver_max_iterations` + are the inner CG solve's, a different convergence question at a + different level. Criterion 3 says "the configured tolerance" and + means it. + - **A tangential boundary velocity** (design question two) -- with + no way to say "this wall moves along itself", neither the cavity + nor Couette can be configured at all. + - **A way to say velocity is solved rather than prescribed.** + `SimulationConfig.velocity`/`velocity_pattern` are documented as + "a prescribed (not solved) constant vector -- Stage 5 is what + eventually solves Navier-Stokes for real". This stage is that, so + that sentence stops being true and something has to express the + difference. + - **Whatever the run-length or steadiness control turns out to be**, + per Criterion 5's steady-state bullet -- a demo that must reach + steady state needs to say so somewhere a config author can see. + - Every field added is validated the way every existing one is (a + rejection test per field, `tests/unit/test_configuration.py`'s own + shape), appears in the regenerated + `docs/implementation/config-template.yaml` with a comment stating + what counts as a valid value -- `make check-config-template` and + `test_every_live_config_field_has_a_comment` both already gate + this -- and does not add a member to any `Literal[...]` closed set + without a real second choice to justify it (P-016, the rule Stage + 4's own Criterion 2 applied). + +### Six design questions, all open Flagged here at stage open rather than left to surface mid-implementation (`docs/practices.md`, "When intent is ambiguous, hold a design session @@ -6170,19 +6340,76 @@ resolve with numerical prototyping before Criterion 3 is turned into a feature file** -- a criterion that cannot be met is worse than one drafted late. -**Four: how is a Reynolds number configured at all? (TASK-031.)** -Criterion 5's cavity bullet compares against Ghia et al. at Re = 100, and -Re = UL/nu needs a kinematic viscosity that is currently -indistinguishable from `NumericsConfig.diffusion_coefficient`, the single -global constant Stage 4's one transported scalar used. Related, and worth -resolving together rather than twice: TASK-040's own Design decision -recorded that PyFlow has *one* global set of boundary conditions shared -across every transported field, "correct for a single transported -scalar... but does not yet express 'field A is 300K at this wall, field B -is 0 at the same wall'". Stage 5 is the first stage with two genuinely -different transported fields in one run -- velocity at a moving lid and a -scalar at the same wall -- so the case that note deferred to Stage 6's -"no new machinery" criterion may bite here first. +**Four: what shape does this stage's new configuration surface take? +(TASK-031, with TASK-034 as its last consumer.)** Criterion 12 lists +what is missing and why each is needed; this question is how it is +expressed, and it wants deciding once rather than five times as each +task hits its own piece. The pieces, in the order they bite: a +kinematic viscosity separable from `NumericsConfig. +diffusion_coefficient` (Criterion 5 compares at Re = UL/nu, and the two +numbers are indistinguishable today); a corrector-loop tolerance and +iteration limit distinct from the inner CG solve's; a tangential +boundary velocity (design question two); a way to say velocity is +*solved* rather than prescribed; and whatever controls run length or +steadiness. Two further constraints on any answer, both already written +down elsewhere and easy to miss: +- TASK-040's own Design decision recorded that PyFlow has *one* global + set of boundary conditions shared across every transported field, + "correct for a single transported scalar... but does not yet express + 'field A is 300K at this wall, field B is 0 at the same wall'". Stage + 5 is the first stage with two genuinely different transported fields + in one run -- velocity at a moving lid and a scalar at the same wall + -- so the case that note deferred to Stage 6's "no new machinery" + criterion may bite here first. +- **How the timestep is chosen under mesh refinement.** + `NumericsConfig.timestep` is a single fixed number, and Criterion 5's + cavity comparison runs the same case at three resolutions. Explicit + RK4 is stability-limited by both the CFL condition (`dt` with `dx`) + and the diffusive limit (`dt` with `dx` squared), so a timestep that + is stable at the coarsest resolution is *guaranteed* to be unstable at + the finest. Either the timestep becomes derivable from the mesh and + the configured viscosity, or every refinement level carries its own + configured timestep and the derivation lives in the scenario. Nothing + in the repository does either today. + +**Five: where does the pressure correction sit relative to RK4's +stages? (TASK-033/034.)** Not surfaced by anything before this pass, and +it decides what Criterion 4's "predictor, corrector loop, corrected +state" sequence actually *is*. `RK4Integrator.advance` evaluates its +`derivative` callable at four states within one timestep +(`adr/ADR-008-time-integrator-derivative-callable.md`), and the +projection has to happen somewhere relative to those four: +- **Outside the integrator** -- RK4 advances momentum with no pressure + term to a provisional velocity, then the corrector loop projects once + per timestep. The classical fractional-step arrangement, one pressure + solve per step, and the reading `docs/handbook/numerical-methods/ + time-integration.md` and TASK-025's own criterion already anticipate + when they say the finished solver's observed order will be "well below + four". Its cost is exactly that: the splitting error caps the temporal + order however good the integrator is, and RK4's intermediate stages + see a velocity field that is not divergence-free. +- **Inside each stage** -- every `derivative` evaluation projects, so + each stage sees a divergence-free field. More accurate, and four + pressure solves per timestep instead of one. +The choice is not free either way, and it interacts with design question +three: whichever arrangement is chosen is what the corrector loop's +monotonic-convergence claim is measured *within*. Resolve it with the +same prototyping-first sequence TASK-027 used, and record which reading +was taken in the scenario, not only in a docstring. + +**Six: does the pressure gradient reach momentum through `SourceTerm`, +or only through the projection? (TASK-032/033.)** `src/pyflow/engine/ +numerics/source.py`'s `SourceTerm` has existed since TASK-018 with no +concrete implementation, no registry entry, and no consumer -- the one +Stage 3 interface nothing has ever used. A momentum equation's pressure +gradient is the obvious candidate for its first implementation; a +projection method equally obviously does not need it, because the +correction *is* the gradient's effect. **Answer it explicitly either +way.** If the answer is no, say so where `SourceTerm` lives, so that an +interface with no implementation two stages after the physics arrived +reads as a decision rather than the oversight it currently resembles -- +the same treatment Gradient/Divergence got at TASK-027, which is what +stopped them being invisible. ### Discharge map @@ -6214,6 +6441,7 @@ obligation, not a reordering. | 9. `make ci` green on a real runner | TASK-034 | | 10. Documentation matches the tree, capability map included | TASK-034 | | 11. `mvp.md`'s Definition of Done discharged item by item | TASK-034, reading Criterion 11's own table back against what landed | +| 12. Configuration surface is real, validated and documented | TASK-031 for viscosity and solved-vs-prescribed velocity; TASK-033 for the corrector tolerance and iteration limit; TASK-034 for the tangential boundary value and whatever run-length control the demos need -- each task adds the fields its own criteria depend on, rather than one task inventing the whole surface up front | **TASK-034 is this stage's last task in build order and therefore owns the stage-level criteria** -- the demonstrations, the CI evidence, the @@ -6262,11 +6490,15 @@ resolves to), TASK-040 (`step`, `accumulate_flux_to_cells`). **One and four above, both open, both blocking this task's own drafting.** One decides how a vector field goes through an interface that returns one value per face -- verified as a real `TypeError`, not -a hypothetical. Four decides how a viscosity is configured separately -from a scalar's diffusivity, without which Criterion 5's Reynolds-number -comparison has nothing to read. Two (tangential boundary values) is -listed against TASK-034 as well as here, because whichever of the two -first needs a no-slip wall is where it has to be answered. +a hypothetical. Four decides the shape of this stage's configuration +surface; this task needs two pieces of it (a viscosity separable from a +scalar's diffusivity, and a way to say velocity is solved rather than +prescribed) and **should settle the shape for all five pieces rather +than adding one field and leaving the next task to invent a different +style for the second** -- that is why it is one question rather than +five. Two (tangential boundary values) is listed against TASK-034 as +well as here, because whichever of the two first needs a no-slip wall is +where it has to be answered. ### Artifacts Produced @@ -6285,8 +6517,17 @@ first needs a no-slip wall is where it has to be answered. restated here as prose. Written to cover, at minimum: - Velocity is advanced by the same `step` call, over the same timestep, - that advances a scalar -- and a run carrying both advances both, with - no engine change between the one-field and two-field cases. + that advances a scalar -- and a run carrying both advances both. +- **The scalar's own result is identical whether the velocity carrying + it was solved or prescribed** (Criterion 1's own executable form of + "no engine change"): capture a solved velocity field, re-run the + scalar against that same field supplied as Stage 4 supplies one, and + compare to floating-point tolerance. +- The orchestrator module's source contains no `"velocity"` string + literal and no `VectorField` `isinstance` check, asserted the way + `tests/unit/test_simulation.py` already asserts the absence of + `is_boundary_face` -- Criterion 1's structural clause, using the + mechanism Stage 4 established for it rather than a new one. - A momentum diffusion coefficient (viscosity) distinct from a scalar's diffusivity: setting one leaves the other's computed flux unchanged. - Velocity advected by itself reproduces a hand-derived result on a @@ -6297,10 +6538,16 @@ restated here as prose. Written to cover, at minimum: dimensionality is rejected with the existing named error (`IncompatibleVelocityFieldError`), exercised against this task's own new path rather than assumed inherited (Criterion 6). +- Each configuration field this task adds rejects bad input with a named + error, the same shape every other config section's tests already take + (`tests/unit/test_configuration.py`), and appears in the regenerated + `docs/implementation/config-template.yaml` with a comment saying what + counts as valid (Criterion 12). ### Discharges -Criterion 1, entirely. Criterion 6 and Criterion 7, its own share. +Criterion 1, entirely. Criterion 12, its viscosity and +solved-vs-prescribed share. Criterion 6 and Criterion 7, its own share. --- @@ -6326,6 +6573,16 @@ TASK-031 (a transported velocity field to solve against), TASK-026 `GreenGaussDivergence`, and the compact symmetric Laplacian that task already builds the Poisson matrix from). +### Open design question + +**Six above** -- whether the pressure gradient reaches the momentum +equation through `SourceTerm`, the one Stage 3 interface nothing has +ever implemented, or only through the projection correction. This is the +task that has cause to answer it, and the answer is owed in writing +either way: an unimplemented interface is fine while nothing needs it, +and reads as an oversight the moment the physics it was built for +arrives without using it. + ### Artifacts Produced - `tests/features/pressure_field.feature` -- this task's Acceptance @@ -6400,7 +6657,7 @@ TASK-031, TASK-032, and TASK-027's own finding -- which is a dependency in the literal sense: this task starts from a measured negative result about what does not work, not from a blank page. -### Open design question +### Open design questions **Three above, open, and the one most likely to move this stage's scope.** What carries the momentum-equation coefficients Rhie-Chow @@ -6413,6 +6670,13 @@ it is unreachable second is precisely the failure `docs/practices.md`'s "A criterion whose strong reading depends on a later task must say so when drafted" was added to prevent, and this is the later task. +**Five above, and it has to be answered before three can be measured.** +Where the correction sits relative to RK4's four derivative evaluations +decides what "within a single timestep" means for this task's own +monotonic-convergence sequence -- one projection per step, or one per +stage. Resolve the two together; measuring convergence inside an +arrangement nobody has chosen is measuring nothing. + ### Artifacts Produced - `tests/features/pressure_correction_loop.feature` -- this task's @@ -6445,7 +6709,8 @@ Written to cover, at minimum: ### Discharges Criterion 3, entirely. Criterion 5's Couette bullet, jointly with -TASK-034. Criterion 6 and Criterion 7, its own share. **Stage 4 +TASK-034. Criterion 12, its corrector-tolerance and iteration-limit +share. Criterion 6 and Criterion 7, its own share. **Stage 4 Completion Criterion 4's Pressure-Velocity Coupling deferral**, which is re-read at this task's close rather than assumed discharged. @@ -6503,12 +6768,22 @@ own visual output); TASK-030's own demo wiring mechanism a live demo already uses and this one extends rather than replaces. -### Open design question +### Open design questions **Two above** -- tangential boundary values -- if TASK-031 has not already had to answer it. A no-slip wall and a moving lid are both tangential, and both of this task's own validation cases need them. +**Four's timestep half**, which lands squarely on this task even if the +rest of the configuration surface was settled at TASK-031: Criterion 5 +runs the cavity at three resolutions, and explicit RK4's stability limit +tightens with the mesh (with `dx` for advection, with `dx` squared for +diffusion). A single fixed `numerics.timestep` cannot serve all three, +so either it becomes derivable or each resolution carries its own -- and +the derivation is stated in the scenario either way, since a silently +hand-tuned timestep per resolution is a convergence study measuring the +tuning. + ### Artifacts Produced - `tests/features/navier_stokes_timestep.feature` -- this task's own @@ -6531,26 +6806,36 @@ feature file, are the criteria. Written to cover, at minimum: - The predictor/corrector/corrected sequence, each part observable -- not only the end state (Criterion 4). -- The null test: a divergence-free field on a closed domain, no forcing, - zero viscosity, stays divergence-free and stays put over many steps. +- Both null tests, per Criterion 4: uniform flow on a fully periodic + domain translates unchanged and stays divergence-free; fluid at rest + in a closed, no-slip domain stays at rest. Not one combined scenario + -- a uniform flow cannot exist inside a closed domain at all. - Determinism: the same configuration run twice produces identical state. -- Couette flow against its exact linear profile (jointly with TASK-033). +- Couette flow against its exact linear profile, at solver tolerance + rather than a loose one (jointly with TASK-033, and see Criterion 5's + own note on why the nonlinear term vanishing makes that reachable). - Lid-driven cavity against Ghia, Ghia & Shin (1982) at Re = 100: monotonically decreasing error across at least three mesh resolutions, plus the qualitative structure at the finest -- **not** a fixed percentage, per Criterion 5's own bullet and the 2026-08-28 decision - recorded there. + recorded there. The reference values come from a committed fixture + citing the paper's own table, and steadiness is a measured residual + rather than a step count. - The emergent-phenomenon pair: the instability under a configuration - that should produce it, and its absence under one that should not. -- Kinetic energy does not grow for an inviscid, unforced, closed-domain - flow. + that should produce it, and its absence under one that should not -- + with the candidate chosen by measurement, per Criterion 5's note on + what the MVP's numerical diffusion may suppress. +- No single step increases total kinetic energy for an inviscid, + unforced, closed-domain flow -- step by step, not net over the run, + since upwind's own dissipation makes the net check pass regardless. ### Discharges Criteria 4, 8, 9, 10 and 11, entirely. Criterion 5, all bullets (its -Couette bullet jointly with TASK-033). Criterion 6 and Criterion 7, its -own share. +Couette bullet jointly with TASK-033). Criterion 12, its +tangential-boundary and run-length share. Criterion 6 and Criterion 7, +its own share. Golden Demo diff --git a/docs/planning/status.md b/docs/planning/status.md index 52ec673..a4f9b4b 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -115,7 +115,7 @@ pie showData ### Stage 5 -- First Fluid Solver -**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 11 criteria defined, no status line yet +**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 12 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| From 2121bf40d6f4699a9af2214ad9a5b9dfa36b299d Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Fri, 28 Aug 2026 22:30:01 +0100 Subject: [PATCH 3/6] Third audit pass: the criterion an otherwise-passing Stage 5 could fail silently The second pass sharpened what was written; this one asked what was still absent, and found the criterion that matters most. Criterion 13 is new: the finished solver must run through ADR-003's seams, checked by substituting a registered test double rather than by reading call sites. mvp.md states the MVP's purpose as "correctness, understandability, and architectural validation", and Stage 5 is the first time all six configuration-selected components run together -- yet nothing in twelve criteria checked that the *configured* coupling is the object the timestep calls. simulation.step consumes advection/diffusion/ time_integration and has never touched numerics.pressure_coupling or .linear_solver, because Stage 4 had nothing to correct. A timestep that constructs PISO directly would pass every other criterion -- same Ghia comparison, same convergence, same demos -- while quietly retiring the architectural claim the MVP exists to validate. Three smaller corrections from the same pass: - Criterion 12's P-016 clause forbade something it should not: as drafted it barred adding a member to any Literal[...] closed set, which would block the initial-condition patterns Criterion 5's own cases need. P-016 is about not building a second scheme nobody needs; a pattern added because a demo needs it is the justification P-016 asks for. The two cases look identical in the type system and opposite in intent, so they are now distinguished explicitly. - Criterion 4's determinism clause gained a cross-platform half. Criterion 9 requires green CI on Ubuntu and Windows, and this is the first stage whose result depends on accumulated floating-point arithmetic in an iterative solve; an exact-value demo assertion is a one-platform flake waiting to happen. Determinism is same-process, cross-run comparisons state a tolerance. - Criterion 8's Heat Diffusion demo has to validate something quantitative, since mvp.md lists it under Validation rather than Components. A sinusoidal mode on a periodic domain decays at a rate set by the diffusion coefficient and wavenumber -- checkable against the analytic rate, and distinct from Stage 4's convergence-order and conservation checks, neither of which is a rate. A seventh design question: does anything this stage builds belong in src/pyflow/physics/? The repository already contradicts itself and nobody had cause to look -- physics/__init__.py's docstring says the package is for physical models, "incompressible flow first", while physics/CLAUDE.md opens "Empty until Stage 6, and empty on purpose". Both predate anyone having a momentum equation to file. Stage 6's own measurable claim depends on the answer: TASK-035's intent proposes counting lines added outside physics/, which measures little if physics/ is still empty when Stage 6 opens. Also recorded in the changelog: a str.index-based splice in this pass matched Stage 3's "### Discharge map" rather than Stage 5's and duplicated ~3,900 roadmap lines into the file. Caught by the line count, reverted, and redone through anchored replacements. In a document with repeating headings, position-based splicing cannot know which section it found. make ci clean: 614 tests, 99%, all twelve gates; status.md regenerated (Stage 5 now 13 criteria defined, no status line yet). Co-Authored-By: Claude Opus 5 --- README.md | 4 +- docs/CHANGELOG-DESIGN.md | 74 ++++++++++++++++++++++++++++--- docs/planning/roadmap.md | 96 +++++++++++++++++++++++++++++++++++++--- docs/planning/status.md | 2 +- 4 files changed, 161 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index abbd93f..53e57f6 100644 --- a/README.md +++ b/README.md @@ -164,8 +164,8 @@ criteria were written on 2026-08-28, before its first task started** every stage since Stage 2 has followed -- including the reconciliation against `docs/implementation/mvp.md`'s own Definition of Done that Stage 5 has owed since 2026-08-22, since -this is the stage that defines the MVP. Six design questions are -recorded there as open, four of them gaps in the current code -- +this is the stage that defines the MVP. Seven design questions are +recorded there as open, five of them gaps in the current code -- verified against it rather than anticipated. Try the most recent demonstration -- a scalar blob advected and diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index 8a9ca2d..e35db22 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -5834,7 +5834,7 @@ whole point of the scenario making the call. conftest-scoping claim confirmed by a throwaway probe module that was run and then deleted, not by reading documentation. -### Stage 5 opened: twelve completion criteria, two maintainer decisions, six open design questions +### Stage 5 opened: thirteen completion criteria, two maintainer decisions, seven open design questions Written before TASK-031 starts, per `docs/practices.md`'s "A stage gets completion criteria before its first task" -- the same point in the @@ -5881,10 +5881,10 @@ qualitative structure (primary vortex centre, both secondary corner vortices) at the finest, with the absolute tolerance stated and defended in the feature file against the mesh actually used. -**Six design questions, recorded open rather than answered.** Stage 4 +**Seven design questions, recorded open rather than answered.** Stage 4 set the precedent of flagging these on the day the criteria are drafted -and resolving each before the task that needs it starts; four of Stage -5's six are gaps in the code, verified against it rather than +and resolving each before the task that needs it starts; five of Stage +5's seven are gaps in the code, verified against it rather than anticipated: - **A vector field cannot go through the Stage 4 transport path at @@ -6023,10 +6023,74 @@ was extended to cover papers a completion criterion is checked against, with `docs/references/CLAUDE.md` amended in the same change rather than the rule quietly broken. +**A third pass over the audited draft found the criterion that mattered +most, which is the argument for having run it.** The second pass had +sharpened what was written; this one asked what was still absent. + +**Criterion 13 is new: the finished solver has to run through +`adr/ADR-003`'s seams, checked by substituting a registered test double +rather than by reading the call sites.** `docs/implementation/mvp.md` +states the MVP's purpose as "correctness, understandability, and +architectural validation", and Stage 5 is the first time all six +configuration-selected components run together -- yet nothing in twelve +criteria checked that the configured coupling is the object the timestep +actually calls. `simulation.step` consumes +`numerics.advection`/`.diffusion`/`.time_integration` and has never +touched `numerics.pressure_coupling` or `.linear_solver`, because Stage +4 had nothing to correct. A timestep that constructs `PISO` directly +would pass every other criterion in the list -- same Ghia comparison, +same convergence, same demos -- while quietly retiring the architectural +claim the MVP exists to validate. Stage 4's Criterion 2 checked that a +name *resolves* to a real instance; this checks that the resolved +instance is what runs. + +Three smaller corrections from the same pass: + +- **Criterion 12's P-016 clause forbade something it should not.** As + drafted it barred adding a member to any `Literal[...]` closed set, + which would have blocked the initial-condition patterns Criterion 5's + own cases need (a lid-driven configuration, a shear layer). P-016 is + about not building a second *scheme* nobody needs; a pattern added + because a demo needs it is the justification P-016 asks for, not a + violation of it. The two cases are now distinguished explicitly, + because they look identical in the type system and opposite in intent. +- **Criterion 4's determinism clause needed a cross-platform half.** + Criterion 9 requires green CI on Ubuntu *and* Windows, and this is the + first stage whose result depends on accumulated floating-point + arithmetic in an iterative solve. An exact-value demo assertion is a + one-platform flake waiting to happen. Determinism is same-process; + cross-run comparisons state a tolerance. +- **A seventh design question: does anything this stage builds belong in + `src/pyflow/physics/`?** The repository already contradicts itself + here and nobody had noticed, because nobody had cause to look: + `physics/__init__.py`'s docstring says the package is for physical + models, "incompressible flow first" -- this stage's exact subject -- + while `physics/CLAUDE.md` opens "Empty until Stage 6, and empty on + purpose". Both predate anyone having a momentum equation to file. The + boundary `CLAUDE.md` draws (phenomena here, discretisation in + `engine/numerics/`) is worth keeping and does not settle the case, + since the equation is physics and what this stage writes is its + discretisation. Stage 6's own measurable claim depends on the answer: + its TASK-035 intent proposes counting the lines added *outside* + `physics/`, which measures little if `physics/` is still empty when + Stage 6 opens. + +**One process note, recorded because the outcome was nearly bad.** The +reordering of two design questions in this pass was attempted with a +`str.index`-based splice, and `index` matched Stage 3's "### Discharge +map" rather than Stage 5's -- duplicating roughly 3,900 lines of the +roadmap into itself. Caught immediately by the line count and reverted +from the last commit, with the four post-commit edits redone through +anchored replacements instead. The lesson is narrow and worth keeping: +in a document with repeating section headings, position-based splicing +has no way to know which section it found, and a uniquely-anchored +replacement does. `wc -l` before and after a structural edit is a +two-second check that catches the whole class. + - *Verified by:* `make ci` clean on this branch; `make check-references` reporting the four planned Stage 5 feature files as promises rather than errors; `make check-graph` clean after the `demos.yaml` edit; - `make status-report` regenerated, showing Stage 5 with 12 criteria + `make status-report` regenerated, showing Stage 5 with 13 criteria defined and no status line yet. The `TypeError` above was produced by running both schemes against a real `VectorField` on a real mesh, not inferred from reading the code. Criterion 12's own list of missing diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index 8af3013..8b12a59 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -5930,6 +5930,16 @@ actual bar (Criterion 5). (`docs/implementation/golden-demos.md`'s Definition of Done), restated here because Criterion 8's demo and any future checkpoint/replay both rest on it. + - **Identical within a process; agreeing to a stated tolerance + across platforms.** Criterion 9 requires green CI on both Ubuntu + and Windows, and this stage is the first to run an iterative + solve whose result depends on accumulated floating-point + arithmetic -- BLAS ordering and library versions differ between + the two runners, so a demo regression test asserting exact values + is a cross-platform flake waiting to happen rather than a + determinism check. Stated in advance because the failure mode is + a red build on one platform only, which reads like a real defect + and costs a session to diagnose. - **Checkpoint/pause/rewind is explicitly not a criterion of this stage**, stated so its absence is not later mistaken for a gap: it is recorded as future scope under TASK-034 below, and @@ -6139,6 +6149,19 @@ actual bar (Criterion 5). one repeated. `implementation-plan.md` and `planning/data/demos.yaml` are amended in the same change as this criterion, not left to be rediscovered. + - **It validates something quantitative, because `mvp.md` lists it + under Validation rather than under Components.** A demo that + shows heat spreading and nothing more would satisfy the letter of + this bullet and none of its purpose -- + `docs/implementation/golden-demos.md`'s Definition of Done + already asks for "meaningful behaviour, not just 'it ran without + crashing'", and this case has an exact answer available: a single + sinusoidal mode on a periodic domain decays exponentially at a + rate set by the diffusion coefficient and the mode's wavenumber, + so the measured decay rate is checkable against the analytic one + rather than against a picture. Distinct from Stage 4's own + diffusion criteria, which measured spatial convergence order and + conservation -- neither of which is a decay *rate*. - **Poiseuille flow, Taylor-Green vortex and Kelvin-Helmholtz instability are not required as demos here**, stated so their absence is not read as a gap: all three are Level 2 catalog entries @@ -6264,15 +6287,50 @@ actual bar (Criterion 5). steady state needs to say so somewhere a config author can see. - Every field added is validated the way every existing one is (a rejection test per field, `tests/unit/test_configuration.py`'s own - shape), appears in the regenerated + shape) and appears in the regenerated `docs/implementation/config-template.yaml` with a comment stating what counts as a valid value -- `make check-config-template` and `test_every_live_config_field_has_a_comment` both already gate - this -- and does not add a member to any `Literal[...]` closed set - without a real second choice to justify it (P-016, the rule Stage - 4's own Criterion 2 applied). - -### Six design questions, all open + this. + - **P-016 applies to the six component-name sets, not to the pattern + sets, and the difference is worth stating before someone applies + the wrong one.** Stage 4's Criterion 2 forbade adding a member to + `AdvectionSchemeName` and its five siblings, because a second + scheme nobody needs is speculation. `ScalarTransportPattern`/ + `VelocityPrescriptionPattern` are the opposite case: a member is + added there precisely *because* a demo needs it, which is the + justification P-016 asks for, and Criterion 5's cases will need + several (a lid-driven configuration, a shear layer, whatever the + emergent-phenomenon pair settles on). Adding those is expected; + adding a second advection scheme is not. +13. **The finished solver runs through `adr/ADR-003`'s seams, not around + them** -- checked by substitution, not by reading the call sites. + `docs/implementation/mvp.md` states the MVP's purpose as + "correctness, understandability, and **architectural validation**", + and this stage is the first time all six configuration-selected + components are used together in one running simulation. Nothing + before it could have checked the claim: `step` consumes + `numerics.advection`/`.diffusion`/`.time_integration` and has never + touched `numerics.pressure_coupling` or `.linear_solver` at all, + because Stage 4 had nothing to correct. + - A `PressureCoupling` test double, registered under its own name + through the existing `register_pressure_coupling` registry and + selected by configuration, is demonstrably the object the timestep + calls -- so a solver that constructs `PISO` directly fails, + however correct its physics. The same substitution check for + `LinearSolver`, which reaches the timestep only through the + coupling and has never been exercised end-to-end either. + - **This is the criterion an otherwise-passing Stage 5 is most + likely to fail silently**, which is why it is stated separately + rather than assumed to follow from Criterion 4. A hardcoded `PISO` + inside the timestep passes Criteria 1-5 exactly as well as a + configured one does, produces the same Ghia comparison, and + quietly retires the architectural claim the MVP exists to + validate. Stage 4's Criterion 2 checked that a *name resolves* to + a real instance; this checks that the resolved instance is what + actually runs. + +### Seven design questions, all open Flagged here at stage open rather than left to surface mid-implementation (`docs/practices.md`, "When intent is ambiguous, hold a design session @@ -6411,6 +6469,29 @@ reads as a decision rather than the oversight it currently resembles -- the same treatment Gradient/Divergence got at TASK-027, which is what stopped them being invisible. +**Seven: does anything this stage builds belong in +`src/pyflow/physics/`? (TASK-031/034.)** The repository already +contradicts itself here, and the contradiction was found by asking the +question rather than by either document being reviewed: +`src/pyflow/physics/__init__.py`'s own docstring says the package is for +"physical models governing the fields the engine transports -- +incompressible flow first", which is precisely this stage's subject, +while `src/pyflow/physics/CLAUDE.md` opens "**Empty until Stage 6, and +empty on purpose**" and lists only Stage 6's phenomena. Both were +written before anyone had a momentum equation to file. The boundary +that `CLAUDE.md` states is a good one and worth keeping -- phenomena +here, discretisation in `engine/numerics/`, because `adr/ADR-003`'s +swappability claim and Stage 6's field-centric claim both become +untestable if the two mix -- but it does not by itself settle where an +incompressible-flow momentum equation sits, since the equation is +physics and nearly everything this stage actually writes is its +discretisation. **Whichever way it goes, those two files stop +contradicting each other in the same change.** Stage 6's own measurable +claim depends on the answer too: its TASK-035 intent proposes "counting +the lines this task adds outside `physics/`" as the test of that +stage's goal, which measures very little if `physics/` is still empty +when Stage 6 starts. + ### Discharge map Every criterion has an owning task, assigned now rather than @@ -6442,6 +6523,7 @@ obligation, not a reordering. | 10. Documentation matches the tree, capability map included | TASK-034 | | 11. `mvp.md`'s Definition of Done discharged item by item | TASK-034, reading Criterion 11's own table back against what landed | | 12. Configuration surface is real, validated and documented | TASK-031 for viscosity and solved-vs-prescribed velocity; TASK-033 for the corrector tolerance and iteration limit; TASK-034 for the tangential boundary value and whatever run-length control the demos need -- each task adds the fields its own criteria depend on, rather than one task inventing the whole surface up front | +| 13. The solver runs through ADR-003's seams, checked by substitution | TASK-034, the first task with a whole timestep to substitute into -- though TASK-033 is where the coupling first reaches the step at all, so a substitution check landing there instead discharges it equally well | **TASK-034 is this stage's last task in build order and therefore owns the stage-level criteria** -- the demonstrations, the CI evidence, the @@ -6832,7 +6914,7 @@ feature file, are the criteria. Written to cover, at minimum: ### Discharges -Criteria 4, 8, 9, 10 and 11, entirely. Criterion 5, all bullets (its +Criteria 4, 8, 9, 10, 11 and 13, entirely. Criterion 5, all bullets (its Couette bullet jointly with TASK-033). Criterion 12, its tangential-boundary and run-length share. Criterion 6 and Criterion 7, its own share. diff --git a/docs/planning/status.md b/docs/planning/status.md index a4f9b4b..e3fde6d 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -115,7 +115,7 @@ pie showData ### Stage 5 -- First Fluid Solver -**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 12 criteria defined, no status line yet +**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 13 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| From da802c2dda20147d7a24d5f140c29f45cb8e972f Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Fri, 28 Aug 2026 22:43:06 +0100 Subject: [PATCH 4/6] Resolve six of Stage 5's seven design questions; leave the seventh for numbers Put to the maintainer the day they were raised. Recorded in the roadmap as questions-with-answers rather than rewritten into decisions that read as though they were never in doubt, following Stage 4's own precedent. - A vector field is transported as per-component ScalarFields, with a VectorField assembled for consumers that need one. No Stage 3 interface changes. Makes Criterion 1's claim literal rather than analogous: velocity goes through the scalar path because in transport it is scalars. Inherited wrinkle recorded: u and v need different values at the same wall, the first real consumer of the single-global-boundary-set limitation TASK-040 deferred. - The pressure correction sits outside the time integrator, once per timestep -- classical fractional step. The splitting error caps the coupled solver's temporal order, which time-integration.md, icds.md and TASK-025's own criterion already expect. Settled on the error budget: upwind's numerical diffusion dominates at every mesh this stage runs, so four pressure solves per step would buy accuracy nothing here could measure. - No SourceTerm implementation, with the reason now in source.py rather than owed to TASK-032. Downstream of the previous answer and would have gone the other way under it: projecting inside RK4's stages would have put the gradient in the derivative evaluation, where a source term belongs. Stage 6's buoyancy stays its natural first consumer. - Stage 5 builds nothing in src/pyflow/physics/, and __init__.py's docstring is corrected. It had said the package was for physical models, "incompressible flow first" -- this stage's exact subject -- while CLAUDE.md beside it said "empty until Stage 6, and empty on purpose". Contradictory since TASK-000, unnoticed because nothing had cause to ask. Both files now agree. - A moving or no-slip wall is configured with velocity_tangential, a second scalar beside the normal one: in 2D a face has exactly one tangential direction, so a scalar carries what a vector would. - The new configuration surface gets its own top-level fluid: section, and numerics.diffusion_coefficient migrates into it -- the project's first breaking configuration change. Viscosity is a fluid property, not a numerical parameter, and filing it under numerics: would make in configuration the category error physics/CLAUDE.md refuses in code. Leaving diffusion_coefficient behind would split two quantities of the same kind across two sections, which is worse than either arrangement chosen consistently. Blast radius enumerated in the roadmap, assigned to TASK-031. The seventh -- what carries the momentum coefficients a converging pressure-correction loop needs -- is deliberately open. It is the only one whose answer depends on a measurement nobody has taken, and TASK-027 is the standing demonstration of what deciding it from an armchair costs: three plausible strategies, each leaving most of the divergence in place, for a reason that only appeared once someone measured. Criteria 1, 6 and 12 and all four task entries were updated to read against the answers rather than against the open questions. make ci clean: 614 tests, 99%, all twelve gates. Co-Authored-By: Claude Opus 5 --- README.md | 10 +- docs/CHANGELOG-DESIGN.md | 94 +++++++- docs/planning/roadmap.md | 306 +++++++++++++++++++++------ src/pyflow/engine/numerics/source.py | 18 ++ src/pyflow/physics/CLAUDE.md | 12 ++ src/pyflow/physics/__init__.py | 24 ++- 6 files changed, 393 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 53e57f6..72335b8 100644 --- a/README.md +++ b/README.md @@ -164,9 +164,13 @@ criteria were written on 2026-08-28, before its first task started** every stage since Stage 2 has followed -- including the reconciliation against `docs/implementation/mvp.md`'s own Definition of Done that Stage 5 has owed since 2026-08-22, since -this is the stage that defines the MVP. Seven design questions are -recorded there as open, five of them gaps in the current code -- -verified against it rather than anticipated. +this is the stage that defines the MVP. Seven design questions were +raised while drafting them -- five of the seven gaps in the current +code, verified against it rather than anticipated -- and six were +decided the same day. The seventh (what carries the momentum +coefficients a converging pressure-correction loop needs) is +deliberately left open for TASK-033 to answer with measurements, since +TASK-027 already showed what deciding that one from an armchair costs. Try the most recent demonstration -- a scalar blob advected and diffused across a periodic domain, stepped live: diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index e35db22..6022e6d 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -5834,7 +5834,7 @@ whole point of the scenario making the call. conftest-scoping claim confirmed by a throwaway probe module that was run and then deleted, not by reading documentation. -### Stage 5 opened: thirteen completion criteria, two maintainer decisions, seven open design questions +### Stage 5 opened: thirteen completion criteria, and eight maintainer decisions Written before TASK-031 starts, per `docs/practices.md`'s "A stage gets completion criteria before its first task" -- the same point in the @@ -5881,7 +5881,8 @@ qualitative structure (primary vortex centre, both secondary corner vortices) at the finest, with the absolute tolerance stated and defended in the feature file against the mesh actually used. -**Seven design questions, recorded open rather than answered.** Stage 4 +**Seven design questions were raised, six of them answered the same +day** (below, after the audit passes that found the last two). Stage 4 set the precedent of flagging these on the day the criteria are drafted and resolving each before the task that needs it starts; five of Stage 5's seven are gaps in the code, verified against it rather than @@ -6087,6 +6088,95 @@ has no way to know which section it found, and a uniquely-anchored replacement does. `wc -l` before and after a structural edit is a two-second check that catches the whole class. +**Six of the seven design questions were then put to the maintainer and +decided the same day.** Recorded here in the order they were asked, with +what each answer costs, because several of them constrain each other and +the constraints are not obvious from the answers alone. + +- **A vector field is transported as per-component `ScalarField`s**, with + a `VectorField` assembled for the consumers that need one. Chosen over + widening `flux` to return `(num_faces, n_components)` -- which is the + more uniform answer and the better fit for 3D, but changes two Stage 3 + interfaces and their contract suites before a second consumer exists to + justify it -- and over an adapter inside `step`, which is the + special-casing Criterion 1 exists to forbid. The answer makes + Criterion 1's claim literal rather than analogous: velocity goes + through the scalar path because in transport it *is* scalars. Its + inherited wrinkle is recorded rather than glossed: `u` and `v` need + different values at the same wall, which is the first real consumer of + the single-global-boundary-set limitation TASK-040 deferred to Stage 6. +- **The pressure correction sits outside the time integrator, once per + timestep** -- the classical fractional-step arrangement, over + projecting inside each of RK4's four derivative evaluations. The + splitting error caps the coupled solver's temporal order well below + RK4's own fourth, which `time-integration.md`, `icds.md` and TASK-025's + own criterion have all already written down as expected. The argument + that settled it is the error budget: first-order upwind's numerical + diffusion dominates at every mesh this stage will run, so four pressure + solves per step would buy a reduction in splitting error that nothing + in Stage 5 could measure. Revisit when Stage 7's less diffusive schemes + make it visible. +- **No `SourceTerm` implementation, and the reason is now in + `source.py`** rather than owed to TASK-032. A projection method does + not route the pressure gradient through a source term -- the correction + *is* that gradient's effect. This one is downstream of the previous + answer and would have gone the other way under it: had the projection + gone inside RK4's stages, the gradient would have had to enter the + derivative evaluation, which is exactly where a source term belongs. + Stage 6's buoyancy coupling stays its natural first consumer. +- **Stage 5 builds nothing in `src/pyflow/physics/`, and + `physics/__init__.py`'s docstring is corrected**, both done in the same + change. The package's docstring had said it was for physical models, + "incompressible flow first" -- Stage 5's exact subject -- while + `physics/CLAUDE.md` in the same directory said "empty until Stage 6, + and empty on purpose". The two had contradicted each other since + TASK-000 with nobody noticing, because until this stage nothing had + cause to ask. `CLAUDE.md` wins: it defends the phenomena/machinery + boundary as the thing that keeps ADR-003's swappability claim testable, + and what Stage 5 writes is discretisation. Both files now say so, and + Stage 6's "lines added outside `physics/`" metric keeps a clean zero + baseline. +- **A moving or no-slip wall is configured with `velocity_tangential`**, + a second scalar beside the existing normal `BoundaryFaceConfig. + velocity`. In 2D a boundary face has exactly one tangential direction, + so a scalar carries everything a vector would and the face geometry + supplies the rest. Chosen over making `velocity` a component pair + (which moves every config, demo, template and the joint net-flux + validation with it) and over a separate `wall_velocity` describing the + wall's own motion (the most honest physics, at the cost of a new + concept and consistency validation between two fields). +- **The new configuration surface gets its own top-level `fluid:` + section, and this is the project's first breaking configuration + change.** Viscosity is a property of the simulated fluid rather than a + numerical parameter, and the repository already defends that exact + boundary in its package layout (`physics/CLAUDE.md`) -- filing + viscosity under `numerics:` would make in configuration the category + error the code refuses. **`numerics.diffusion_coefficient` migrates + with it**, which is the cost of the answer rather than an optional + tidy-up: a scalar's diffusivity and a fluid's viscosity are the same + kind of quantity, and splitting them across two sections is worse than + either arrangement chosen consistently. The blast radius is enumerated + in the roadmap rather than left to be discovered -- + `examples/golden-demos/passive_scalar_transport.yaml` is the one + committed config that sets it, plus the schema, loader, the template + generator's own comment tables and `pyflow generate-config`. Named + `fluid:` rather than `physics:` because that word already means + something narrower here, and reusing it for two meanings in one project + is how `docs/planning/backlog.md`'s competing-vocabularies problem + starts. The corrector-loop tolerance and iteration limit stay in + `numerics:`, where they belong, which is the whole point of drawing the + line. + +**The seventh was deliberately left open**: what carries the +momentum-equation coefficients a converging pressure-correction loop +needs. It is the only one whose answer depends on a measurement nobody +has taken, and TASK-027 is the standing demonstration of what deciding +it from an armchair costs -- three correction strategies that all looked +reasonable on paper, each leaving most of the original divergence in +place, for a reason (the composed Green-Gauss Poisson matrix is provably +not symmetric) that only appeared once someone measured. TASK-033 +answers it with numbers and records it in the same place. + - *Verified by:* `make ci` clean on this branch; `make check-references` reporting the four planned Stage 5 feature files as promises rather than errors; `make check-graph` clean after the `demos.yaml` edit; diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index 8b12a59..13d78a0 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -5808,11 +5808,19 @@ actual bar (Criterion 5). constant serving the one transported scalar Stage 4 had. A configuration that sets a fluid's viscosity must not silently set an unrelated scalar's diffusivity, and a Reynolds number cannot be - configured at all until the two are distinguishable -- see design - question four below. - - Whatever resolves design question one applies to *any* vector - field, not to a field named `"velocity"`. A name-keyed branch - anywhere in the engine fails this criterion however well it works. + configured at all until the two are distinguishable. Design + question four settles where they live (a new `fluid:` section, with + `diffusion_coefficient` migrating into it); this criterion is the + behavioural half, and holds whatever the section is called. + - **Design question one is resolved (per-component `ScalarField`s), + and the criterion is what stops that answer being implemented as a + special case.** Velocity is transported as its components, so the + transport path sees scalars and nothing about it may know that two + particular scalars are somebody's velocity: no name-keyed branch, + and the assembly back into a `VectorField` happens at the + consumers that need one, not inside the transport path. An + implementation that hardcodes the pair `("u", "v")` has moved the + special case rather than removed it. 2. **Pressure is solved from the incompressibility constraint, not transported.** A criterion that treats pressure as another advected scalar has misunderstood the task @@ -6088,8 +6096,12 @@ actual bar (Criterion 5). specific new surfaces this stage creates: a velocity field whose component count disagrees with the mesh's dimensionality, a configuration that names a boundary treatment velocity has no meaning - for, a corrector loop that exhausts its iteration limit, and whatever - rejection path design question one's resolution introduces. + for, a corrector loop that exhausts its iteration limit, and -- from + design question one's per-component answer -- a component set whose + size disagrees with the mesh's dimensionality, or components defined + over different meshes. `MismatchedMeshError` already has the shape + for the last of those but has never been asked about a reassembled + vector. 7. **Every task's acceptance criteria are a Gherkin `.feature` file under `tests/features/`, and `make check-scenarios` gates that every scenario it contains actually runs.** The mechanism @@ -6264,18 +6276,28 @@ actual bar (Criterion 5). than anticipated -- checked field by field against `src/pyflow/configuration/schema.py` while drafting these criteria, not assumed: - - **A viscosity.** `NumericsConfig.diffusion_coefficient` is one - global constant serving the one transported scalar Stage 4 had. - Criterion 5's Reynolds-number comparison cannot be configured - without separating the two (design question four). + - **A viscosity, in a new `fluid:` section** (design question four, + resolved 2026-08-28). `NumericsConfig.diffusion_coefficient` is + one global constant serving the one transported scalar Stage 4 + had, and Criterion 5's Reynolds-number comparison cannot be + configured without separating the two. **This is the project's + first breaking configuration change**, because + `diffusion_coefficient` migrates into the same new section rather + than being left behind in `numerics:` -- the migration and its + enumerated blast radius are recorded with that question's answer, + and `examples/golden-demos/passive_scalar_transport.yaml` is the + one committed config that has to move with it. - **A corrector-loop tolerance, and an iteration limit.** Neither exists; `linear_solver_tolerance`/`linear_solver_max_iterations` are the inner CG solve's, a different convergence question at a different level. Criterion 3 says "the configured tolerance" and means it. - - **A tangential boundary velocity** (design question two) -- with - no way to say "this wall moves along itself", neither the cavity - nor Couette can be configured at all. + - **A tangential boundary velocity**, `velocity_tangential` beside + the existing normal one (design question two, resolved + 2026-08-28) -- with no way to say "this wall moves along itself", + neither the cavity nor Couette can be configured at all. It also + has to reach *two* component fields with different values at the + same wall, which is design question one's recorded wrinkle. - **A way to say velocity is solved rather than prescribed.** `SimulationConfig.velocity`/`velocity_pattern` are documented as "a prescribed (not solved) constant vector -- Stage 5 is what @@ -6330,14 +6352,23 @@ actual bar (Criterion 5). a real instance; this checks that the resolved instance is what actually runs. -### Seven design questions, all open +### Seven design questions: six resolved, one open Flagged here at stage open rather than left to surface mid-implementation (`docs/practices.md`, "When intent is ambiguous, hold a design session before implementing"), following Stage 4's own precedent of recording its two on the day its criteria were drafted and resolving each before the -task that needed it started. **None of these is resolved yet**, and each -names the task that cannot start drafting without the answer. +task that needed it started. + +**Six were put to the maintainer and decided on 2026-08-28, the same day +they were raised; the seventh is deliberately still open.** Each is kept +below as the question it was, with its answer, rather than rewritten into +a decision that reads as though it was never in doubt -- the same +treatment Stage 4's "Two design questions, both now resolved" section +gives its own pair. The one still open is question three (Rhie-Chow's +momentum coefficients), because it is the only one whose answer depends +on a measurement nobody has taken: TASK-027 already established that +reasoning about it without numbers produces a confident wrong answer. **One: how does a vector field go through an interface that returns one value per face? (TASK-031.)** `AdvectionScheme.flux(field, velocity)` and @@ -6365,6 +6396,24 @@ none yet chosen: structural clause, since the adapter is exactly the special-casing that criterion forbids. +**Resolved 2026-08-28, maintainer's call: per-component `ScalarField`s.** +Momentum is transported as one `ScalarField` per component, with a +`VectorField` assembled from them for the consumers that genuinely need +one -- `AdvectionScheme.flux`'s own `velocity` argument, +`GreenGaussDivergence`, and rendering. It changes no Stage 3 interface, +works with both concrete schemes exactly as they stand, and makes +Criterion 1's claim literally rather than analogously true: velocity is +transported by the same code path a scalar is because it *is* a set of +scalars while in transport. **The wrinkle it inherits, recorded now +rather than met later:** `u` and `v` need different boundary values at +the same wall -- a no-slip moving lid is `u = U, v = 0` -- which is +precisely the "one global set of boundary conditions... does not yet +express field A and field B at the same wall" limitation TASK-040 +deferred. Question two's answer supplies the wall's own tangential +value; how that reaches two component fields with different numbers is +TASK-031's to resolve, and it is the first real consumer of that +deferred case rather than a new problem. + **Two: `BoundaryFaceConfig.velocity` is the boundary-normal component only, and a lid-driven cavity's lid is tangential. (TASK-031/034.)** That field's own docstring already flagged this and deferred it "to whichever @@ -6380,6 +6429,20 @@ field and is about normal flux specifically, and its own `FIELD_COMMENTS`, and `pyflow generate-config` (TASK-039) emits it. +**Resolved 2026-08-28, maintainer's call: a second scalar, +`velocity_tangential`, beside the existing normal one.** In 2D each +boundary face has exactly one tangential direction, so a scalar is +sufficient and a vector would be a representation carrying information +the face geometry already supplies. It introduces no new concept, leaves +`velocity`'s meaning and the zero-net-flux validation that reads it +untouched, and the generated config template picks the field up on its +own. The two alternatives considered and not taken: making `velocity` a +component pair (cleanest single representation, but it changes an +existing field's type and moves every config, demo, template and the +joint validation with it), and a separate `wall_velocity` describing the +wall's own motion (arguably the most honest physics, at the cost of a +new concept plus consistency validation between it and `velocity`). + **Three: what does the corrector loop need that TASK-027's interface could not supply? (TASK-033.)** Criterion 3's monotonic-convergence claim is the one TASK-027 measured and could not make: on PyFlow's collocated @@ -6398,6 +6461,19 @@ resolve with numerical prototyping before Criterion 3 is turned into a feature file** -- a criterion that cannot be met is worse than one drafted late. +**Deliberately still open, 2026-08-28** -- the only one of the seven not +decided when the other six were. Not an omission: TASK-027 already +demonstrated what deciding this from an armchair costs. Three correction +strategies that all looked reasonable on paper each left most of the +original divergence in place, and the reason (the composed Green-Gauss +Poisson matrix is provably not symmetric) only became visible once +someone measured. Whichever mechanism carries the coefficients, the +choice is between three shapes that differ in cost and blast radius but +not in whether they *can* work -- and which of them is needed at all +depends on how far a momentum-coupled iteration actually converges, +which nobody yet knows. Answer it at TASK-033, with numbers, and record +it here. + **Four: what shape does this stage's new configuration surface take? (TASK-031, with TASK-034 as its last consumer.)** Criterion 12 lists what is missing and why each is needed; this question is how it is @@ -6430,6 +6506,49 @@ down elsewhere and easy to miss: configured timestep and the derivation lives in the scenario. Nothing in the repository does either today. +**Resolved 2026-08-28, maintainer's call, and this one costs a +migration: a new top-level configuration section for the fluid's own +physical properties, separate from `numerics:`.** Viscosity is a +property of the simulated fluid, not a numerical parameter, and the +repository defends that exact boundary in code +(`src/pyflow/physics/CLAUDE.md`: "phenomena here, numerical machinery +there"); a configuration that files viscosity under `numerics:` makes +the same category error the package layout refuses. Chosen over the +smaller option of flat additions to `NumericsConfig` beside +`linear_solver_tolerance`, which would have matched the existing style +and touched nothing. +- **`numerics.diffusion_coefficient` moves into the new section too**, + and that is the real cost of this answer rather than an optional + tidy-up: leaving it behind would put a scalar's diffusivity and a + fluid's viscosity -- the same kind of quantity -- in two different + sections, which is worse than either arrangement chosen + consistently. That makes this a **breaking configuration change**, the + first the project has made. Its blast radius, enumerated rather than + discovered: `examples/golden-demos/passive_scalar_transport.yaml` + (the one committed config that sets it), + `tools/generators/generate_config_template.py`'s own + `FIELD_COMMENTS`/`SECTION_COMMENTS` and the regenerated + `docs/implementation/config-template.yaml`, + `src/pyflow/configuration/schema.py`'s `PyFlowConfig` and loader, and + `pyflow generate-config` (TASK-039). `make check-config-template` + gates the template half automatically; the rest is a grep for the + field name. +- **Named `fluid:`, not `physics:`** -- the section holds material + properties (viscosity now, density when Stage 6 needs it), and + `physics` already means something narrower and specific in this + repository (`src/pyflow/physics/`: phenomena, not properties). Reusing + the word for two different meanings in the same project is how the + competing-vocabularies problem `docs/planning/backlog.md` records + starts. A naming call rather than a structural one, and TASK-031 may + overturn it with a better name as long as it does not reuse + `physics`. +- The corrector-loop tolerance and iteration limit stay in `numerics:`, + beside `linear_solver_tolerance`/`linear_solver_max_iterations`: those + genuinely are numerical parameters, and the split this answer draws is + the whole point of drawing it. Solved-vs-prescribed velocity and any + run-length control stay in `simulation:`, which already describes what + a run does rather than what the fluid is. + **Five: where does the pressure correction sit relative to RK4's stages? (TASK-033/034.)** Not surfaced by anything before this pass, and it decides what Criterion 4's "predictor, corrector loop, corrected @@ -6451,9 +6570,24 @@ projection has to happen somewhere relative to those four: pressure solves per timestep instead of one. The choice is not free either way, and it interacts with design question three: whichever arrangement is chosen is what the corrector loop's -monotonic-convergence claim is measured *within*. Resolve it with the -same prototyping-first sequence TASK-027 used, and record which reading -was taken in the scenario, not only in a docstring. +monotonic-convergence claim is measured *within*. + +**Resolved 2026-08-28, maintainer's call: once per timestep, outside the +integrator** -- the classical fractional-step arrangement. RK4 advances +momentum with no pressure term to a provisional velocity, then the +corrector loop projects once. The splitting error caps the coupled +solver's temporal order well below RK4's own fourth, which is not a +concession this decision makes but a consequence +`docs/handbook/numerical-methods/time-integration.md`, +`docs/architecture/icds.md` and TASK-025's own criterion have all +already written down and expected. **The argument that settles it is +the error budget:** first-order upwind's numerical diffusion is the +dominant error term at every mesh this stage will run (Criterion 5's own +cavity bullet turns on exactly that), so paying four pressure solves per +timestep instead of one buys a reduction in splitting error that nothing +in this stage could measure. Revisit when Stage 7's less diffusive +schemes make the splitting error visible, not before. Recorded in the +scenario as well as here, per this question's own original instruction. **Six: does the pressure gradient reach momentum through `SourceTerm`, or only through the projection? (TASK-032/033.)** `src/pyflow/engine/ @@ -6469,6 +6603,19 @@ reads as a decision rather than the oversight it currently resembles -- the same treatment Gradient/Divergence got at TASK-027, which is what stopped them being invisible. +**Resolved 2026-08-28, maintainer's call: no -- and the reason is +recorded in `source.py` itself**, done in the same change as this +answer rather than left as an obligation for TASK-032 to remember. A +projection method does not route the pressure gradient through a source +term: the correction *is* that gradient's effect on the velocity field, +applied after the predictor, so implementing `SourceTerm` here would add +machinery nothing calls. Question five's answer is what makes this +clean-cut -- had the projection gone inside RK4's stages, the pressure +gradient would have had to enter the derivative evaluation, which is +exactly where a source term belongs. Stage 6's buoyancy coupling +(TASK-035) remains the natural first consumer: a body force *is* a +source term in the way a projection correction is not. + **Seven: does anything this stage builds belong in `src/pyflow/physics/`? (TASK-031/034.)** The repository already contradicts itself here, and the contradiction was found by asking the @@ -6492,6 +6639,23 @@ the lines this task adds outside `physics/`" as the test of that stage's goal, which measures very little if `physics/` is still empty when Stage 6 starts. +**Resolved 2026-08-28, maintainer's call: everything Stage 5 builds +stays in `engine/`, and `physics/__init__.py`'s docstring is corrected +-- done in the same change as this answer**, so the contradiction does +not outlive the session that found it. Nearly all of what this stage +writes is discretisation and orchestration, which +`src/pyflow/physics/CLAUDE.md` explicitly excludes, and that file is the +more carefully reasoned of the two: it defends the phenomena/machinery +line as the thing that keeps `adr/ADR-003`'s swappability claim and +Stage 6's field-centric claim testable at all. The docstring's +"incompressible flow first" was written at TASK-000, before either claim +existed, and describing the package by the physics PyFlow simulates +rather than by the code that belongs in it is what made it read as a +Stage 5 obligation. Stage 6's own metric is unaffected: counting lines +added outside `physics/` works from an empty baseline, and arguably +works better, since every line TASK-035 adds inside it is then genuinely +attributable to that task. + ### Discharge map Every criterion has an owning task, assigned now rather than @@ -6567,20 +6731,30 @@ TASK-023 (`FirstOrderUpwindAdvection`), TASK-024 TASK-028/029 (the real Dirichlet/Neumann conditions a velocity boundary resolves to), TASK-040 (`step`, `accumulate_flux_to_cells`). -### Open design questions - -**One and four above, both open, both blocking this task's own -drafting.** One decides how a vector field goes through an interface -that returns one value per face -- verified as a real `TypeError`, not -a hypothetical. Four decides the shape of this stage's configuration -surface; this task needs two pieces of it (a viscosity separable from a -scalar's diffusivity, and a way to say velocity is solved rather than -prescribed) and **should settle the shape for all five pieces rather -than adding one field and leaving the next task to invent a different -style for the second** -- that is why it is one question rather than -five. Two (tangential boundary values) is listed against TASK-034 as -well as here, because whichever of the two first needs a no-slip wall is -where it has to be answered. +### Design questions, all resolved for this task + +**One, two and four, all answered 2026-08-28 (maintainer's calls, above) +-- so nothing blocks this task's drafting.** In the order they bite +here: +- **One:** momentum is transported as one `ScalarField` per component, + with a `VectorField` assembled for the consumers that need one. No + Stage 3 interface changes. +- **Four:** a new `fluid:` configuration section holds viscosity, and + `numerics.diffusion_coefficient` migrates into it -- **the project's + first breaking configuration change, and this task's to carry out**, + including `examples/golden-demos/passive_scalar_transport.yaml` and + `tools/generators/generate_config_template.py`'s own comment tables. + Solved-vs-prescribed velocity goes in `simulation:`. +- **Two:** `velocity_tangential` beside the existing normal + `BoundaryFaceConfig.velocity`. Listed against TASK-034 as well, + because whichever task first needs a no-slip wall is where it lands; + if that turns out to be this one, it lands here. + +**The one thing these answers leave for this task to work out** is how a +single wall's tangential value reaches two component fields that need +different numbers there (`u = U`, `v = 0` at a moving lid) -- design +question one's own recorded wrinkle, and the first real consumer of the +"one global set of boundary conditions" limitation TASK-040 deferred. ### Artifacts Produced @@ -6655,15 +6829,16 @@ TASK-031 (a transported velocity field to solve against), TASK-026 `GreenGaussDivergence`, and the compact symmetric Laplacian that task already builds the Poisson matrix from). -### Open design question +### Design question, resolved -**Six above** -- whether the pressure gradient reaches the momentum -equation through `SourceTerm`, the one Stage 3 interface nothing has -ever implemented, or only through the projection correction. This is the -task that has cause to answer it, and the answer is owed in writing -either way: an unimplemented interface is fine while nothing needs it, -and reads as an oversight the moment the physics it was built for -arrives without using it. +**Six above, answered 2026-08-28: no `SourceTerm` implementation.** The +pressure gradient reaches velocity as the projection correction, not as +a source term, so this task adds no concrete `SourceTerm` -- and the +reason is already recorded in `src/pyflow/engine/numerics/source.py`'s +own module docstring, written when the decision was taken rather than +left as an obligation for this task to remember. Nothing to decide here; +the note exists so that a reader who wonders why the interface is still +empty finds an answer next to it. ### Artifacts Produced @@ -6741,23 +6916,28 @@ about what does not work, not from a blank page. ### Open design questions -**Three above, open, and the one most likely to move this stage's -scope.** What carries the momentum-equation coefficients Rhie-Chow -needs: a widened `PressureCoupling.correct`, a momentum operator handed -in at construction, or outer-loop state. **Resolve it with numerical -prototyping before writing this task's feature file**, the same -sequence TASK-027 used (five prototype scripts, then a scoping +**Three above -- the only one of this stage's seven still open, and this +task owns it.** What carries the momentum-equation coefficients +Rhie-Chow needs: a widened `PressureCoupling.correct`, a momentum +operator handed in at construction, or outer-loop state. **Resolve it +with numerical prototyping before writing this task's feature file**, +the same sequence TASK-027 used (five prototype scripts, then a scoping decision, then the tests) -- writing a criterion first and discovering it is unreachable second is precisely the failure `docs/practices.md`'s "A criterion whose strong reading depends on a later task must say so -when drafted" was added to prevent, and this is the later task. - -**Five above, and it has to be answered before three can be measured.** -Where the correction sits relative to RK4's four derivative evaluations -decides what "within a single timestep" means for this task's own -monotonic-convergence sequence -- one projection per step, or one per -stage. Resolve the two together; measuring convergence inside an -arrangement nobody has chosen is measuring nothing. +when drafted" was added to prevent, and this is the later task. It was +left open deliberately when the other six were decided, because +TASK-027 already demonstrated what deciding this one without +measurements costs. + +**Five above is no longer open, and that is what makes three +measurable.** The correction sits outside the integrator, once per +timestep (resolved 2026-08-28, maintainer's call): RK4 advances momentum +to a provisional velocity with no pressure term, then this task's +corrector loop projects. So "within a single timestep" in the +monotonic-convergence claim above means *within that one projection's +corrector passes* -- an unambiguous sequence to record and assert on, +which is exactly what it was not while the arrangement was undecided. ### Artifacts Produced @@ -6850,14 +7030,16 @@ own visual output); TASK-030's own demo wiring mechanism a live demo already uses and this one extends rather than replaces. -### Open design questions +### Design questions: the shape is settled, two pieces land here -**Two above** -- tangential boundary values -- if TASK-031 has not -already had to answer it. A no-slip wall and a moving lid are both -tangential, and both of this task's own validation cases need them. +**Two above is answered** (`velocity_tangential`, 2026-08-28) -- but +whether this task or TASK-031 *builds* it depends on which first needs a +no-slip wall. Both of this task's validation cases do, so if TASK-031 +did not need one, it lands here. -**Four's timestep half**, which lands squarely on this task even if the -rest of the configuration surface was settled at TASK-031: Criterion 5 +**Four's timestep half**, which lands squarely on this task even though +the rest of the configuration surface was settled at TASK-031: +Criterion 5 runs the cavity at three resolutions, and explicit RK4's stability limit tightens with the mesh (with `dx` for advection, with `dx` squared for diffusion). A single fixed `numerics.timestep` cannot serve all three, diff --git a/src/pyflow/engine/numerics/source.py b/src/pyflow/engine/numerics/source.py index 57c7f39..16b8ef6 100644 --- a/src/pyflow/engine/numerics/source.py +++ b/src/pyflow/engine/numerics/source.py @@ -8,6 +8,24 @@ field it's handed rather than a fixed one. No concrete scheme lives here -- Stage 3 Completion Criterion 1. + +**Still no concrete scheme after Stage 5, and that is a decision rather +than an oversight** (2026-08-28, maintainer's call -- Stage 5's own +design question six, `docs/planning/roadmap.md`). This is the only Stage +3 interface with no implementation, no registry entry and no consumer, +and the obvious candidate was the momentum equation's pressure gradient. +It is not one: Stage 5 applies the pressure correction as a projection +after the momentum predictor (design question five, resolved the same +day), so the correction *is* that gradient's effect on the velocity +field and there is nothing left for a source term to contribute. Had the +projection been placed inside the time integrator's own stages instead, +the gradient would have had to enter the derivative evaluation, which is +exactly where a source term belongs -- the two answers are linked, and +the other pairing would have given this interface its first consumer. + +Stage 6's buoyancy coupling (TASK-035) is the natural first +implementation: a body force is a source term in the way a projection +correction is not. """ from __future__ import annotations diff --git a/src/pyflow/physics/CLAUDE.md b/src/pyflow/physics/CLAUDE.md index e7dd344..cbd4aaa 100644 --- a/src/pyflow/physics/CLAUDE.md +++ b/src/pyflow/physics/CLAUDE.md @@ -7,6 +7,18 @@ placeholder the root `CLAUDE.md` allows "only until something specific is known about that directory" until 2026-08-22, when drafting Stage 3 made the boundary below specific. +**Confirmed against Stage 5 on 2026-08-28, which is the first stage that +could plausibly have filled this package.** `__init__.py`'s own +docstring said the package was for physical models, "incompressible flow +first" -- Stage 5's exact subject -- and so contradicted this file's +opening sentence for as long as both existed, unnoticed because nothing +had cause to ask. Stage 5's own design question seven put it to the +maintainer: everything that stage builds stays in `engine/`, because +what it writes is discretisation and orchestration, which is the half +the boundary below excludes. `__init__.py` was corrected in the same +change. The line stands, now with a stage's worth of evidence behind it +rather than an assumption. + **What belongs here: phenomena. What does not: numerical machinery.** - Here, eventually: temperature, density, humidity, passive tracers diff --git a/src/pyflow/physics/__init__.py b/src/pyflow/physics/__init__.py index e0f9590..11c83ea 100644 --- a/src/pyflow/physics/__init__.py +++ b/src/pyflow/physics/__init__.py @@ -1,6 +1,22 @@ -"""Physical models governing the fields the engine transports -- -incompressible flow first, per adr/ADR-002-fvm-first.md and -docs/implementation/mvp.md. +"""Phenomena the engine transports -- temperature, density, humidity, +passive tracers (Stage 6, docs/planning/roadmap.md TASK-035..038) and +the couplings between them, such as buoyancy. -Placeholder package (TASK-000) -- no implementation yet. +Empty on purpose; see CLAUDE.md in this directory for the boundary this +package defends (phenomena here, numerical machinery in +engine/numerics/) and why adr/ADR-003-modular-numerical-strategies.md's +swappability claim depends on it. + +This docstring read "Physical models governing the fields the engine +transports -- incompressible flow first" until 2026-08-28, which named +Stage 5's own subject and so read as a promise that Stage 5 would fill +this package. It contradicted CLAUDE.md's "empty until Stage 6, and +empty on purpose" in the same directory, and the contradiction had gone +unnoticed since TASK-000 because nothing had ever had cause to ask. +Resolved when Stage 5's completion criteria were drafted (that stage's +own design question seven, maintainer's call): Stage 5 stays entirely in +engine/, because what it writes is discretisation and orchestration, +which is the half CLAUDE.md excludes. Describing this package by the +physics PyFlow simulates, rather than by the code that belongs in it, is +what made the two disagree. """ From ccfedb41a36788af25dede03fa8d781316984048 Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Fri, 28 Aug 2026 22:54:39 +0100 Subject: [PATCH 5/6] Fix two artifact bullets left stale by resolving the design questions Both described a state that stopped existing when question one was answered. TASK-031's said its source artifacts were "deliberately not named yet" because that question decided them -- it is decided, so they are named: schema.py (the fluid: section, velocity_tangential, the solve-mode control, diffusion_coefficient's migration), simulation.py, the config-template generator's comment tables, and the one committed demo config that carries diffusion_coefficient. What stays open there is smaller and stated as such: where the component-to-VectorField assembly helper lives, which is an implementation choice rather than a design question to escalate. TASK-032's said its artifacts wait on "TASK-031's own design questions being answered". They are. Its one genuinely local question -- whether pressure needs a type of its own or is a ScalarField the coupling owns -- is restated as its own rather than inherited. Found by checking what the resolutions invalidated rather than what they changed, which is the rule Stage 4's exit audit produced. make ci clean. Co-Authored-By: Claude Opus 5 --- docs/planning/roadmap.md | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index 13d78a0..6bf38af 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -6761,11 +6761,28 @@ question one's own recorded wrinkle, and the first real consumer of the - `tests/features/velocity_field_support.feature` -- this task's Acceptance Criteria, per `adr/ADR-007-executable-acceptance-criteria.md`. -- Source artifacts are deliberately not named yet: which modules change, - and whether any new one appears at all, is exactly what design - question one decides. Naming them now would be the speculation P-016 - refuses, and a wrong name here is a `make check-references` failure - rather than a harmless guess. +- **The modules this touches are now knowable, because design question + one is answered** (this bullet said they were deliberately unnamed + while it was open; that stopped being true on 2026-08-28): + - `src/pyflow/configuration/schema.py` -- the new `fluid:` section, + `BoundaryFaceConfig.velocity_tangential`, the solved-vs-prescribed + control on `SimulationConfig`, and `diffusion_coefficient`'s + migration out of `NumericsConfig`. + - `src/pyflow/engine/simulation.py` -- `step` advancing velocity's + components alongside any scalar, with no branch that knows which + ones they are. + - `tools/generators/generate_config_template.py` -- its + `FIELD_COMMENTS`/`SECTION_COMMENTS` for every field above, which + `make check-config-template` gates. + - `examples/golden-demos/passive_scalar_transport.yaml` -- the one + committed config carrying `diffusion_coefficient` today. +- **Still genuinely open, and small: where the component-to-`VectorField` + assembly helper lives.** `vector_field.py` and `simulation.py` are both + defensible homes and no new module is obviously needed; that is a + choice to make while implementing rather than a design question to + escalate, and it is left unnamed here for the reason this bullet + originally gave -- a wrong path in prose is a `make check-references` + failure rather than a harmless guess. ### Acceptance Criteria @@ -6844,10 +6861,12 @@ empty finds an answer next to it. - `tests/features/pressure_field.feature` -- this task's Acceptance Criteria. -- Source artifacts named when this task is drafted, after TASK-031's own - design questions are answered: whether pressure needs a type of its - own, or is a `ScalarField` the coupling owns, is a real question this - task decides rather than one to guess now. +- Source artifacts named when this task is drafted. **TASK-031's own + design questions are no longer what this waits on** -- all three were + answered on 2026-08-28 -- but one local question genuinely remains + this task's to decide: whether pressure needs a type of its own, or is + a `ScalarField` the coupling owns. Nothing before it has cause to + choose, so it is not escalated as a stage-level design question. ### Acceptance Criteria From e2b30e4f30b258c41b625543463f8a154785d40c Mon Sep 17 00:00:00 2001 From: Adam Clemens Date: Fri, 28 Aug 2026 23:11:28 +0100 Subject: [PATCH 6/6] Split TASK-041 out of TASK-031, and break TASK-031 into four subtasks Both from asking whether Stage 5 was actually ready to start. TASK-041 (Fluid Configuration Section), built first. Design question four's answer -- a new fluid: section with numerics.diffusion_coefficient migrating into it -- had landed inside TASK-031 by default, making "Velocity Field Support" responsible for the project's first breaking configuration change: a cross-cutting edit across schema, loader, template generator, pyflow generate-config and a committed demo config, with no velocity in any of it. Same late addition Stage 4 made with TASK-040, same reasoning: real work, independently verifiable, and everything after it is easier once it exists. Keeps its number, takes first position. It ships viscosity rather than migrating alone, and says why: a pure move would be a breaking change buying nothing until TASK-031 arrives. The cost is stated instead of hidden -- fluid.viscosity is configurable and unread until TASK-031b, a narrow deliberate P-016 exception. Its central criterion is that the break is loud: a config still setting numerics.diffusion_coefficient must fail with an error saying where the field went, not be silently defaulted. Its entry also records why a task that computes nothing still owes a .feature file, since that looked like an exemption candidate: Criterion 7 says every task, and ADR-007's Stage 3 exemption is for criteria with no user-observable behaviour. Loading a config is user-observable, and the real claim -- old config fails loudly, new one works, demo still runs -- is scenario-shaped. Per-field type validation stays in test_configuration.py. TASK-031 is now four subtasks meant for one session, sharing a branch, test module and review cycle: (a) velocity as component fields, (b) viscosity distinct from a scalar's diffusivity, (c) per-field boundary values at one wall, (d) velocity advanced by step. Each carries its own acceptance criteria, written where the work is rather than gathered into a second list that would drift. The split exists because "velocity works now" is the shape of task that gets called done while one of its claims was never checked -- the failure Stage 4's exit audit found three times. Subtask (c)'s criteria require its mechanism to be exercised by two scalars, not only a velocity pair, so it cannot be built velocity-specific -- it is the first real consumer of the single global boundary-condition set TASK-040 deferred to Stage 6. Build order, discharge map, criterion 12, design question four, README and PLANNED all updated. status.md regenerated: 42 tasks total, Stage 5 at 0/5. make ci clean: 614 tests, 99%, all twelve gates. Co-Authored-By: Claude Opus 5 --- README.md | 4 + docs/CHANGELOG-DESIGN.md | 83 ++++++ docs/planning/roadmap.md | 386 +++++++++++++++++++++------ docs/planning/status.md | 9 +- tools/validators/check_references.py | 1 + 5 files changed, 402 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index 72335b8..ca1534a 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,10 @@ decided the same day. The seventh (what carries the momentum coefficients a converging pressure-correction loop needs) is deliberately left open for TASK-033 to answer with measurements, since TASK-027 already showed what deciding that one from an armchair costs. +Stage 5 starts with TASK-041 (Fluid Configuration Section), split out +from TASK-031 once those answers made it clear that a task about +velocity transport had quietly acquired the project's first breaking +configuration change. Try the most recent demonstration -- a scalar blob advected and diffused across a periodic domain, stepped live: diff --git a/docs/CHANGELOG-DESIGN.md b/docs/CHANGELOG-DESIGN.md index 6022e6d..48a3210 100644 --- a/docs/CHANGELOG-DESIGN.md +++ b/docs/CHANGELOG-DESIGN.md @@ -6189,3 +6189,86 @@ answers it with numbers and records it in the same place. mechanism Criterion 1 now names was read out of `tests/unit/test_simulation.py` rather than proposed -- both are things this repository already does, not things this stage invents. + +### TASK-041 split out, and TASK-031 broken into four subtasks + +Both from the same question, asked once the design answers were +recorded: is Stage 5 actually ready to start? Two things said no. + +**The configuration migration had landed inside TASK-031 by default.** +Design question four's answer -- a new `fluid:` section with +`numerics.diffusion_coefficient` migrating into it -- made "Velocity +Field Support" responsible for the project's first breaking +configuration change, a cross-cutting edit across the schema, loader, +template generator, `pyflow generate-config` and a committed demo config, +with no velocity in any of it. Split out as **TASK-041 (Fluid +Configuration Section)**, built first, the same late addition Stage 4 +made with TASK-040 and for the same kind of reason: the work is real, +independently verifiable, and everything after it is easier once it +exists. It keeps its number and takes first position, since position in +the roadmap says what happens when. + +TASK-041 ships `viscosity` rather than performing the migration alone, +and that decision is recorded in its entry rather than left implicit: a +pure move -- new section, one field relocated, nothing new in it -- would +be a breaking change buying nothing until TASK-031 arrives, which is +hard to justify to a user whose config just broke. The cost is stated +plainly instead: `fluid.viscosity` is configurable and unread until +TASK-031b, a narrow deliberate exception to P-016 rather than one +pretended away. Its central criterion is that the break is *loud* -- a +config still setting `numerics.diffusion_coefficient` must fail with an +error saying where the field went, not be silently defaulted while the +simulation runs on. + +**Why a task that computes nothing still owes a `.feature` file**, since +that looked like a candidate for an exemption and is not one: Stage 5's +Criterion 7 says every task, and Stage 3's exemption covers criteria +"about architecture... with no user-observable behaviour to describe" +(`adr/ADR-007-executable-acceptance-criteria.md`). Loading a +configuration file is user-observable, and this task's real claim -- an +old config fails loudly, a new one works, the demo still runs -- is +scenario-shaped. Per-field type validation stays in +`tests/unit/test_configuration.py` where every other field's already is. + +**TASK-031 is now four subtasks, meant for one session**, not four. +They share a branch, a test module and a review cycle, and none is worth +a `Status:` line of its own. The split exists because the task has four +separable claims and "velocity works now" is exactly the shape of task +that gets called done while one claim was never checked -- the failure +Stage 4's exit audit found three times. Each carries its own acceptance +criteria, written where the work is rather than gathered into a second +list that would drift: + +- **(a) Velocity as component fields** -- the decomposition and + reassembly design question one settled, isolated from anything that + transports or configures. +- **(b) Viscosity, distinct from a scalar's diffusivity** -- the + smallest of the four, separated because its failure mode is silent: a + run using the wrong coefficient produces a plausible flow, not an + error. +- **(c) Per-field boundary values at one wall** -- `u` and `v` are two + fields and a wall gives them different numbers, which makes this the + first real consumer of the "one global set of boundary conditions" + limitation TASK-040 deferred to Stage 6. Its criteria require the + mechanism to be exercised by two *scalars*, not only by a velocity + pair, so it cannot be built velocity-specific. +- **(d) Velocity advanced by `step`** -- the wiring, and the structural + assertion that the orchestrator contains no `"velocity"` literal, no + `VectorField` `isinstance` check, and no hardcoded component-name + pair. + +Two obligations stayed at task level rather than being pushed into a +subtask, because they are about the whole: every configuration field +added rejects bad input and reaches the generated template, and the four +subtasks' scenarios share `tests/unit/_numerics.py`'s building blocks -- +four subtasks in one module being the easiest place in this stage to +re-accumulate the duplication Stage 4's exit audit had to undo. + +- *Verified by:* `make ci` clean; the roadmap parsing to five Stage 5 + tasks in build order TASK-041, 031, 032, 033, 034; + `docs/planning/status.md` regenerated (42 tasks total, up from 41, and + Stage 5 showing 0/5); `make check-references` reporting five planned + artifacts. The splice that inserted TASK-041 asserted every anchor + occurred exactly once before writing anything and printed its own + line-count delta -- the direct lesson from the `str.index` splice + earlier in this session that silently duplicated 3,900 lines. diff --git a/docs/planning/roadmap.md b/docs/planning/roadmap.md index 6bf38af..0c9bb7e 100644 --- a/docs/planning/roadmap.md +++ b/docs/planning/roadmap.md @@ -6286,7 +6286,9 @@ actual bar (Criterion 5). than being left behind in `numerics:` -- the migration and its enumerated blast radius are recorded with that question's answer, and `examples/golden-demos/passive_scalar_transport.yaml` is the - one committed config that has to move with it. + one committed config that has to move with it. **Carried out by + TASK-041, which exists for this** rather than by whichever task + first happened to need a viscosity. - **A corrector-loop tolerance, and an iteration limit.** Neither exists; `linear_solver_tolerance`/`linear_solver_max_iterations` are the inner CG solve's, a different convergence question at a @@ -6532,7 +6534,10 @@ and touched nothing. `src/pyflow/configuration/schema.py`'s `PyFlowConfig` and loader, and `pyflow generate-config` (TASK-039). `make check-config-template` gates the template half automatically; the rest is a grep for the - field name. + field name. **TASK-041 (below) is the task that carries this out**, + split off on 2026-08-28 rather than left inside TASK-031, where it + would have made a task about velocity transport responsible for the + project's first breaking configuration change. - **Named `fluid:`, not `physics:`** -- the section holds material properties (viscosity now, density when Stage 6 needs it), and `physics` already means something narrower and specific in this @@ -6663,30 +6668,43 @@ reconstructed at the exit audit, following Stage 3 and Stage 4's own precedent. A task's own **Discharges** section is authoritative; this table is the index. -**Build order is TASK-031, 032, 033, 034 -- numerical order, for once**, -and structurally rather than conveniently: TASK-032's pressure field has -nothing to be solved against until velocity is transported, TASK-033's -corrector loop has nothing to correct until both exist, and TASK-034 -assembles what the first three build. The only thing that could reorder -this is design question three's answer, if it turns out the momentum -coefficients TASK-033 needs must be produced by TASK-031's own work -rather than read from it -- which would make part of TASK-033 a TASK-031 -obligation, not a reordering. +**Build order is TASK-041, 031, 032, 033, 034**, and structural rather +than convenient: TASK-041 settles the configuration surface everything +after it reads, TASK-032's pressure field has nothing to be solved +against until velocity is transported, TASK-033's corrector loop has +nothing to correct until both exist, and TASK-034 assembles what the +rest build. Numerical order from 031 onward, for once. + +**TASK-041 was added 2026-08-28, after this stage's criteria were +drafted and while auditing whether it was ready to start** -- the same +shape of late addition Stage 4 made with TASK-040, and for the same kind +of reason. Design question four's answer (a new `fluid:` section, with +`numerics.diffusion_coefficient` migrating into it) had landed inside +TASK-031 by default, which gave "Velocity Field Support" a cross-cutting +breaking configuration change with no velocity in it and made it the +widest task in the stage. It keeps its own number and takes first +position: **position in this document says what happens when, the number +does not**, the same reasoning TASK-040 and TASK-021/022 were kept under. + +The only thing that could reorder the rest is design question three's +answer, if it turns out the momentum coefficients TASK-033 needs must be +produced by TASK-031's own work rather than read from it -- which would +make part of TASK-033 a TASK-031 obligation, not a reordering. | Criterion | Discharged by | |-----------|---------------| -| 1. Velocity transported by the same mechanism as every other field | TASK-031 | +| 1. Velocity transported by the same mechanism as every other field | TASK-031, subtasks a, c and d between them | | 2. Pressure solved from the constraint, not transported | TASK-032 | | 3. Divergence decreases monotonically to the configured tolerance | TASK-033 | | 4. One timestep solves momentum and continuity together | TASK-034 | | 5. Physical correctness against a known answer, per case | TASK-034, each for its own bullet; Couette flow jointly with TASK-033, the first task able to run it | -| 6. Rejection paths exercised against real bad input | TASK-031..034, each for its own error conditions | -| 7. Executable Gherkin criteria, `make check-scenarios` gates | TASK-031..034, each for its own `.feature` file | +| 6. Rejection paths exercised against real bad input | TASK-041 and TASK-031..034, each for its own error conditions | +| 7. Executable Gherkin criteria, `make check-scenarios` gates | TASK-041 and TASK-031..034, each for its own `.feature` file -- TASK-041 included, and its entry says why a task that computes nothing still owes one | | 8. Demonstrations: Lid-Driven Cavity and Heat Diffusion | TASK-034 (this stage's last task) | | 9. `make ci` green on a real runner | TASK-034 | | 10. Documentation matches the tree, capability map included | TASK-034 | | 11. `mvp.md`'s Definition of Done discharged item by item | TASK-034, reading Criterion 11's own table back against what landed | -| 12. Configuration surface is real, validated and documented | TASK-031 for viscosity and solved-vs-prescribed velocity; TASK-033 for the corrector tolerance and iteration limit; TASK-034 for the tangential boundary value and whatever run-length control the demos need -- each task adds the fields its own criteria depend on, rather than one task inventing the whole surface up front | +| 12. Configuration surface is real, validated and documented | TASK-041 for the `fluid:` section, viscosity and `diffusion_coefficient`'s migration; TASK-031 for solved-vs-prescribed velocity and whatever its subtask (c) needs; TASK-033 for the corrector tolerance and iteration limit; TASK-034 for the tangential boundary value and whatever run-length control the demos need. Design question four settles the *shape* once, so these are additions in one established style rather than four tasks each inventing their own | | 13. The solver runs through ADR-003's seams, checked by substitution | TASK-034, the first task with a whole timestep to substitute into -- though TASK-033 is where the coupling first reaches the step at all, so a substitution check landing there instead discharges it equally well | **TASK-034 is this stage's last task in build order and therefore owns @@ -6702,6 +6720,125 @@ what the task must not merely *nominally* satisfy (`docs/practices.md`, "The intent lives in the qualifier"). The Completion Criteria above were drafted against these on 2026-08-28, not in place of them. +## TASK-041 + +Fluid Configuration Section + +**Added 2026-08-28, while auditing this stage's own readiness to start +-- not anticipated when Stage 5's criteria were drafted the same day.** +Design question four's answer (a new `fluid:` section, with +`numerics.diffusion_coefficient` migrating into it) landed inside +TASK-031's scope by default, which made "Velocity Field Support" the +widest task in the stage and gave it a cross-cutting breaking change +that has nothing to do with velocity. Split out for the same reason +Stage 4 added TASK-040 out of numerical order: the work is real, it is +independently verifiable, and everything after it is easier once it +exists. **Numbered 041 and placed first in this stage's document order +-- position says what happens when, the number does not**, exactly as +TASK-040 and TASK-021/022 before it. + +**Intent:** this is a configuration change with no physics in it, and +the temptation is to treat it as a rename. It is not. It is the first +change this project has made that **invalidates configuration files +users may already have**, and the criterion that matters is that the +break is complete and visible rather than partial and silent -- a config +still setting `numerics.diffusion_coefficient` must fail loudly, not be +quietly ignored while the simulation runs with a default. + +### Purpose + +Give the physical properties of the simulated fluid a home of their own, +separate from the numerical parameters that act on them, and move the +one property already misfiled there into it. + +### Dependencies + +TASK-005 (the configuration framework), TASK-019 (`NumericsConfig` and +whole-configuration validation), TASK-039 (`pyflow generate-config`), +and the `make config-template` generator added 2026-08-28. No engine +dependency at all -- nothing in `src/pyflow/engine/` reads these fields +until TASK-031b consumes viscosity. + +### Design decision, inherited rather than made here + +Design question four, resolved 2026-08-28 (above): the section is named +`fluid:` rather than `physics:`, viscosity lives in it, and +`diffusion_coefficient` migrates into it rather than being left behind. +That question's own answer records why, including the naming argument +and the enumerated blast radius. **This task does not reopen it**; it +carries it out. + +**One decision this task does make: it ships `viscosity` rather than +performing the migration alone.** A pure move -- creating a section and +relocating one field, with nothing new in it -- would be a breaking +change buying nothing until TASK-031 arrives, which is a poor trade and +a hard thing to justify to a user whose config just broke. Shipping the +field that gives the section its reason to exist makes the break worth +making once. The cost is stated plainly: `fluid.viscosity` is a +configurable value that nothing reads until TASK-031b, which is the +narrow, deliberate exception to P-016 this task takes rather than +pretends away. + +### Artifacts Produced + +- `src/pyflow/configuration/schema.py` -- a `FluidConfig` dataclass + (`viscosity`, `diffusion_coefficient`), its `validate`, and its slot on + `PyFlowConfig`; `diffusion_coefficient` removed from `NumericsConfig`. +- `tools/generators/generate_config_template.py` -- a `SECTION_COMMENTS` + entry for the new section and `FIELD_COMMENTS` for both its fields; + the stale `numerics.diffusion_coefficient` entry removed. +- `docs/implementation/config-template.yaml` -- regenerated, never + hand-edited (`make config-template`). +- `examples/golden-demos/passive_scalar_transport.yaml` -- the one + committed config that sets the migrating field. +- `tests/features/fluid_configuration.feature` and its binding module. + +**Why this task has a `.feature` file when it computes nothing**, stated +rather than assumed: Stage 5 Criterion 7 says *every* task's acceptance +criteria are a Gherkin file, and Stage 3's exemption does not extend +here (`adr/ADR-007-executable-acceptance-criteria.md` -- that exemption +is for criteria "about architecture... with no user-observable behaviour +to describe"). Loading a configuration file *is* user-observable +behaviour, and this task's central claim -- an old config fails loudly, +a new one works, the demo still runs -- is scenario-shaped rather than +assertion-shaped. The per-field rejection tests stay in +`tests/unit/test_configuration.py` where every other field's already +are; the feature file carries the migration's own claims, not a Gherkin +restatement of type validation. + +### Acceptance Criteria + +`tests/features/fluid_configuration.feature` is the criteria. Written to +cover, at minimum: + +- A configuration setting `fluid.viscosity` and `fluid.diffusion_ + coefficient` loads, and both values arrive on the loaded config + object. +- **A configuration still setting `numerics.diffusion_coefficient` is + rejected with a named error that says where the field went** -- not + ignored, not silently defaulted. This is the task's whole point: a + breaking change that fails quietly is worse than one that fails. +- The Passive Scalar Transport golden demo still runs through the real + CLI and still produces its own already-tested result -- the migration + changed where a number lives, not what the simulation does. Checked by + running it, not by inspecting the config. +- `viscosity` and `diffusion_coefficient` are independent fields: + setting one leaves the other at its own default. (That they have + *different effects* is TASK-031b's claim, which cannot be made here -- + nothing reads viscosity yet.) +- Each field rejects bad input with a named error + (`tests/unit/test_configuration.py`, Criterion 12), and the + regenerated template carries a comment for both, which + `make check-config-template` and + `test_every_live_config_field_has_a_comment` already gate. + +### Discharges + +Criterion 12, its viscosity and `diffusion_coefficient`-migration share. +Criterion 6 and Criterion 7, its own share. + +--- + ## TASK-031 Velocity Field Support @@ -6729,7 +6866,9 @@ TASK-012 (`Mesh`), TASK-014..016 (`Field`/`ScalarField`/`VectorField`), TASK-023 (`FirstOrderUpwindAdvection`), TASK-024 (`CentralDifferenceDiffusion`), TASK-025 (`RK4TimeIntegrator`), TASK-028/029 (the real Dirichlet/Neumann conditions a velocity boundary -resolves to), TASK-040 (`step`, `accumulate_flux_to_cells`). +resolves to), TASK-040 (`step`, `accumulate_flux_to_cells`), and +**TASK-041**, which supplies the `fluid.viscosity` this task's own +subtask (b) is the first thing to read. ### Design questions, all resolved for this task @@ -6739,88 +6878,181 @@ here: - **One:** momentum is transported as one `ScalarField` per component, with a `VectorField` assembled for the consumers that need one. No Stage 3 interface changes. -- **Four:** a new `fluid:` configuration section holds viscosity, and - `numerics.diffusion_coefficient` migrates into it -- **the project's - first breaking configuration change, and this task's to carry out**, - including `examples/golden-demos/passive_scalar_transport.yaml` and - `tools/generators/generate_config_template.py`'s own comment tables. - Solved-vs-prescribed velocity goes in `simulation:`. +- **Four:** a new `fluid:` configuration section holds viscosity, with + `numerics.diffusion_coefficient` migrating into it. **Carried out by + TASK-041, not here** -- this task consumes that surface rather than + building it, which is why TASK-041 exists at all. - **Two:** `velocity_tangential` beside the existing normal `BoundaryFaceConfig.velocity`. Listed against TASK-034 as well, - because whichever task first needs a no-slip wall is where it lands; - if that turns out to be this one, it lands here. + because whichever task first needs a no-slip *moving* wall is where it + lands; subtask (c) below needs per-component wall values but not + necessarily a moving one. **The one thing these answers leave for this task to work out** is how a -single wall's tangential value reaches two component fields that need +single wall's boundary values reach two component fields that need different numbers there (`u = U`, `v = 0` at a moving lid) -- design -question one's own recorded wrinkle, and the first real consumer of the -"one global set of boundary conditions" limitation TASK-040 deferred. +question one's own recorded wrinkle, the first real consumer of the "one +global set of boundary conditions" limitation TASK-040 deferred, and the +reason subtask (c) exists as its own step rather than as part of (d). + +### Subtasks + +**Four steps, meant to be done in one session**, not four sessions: +they share a branch, a test module and a review cycle, and none of them +is worth a `Status:` line of its own. The split exists because this task +has four separable claims and a single undifferentiated "velocity works +now" is exactly the shape of task that gets called done while one of its +claims was never checked. **Each subtask below is done when its own +acceptance criteria pass**; the task is done when all four do, plus the +whole-task obligations under Acceptance Criteria at the end. + +Strict TDD applies within each (`docs/practices.md`): the subtask's +failing scenarios first, then its implementation, before moving to the +next. Build them in order -- (b) needs (a)'s decomposition, (c) needs +something to give boundary values *to*, and (d) is the wiring the first +three make possible. + +#### TASK-031a -- Velocity as component fields + +A `VectorField` decomposes into one `ScalarField` per component, and +those components reassemble into an equal `VectorField`. This is the +whole of design question one's answer, isolated from anything that +transports or configures. + +*Acceptance criteria:* +- Round trip: decompose then reassemble reproduces the original field's + values exactly, on a non-square mesh with non-unit spacing and a + non-trivial origin, for values that are not 0 or 1 anywhere. +- Component fields carry the mesh they came from, and each is a real + `ScalarField` -- usable by any existing scheme with no adapter. +- The naming convention for a component field is fixed and stated once, + in the code, not re-derived per caller. +- Reassembly rejects, with named errors: a component count that + disagrees with the mesh's dimensionality, and components defined over + different meshes (Criterion 6). + +#### TASK-031b -- Viscosity, distinct from a scalar's diffusivity + +`fluid.viscosity` (TASK-041) becomes the diffusion coefficient the +*momentum* components are diffused with, while a transported scalar +keeps using `fluid.diffusion_coefficient`. This is the smallest subtask +and it is separated deliberately: it is the one whose failure mode is +silent, since a run using the wrong coefficient produces a plausible +flow rather than an error. + +*Acceptance criteria:* +- Changing `fluid.viscosity` changes the diffusive flux computed for a + velocity component and leaves a scalar's own diffusive flux + unchanged; changing `fluid.diffusion_coefficient` does the reverse. + Both directions, since either alone passes if the two are wired to the + same number. +- The two configured values are different from each other and neither is + 1 in the fixture -- Criterion 7's degenerate-fixture rule, which this + subtask is the easiest place in the stage to violate. + +#### TASK-031c -- Per-field boundary values at one wall + +Two transported fields get different boundary values at the *same* wall. +Required by (a)'s answer -- `u` and `v` are two fields and a wall gives +them different numbers -- and the first real consumer of the limitation +TASK-040 recorded ("does not yet express 'field A is 300K at this wall, +field B is 0 at the same wall'"). + +*Acceptance criteria:* +- Two fields transported in one run, with different prescribed values at + the same named boundary, each see their own -- checked in what the + interior scheme computes at that boundary face, not only in what + `evaluate()` returns (the TASK-028 rule, restated because it is the + one that catches wiring errors). +- A velocity component's wall value is not read from the field + `scalar_value` that a transported scalar uses, and vice versa, + demonstrated by changing one and observing the other's flux is + unchanged. +- Whatever mechanism this introduces is not velocity-specific: it is + exercised by two *scalars* with different wall values, not only by a + velocity pair (Criterion 1's "applies to any field" clause, and the + thing that makes Stage 6's four fields cheap). + +#### TASK-031d -- Velocity advanced by `step` + +The wiring: `step` advances velocity's components alongside any scalar, +through the same schemes, with no branch that knows which fields are +whose. + +*Acceptance criteria:* +- Velocity is advanced by the same `step` call, over the same timestep, + that advances a scalar -- and a run carrying both advances both. +- **The scalar's own result is identical whether the velocity carrying + it was solved or prescribed** (Criterion 1's executable form of "no + engine change"): capture a solved velocity field, re-run the scalar + against that same field supplied the way Stage 4 supplies one, and + compare to floating-point tolerance. +- The orchestrator module's source contains no `"velocity"` string + literal, no `VectorField` `isinstance` check, and no hardcoded + component-name pair, asserted the way `tests/unit/test_simulation.py` + already asserts the absence of `is_boundary_face` -- Criterion 1's + structural clause, using the mechanism Stage 4 established rather than + a new one. +- Velocity advected by itself reproduces a hand-derived result on a + small non-square mesh with non-unit spacing -- self-advection is the + one nonlinearity in the momentum equation, and the case a scalar + transported by a *prescribed* velocity never exercises. Hand-derived + means derived by hand: `docs/practices.md`'s standing rule, and the + only check here that would catch a sign error. +- A velocity field whose component count disagrees with the mesh's + dimensionality is rejected with the existing named error + (`IncompatibleVelocityFieldError`), exercised against this task's own + new path rather than assumed inherited (Criterion 6). ### Artifacts Produced - `tests/features/velocity_field_support.feature` -- this task's Acceptance Criteria, per - `adr/ADR-007-executable-acceptance-criteria.md`. -- **The modules this touches are now knowable, because design question - one is answered** (this bullet said they were deliberately unnamed - while it was open; that stopped being true on 2026-08-28): - - `src/pyflow/configuration/schema.py` -- the new `fluid:` section, - `BoundaryFaceConfig.velocity_tangential`, the solved-vs-prescribed - control on `SimulationConfig`, and `diffusion_coefficient`'s - migration out of `NumericsConfig`. - - `src/pyflow/engine/simulation.py` -- `step` advancing velocity's - components alongside any scalar, with no branch that knows which - ones they are. - - `tools/generators/generate_config_template.py` -- its - `FIELD_COMMENTS`/`SECTION_COMMENTS` for every field above, which - `make check-config-template` gates. - - `examples/golden-demos/passive_scalar_transport.yaml` -- the one - committed config carrying `diffusion_coefficient` today. + `adr/ADR-007-executable-acceptance-criteria.md`. One feature file with + its scenarios grouped by subtask, not four files: the subtasks are one + session's work and one task's claim, and `make check-scenarios` cares + that every scenario runs, not how many files they live in. +- `src/pyflow/engine/simulation.py` -- `step` advancing velocity's + components alongside any scalar (subtask d). +- `src/pyflow/configuration/schema.py` -- the solved-vs-prescribed + control on `SimulationConfig`, and whatever subtask (c)'s per-field + boundary values require. **Not the `fluid:` section itself, which is + TASK-041's.** +- `tools/generators/generate_config_template.py` -- comment entries for + any field the two bullets above add, which + `make check-config-template` gates. - **Still genuinely open, and small: where the component-to-`VectorField` assembly helper lives.** `vector_field.py` and `simulation.py` are both defensible homes and no new module is obviously needed; that is a choice to make while implementing rather than a design question to - escalate, and it is left unnamed here for the reason this bullet - originally gave -- a wrong path in prose is a `make check-references` - failure rather than a harmless guess. + escalate, and it is left unnamed here because a wrong path in prose is + a `make check-references` failure rather than a harmless guess. ### Acceptance Criteria `tests/features/velocity_field_support.feature` is the criteria, not -restated here as prose. Written to cover, at minimum: - -- Velocity is advanced by the same `step` call, over the same timestep, - that advances a scalar -- and a run carrying both advances both. -- **The scalar's own result is identical whether the velocity carrying - it was solved or prescribed** (Criterion 1's own executable form of - "no engine change"): capture a solved velocity field, re-run the - scalar against that same field supplied as Stage 4 supplies one, and - compare to floating-point tolerance. -- The orchestrator module's source contains no `"velocity"` string - literal and no `VectorField` `isinstance` check, asserted the way - `tests/unit/test_simulation.py` already asserts the absence of - `is_boundary_face` -- Criterion 1's structural clause, using the - mechanism Stage 4 established for it rather than a new one. -- A momentum diffusion coefficient (viscosity) distinct from a scalar's - diffusivity: setting one leaves the other's computed flux unchanged. -- Velocity advected by itself reproduces a hand-derived result on a - small non-square mesh with non-unit spacing -- self-advection is the - one nonlinearity in the momentum equation, and the case a scalar - transported by a *prescribed* velocity never exercises. -- A velocity field whose component count disagrees with the mesh's - dimensionality is rejected with the existing named error - (`IncompatibleVelocityFieldError`), exercised against this task's own - new path rather than assumed inherited (Criterion 6). -- Each configuration field this task adds rejects bad input with a named - error, the same shape every other config section's tests already take - (`tests/unit/test_configuration.py`), and appears in the regenerated - `docs/implementation/config-template.yaml` with a comment saying what - counts as valid (Criterion 12). +restated here as prose: the four subtasks' own criteria above are what +it covers, and they are written where the work is rather than gathered +into a second list that would drift from them. + +**Two obligations belong to the task as a whole rather than to any +subtask**, and are the reason this section still exists: +- Every configuration field this task adds rejects bad input with a + named error, the same shape every other config section's tests already + take (`tests/unit/test_configuration.py`), and appears in the + regenerated template with a comment saying what counts as valid + (Criterion 12). +- The four subtasks' scenarios share the fixtures and building blocks in + `tests/unit/_numerics.py` rather than each re-deriving a mesh and a + boundary condition (Criterion 7) -- four subtasks in one module is the + easiest place in this stage to accumulate exactly the duplication + Stage 4's exit audit had to undo. ### Discharges -Criterion 1, entirely. Criterion 12, its viscosity and -solved-vs-prescribed share. Criterion 6 and Criterion 7, its own share. +Criterion 1, entirely (subtasks a, c and d between them). Criterion 12, +its solved-vs-prescribed share -- **its viscosity and migration share +is TASK-041's**. Criterion 6 and Criterion 7, its own share. --- diff --git a/docs/planning/status.md b/docs/planning/status.md index e3fde6d..51b53af 100644 --- a/docs/planning/status.md +++ b/docs/planning/status.md @@ -15,14 +15,14 @@ demand, not part of this file. ## Progress -**33/41 tasks complete (80%)** across 14 planned stages. For the full plan, including +**33/42 tasks complete (79%)** across 14 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" : 33 - "Not started" : 8 + "Not started" : 9 ``` ### Milestones @@ -35,7 +35,7 @@ pie showData ### Up next -**Stage 5 -- First Fluid Solver** is next, starting with TASK-031 (Velocity Field Support), 3 more not yet started in this stage. +**Stage 5 -- First Fluid Solver** is next, starting with TASK-041 (Fluid Configuration Section), 4 more not yet started in this stage. ## Live repository facts @@ -115,10 +115,11 @@ pie showData ### Stage 5 -- First Fluid Solver -**no status recorded** -- `░░░░░░░░░░` 0/4 tasks; 13 criteria defined, no status line yet +**no status recorded** -- `░░░░░░░░░░` 0/5 tasks; 13 criteria defined, no status line yet | Task | Status | Date | Artifact | |------|--------|------|----------| +| TASK-041 -- Fluid Configuration Section | Not started | | | | TASK-031 -- Velocity Field Support | Not started | | | | TASK-032 -- Pressure Field | Not started | | | | TASK-033 -- Pressure Correction Loop | Not started | | | diff --git a/tools/validators/check_references.py b/tools/validators/check_references.py index 18e6060..2e474b9 100644 --- a/tools/validators/check_references.py +++ b/tools/validators/check_references.py @@ -110,6 +110,7 @@ # change is what that Stage's own design question one decides, and a # guess here would be the speculation P-016 refuses. PLANNED: dict[str, str] = { + "tests/features/fluid_configuration.feature": "TASK-041", "tests/features/velocity_field_support.feature": "TASK-031", "tests/features/pressure_field.feature": "TASK-032", "tests/features/pressure_correction_loop.feature": "TASK-033",