Fix #348: discriminate symbols across domains in expression identity

include/numsim_cas/core/assumptions.h

@@ -199,18 +199,17 @@ template <typename LHSExpr, typename RHSExpr> 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;
   }
 };
 

include/numsim_cas/core/symbol_base.h

@@ -55,7 +55,10 @@ template <typename BaseExpr> class symbol_base : public BaseExpr {
 template <typename BaseExprT>
 bool operator<(symbol_base<BaseExprT> const &lhs,
                symbol_base<BaseExprT> 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 <typename BaseExprT>
@@ -67,7 +70,7 @@ bool operator>(symbol_base<BaseExprT> const &lhs,
 template <typename BaseExprT>
 bool operator==(symbol_base<BaseExprT> const &lhs,
                 symbol_base<BaseExprT> const &rhs) {
-  return lhs.hash_value() == rhs.hash_value();
+  return lhs.hash_value() == rhs.hash_value() && lhs.name() == rhs.name();
 }
 
 template <typename BaseExprT>

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 <cassert>
 #include <numsim_cas/core/binary_op.h>
 #include <numsim_cas/scalar/scalar_functions.h>
 #include <numsim_cas/tensor/structural_propagation.h>
@@ -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<tensor_scalar_mul const *>(&rhs) != nullptr);
       auto const &r = static_cast<tensor_scalar_mul const &>(rhs);
       return this->expr_rhs() == r.expr_rhs();
     }

src/numsim_cas/core/expression.cpp

@@ -1,5 +1,7 @@
 #include <numsim_cas/core/expression.h>
 
+#include <typeinfo>
+
 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);
 }
 

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<expression>, 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<double> 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<expression_holder<expression>, double> keys;
+  keys[expression_holder<expression>(
+      std::static_pointer_cast<expression>(xs.data()))] = 2.0;
+  keys[expression_holder<expression>(
+      std::static_pointer_cast<expression>(xt.data()))] = 3.0;
+  EXPECT_EQ(keys.size(), 2u);
+  EXPECT_DOUBLE_EQ(keys.at(expression_holder<expression>(
+                       std::static_pointer_cast<expression>(xs.data()))),
+                   2.0);
+  EXPECT_DOUBLE_EQ(keys.at(expression_holder<expression>(
+                       std::static_pointer_cast<expression>(xt.data()))),
+                   3.0);
+}
+
 // #93 — a tensor_mul's space() must survive copy reconstruction
 // (tensor_add did this; mul dropped it).
 TEST(CoreBugFix, TensorMulCopyPreservesSpaceAnnotation) {

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::scalar_expression,
+                                                numsim::cas::scalar_expression>;
+  numsim::cas::relation::set<numsim::cas::scalar_expression,
+                             numsim::cas::scalar_expression>
+      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