W1.1b: close the three $kind ops that failed open - #505
Merged
Merged
Conversation
Not the item the plan describes, because the plan's premise did not survive
measurement. It says "EnumAttr for every semantic StrAttr (4 x $kind)". Measured:
**17** ops carry `$kind`, **3 of them are I64Attr** rather than strings, and 14
of the 17 ALREADY fail closed with a named error in their generator.
Three did not. Probed by feeding each a bogus kind and inspecting the OUTPUT
rather than the exit code, `tessera_rocm.{predicate,optimizer,clifford}`
returned 0 AND emitted a complete GPU kernel.
All three failed the same way: a trailing `else` doing double duty as a real
branch and an unnamed semantic default.
predicate else { /* isfinite */ } -> a typo silently tests isfinite
clifford else { *t = GP; } -> a typo silently computes the GEOMETRIC
PRODUCT, a different algebra operation
returning plausible numbers
optimizer falls through to Adam -> a typo silently trains with Adam
Decision #21a: a semantic key fails closed; it may never be silently defaulted.
── The sets, and why deriving them is the hard part ──
Each legal set includes the ELSE BRANCH'S OWN NAME, which is exactly what a
`kind ==` scan of the consumer does not see. That scan yields {isnan, isinf} and
{wedge, left_contraction} and omits `isfinite`, `geometric_product` and `adam` --
all real values producers emit (`runtime.py:4000`, `_CLIFFORD_KIND_NAME`, and 4
uses of `"adam"`). A constraint built that way REJECTS VALID PROGRAMS, which is
the same defect as the earlier `mode` set that omitted "set".
predicate isnan, isinf, isfinite
clifford geometric_product, wedge, left_contraction
optimizer sgd, momentum, nesterov, lion, adam, adamw, adafactor
── And I made that exact mistake again, in this change ──
The first version of the optimizer enum omitted `adafactor`, and the negative
fixture used `adafactor` as its example of a plausible-but-unimplemented value.
It is neither: the pass implements it and
`test_adafactor_full_backward_executes_on_gfx1151` runs it on hardware. Six unit
tests failed and are what caught it -- I had read a 15-line window of the
consumer and taken it for the vocabulary; the pass's full literal set is seven.
The durable lesson, now written into the .td: the existing tests encode producer
reality better than any scan of the consumer, so run them before trusting a set.
A later scare (tests appearing to emit `isnormal`/`inner`) turned out to be my
own negative fixtures matching a grep that lacked `--include=*.py` -- checked
rather than reacted to, and the sets stand.
298 lit, 14425 unit, mypy 0, ruff clean, docs in sync.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gstoner
pushed a commit
that referenced
this pull request
Aug 5, 2026
…ls OPEN Review was right, and checking it invalidated my own framing. I wrote "11 already-fail-closed `$kind` sites". I measured the COUNT and the dialect spread, then inherited "already fail closed" from the previous plan row without checking it. Same error shape as #520: measure one thing, carry an unverified adjective attached to it. Verified both directions rather than assuming either: * `ROCM_Int4PackKernelOp` DOES fail closed. Its generator validates `kind` against an explicit set and errors naming `kind=pack|unpack|relu|sparse_gather|cache_append`. A genuine hoist candidate. * `tessera.neighbors.topology.create` fails OPEN, and it is a Decision #21a violation rather than a layering nit. `CreateTopologyOp::verify()` checks only that `kind` EXISTS. `DynamicTopologyPass::isMutableKind` then dispatches by SUBSTRING — `contains("dynamic") || contains("adaptive") || contains("fault") || contains("custom_graph")`. So a typo (`2d_mseh`) matches nothing and silently becomes a STATIC topology. The substring test is also wrong in the other direction: `not_dynamic` would classify as MUTABLE. `kind` drives `topology.dynamic`, `topology.replan`, and `topology.replan_hook`, so an unrecognized value silently selects different execution semantics — the same unnamed-semantic-default class #505 closed for `predicate` / `optimizer` / `clifford`. So W1.1b is per-site triage first — fail-open sites are correctness fixes owing a negative fixture (Decision #10a), fail-closed sites are ODS hoists — and its remaining size is not yet known. The row no longer calls it a layering improvement, because at least one site is a live fail-open defect. Not fixed here: this PR is scoping, and the Neighbors fix wants its own change with a `CHECK-NOT`-style negative fixture and the substring-vs-equality correction, which is a behavioural change to topology classification. Docs only. 14448 unit passed, ruff clean, 24 generated docs in sync. 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
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.
Not the item the plan describes, because its premise didn't survive measurement.
The plan says "EnumAttr for every semantic StrAttr (4 ×
$kind)". Measured: 17 ops carry$kind, 3 of them areI64Attrrather than strings, and 14 of 17 already fail closed with a named error in their generator.Three did not. Probed by feeding each a bogus kind and inspecting the output rather than the exit code,
tessera_rocm.{predicate,optimizer,clifford}returned 0 and emitted a complete GPU kernel.The defect
All three the same way — a trailing
elsedoing double duty as a real branch and an unnamed semantic default:predicateisfinitecliffordoptimizerDecision #21a: a semantic key fails closed; it may never be silently defaulted.
Deriving the sets is the hard part
Each legal set includes the else branch's own name — exactly what a
kind ==scan of the consumer cannot see. That scan yields{isnan, isinf}and{wedge, left_contraction}, omittingisfinite,geometric_productandadam, all of which producers really emit. A constraint built that way rejects valid programs — the same defect as the earliermodeset that omitted"set".I made that mistake again inside this change
The first optimizer enum omitted
adafactor— and the negative fixture usedadafactoras its example of a plausible-but-unimplemented value. It's neither: the pass implements it andtest_adafactor_full_backward_executes_on_gfx1151runs it on hardware. Six unit tests failed and are what caught it. I'd read a 15-line window of the consumer and taken it for the vocabulary; the pass's full literal set is seven.That's written into the
.tdas the durable lesson: the existing tests encode producer reality better than any scan of the consumer, so run them before trusting a set. A later scare (tests appearing to emitisnormal/inner) turned out to be my own negative fixtures matching a grep missing--include=*.py— checked rather than reacted to; the sets stand.Not in scope
The other 14 already fail closed in their generators. Hoisting their sets into ODS is a layering improvement, not a correctness fix, and is better done per-op with the same derivation discipline than in bulk here.
298 lit, 14425 unit, mypy 0, ruff clean, docs in sync.
🤖 Generated with Claude Code