evidence(rocm): re-race the gfx1151 corpus; refuse unproven rankings at dispatch - #671
Conversation
The 12 committed rocm:gfx1151 rows predate #663 and carry separation: None. They cannot be re-raced by re-running a recorder: load_corpus makes every key a hit and _record_raced_the_live_field never consults separation, so a present row is returned unchanged. This evicts the rows it owns first. Carries both rules the NVIDIA half learned by nearly breaking them -- warm-start always (save_corpus writes the WHOLE cache, so a fresh start deletes every other device's rows), and print a before/after row-and-evidence diff, because both near-misses were regenerations that SUCCEEDED while producing weaker evidence. It also refuses to write when the other-device row count would drop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ered Registration is a side effect of importing the backend emit module. Without it the registry is empty and measured_arbitrate returns None for every shape -- which prints as 'no verified candidate', reading as 'this hardware cannot do it' rather than 'the driver forgot an import'. A confident wrong answer, not an error. The driver now imports rocm_hip explicitly and refuses to record at all when the registry is empty, rather than writing a corpus that claims the hardware has no candidates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…verdict That recorder built MeasureRecord directly from a SINGLE measurement, so every row it wrote carried separation: None -- 'never asked' -- which corpus_winner accepts as a dispatch hint on trust. It now measures --repeats times (default 7), takes a per-candidate median with the spread beside it, and records a real separation verdict. One pass would report a spread of zero and earn separated: True on every row automatically, which is worse than recording nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…at dispatch
AUTOTUNE-SEPARATION, ROCm half, plus the tightening it unblocks.
ALL 16 gfx1151 ROCm ROWS NOW CARRY A VERDICT -- zero never-asked, where all 12
previously had `separation: None`. 15 separate cleanly (margins 34-99.9% against
0.10-14.24% noise); the one refusal is paged_kv_decode 8192 end_to_end at 6.52%
margin vs 5.59% noise, which is genuinely marginal.
That is the OPPOSITE of sm_120, where 82% of rankings were unsupported, and the
reason is structural rather than a hardware difference: ROCm races two
candidates that are far apart (generic HIP vs WMMA), NVIDIA races four that
often sit within a few percent. Verified the numbers come from
`device_event`, not a wall-clock fallback -- the hazard recorded for this
backend -- so the noise floor is genuine.
Two recorder changes were needed and both were bugs in their own right:
* `record_paged_kv_corpus` built MeasureRecord from a SINGLE measurement, so
it had no spread and every row it wrote was unprovable by construction. It
now measures --repeats times (default 7) and records a real verdict. One
pass would report a spread of zero and earn `separated: True` on every row
automatically, which is worse than recording nothing.
* A new `record_autotune_separation` driver for the fused rows, because
re-running a recorder cannot re-race them: `load_corpus` makes every key a
hit and `_record_raced_the_live_field` never consults `separation`. It
evicts the rows it owns, warm-starts everything else, prints a before/after
row-and-evidence diff, and refuses to write if another device's row count
would drop.
THE TIGHTENING, now that every fleet row has had its chance: `corpus_winner`
also refuses a row that ranks two or more candidates and has no verdict.
NOT a literal "reject None", and the difference matters. `separation_verdict`
returns None when fewer than two candidates were timed, because a sole
candidate is chosen by applicability rather than by a race and has no margin to
defend -- 12 of the 23 remaining None rows are exactly that shape. Refusing
them would be a category error, not caution. `inf` is likewise not a
competitor: it marks "could not be timed", so a row with one latency and one
inf is a sole-candidate row wearing a pair's clothes.
Committed corpus after this: 113 rows -- 67 refused as dispatch hints (11 of
them unproven rankings), 34 with a supported verdict, 12 sole-candidate.
Mutation-verified three ways, including that the literal "refuse all None" rule
FAILS because it rejects sound sole-candidate rows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… backends
Key AUTOTUNE-SEPARATION-ROCM-2026-09-01.
ROCm parity validated -- all 16 rows carry a verdict, 15 separated; two
recorder defects fixed (single-measurement
records, and rows that a re-run cannot re-race)
NVIDIA follow-up required -- 11 rows now inert, recoverable by widening the
recorder's shape flags on Super-Bear
Apple not applicable -- but the entry bar changed: a first device timer
x86 not applicable with one sample earns separated:True for free
Records the contrast honestly: 15/16 vs 19/74 is field width (two far-apart
candidates vs four close ones), NOT evidence that one backend measures better.
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: f20d857a06
ℹ️ 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".
… timer gate Three review findings on #671. All three verified against the code before fixing; all three correct. P1 -- A SECOND CONSUMER BYPASSED THE RULE. `corpus_winner` is not the only path from the committed corpus to production dispatch: `cache/paged_kv.py::_rocm_paged_attention_corpus_winner` reads the same file directly and checked only that the winner was a known NAME. So the admission rules landed in one consumer while the other kept serving `direct` for the gfx1151 8192-token bucket on a ranking THIS BRANCH marks separated: false (6.52% margin vs 5.59% noise). Reproduced exactly, then fixed. This is the #670 finding one level down: there, a verdict with no consumer; here, a rule with only some of its consumers. The checks are now one predicate -- `record_is_admissible` -- called by both, rather than two copies to drift. P1 -- THE TIMER GATE WAS DOCUMENTED AND ABSENT. The module docstring said "the reason `--check-timer` exists"; no such flag was registered and `rocm_last_timer_source` was never consulted. Rows recorded during a wall-clock fallback would have been saved as timing="device" -- the wrong timing domain wearing the right label, in published evidence. Documenting a gate that does not exist is worse than not mentioning one, because a reader believes the write was checked; it was checked by hand in a separate probe, which is how the claim came to be written. Now implemented, on by default. P2 -- THE GUARD HAD THE BLIND SPOT IT GUARDS AGAINST. Eviction matched startswith("rocm") while `_summarise` counted every ROCm device as one bucket, so the "did another device lose rows?" check could not see the loss of a sibling ROCm device's fused rows. Both now key on the exact rocm:gfx1151 device. No live damage -- gfx1151 is the only ROCm device in the corpus -- but the guard existed precisely to prevent silent evidence deletion. Mutation-verified: removing the paged-KV admission check fails two tests, and _summarise now classifies a sibling ROCm device as `other`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Evaluated all three against the code before touching anything. All three are correct, and one of them lands harder than the reviewer stated. P1 — the second consumer. Correct, and reproduced
This is the #670 finding one level down: there, a verdict with no consumer; here, a rule with only some of its consumers. I fixed one call site and assumed it was the boundary. Fixed by extracting one predicate — P1 — the phantom flag. Correct, and worse than describedMy docstring said "the reason The reviewer frames it as a missing option. I'd put it more sharply: documenting a gate that doesn't exist is worse than not mentioning one, because a reader believes the write was checked. It was checked — by me, by hand, in a separate probe — which is exactly how the claim came to be written as if the script did it. Now implemented and on by default, refusing to write device-timed rows when the timer wasn't P2 — the guard's blind spot. Correct, and the irony is the pointEviction matched That guard exists specifically to prevent silent evidence deletion, after I nearly deleted 12 ROCm rows on the NVIDIA half. It had the same blind spot as the bug it guards against. Both now key on the exact No live damage — gfx1151 is the only ROCm device in the corpus — but that's luck, not design. VerificationMutation-verified: removing the paged-KV admission check fails two tests. 279 passed across corpus/autotune/arbiter/paged suites; The pattern worth namingAcross #670 and #671 the same shape has now appeared three times: a rule that exists in fewer places than it applies. A verdict nothing read; a rule only one of two consumers applied; a guard that couldn't see the case it guarded. Each time the fix was to have one statement of the rule rather than a copy — which is Decision #31's "one implementation per boundary" showing up in admission logic rather than lowering paths. |
1 participant
The ROCm half of
AUTOTUNE-SEPARATION, plus the tightening you approved to follow it.All 16 gfx1151 rows now carry a verdict
Zero never-asked, where all 12 previously had
separation: None. 15 separate cleanly — margins 34–99.9% against 0.10–14.24% noise. The one refusal ispaged_kv_decode 8192 end_to_endat 6.52% margin vs 5.59% noise, genuinely marginal.Verified the numbers come from
device_event, not the wall-clock fallback this backend is prone to — the hazard I'd recorded — so the noise floor is genuine rather than inflated.This is the opposite of sm_120's 82%-unsupported result, and I'd flag how to read it. ROCm races two candidates that are far apart (generic HIP vs WMMA); NVIDIA races four that often sit within a few percent. 15/16 vs 19/74 is field width, not measurement quality. A backend that adds candidates should expect its separated fraction to fall — that's the field getting more honest, not the hardware getting worse.
Two recorder defects, both bugs in their own right
record_paged_kv_corpusbuiltMeasureRecordfrom a single measurement. It had no spread, so every row it wrote was unprovable by construction —separation: Nonewasn't an omission, it was the only possible outcome. Now measures--repeatstimes (default 7). One pass would report a spread of zero and earnseparated: Trueon every row automatically, which is worse than recording nothing.load_corpusmakes every key a hit and_record_raced_the_live_fieldnever consultsseparation, so a present row comes back unchanged. New driver evicts the rows it owns, warm-starts the rest, prints a before/after row-and-evidence diff, and refuses to write if another device's row count would drop.One trap worth carrying: candidate registration is a side effect of importing the backend's emit module. A driver that forgets it gets an empty registry, and
measured_arbitratereturnsNonefor every shape — which prints as "no verified candidate", reading as "this hardware cannot do it" rather than "I forgot an import". A confident wrong answer, not an error. The driver now imports explicitly and refuses to record at all on an empty registry.The tightening — and it is deliberately not "reject
None"corpus_winnernow refuses a row that ranks two or more candidates and has no verdict.separation_verdictreturnsNonewhen fewer than two were timed, because a sole candidate is chosen by applicability, not by a race, and has no margin to defend. 12 of the 23 remainingNonerows are exactly that shape — refusing them would be a category error, not caution.inflikewise isn't a competitor (it marks "could not be timed"), so a row with one latency and oneinfis a sole-candidate row wearing a pair's clothes.Committed corpus: 113 rows — 67 refused as dispatch hints, 34 with a supported verdict, 12 sole-candidate.
Mutation-verified three ways, including that the literal "refuse all
None" rule fails, because it rejects sound sole-candidate rows.Cross-backend, key
AUTOTUNE-SEPARATION-ROCM-2026-09-01ROCm parity validated. NVIDIA follow-up required — 11 sm_120 rows are now inert at shapes the recorder's default flags don't cover; recoverable by widening those flags on Super-Bear, and until then those buckets fall back to lead-safe tier priority, which is correct degraded behaviour rather than a regression. Apple/x86 not applicable, but the entry bar changed: a first device timer with one sample earns
separated: Truefor free.🤖 Generated with Claude Code