Open Stage 5: thirteen completion criteria, eight decisions, and a task split out - #42
Merged
Conversation
…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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…il 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 <noreply@anthropic.com>
…r 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opens Stage 5 (First Fluid Solver) with real completion criteria, written
before its first task starts per
docs/practices.md's standing rule —the sixth stage in a row. No implementation: this is planning, plus two
docstring corrections and one validator change.
Stage 5 defines the MVP, so its criteria are reconciled item by item
against
docs/implementation/mvp.md's own Definition of Done — acondition the Stage 5 section has carried since 2026-08-22.
What's here
Thirteen completion criteria covering the stage's goal rather than
the union of its tasks: velocity transported by the same mechanism as
any other field, pressure solved from the constraint rather than
transported, monotonic corrector convergence measured across iterations,
a timestep coupling momentum and continuity, physical correctness per
case, the configuration surface, and the solver actually running through
ADR-003's seams.
Four task specs plus a new one. TASK-041 (Fluid Configuration
Section) was split out of TASK-031 and built first; TASK-031 is broken
into four subtasks (a–d) intended for a single session, each with its own
acceptance criteria.
Eight maintainer decisions, all recorded where they were found.
Reviewer notes
Three things are worth a reviewer's attention specifically:
1. This commits the project to its first breaking configuration
change. Design question four settled on a new top-level
fluid:section, and
numerics.diffusion_coefficientmigrates into it ratherthan being left behind — splitting two quantities of the same kind
across two sections would be worse than either arrangement chosen
consistently. Blast radius is enumerated in the roadmap rather than left
to be discovered: the schema, loader, template generator's comment
tables,
pyflow generate-config, andexamples/golden-demos/passive_scalar_transport.yaml. TASK-041 owns it,and its central criterion is that the break is loud — an old config
must fail with an error saying where the field went, not be silently
defaulted.
2. Ghia et al.'s illustrative 2% is deliberately not adopted.
adr/ADR-007anddocs/glossary.mdboth use "matches Ghia et al. within2%" to illustrate what an executable physics criterion looks like. 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 documented numerics are not built to clear. Criterion 5 requires
convergence instead — error against Ghia's tabulated profiles decreasing
monotonically across at least three mesh resolutions, plus qualitative
structure at the finest.
3. Two contradictions in the existing repository were found and
fixed.
src/pyflow/physics/__init__.pysaid the package was forphysical models, "incompressible flow first" — Stage 5's exact subject —
while
physics/CLAUDE.mdbeside it said "empty until Stage 6, and emptyon purpose". Contradictory since TASK-000, unnoticed because nothing had
cause to ask. And
docs/planning/backlog.md's conservation item stillread as though TASK-027 would discharge the divergence-free claim; it
landed deliberately weaker, and the strong claim is now Criterion 3.
How this was reviewed
Three audit passes under
prompts/common/AUDITOR.md's stance, and thesecond and third both found real defects in the first draft:
constant sequence — exactly what a corrector doing nothing produces.
field isn't automatically steady, and uniform flow can't exist inside
no-penetration walls.
names which fields are transported.
upwind's dissipation guarantees net decay. Now: no single step may
increase it.
stephas never touchednumerics.pressure_couplingor.linear_solver, so a timestepconstructing
PISOdirectly would pass every other criterion — sameGhia comparison, same demos — while quietly retiring the architectural
claim
mvp.mdsays the MVP exists to validate.Still open by design
One of seven design questions is deliberately unanswered: 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 plausible strategies, each leaving most of
the divergence in place, for a reason that only appeared once someone
measured. TASK-033 answers it with numbers.
Verification
make ciclean locally at every commit: 614 tests, 99% coverage, alltwelve gates.
make check-claimsreturns only its two known falsepositives. The
TypeErrorbehind design question one was produced byrunning both concrete schemes against a real
VectorFieldon a realmesh, not inferred from reading the code; Criterion 12's list of missing
configuration fields was checked field by field against
schema.py.CI on both platforms is what this PR is for — the Merge Gate requires it
for anything touching code, and this touches
tools/validators/check_references.py.🤖 Generated with Claude Code