From f28f5343be8f0df0caa23cc8f09cb732e37b66d8 Mon Sep 17 00:00:00 2001 From: petlenz Date: Thu, 17 Sep 2026 09:35:51 +0200 Subject: [PATCH 1/4] Fix #443: tensor symbol identity includes dim and rank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tensor symbol compared equal to any symbol of the same name, so A(3,2) and A(2,2) shared one key: a map held one entry for three shapes, and an evaluator holding both bindings served whichever it stored first. Equality and ordering now compare the shape after the name, via a requires-guarded helper so scalar symbols (which carry no shape) are unaffected. The hash is deliberately unchanged — it stays a name-only fast reject, so hash-driven print order and the tensor_scalar_mul / tensor_pow hash invariants are untouched. Signed-off-by: petlenz --- include/numsim_cas/core/symbol_base.h | 28 ++++++++++++- tests/CoreBugFixTest.h | 60 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/include/numsim_cas/core/symbol_base.h b/include/numsim_cas/core/symbol_base.h index 1223e139..720e2ae6 100644 --- a/include/numsim_cas/core/symbol_base.h +++ b/include/numsim_cas/core/symbol_base.h @@ -3,6 +3,8 @@ #include +#include + namespace numsim::cas { template class symbol_base : public BaseExpr { @@ -53,13 +55,34 @@ template class symbol_base : public BaseExpr { std::string m_name; }; +namespace detail { + +// Shape belongs to a tensor symbol's identity: the same name at another dim +// or rank denotes a different tensor. Scalar symbols carry no shape. +template +[[nodiscard]] inline std::pair +symbol_shape(Symbol const &symbol) { + if constexpr (requires { + symbol.dim(); + symbol.rank(); + }) { + return {symbol.dim(), symbol.rank()}; + } else { + return {0, 0}; + } +} + +} // namespace detail + template bool operator<(symbol_base const &lhs, symbol_base const &rhs) { if (lhs.hash_value() != rhs.hash_value()) return lhs.hash_value() < rhs.hash_value(); // The hash covers only the name, so colliding names need a real tiebreak. - return lhs.name() < rhs.name(); + if (lhs.name() != rhs.name()) + return lhs.name() < rhs.name(); + return detail::symbol_shape(lhs) < detail::symbol_shape(rhs); } template @@ -71,7 +94,8 @@ bool operator>(symbol_base const &lhs, template bool operator==(symbol_base const &lhs, symbol_base const &rhs) { - return lhs.hash_value() == rhs.hash_value() && lhs.name() == rhs.name(); + return lhs.hash_value() == rhs.hash_value() && lhs.name() == rhs.name() && + detail::symbol_shape(lhs) == detail::symbol_shape(rhs); } template diff --git a/tests/CoreBugFixTest.h b/tests/CoreBugFixTest.h index a8cfede6..3b800858 100644 --- a/tests/CoreBugFixTest.h +++ b/tests/CoreBugFixTest.h @@ -1034,6 +1034,66 @@ TEST(SymbolIdentity, EvaluatorKeepsBothDomainBindings) { 3.0); } +// A tensor symbol's shape is part of its identity: the same name at a +// different dim or rank denotes a different tensor. +TEST(SymbolIdentity, TensorShapeDistinguishesSameName) { + auto A32 = make_expression("A", 3, 2); + auto A22 = make_expression("A", 2, 2); + auto A34 = make_expression("A", 3, 4); + auto B32 = make_expression("B", 3, 2); + auto A32b = make_expression("A", 3, 2); + + EXPECT_FALSE(*A32 == *A22) << "differing dim"; + EXPECT_FALSE(*A32 == *A34) << "differing rank"; + EXPECT_TRUE(*A32 == *A32b) << "same name and shape"; + EXPECT_FALSE(*A32 == *B32) << "differing name"; + + // Distinct shapes need a total order, equal ones must stay incomparable. + EXPECT_TRUE((*A32 < *A22) != (*A22 < *A32)); + EXPECT_TRUE((*A32 < *A34) != (*A34 < *A32)); + EXPECT_FALSE(*A32 < *A32b); + EXPECT_FALSE(*A32b < *A32); + + std::map, int> keys; + keys[A32] = 1; + keys[A22] = 2; + keys[A34] = 3; + EXPECT_EQ(keys.size(), 3u); + EXPECT_EQ(keys.at(A32), 1); + EXPECT_EQ(keys.at(A22), 2); + EXPECT_EQ(keys.at(A34), 3); +} + +// Each shape carries its own binding; the evaluator must not serve one +// symbol's data for another. +TEST(SymbolIdentity, EvaluatorKeepsBindingsPerTensorShape) { + auto A32 = make_expression("A", 3, 2); + auto A22 = make_expression("A", 2, 2); + + auto d32 = std::make_shared>(); + d32->data() = tmech::eye(); + auto d22 = std::make_shared>(); + d22->data() = 7.0 * tmech::eye(); + + tensor_evaluator ev; + ev.set(A32, d32); + ev.set(A22, d22); + + auto r32 = ev.apply(A32); + ASSERT_NE(r32, nullptr); + EXPECT_EQ(r32->dim(), 3u); + EXPECT_DOUBLE_EQ(r32->raw_data()[0], 1.0); + + auto r22 = ev.apply(A22); + ASSERT_NE(r22, nullptr); + EXPECT_EQ(r22->dim(), 2u); + EXPECT_DOUBLE_EQ(r22->raw_data()[0], 7.0); + + tensor_to_scalar_evaluator t2s; + t2s.set(A22, d22); + EXPECT_DOUBLE_EQ(t2s.apply(trace(A22)), 14.0); +} + // #93 — a tensor_mul's space() must survive copy reconstruction // (tensor_add did this; mul dropped it). TEST(CoreBugFix, TensorMulCopyPreservesSpaceAnnotation) { From d4c7e87a07e3318f786c9bdd8d45c06cd35f5e70 Mon Sep 17 00:00:00 2001 From: petlenz Date: Thu, 17 Sep 2026 18:20:59 +0200 Subject: [PATCH 2/4] Pin that mixed-rank factors of one name do not merge A(3,2) * A(3,4) collapsed to pow(A,2) while symbol identity ignored shape. Tensor multiplication validates dim only, so the like-term path is reachable and deserves its own lock-in. Signed-off-by: petlenz --- tests/CoreBugFixTest.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/CoreBugFixTest.h b/tests/CoreBugFixTest.h index 3b800858..705066b7 100644 --- a/tests/CoreBugFixTest.h +++ b/tests/CoreBugFixTest.h @@ -1036,6 +1036,16 @@ TEST(SymbolIdentity, EvaluatorKeepsBothDomainBindings) { // A tensor symbol's shape is part of its identity: the same name at a // different dim or rank denotes a different tensor. +// Mixed-rank factors sharing a name must not merge into a power: tensor +// multiplication validates dim only, so this reaches the like-term path. +TEST(SymbolIdentity, MixedRankFactorsDoNotMerge) { + auto A32 = make_expression("A", 3, 2); + auto A34 = make_expression("A", 3, 4); + auto product = A32 * A34; + EXPECT_FALSE(is_same(product)) << to_string(product); + EXPECT_EQ(to_string(product), to_string(A32 * A34)); +} + TEST(SymbolIdentity, TensorShapeDistinguishesSameName) { auto A32 = make_expression("A", 3, 2); auto A22 = make_expression("A", 2, 2); From 53e5ed3db4294ad62617d8fe7d76f1523398b013 Mon Sep 17 00:00:00 2001 From: petlenz Date: Sun, 20 Sep 2026 03:02:06 +0200 Subject: [PATCH 3/4] Compare shape where the shape lives symbol_base had to detect dim() and rank() to compare them, so the base of every symbol knew what only a tensor carries. tensor now defines its own comparison operators; equals_same_type casts to the derived type first, so they win by exact match and every path reaches them. Signed-off-by: petlenz --- include/numsim_cas/core/symbol_base.h | 30 ++++----------------------- include/numsim_cas/tensor/tensor.h | 29 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/include/numsim_cas/core/symbol_base.h b/include/numsim_cas/core/symbol_base.h index 720e2ae6..83a28feb 100644 --- a/include/numsim_cas/core/symbol_base.h +++ b/include/numsim_cas/core/symbol_base.h @@ -3,8 +3,6 @@ #include -#include - namespace numsim::cas { template class symbol_base : public BaseExpr { @@ -55,34 +53,15 @@ template class symbol_base : public BaseExpr { std::string m_name; }; -namespace detail { - -// Shape belongs to a tensor symbol's identity: the same name at another dim -// or rank denotes a different tensor. Scalar symbols carry no shape. -template -[[nodiscard]] inline std::pair -symbol_shape(Symbol const &symbol) { - if constexpr (requires { - symbol.dim(); - symbol.rank(); - }) { - return {symbol.dim(), symbol.rank()}; - } else { - return {0, 0}; - } -} - -} // namespace detail - template bool operator<(symbol_base const &lhs, symbol_base const &rhs) { if (lhs.hash_value() != rhs.hash_value()) return lhs.hash_value() < rhs.hash_value(); // The hash covers only the name, so colliding names need a real tiebreak. - if (lhs.name() != rhs.name()) - return lhs.name() < rhs.name(); - return detail::symbol_shape(lhs) < detail::symbol_shape(rhs); + // A derived symbol carrying more identity state (a tensor's shape) defines + // its own comparison; equals_same_type casts to it before comparing. + return lhs.name() < rhs.name(); } template @@ -94,8 +73,7 @@ bool operator>(symbol_base const &lhs, template bool operator==(symbol_base const &lhs, symbol_base const &rhs) { - return lhs.hash_value() == rhs.hash_value() && lhs.name() == rhs.name() && - detail::symbol_shape(lhs) == detail::symbol_shape(rhs); + return lhs.hash_value() == rhs.hash_value() && lhs.name() == rhs.name(); } template diff --git a/include/numsim_cas/tensor/tensor.h b/include/numsim_cas/tensor/tensor.h index 5d77b038..809ebade 100644 --- a/include/numsim_cas/tensor/tensor.h +++ b/include/numsim_cas/tensor/tensor.h @@ -4,6 +4,7 @@ #include #include #include +#include namespace numsim::cas { @@ -30,6 +31,34 @@ class tensor final : public symbol_base> { // re-override here; symbol_base is the single source of truth for the // Symbol classification. + // Shape is part of a tensor symbol's identity: the same name at another + // dim or rank denotes a different tensor. These beat the symbol_base + // templates by exact match, and equals_same_type casts to tensor before + // comparing, so every path reaches them. The hash stays name-only — it + // is a fast reject, and widening it would churn hash-driven print order. + friend bool operator==(tensor const &lhs, tensor const &rhs) { + return static_cast(lhs) == static_cast(rhs) && + lhs.dim() == rhs.dim() && lhs.rank() == rhs.rank(); + } + + friend bool operator!=(tensor const &lhs, tensor const &rhs) { + return !(lhs == rhs); + } + + friend bool operator<(tensor const &lhs, tensor const &rhs) { + auto const &l = static_cast(lhs); + auto const &r = static_cast(rhs); + if (l < r) + return true; + if (r < l) + return false; + return std::pair{lhs.dim(), lhs.rank()} < std::pair{rhs.dim(), rhs.rank()}; + } + + friend bool operator>(tensor const &lhs, tensor const &rhs) { + return rhs < lhs; + } + // const tensor &operator=(expression_holder &&data) { // this->m_expr = std::move(data); // return *this; From 57878c6a376f230b17b12af5ce13fb76f18d2a64 Mon Sep 17 00:00:00 2001 From: petlenz Date: Sun, 20 Sep 2026 21:18:58 +0200 Subject: [PATCH 4/4] Shorten the identity comments Signed-off-by: petlenz --- include/numsim_cas/core/symbol_base.h | 3 +-- include/numsim_cas/tensor/tensor.h | 7 ++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/include/numsim_cas/core/symbol_base.h b/include/numsim_cas/core/symbol_base.h index 83a28feb..2d11e4d2 100644 --- a/include/numsim_cas/core/symbol_base.h +++ b/include/numsim_cas/core/symbol_base.h @@ -59,8 +59,7 @@ bool operator<(symbol_base const &lhs, if (lhs.hash_value() != rhs.hash_value()) return lhs.hash_value() < rhs.hash_value(); // The hash covers only the name, so colliding names need a real tiebreak. - // A derived symbol carrying more identity state (a tensor's shape) defines - // its own comparison; equals_same_type casts to it before comparing. + // A symbol with more identity state defines its own comparison. return lhs.name() < rhs.name(); } diff --git a/include/numsim_cas/tensor/tensor.h b/include/numsim_cas/tensor/tensor.h index 809ebade..faf0edf4 100644 --- a/include/numsim_cas/tensor/tensor.h +++ b/include/numsim_cas/tensor/tensor.h @@ -31,11 +31,8 @@ class tensor final : public symbol_base> { // re-override here; symbol_base is the single source of truth for the // Symbol classification. - // Shape is part of a tensor symbol's identity: the same name at another - // dim or rank denotes a different tensor. These beat the symbol_base - // templates by exact match, and equals_same_type casts to tensor before - // comparing, so every path reaches them. The hash stays name-only — it - // is a fast reject, and widening it would churn hash-driven print order. + // Shape is part of the identity: the same name at another dim or rank is + // a different tensor. The hash stays name-only, as a fast reject. friend bool operator==(tensor const &lhs, tensor const &rhs) { return static_cast(lhs) == static_cast(rhs) && lhs.dim() == rhs.dim() && lhs.rank() == rhs.rank();