compiler: interleaved backward runs from the last virtual stage back; the spectral policy is bound by a hash chain - #627
Conversation
… the spectral policy is bound by a hash chain Two follow-ups recorded during the #626 review, plus a third defect the first one exposed. 1. Interleaved backward order. `_build_interleaved` computed `bwd_clock = fwd_clock + p*v` — a CONSTANT offset — so backward inherited forward's ASCENDING virtual-stage order, the opposite of gradient flow: stage s's backward ran before the stage s+1 backward that produces its input gradient (p=2/v=2, micro-batch 0: stage 0 B at clock 4 ... stage 3 B at clock 7). The stage term is now mirrored: `(V-1-stage) + mb + V`. Gradients flow from the last virtual stage back to the first, and the makespan is BIT-IDENTICAL (first backward clock still V, last still 2V+m-2), so this is an ordering fix, not a schedule change. Verified over five configurations: zero dependency violations, makespan unchanged, and the rank/clock collision count identical to the old generator (a pre-existing modelling property of this analytical builder, deliberately untouched). The backward gradient edge is therefore expressible again, so the "known limitation" recorded in #626 is deleted and that edge is REQUIRED like any other — an inversion now fails closed. 2. Decoupled schedules carried fabricated cross-stage edges — found while making (1) fail closed. A decoupled stage owns a self-contained objective and trains directly from data; zero cross-stage coupling is its defining property, yet the carrier asserted cross-stage forward edges (which silently passed the old ordering filter) and dropped the backward ones. Only per-rank program order and each stage's own forward->backward edge remain. 3. Spectral policy binding. Verifying consumed attributes against a carried declaration stops a lone attribute edit but not an edit of BOTH. It turns out the producer already had a complete chain — `schedule_digest = sha256(payload)`, the payload names `object_id "spectral:<sha256(identity)>"`, and the identity carries workspace_bytes/native_entry/normalization — so rather than invent a scheme, the consumer now RE-DERIVES that chain with llvm::SHA256 over the two preimages the module carries. Editing an attribute and its declaration together now breaks the second hash link. Gates (Strix Halo): ninja all targets clean; IR lit 356 passed / 0 failed; spectral suite 12/12 with tessera-opt live (including the co-edit and payload-swap rejections); pipeline suites 56 passed; full unit suite 16427 passed. The 42 remaining failures are the recorded environmental baseline (Apple lane + rocm fp8 + NVIDIA-dialect-off rows): the failure sets with and without this change are IDENTICAL, verified by stashing. mypy 0; ruff clean; dashboards in sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b756f1ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tity contract (PR #627 review) New sync key SPECTRAL-PAYLOAD-CHAIN-2026-08-25. The change adds two required preimages to the shared Schedule->Tile spectral contract and is verification-only (no kernel, launch parameter, or numerical result changes): rocm parity validated exact-device on gfx1151 and x86 validated on the AVX-512 host (44 device-lane tests plus the 12/12 spectral suite with tessera-opt live); nvidia and apple not-applicable-today with the architecture-specific reason and the obligation they inherit on adoption. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… invariants as tests Mathematical audit of this PR found one of its own claims false. I stated the reordering left the (rank, clock) collision count identical to the old generator; that was generalised from three configurations which happened to be unchanged. Swept over p<=6, v<=5, m<=12: unchanged in 235/360, INCREASED in 125 (at most 9, mean +1), never decreased. The builder is an analytical bubble model, not a resource-feasible timeline — it already placed several steps on one (rank, clock) slot — so this changes no execution claim, but the note was wrong and is corrected in place. Dependency correctness and makespan are unaffected, and are now proved rather than sampled. With V = p*v: clock_F(s,mb) = s + mb, clock_B(s,mb) = 2V-1-s+mb T1 forward chain: clock_F(s) - clock_F(s-1) = 1 > 0 T2 backward chain: clock_B(s) - clock_B(s+1) = 1 > 0 (old: -1, inverted) T3 backward after own forward: difference 2V-1-2s, minimised at s=V-1 -> 1 T4 span endpoints V and 2V+m-2, identical to the pre-fix generator test_pipeline_schedule_carrier.py checks all four exhaustively over the parameter space and pins the generator to the closed forms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Mathematical correctness audit of this PR — including a correction to one of its own claims. Correction: the collision claim was wrongI wrote that the reordering leaves the
What it does and doesn't mean: this builder is an analytical bubble model, not a resource-feasible timeline — it already placed several steps on one The ordering fix is now proved, not sampledThe clock model is exact integer arithmetic, so with
All four are now exhaustive tests, plus one pinning the generator to the closed forms. Audit of the merged polygamma tower (while I was at it)Against independent references — scipy on the positive axis, and the recurrence
CI was green before these two commits; re-running now. 🤖 Generated with Claude Code |
|
Already addressed — the assessment landed in b1e6777, pushed before this review comment was posted (the bot reviewed the earlier commit Sync key
The entry also covers this PR's other shared change — the interleaved pipeline carrier ordering — since ROCm consumes that carrier through the shared pipeline passes (phase4 lit 6/6 green). 🤖 Generated with Claude Code |
The two follow-ups recorded during the #626 review — plus a third defect the first one exposed.
1. Interleaved backward order (the generator defect)
_build_interleavedcomputedbwd_clock = fwd_clock + p*v— a constant offset — so backward inherited forward's ascending virtual-stage order, the opposite of gradient flow. Measured (p=2, v=2, micro-batch 0): stage 0 B at clock 4 … stage 3 B at clock 7, i.e. stages's backward ran before the stages+1backward that produces its input gradient.The stage term is now mirrored —
(V-1-stage) + mb + V— so micro-batch 0 runs F 0→1→2→3, B 3→2→1→0. Crucially the makespan is bit-identical (first backward clock stillV, last still2V+m-2), which is what makes this an ordering fix rather than a schedule change. Verified across five configurations: zero dependency violations, makespan unchanged, and the rank/clock collision count identical to the old generator (a pre-existing modelling property of this analytical builder — measured, and deliberately left alone).Because the edge is expressible again, the "known limitation" recorded in #626 is deleted and the backward gradient edge is now required like any other; an inversion fails closed.
2. Decoupled schedules carried fabricated cross-stage edges
Found while making (1) fail closed. A decoupled stage owns a self-contained objective and trains directly from data — zero cross-stage coupling is its defining property — yet the carrier asserted cross-stage forward edges (which silently passed the old ordering filter) and dropped the backward ones. Now only per-rank program order and each stage's own forward→backward edge remain.
3. Spectral policy: comparison → hash chain
Verifying consumed attributes against a carried declaration stops a lone attribute edit, but not an edit of both. Investigating showed the producer already had a complete chain:
schedule_digest = sha256(payload)→ payload namesobject_id "spectral:<sha256(identity)>"→ identity carriesworkspace_bytes/native_entry/normalization. So instead of inventing a scheme, the consumer now re-derives that chain withllvm::SHA256over the two preimages the module carries. The decisive test edits the attribute and its declaration together — the case the previous fix would have accepted — and it is now rejected at the second hash link.Evidence (Strix Halo)
ninjaall targets clean · IR lit 356 passed / 0 failed · spectral suite 12/12 withtessera-optlive (including co-edit and payload-swap rejections) · pipeline suites 56 passed · full unit suite 16,427 passed.The 42 remaining failures are the recorded environmental baseline (Apple lane + rocm fp8 + NVIDIA-dialect-off rows). I verified this rather than assuming: stashing the change and re-running the affected files gives an identical failure set, so this change introduces none.
mypy 0 · ruff clean · dashboards in sync.
Remaining from the four-area sweep: the W4 admissible-effects packet family, still unstarted.
🤖 Generated with Claude Code