feat(autotune): refuse to call a margin a verdict when it is inside the noise - #663
Merged
Merged
Conversation
…he noise The arbiter took one median per candidate and picked `min`, so a MeasureRecord could assert a winner with nothing to say whether the difference meant anything. It did: on sm_120 at 256^3 the two NVIDIA matmul lanes measured 0.01300 ms (sd 14.5%) and 0.01057 ms (sd 39.1%), and that 18.7% gap was recorded as a clean 1.63x win. The same 1.6x ratio at 2048^3 is real -- spreads there are 2.2% and 0.6%. A record storing only medians cannot tell them apart. Measured over the committed corpus: 75 of 87 rows assert a ranking, none of them declares a separation, and 11 of the 75 (15%) picked a winner that beat the runner-up by under 2% -- down to 0.07%, well inside end-to-end wall jitter. - `measure_latency_samples` / `relative_spread` expose the dispersion that `measure_latency` was discarding; device timing repeats the whole measurement `device_repeats` times (default 3) so it has a noise floor at all. - `separation_verdict` judges the winner's margin against the noisier of the two fastest lanes, at SEPARATION_FACTOR=2. `None` when fewer than two candidates were timed -- a sole candidate is chosen by applicability, not a race, so there is no margin to defend. - `MeasureRecord.separation` + `is_separated()`. `None` means the record never asked, and must be read like `unmeasured`'s `None`: not the favourable answer. Absent from JSON when unset, so the corpus stays diffable. - An unseparated re-race keeps the incumbent rather than adopting the nominally-faster lane, so a tie cannot thrash the cached selection between runs. A separated race still displaces it. A tie never blocks dispatch -- something has to run. It blocks *claiming* one candidate is faster. Tests mutation-checked: forcing separated=True fails 6, removing the incumbent rule fails 1, zeroing the noise floor fails 5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Key AUTOTUNE-SEPARATION-2026-08-31. The corpus is shared by all four backends,
so what counts as a verdict is a shared contract even though the hardware
producing the numbers is not.
NVIDIA parity validated -- defect found here, sm_120 evidence; all 11
under-2% corpus rows are NVIDIA, an artifact of it
racing the widest field rather than anything CUDA
x86 parity validated by construction -- end-to-end timing already
collected the samples and discarded them; no new
measurement needed, no device_repeats cost
ROCm follow-up required -- backend-agnostic change, no gfx1151 row
re-raced; note that a wall-clock fallback raises
the noise floor, so an unseparated ROCm row may
mean the timer degraded, not that lanes match
Apple follow-up required -- compounds with NVIDIA-TIMER-DRAIN: a single
unvalidated timestamp has zero spread by
construction, so it would earn a free separation.
Land the repeated-measurement half WITH the wall
witness, not after it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdd9978299
ℹ️ 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".
gstoner
added a commit
that referenced
this pull request
Sep 1, 2026
…wo stale verdicts AUTOTUNE-SEPARATION, NVIDIA half. 42 of 51 freshly-raced rankings (82%) are NOT separated. The earlier "11 rows under 2%" understated it badly, because a margin cannot be judged without the noise beside it -- which is what #663 added and this measures. TWO RECORDED VERDICTS RETIRED BY EVIDENCE. At 512^3 and 1024^3 device-timed matmul the compiler-EMITTED PTX lane wins by ~38% against 0.15-1.86% noise, racing the full four-candidate field. The prior rows named a TILE lane and pinned the 1024^3 field to exactly two candidates -- encoding the biased race #655/#662 removed, where the GEMM lanes had no device timer, scored `inf`, and lost silently. "The tile lane wins" meant "the tile lanes were the only ones that could be timed". device_repeats 3 -> 10 for the recorder. Measured at 128x512x64 bf16, sd is 48.31% / 30.74% / 19.34% over 3 / 10 / 30 whole measurements. The lane really is ~19% noisy so the verdicts hold, but a published floor 2.5x the truth is a number someone acts on. `measured_arbitrate` keeps 3 -- the right trade for runtime selection, not for evidence. AN INDEPENDENT MECHANISM AGREES. `finalize_test5_corpus` replaces a row only when two runs pick the same winner. The one row it refuses -- bfloat16 [128,256,64] device -- is exactly the row separation flags at 9.92% margin vs 102.96% noise. Two checks built years apart rejecting the same ranking. TWO NEAR-MISSES, both of which destroy evidence silently: * without --warm-start the recorder writes the whole cache and deleted all 12 rocm:gfx1151 rows plus 13 NVIDIA rows -- 25 rows, no error; * recording is two runs plus the finalizer. One run yields no `evidence` block at all, so the corpus carries LESS evidence than it replaces (41 -> 0) while looking like an update. Done properly: 41 -> 84, zero rows lost. Both share a shape worth naming -- a regeneration that succeeds while producing weaker evidence. Only a before/after row-and-evidence diff catches it. Three consumer tests rewritten to the measured reality, each pinning something the old assertion could not express: the full field AND a separated margin, not a winner's name; eligible => stable, rather than "every row is stable", since reproducibility at the launch-overhead floor is not a property this hardware has; and a SUBSET of the candidate pair, because applies_to makes them mutually exclusive by contract. All three mutation-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gstoner
added a commit
that referenced
this pull request
Sep 1, 2026
…ice_repeats Two review findings on #670, both correct, and the first is the important one. P1 -- SEPARATION HAD NO CONSUMER. I added the field in #663, documented it at length, and nothing read it. `corpus_winner` validated that a record raced the live field (#655) and never asked whether the verdict was SUPPORTED, so `run_arbitrated` kept dispatching on rows the corpus itself marks as noise. The committed sm_120 corpus holds a float16 device row whose 2.16% margin sits under 148.55% noise, and before this check that row still changed a production route -- this PR was shipping route changes off a corpus whose unsupported rows were equally live. Decision #29 names it exactly: a declaration with no consumer is worse than a missing one, because it reads as a closed contract in review. It did. `corpus_winner` now refuses `separated: False`, and also `selector_eligible: False` -- the finalizer's own signal, set when two independent runs disagree, which is the same conclusion reached by a different mechanism. `separation is None` is still ALLOWED, deliberately. None means the row predates the field and was never asked -- the state every row was in before #663. Rejecting it would silently deactivate most of the committed corpus as a side effect of adding a check. A row that is KNOWN unsupported is strictly worse than one merely unproven, and only the first is a regression to allow. Measured consequence on the committed corpus: of 109 rows, 55 are now refused as dispatch hints, 19 carry a supported verdict, and 35 remain usable as never-asked legacy. Re-racing is what moves a None row into the first two groups. P2 -- device_repeats now travels with the evidence. It sets the noise floor behind every separation verdict and is distinct from `device_reps` (launches inside one measurement). Runs at 3/10/30 produce materially different verdicts (sd 48.31% / 30.74% / 19.34%), so omitting it left rows whose separation could not be reproduced or compared. Mutation-verified: removing either consumer check fails, and so does tightening `is False` to `is not True` -- which pins the None asymmetry as deliberate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gstoner
added a commit
that referenced
this pull request
Sep 1, 2026
`applies_to(region)` is shape-blind by construction: a region carries structure (epilogue chain, dtype, transpose flags) and the dimensions arrive with the operands. So an aligned-only lane could not decline a ragged shape at the applicability gate, and the F4 oracle could not cover for it -- its probe shape is fixed and its verdict is cached under a key with no shape in it. The lane declined inside `run` instead, by returning the numpy reference, after it had already won. Two harms, reproduced against the real NvidiaMmaGemmEmittedCandidate: - Starvation. It won on tier at a ragged shape and handed back numpy while a lower-tier lane that could serve the shape went untried -- the failure RocmWmmaGemmCandidate.available was hardened against on the availability axis (PR #289 review), one axis over. - A fabricated measurement. `_measure` timed the decline and stored 0.00525 ms of numpy under the kernel's name against a real 0.00196 ms rival. That number is not inert: with the backstop disabled the record comes back `separated: True, margin 0.59`, so #663's separation machinery certifies a 2.4x loss for a kernel that never ran. The execution tag already said so. The D3 arbiter log has described this as "a silent degrade ... an unsupported shape" since it was written, and nothing read it. - `Candidate.applies_to_inputs(region, *inputs)`, additive, default True, fail-OPEN on absent/malformed operands (an operand error must still raise through `run`, not be silently excluded -- Decision #21). - `candidate.live_candidates`, one statement of "who is racing", replacing the copy `arbitrate`, `measured_arbitrate` and `corpus_winner` each kept. - `_measure` reads the tag: a reference decline lands in `unmeasured`, not in `candidates`. Fail-CLOSED backstop for lanes that never adopt the hook. - Producers: the NVIDIA emitted GEMM (aligned-only, as its own docstring and device timer already said) and ROCm flash-attention (head_dim % 16, Tier-3 on the one AMD device that executes -- the worse-placed of the two). All four backends assessed under APPLIES-TO-SHAPE-BLIND-2026-09-01: NVIDIA and ROCm follow-up required (device proof owed), Apple and x86 not applicable with reasons. Mutation-verified: four mutations, each killing only its own tests. Device evidence is NOT claimed -- this Mac has no CUDA or ROCm, so harm 2 was reproduced under a simulated device and labelled. Co-Authored-By: Claude Fable 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
1 participant
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.
Follow-on from #662, which flagged this in the NVIDIA plan: "the arbiter should not record a matmul winner at 256³ without a separation check — a median difference smaller than the spread is not a verdict."
The defect
measured_arbitratetook one median per candidate and pickedmin. A margin was a margin; nothing recorded how noisy the lanes were. Measured on sm_120 at 256³:Recorded as a clean 1.63× win. The same ratio at 2048³ is real — spreads there are 2.2% and 0.6%. The ratio isn't what distinguishes them; the spread is, and the record didn't keep it.
It's systemic, not one shape
Measured over the committed corpus — 87 rows:
That last group is inside ordinary end-to-end wall jitter. Those eleven verdicts record which lane was luckier.
(I nearly reported something stronger — pairs of same-bucket rows with different winners. They turned out to be different dtypes, so that would have been an overclaim. The 11-of-75 figure is the one that holds.)
The change
measure_latency_samples/relative_spreadexpose dispersionmeasure_latencywas discarding. Device timing repeats the whole measurementdevice_repeatstimes (default 3) so it has a noise floor at all.separation_verdictjudges the margin against the noisier of the two fastest lanes, atSEPARATION_FACTOR = 2. ReturnsNonewhen fewer than two candidates were timed — a sole candidate is chosen by applicability, not a race, so there's no margin to defend.MeasureRecord.separation+is_separated().Nonemeans the record never asked, and reads likeunmeasured'sNone: not the favourable answer. Omitted from JSON when unset so the corpus stays diffable.A tie never blocks dispatch — something has to run. It blocks claiming one candidate is faster.
Verification
separated=Truefails 6, removing the incumbent rule fails 1, zeroing the noise floor fails 5.mypyclean.separation: None(never asked), not a wrong verdict. Re-racing the 11 tight rows needs sm_120 and is the follow-up.Cross-backend (AGENTS.md:81-85), key
AUTOTUNE-SEPARATION-2026-08-31device_repeatscostNVIDIA-TIMER-DRAIN: a single unvalidated timestamp has zero spread by construction, so it would earn a free separation. Land the repeated-measurement half with the wall witness, not after it🤖 Generated with Claude Code