Skip to content

Commit f3cab22

Browse files
committed
Fix #354: t2s constant_mul keeps a symbolic scalar_wrapper factor
constant_mul::dispatch(tensor_to_scalar_mul) handled a numeric LHS by merging it into the coefficient, but a non-numeric scalar_wrapper LHS fell through the same path: the factor was never inserted and the coefficient was reset to the wrapped default, so wrapper(x) * (trace(A)*det(A)) evaluated as if x were 1. The promoted route (plain x * (f*g)) goes through mul_base and was correct, which is why tests missed it - the bug fires whenever the wrapper is the visitor's LHS. Non-numeric wrappers are now inserted as factors via push_or_combine (merging with an existing wrapper child through the unwrap-multiply- rewrap path). Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
1 parent e5ca023 commit f3cab22

2 files changed

Lines changed: 26 additions & 3 deletions

File tree

src/numsim_cas/tensor_to_scalar/simplifier/tensor_to_scalar_simplifier_mul.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ namespace simplifier {
1212
// --- constant_mul ---
1313
using t2s_traits = domain_traits<tensor_to_scalar_expression>;
1414

15+
static void push_or_combine(tensor_to_scalar_mul &mul,
16+
mul_base::expr_holder_t const &child);
17+
1518
constant_mul::constant_mul(expr_holder_t lhs, expr_holder_t rhs)
1619
: base(std::move(lhs), std::move(rhs)),
1720
lhs_val{t2s_traits::try_numeric(base::m_lhs)} {}
@@ -36,10 +39,14 @@ constant_mul::dispatch(tensor_to_scalar_mul const &rhs) {
3639
}
3740
auto mul_expr{make_expression<tensor_to_scalar_mul>(rhs)};
3841
auto &mul{mul_expr.template get<tensor_to_scalar_mul>()};
39-
auto coeff{get_coefficient<t2s_traits>(mul, 1)};
40-
if (lhs_val) {
41-
coeff = coeff * *lhs_val;
42+
if (!lhs_val) {
43+
// symbolic scalar_wrapper: keep it as a factor instead of silently
44+
// resetting the coefficient (#354)
45+
push_or_combine(mul, base::m_lhs);
46+
return mul_expr;
4247
}
48+
auto coeff{get_coefficient<t2s_traits>(mul, 1)};
49+
coeff = coeff * *lhs_val;
4350
mul.set_coeff(t2s_traits::make_constant(coeff));
4451
return mul_expr;
4552
}

tests/TensorToScalarExpressionTest.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,4 +921,20 @@ TYPED_TEST(TensorToScalarExpressionTest,
921921
EXPECT_EQ(d.get().dim(), X.get().dim());
922922
}
923923

924+
// #354 — a symbolic scalar_wrapper multiplied into an existing t2s mul must
925+
// survive as a factor (it was silently dropped and the coefficient reset).
926+
TYPED_TEST(TensorToScalarExpressionTest,
927+
SymbolicWrapperFactorSurvivesMulMerge) {
928+
auto &X = this->X;
929+
auto &x = this->x;
930+
using numsim::cas::det;
931+
using numsim::cas::trace;
932+
auto f = trace(X) * det(X); // tensor_to_scalar_mul
933+
auto w = numsim::cas::make_expression<
934+
numsim::cas::tensor_to_scalar_scalar_wrapper>(x);
935+
auto e = w * f; // wrapper-first: hits constant_mul::dispatch(mul)
936+
auto const s = numsim::cas::to_string(e);
937+
EXPECT_NE(s.find("x"), std::string::npos) << s;
938+
}
939+
924940
#endif // TENSORTOSCALAREXPRESSIONTEST_H

0 commit comments

Comments
 (0)