Conversation
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 25, 2026 12:29
e5ca023 to
e787e54
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 25, 2026 13:38
e787e54 to
826c934
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 25, 2026 22:33
bdeeb63 to
917739d
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 26, 2026 12:14
917739d to
1de7956
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 26, 2026 12:43
1de7956 to
9bb3d1b
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
2 times, most recently
from
July 26, 2026 17:42
78e38ed to
90f577f
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 26, 2026 18:16
90f577f to
3c011d9
Compare
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 26, 2026 18:40
3c011d9 to
aef27fc
Compare
tensor_rebuild_visitor::apply restored the source expression's space() onto any rebuilt result lacking one (the #93 fix for variadic-ctor reconstruction dropping post-construction annotations). Sound for pure rebuilds, wrong when a subclass swapped children: substitute( trans(A)-A, trans(A), C) returned C-A tagged Skew, after which sym() folded it to zero and skew() returned it unchanged - every consumer of the tag (projector guards, trans, inv, diff kernels) trusted it. The restore is now gated on structural equality (m_result == expr, honest since #339), with one deliberate exception: projector contractions (skew(X), sym(X), ...) restore across argument substitution because their space is derived from the projector, not the argument - substituting inside skew(A) keeps Skew (the original deterministic #93 reproducer stays green). Structure-derived tags (trans(X)-X) are re-derived by construction when the substituted form still qualifies. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
The projector-restore exception compared only the projectors; a substitution that changed the argument's rank or dim could restore a rank-2-style tag onto a shape-inconsistent result. Rank and dim must match before any restore. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
The node-level dim guard was inert: inner_product_wrapper reports the projector's dim, so substitute(sym(A_dim3), A, E_dim2) passed the guard, restored the stale Symmetric tag onto a shape-broken node, and evaluation ran the projector at dim 3 over a dim-2 buffer (ASan-confirmed heap-buffer-overflow read). The guard now compares the actual contraction arguments' rank and dim. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
…nd shape Round 3 proved the round-2 commit closed only the tag restoration: the heap overflow it cited lived in eval_projector_unary, which drives the unary wrapper with the PROJECTOR's dim and never compares the operand's - substitute(sym(A_dim3), A, E_dim2) still over-read the dim-2 buffer at evaluation (UBSan vptr abort). The short-circuit now throws evaluation_error on operand dim/rank mismatch, and the regression test evaluates the reproducer instead of only checking the tag. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
petlenz
force-pushed
the
fix-352-substitution-space
branch
from
July 26, 2026 19:03
aef27fc to
a703520
Compare
petlenz
added this pull request to stack #425
September 15, 2026 20:15
Member
Author
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.
Summary
Fixes #352. Stacked on #404.
substitute(trans(A)-A, trans(A), C)→C-Atagged Skew;sym→0{2},skew→ unchangedsym/skewbehave correctlysubstitute(skew(A), A, B)keeps Skew (#93 reproducer)substitute(trans(A)-A, A, B)→trans(B)-BDesign
The #93 restore in
tensor_rebuild_visitor::applyis now gated on structural equality (m_result == expr— honest since #339), with one deliberate exception: projector contractions restore across argument substitution, becauseskew(X)/sym(X)'s space is derived from the projector, not the argument. That exception is exactly what keeps the original #93 deterministic reproducer green — a plain equality gate broke it, which is how the exception was found.Note: the equality check runs per rebuilt subtree (recursive
apply), so worst-case rebuild cost gains an==per node; the hash fast-reject makes the changed-subtree case cheap. Flagged for the perf epic's benchmark once #382 lands.Tests
2 lock-ins: stale-tag drop (the #352 repro) + operator-derived survival and structural re-derivation. Existing
SkewSpacePreservedThroughRebuild(#93) passes unchanged. Full suite: 2442/2442 pass. gcc-14 clean.