You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Commit 7e962e5 on the remove-flaky-skip-list branch (now merged via PR #90) added structural skew detection to the fuzz test with this rationale:
The Skew space on skew(G) is not preserved on all platforms when stored inside tensor_mul. Detect skew structurally by checking if the expression is inner_product(P_skew, ...) where the LHS is a tensor_projector with Skew permutation.
PR #90 moved the structural classifier into the library (include/numsim_cas/tensor/skew_classification.h) and now uses it in inv()'s rejection. The structural path is the authoritative check; the space-annotation fast path is a hint.
But the root cause of the platform-dependent annotation loss was never identified, only worked around. As long as the annotation is unreliable, every other consumer that reads data()->space() is suspect — not just inv().
tensor_mul does not carry a Skew annotation derived from its children — it isn't generally true that a * of skew is skew, so that's correct.
The platform dependence kicks in when a skew-annotated child is stored inside a tensor_mul. Per the commit message, the annotation on that child is sometimes not preserved on retrieval on some platforms.
Hypotheses worth investigating
symbol_map iteration order across stdlib implementations. If any rewrite path reconstructs an annotated expression by iterating its children, libc++ vs libstdc++ could surface a different "first" child whose annotation gets preserved while others get dropped.
Annotation lost during a rebuild path (tensor_rebuild_visitor or similar). If the rebuild copies structure but doesn't propagate space(), then anything that goes through it loses the annotation.
make_expression<tensor_mul> constructor doesn't read children's space(), and a child whose annotation was set via set_space()after its hash was computed could lose it across some intermediate operation (related to the post-construction mutation we documented in PR Remove flaky tensor diff seed skip list by fixing underlying bugs #90).
Specific simplifier paths in tensor_simplifier_mul.cpp / tensor_simplifier_add.cpp that recurse via accept and rebuild — they might not be preserving the annotation on the result.
Acceptance criteria
Reproduce the platform dependence in a deterministic test. Likely needs a custom matrix to isolate the iteration-order angle — e.g., build the same expression with the same calls on libstdc++ and libc++ and diff the space() annotations through each rewrite step.
Identify the specific code path that drops the annotation.
Fix at the source so data()->space() is a reliable contract again. Structural detection in skew_classification.h then becomes belt-and-braces rather than the only reliable check.
Re-evaluate whether is_provably_skew still needs the structural fallback once the fast path is reliable.
References
Commit 7e962e5 ("Detect skew() via projector node instead of space annotation").
include/numsim_cas/tensor/skew_classification.h — the current structural classifier.
include/numsim_cas/tensor/tensor_operators.h — sub and add branches annotate trans/neg patterns at construction with a comment about the post-construction set_space dependence.
include/numsim_cas/tensor/visitors/tensor_rebuild_visitor.h — candidate suspect for annotation loss.
The Skew-on-tensor_mul case in this issue is the known annotation-loss bug. The broader concern: every consumer that reads data()->space() is currently betting that the annotation survives the path the expression took to reach them. Today that bet is unreliable enough that PR #90 had to add structural classification (is_provably_skew, contains_skew_factor) as the authoritative check.
Once the root cause of the Skew/tensor_mul loss is identified, the natural follow-up is an end-to-end audit of space propagation across all spaces (Skew, Symmetric, Volumetric, Deviatoric, Harmonic) and all composition paths (tensor_add, tensor_mul, tensor_scalar_mul, tensor_negative, basis_change_imp, inner_product_wrapper, tensor_inv, tensor_projector, pow, tensor_rebuild_visitor).
Extra acceptance criteria for that broader audit:
Build a property test: for each space S and each composition path P, an expression built via P from an S-annotated child should be re-classified by both space() and the structural classifier; the two must agree.
Document the contract on tensor_node_base::space() — what propagation properties callers can rely on, and which composition paths are "join" (intersect the input spaces), "preserve" (carry through), or "lose" (no annotation on result).
Remove the structural fallbacks where the annotation is reliable; keep them where the algebra means the annotation legitimately can't be derived.
Summary
Commit
7e962e5on theremove-flaky-skip-listbranch (now merged via PR #90) added structural skew detection to the fuzz test with this rationale:PR #90 moved the structural classifier into the library (
include/numsim_cas/tensor/skew_classification.h) and now uses it ininv()'s rejection. The structural path is the authoritative check; the space-annotation fast path is a hint.But the root cause of the platform-dependent annotation loss was never identified, only worked around. As long as the annotation is unreliable, every other consumer that reads
data()->space()is suspect — not justinv().What we know
tensor_scalar_mulandtensor_negativeconstructors propagate thespace()from their inner expression (verified during the PR Remove flaky tensor diff seed skip list by fixing underlying bugs #90 review).tensor_add::suboperator annotatestrans(A) − A/A − trans(A)andtrans(A) + (−A)/(−A) + trans(A)as Skew at construction (added in PR Remove flaky tensor diff seed skip list by fixing underlying bugs #90).tensor_muldoes not carry a Skew annotation derived from its children — it isn't generally true that a*of skew is skew, so that's correct.tensor_mul. Per the commit message, the annotation on that child is sometimes not preserved on retrieval on some platforms.Hypotheses worth investigating
symbol_mapiteration order across stdlib implementations. If any rewrite path reconstructs an annotated expression by iterating its children, libc++ vs libstdc++ could surface a different "first" child whose annotation gets preserved while others get dropped.tensor_rebuild_visitoror similar). If the rebuild copies structure but doesn't propagatespace(), then anything that goes through it loses the annotation.make_expression<tensor_mul>constructor doesn't read children'sspace(), and a child whose annotation was set viaset_space()after its hash was computed could lose it across some intermediate operation (related to the post-construction mutation we documented in PR Remove flaky tensor diff seed skip list by fixing underlying bugs #90).tensor_simplifier_mul.cpp/tensor_simplifier_add.cppthat recurse viaacceptand rebuild — they might not be preserving the annotation on the result.Acceptance criteria
space()annotations through each rewrite step.data()->space()is a reliable contract again. Structural detection inskew_classification.hthen becomes belt-and-braces rather than the only reliable check.is_provably_skewstill needs the structural fallback once the fast path is reliable.References
7e962e5("Detect skew() via projector node instead of space annotation").include/numsim_cas/tensor/skew_classification.h— the current structural classifier.include/numsim_cas/tensor/tensor_operators.h—subandaddbranches annotate trans/neg patterns at construction with a comment about the post-constructionset_spacedependence.include/numsim_cas/tensor/visitors/tensor_rebuild_visitor.h— candidate suspect for annotation loss.Broader scope (added post-PR #90)
The Skew-on-
tensor_mulcase in this issue is the known annotation-loss bug. The broader concern: every consumer that readsdata()->space()is currently betting that the annotation survives the path the expression took to reach them. Today that bet is unreliable enough that PR #90 had to add structural classification (is_provably_skew,contains_skew_factor) as the authoritative check.Once the root cause of the Skew/
tensor_mulloss is identified, the natural follow-up is an end-to-end audit of space propagation across all spaces (Skew, Symmetric, Volumetric, Deviatoric, Harmonic) and all composition paths (tensor_add,tensor_mul,tensor_scalar_mul,tensor_negative,basis_change_imp,inner_product_wrapper,tensor_inv,tensor_projector,pow,tensor_rebuild_visitor).Extra acceptance criteria for that broader audit:
space()and the structural classifier; the two must agree.tensor_node_base::space()— what propagation properties callers can rely on, and which composition paths are "join" (intersect the input spaces), "preserve" (carry through), or "lose" (no annotation on result).