diff --git a/include/numsim_cas/core/assumptions.h b/include/numsim_cas/core/assumptions.h index 6cc0973a..a48f7204 100644 --- a/include/numsim_cas/core/assumptions.h +++ b/include/numsim_cas/core/assumptions.h @@ -199,18 +199,17 @@ template struct relation { RHS rhs; kind k; - // order by (kind_index, lhs_hash, rhs_hash) friend bool operator<(relation const &a, relation const &b) { auto ai = a.k.index(), bi = b.k.index(); if (ai != bi) return ai < bi; - auto al = a.lhs.get().hash_value(); - auto bl = b.lhs.get().hash_value(); - if (al != bl) - return al < bl; - auto ar = a.rhs.get().hash_value(); - auto br = b.rhs.get().hash_value(); - return ar < br; + // Compare the operands themselves: hash-only ordering treats colliding + // relations as duplicates, which drops them from relation::set. + if (a.lhs.get() != b.lhs.get()) + return a.lhs.get() < b.lhs.get(); + if (a.rhs.get() != b.rhs.get()) + return a.rhs.get() < b.rhs.get(); + return false; } }; diff --git a/include/numsim_cas/core/symbol_base.h b/include/numsim_cas/core/symbol_base.h index 52143adb..e0d3aa2b 100644 --- a/include/numsim_cas/core/symbol_base.h +++ b/include/numsim_cas/core/symbol_base.h @@ -55,7 +55,10 @@ template class symbol_base : public BaseExpr { template bool operator<(symbol_base const &lhs, symbol_base const &rhs) { - return lhs.hash_value() < rhs.hash_value(); + 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(); } template @@ -67,7 +70,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(); + return lhs.hash_value() == rhs.hash_value() && lhs.name() == rhs.name(); } template diff --git a/include/numsim_cas/tensor/operators/scalar/tensor_scalar_mul.h b/include/numsim_cas/tensor/operators/scalar/tensor_scalar_mul.h index 7ff6724b..3af8c995 100644 --- a/include/numsim_cas/tensor/operators/scalar/tensor_scalar_mul.h +++ b/include/numsim_cas/tensor/operators/scalar/tensor_scalar_mul.h @@ -1,6 +1,7 @@ #ifndef TENSOR_SCALAR_MUL_H #define TENSOR_SCALAR_MUL_H +#include #include #include #include @@ -45,6 +46,7 @@ class tensor_scalar_mul final if (this->hash_value() != rhs.hash_value()) return false; if (rhs.id() == this->id()) { + assert(dynamic_cast(&rhs) != nullptr); auto const &r = static_cast(rhs); return this->expr_rhs() == r.expr_rhs(); } diff --git a/src/numsim_cas/core/expression.cpp b/src/numsim_cas/core/expression.cpp index 417f40d0..176440fb 100644 --- a/src/numsim_cas/core/expression.cpp +++ b/src/numsim_cas/core/expression.cpp @@ -1,5 +1,7 @@ #include +#include + namespace numsim::cas { expression::hash_type const &expression::hash_value() const { @@ -21,6 +23,11 @@ bool expression::operator==(expression const &rhs) const noexcept { if (hash_value() != rhs.hash_value()) return false; + // id() indexes the node's own domain type list, so nodes from different + // domains share ids; the downcast in equals_same_type needs the exact type. + if (typeid(*this) != typeid(rhs)) + return false; + // same type => do the real compare return equals_same_type(rhs); } @@ -34,6 +41,10 @@ bool expression::operator<(expression const &rhs) const noexcept { return hash_value() < rhs.hash_value(); if (id() != rhs.id()) return id() < rhs.id(); + // Cross-domain ties need an order before the same-type downcast. before() + // is arbitrary but consistent within a run, which is all a key needs. + if (typeid(*this) != typeid(rhs)) + return typeid(*this).before(typeid(rhs)); return less_than_same_type(rhs); } diff --git a/tests/CoreBugFixTest.h b/tests/CoreBugFixTest.h index 48b30406..da308f7d 100644 --- a/tests/CoreBugFixTest.h +++ b/tests/CoreBugFixTest.h @@ -949,6 +949,49 @@ TEST(TensorConstHashInvariant, EXPECT_EQ(m.get().hash_value(), A.get().hash_value()); } +// Node ids are per-domain indices and a symbol's hash covers only its name, +// so a scalar and a tensor named alike tie on both and used to reach the +// same-type downcast. +TEST(SymbolIdentity, SameNameAcrossDomainsIsDistinct) { + auto [xs] = make_scalar_variable("x"); + auto [xt] = + make_tensor_variable(std::tuple{"x", std::size_t{3}, std::size_t{2}}); + expression const &a = xs.get(); + expression const &b = xt.get(); + + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); + EXPECT_TRUE((a < b) != (b < a)) << "cross-domain order must be total"; +} + +// evaluator_base keys one map by expression_holder, so symbols +// from different domains share it: binding both must keep both values. +TEST(SymbolIdentity, EvaluatorKeepsBothDomainBindings) { + auto [xs] = make_scalar_variable("x"); + auto [xt] = + make_tensor_variable(std::tuple{"x", std::size_t{3}, std::size_t{2}}); + + scalar_evaluator ev; + ev.set(xs, 2.0); + ev.set(xt, 3.0); + + EXPECT_DOUBLE_EQ(ev.apply(xs), 2.0); + + // Same keying as evaluator_base, so both entries must coexist. + std::map, double> keys; + keys[expression_holder( + std::static_pointer_cast(xs.data()))] = 2.0; + keys[expression_holder( + std::static_pointer_cast(xt.data()))] = 3.0; + EXPECT_EQ(keys.size(), 2u); + EXPECT_DOUBLE_EQ(keys.at(expression_holder( + std::static_pointer_cast(xs.data()))), + 2.0); + EXPECT_DOUBLE_EQ(keys.at(expression_holder( + std::static_pointer_cast(xt.data()))), + 3.0); +} + // #93 — a tensor_mul's space() must survive copy reconstruction // (tensor_add did this; mul dropped it). TEST(CoreBugFix, TensorMulCopyPreservesSpaceAnnotation) { diff --git a/tests/ScalarAssumptionTest.h b/tests/ScalarAssumptionTest.h index df0872c3..b5d0db27 100644 --- a/tests/ScalarAssumptionTest.h +++ b/tests/ScalarAssumptionTest.h @@ -911,4 +911,20 @@ TEST_F(AssumptionFixture, Step6_ScalarAssumeUniformGuardSampling) { numsim::cas::invalid_assumption_error); } +// relation::set orders by its operands; relations differing only in an +// operand must stay distinct members. +TEST_F(AssumptionFixture, RelationSetKeepsDistinctOperands) { + using rel_t = numsim::cas::relation::relation; + numsim::cas::relation::set + set; + set.insert(rel_t{x, y, numsim::cas::relation::equal{}}); + set.insert(rel_t{x, z, numsim::cas::relation::equal{}}); + set.insert(rel_t{x, y, numsim::cas::relation::less{}}); + set.insert(rel_t{x, y, numsim::cas::relation::equal{}}); + + EXPECT_EQ(set.size(), 3u); +} + #endif // SCALARASSUMPTIONTEST_H