From fefdc35dae971273058dbaafec52b2c3b3dfe206 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 12 Nov 2026 14:27:42 +0100 Subject: [PATCH] Fix non-constant numbers in Boolean context `num2bool` fell back to `Piecewise((True, x != 0.0), (False, True))`, where `x != 0.0` is Python's structural inequality, so any non-constant operand of `&&`, `||`, `!` or a `piecewise` condition was treated as `true`. Return `Ne(x, 0)` instead. Also convert symbols (which are sympy `Boolean`s as well) and Boolean-valued `Piecewise` operands. Fixes #581. Co-Authored-By: Claude Opus 5.5 --- petab/v1/math/SympyVisitor.py | 17 ++++++++++++----- tests/v1/math/test_math.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/petab/v1/math/SympyVisitor.py b/petab/v1/math/SympyVisitor.py index ddf0891b..6803eae6 100644 --- a/petab/v1/math/SympyVisitor.py +++ b/petab/v1/math/SympyVisitor.py @@ -327,14 +327,21 @@ def bool2num(x: sp.Basic | sp.Expr) -> sp.Basic | sp.Expr: def num2bool(x: sp.Basic | sp.Expr) -> sp.Basic | sp.Expr: - """Convert sympy Floats to booleans.""" - if isinstance(x, BooleanTrue | BooleanFalse): + """Convert sympy numeric expressions to booleans. + + Zero is converted to false, any other value to true. + """ + # sp.Symbol is both Boolean and Expr; PEtab symbols are numeric + if isinstance(x, Boolean) and not isinstance(x, sp.Expr): return x # Note: sp.Float(0) == 0 is False in sympy>=1.13 if x.is_zero is True: return sp.false if x.is_zero is False: return sp.true - if isinstance(x, Boolean): - return x - return sp.Piecewise((True, x != 0.0), (False, True)) + if isinstance(x, sp.Piecewise): + # Boolean-valued pieces have to be converted to numbers first + x = sp.Piecewise( + *((bool2num(expr), cond) for expr, cond in x.args), + ) + return sp.Ne(x, 0) diff --git a/tests/v1/math/test_math.py b/tests/v1/math/test_math.py index 019a6163..47fa596a 100644 --- a/tests/v1/math/test_math.py +++ b/tests/v1/math/test_math.py @@ -112,6 +112,38 @@ def test_print_parse_round_trip(expr_str, evaluate): assert np.isclose(actual, expected, rtol=1e-15, atol=0), printed +@pytest.mark.parametrize( + "expr_str, expected", + [ + # expected values for x = -1, 0, 1 (y = 3) + ("(x + 1) && (y > 2)", [False, True, True]), + ("(x + 1) || (y < 2)", [False, True, True]), + ("!(x + 1)", [True, False, False]), + ("piecewise(1, x + 1, 0)", [0.0, 1.0, 1.0]), + ("x && y", [True, False, True]), + ("!x", [False, True, False]), + ("piecewise(1, x > 0, 0) && true", [False, False, True]), + ("!piecewise(1, x < 0, 0)", [False, True, True]), + ("piecewise(true, x > 0, false) && true", [False, False, True]), + ("!piecewise(true, x < 0, false)", [False, True, True]), + ("piecewise(1, piecewise(true, x > 0, false), 0)", [0.0, 0.0, 1.0]), + ], +) +@pytest.mark.parametrize("evaluate", [True, False]) +def test_num2bool(expr_str, expected, evaluate): + """Non-constant numbers in Boolean context: 0 is false, else true.""" + x, y = sp.symbols("x y", real=True) + expr = sympify_petab(expr_str, evaluate=evaluate) + reparsed = sympify_petab(petab_math_str(expr), evaluate=evaluate) + for e in (expr, reparsed): + assert {x, y} & e.free_symbols, e + actual = [ + e.subs({x: sp.Float(x_val), y: sp.Float(3.0)}) + for x_val in (-1.0, 0.0, 1.0) + ] + assert actual == expected, e + + def read_cases(): """Read test cases from YAML file in the petab_test_suite package.""" yaml_file = importlib.resources.files("petabtests.cases").joinpath(