fix(autodiff): name the offending node when lowering fails closed - #135
Open
AlekseiChirkovVention wants to merge 2 commits into
Open
fix(autodiff): name the offending node when lowering fails closed#135AlekseiChirkovVention wants to merge 2 commits into
AlekseiChirkovVention wants to merge 2 commits into
Conversation
Pin the two properties the expansion exists to deliver. A backend missing a handler an expanded artifact requires is rejected before any handler runs. The error alone does not establish that, so each case also asserts the invocation recorder is empty, and first asserts the registry does hold other handlers -- an empty recorder proves nothing if nothing could have been recorded. Equivalence is asserted against a control registry that keeps its native reduction, over both tiers, both differentiable dtypes, and square and non-square shapes: dtype and shape exactly equal, values within the documented tolerance. The control that the unexpanded artifact fails against the limited registry is what makes those comparisons meaningful rather than incidental. A scalar and a one-by-one value are asserted never interchangeable, each tier checked against the other's expectation inside a raises block, so the distinction is executed rather than described. One assertion pins the registry lookup message byte-for-byte: only the pre-flight check may name the offending node, and every other caller must keep seeing what it sees today.
A backend missing a handler was told which operator type it could not lower, but not which node carried it. In a graph with several nodes of that type the message left the reader to guess, and an expanded artifact introduces generated nodes the author never wrote by hand. The registry can only name the type, since that is all a lookup is given. The pre-flight check is the one place the node is in scope, so it re-raises the same failure with the node id appended. The category is carried through unchanged and the registry's own message is untouched, so every other caller observes exactly what it did before.
code-tc
approved these changes
Aug 28, 2026
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
3 participants
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.
Prove the two properties the expansion exists to deliver, and name the offending node when
lowering fails closed.
Goal
A backend that does not understand a construct an expanded artifact introduces must be
rejected before any handler runs, and an expanded artifact must compute what the
unexpanded one computes. Neither property is worth much asserted loosely, so this branch
establishes both under conditions that would expose a vacuous proof.
The production change
_require_every_operation_supportedre-raises its pre-flight failure with the offending nodeid appended. The registry can only name the operator type — that is all a lookup is given —
and in a graph with several nodes of that type the message left the reader to guess. An
expanded artifact makes that worse, because it introduces generated nodes the author never
wrote by hand.
The registry's own message is untouched, so every other caller observes exactly what it did
before; one test pins that message byte-for-byte. No signature, argument, return type, or
error category changed.
Why the fail-closed proofs are not vacuous
The evidence that no handler ran is an empty invocation recorder. That is only evidence if
the recorder could have recorded, so each case additionally asserts the registry does hold
handlers for the operator types that remain — a registry wrapping nothing would leave an empty
record no matter what the code did. The recorder captures at handler call time, not at lookup
time, which is what makes an empty record mean "nothing executed" rather than "nothing was
looked up".
Why the equivalence comparison means something
The expanded artifact is lowered by a registry with no reduction, broadcast, or division
handler; the unexpanded one by a control registry that keeps its native reduction and reaches
numpy directly. The two sides share no code path, so a defect in the expansion cannot be
mirrored by the control and cancel out.
The control case is what makes the comparison worth making: the unexpanded artifact fails
against the limited registry with
unsupported_operator. Without it, "the expanded artifactlowers successfully" would be unremarkable rather than the point.
dtype and shape are compared exactly, rank included; only values are compared within
tolerance —
1e-6forf32,1e-12forf64— because substituting a reciprocal multiplyfor a divide is not required to agree bit for bit. A scalar result and a one-by-one result are
asserted never interchangeable, each tier checked against the other's expectation inside a
raises block, so the distinction is executed rather than described.
Test evidence
Level: unit and integration. Tests first: the failing set landed in
7353495;c5899cdmade it pass. The test file is byte-identical between the two.44 cases: 24 equivalence cases spanning both tiers, both differentiable dtypes, and square and
non-square shapes, for the forward result and the gradient with respect to the traced
parameter; the fail-closed and control cases; and the dependency-analysis compatibility checks.
Only three cases were failing at the start — the node-naming half — which is correct for a
branch whose sole production change is one error message. Review reproduced that by reverting
the pre-flight and re-running: exactly 3 failed, 41 passed, each for the right reason.
Pre-existing lowering tests pass unmodified. Focused autodiff suite: 885 → 888. Broad
non-integration suite: 1139 passed, 9 failed, 1 skipped — the 9 pre-existing and unrelated,
unchanged in count across the whole series.
Note for backend authors
The rank-reducing tier requires a trivial reshape handler in both the forward artifact and
the derivative program: the leading seed reshape survives expansion untouched. The
rank-preserving tier requires none. This is the easiest thing to get wrong when adopting an
expanded artifact.
Stack position
Stack position 6 of 8 — based on
feat/mean-expansion-provenance.Cannot merge into
mainbeforefeat/generated-constant-operator,chore/generic-lowering-reference-consumer,feat/matmul-based-mean-expansion,feat/broadcast-scale-gradient-expansion, andfeat/mean-expansion-provenance.