Two latent instances of the same root cause as #451: operator== bottoming out in a hash that does not encode the node's full structure. Neither produces a wrong value today; both are landmines for work already planned.
1. tensor_zero identity ignores shape. include/numsim_cas/tensor/tensor_zero.h:37-40 hashes the node id alone, and :29-31 compares hashes, so every zero of every shape is one key:
0{3,2} == 0{2,4} -> true
set{0{3,2}, 0{2,4}, 0{3,4}} -> size 1
map lookup with one shape -> returns another shape's value
No end-user wrong value today: #438 now rejects mixed-shape add/sub, and zeros fold away before reaching a container. It goes live the moment a zero becomes a map/set key. Note for #441: the proposed memoization must key on the node pointer, not on expression_holder, or it will collide here. Same family as #443 (tensor symbols ignoring dim/rank).
2. Dead projector simplifier groups by argument hash alone. src/numsim_cas/tensor/simplifier/tensor_projector_simplifier.cpp:112 uses std::map<std::size_t, std::vector<ProjEntry>> keyed by the argument's hash. By the deliberate design rules (2A).hash == A.hash and pow(A,2).hash == A.hash (both confirmed), so sym(A) and dev(2A) land in one group and a combining rule could fire across different arguments. The file is wired nowhere, so there is no live impact — but whoever wires it inherits the bug. Either fix the keying (pointer or deep comparison) when wiring, or delete the file.
The invariant worth enforcing repo-wide: no operator== may reduce to hash equality unless the hash provably encodes the node's complete structure. That holds for identity_tensor, levi_civita_tensor and tensor_projector; it did not hold for the t2s wrapper (#451) and does not hold for tensor_zero.
Bearing on #379 (canonical form + hash-consing): hash-consing makes pointer identity equal structural identity only if the interning key is the full structure. Interning on today's lossy hashes would merge 2*x·tr(A) with 3*x·tr(A) permanently — turning #451 from a comparison bug into a construction bug — and the deliberate aliasing rules (tensor_scalar_mul/tensor_pow hashing to the inner tensor) are directly incompatible with it. Fixing the lossy-hash/identity conflation is a prerequisite for #379, not a consequence of it.
Signed-off-by: petlenz [email protected]
Two latent instances of the same root cause as #451:
operator==bottoming out in a hash that does not encode the node's full structure. Neither produces a wrong value today; both are landmines for work already planned.1.
tensor_zeroidentity ignores shape.include/numsim_cas/tensor/tensor_zero.h:37-40hashes the node id alone, and:29-31compares hashes, so every zero of every shape is one key:No end-user wrong value today: #438 now rejects mixed-shape add/sub, and zeros fold away before reaching a container. It goes live the moment a zero becomes a map/set key. Note for #441: the proposed memoization must key on the node pointer, not on
expression_holder, or it will collide here. Same family as #443 (tensor symbols ignoring dim/rank).2. Dead projector simplifier groups by argument hash alone.
src/numsim_cas/tensor/simplifier/tensor_projector_simplifier.cpp:112usesstd::map<std::size_t, std::vector<ProjEntry>>keyed by the argument's hash. By the deliberate design rules(2A).hash == A.hashandpow(A,2).hash == A.hash(both confirmed), sosym(A)anddev(2A)land in one group and a combining rule could fire across different arguments. The file is wired nowhere, so there is no live impact — but whoever wires it inherits the bug. Either fix the keying (pointer or deep comparison) when wiring, or delete the file.The invariant worth enforcing repo-wide: no
operator==may reduce to hash equality unless the hash provably encodes the node's complete structure. That holds foridentity_tensor,levi_civita_tensorandtensor_projector; it did not hold for the t2s wrapper (#451) and does not hold fortensor_zero.Bearing on #379 (canonical form + hash-consing): hash-consing makes pointer identity equal structural identity only if the interning key is the full structure. Interning on today's lossy hashes would merge
2*x·tr(A)with3*x·tr(A)permanently — turning #451 from a comparison bug into a construction bug — and the deliberate aliasing rules (tensor_scalar_mul/tensor_powhashing to the inner tensor) are directly incompatible with it. Fixing the lossy-hash/identity conflation is a prerequisite for #379, not a consequence of it.Signed-off-by: petlenz [email protected]