symengine.py
symengine.py copied to clipboard
Evaluate Relationals when casting to bool
References to other Issues or PRs
Fixes #312. Before, a Relational operator would always evaluate to True when cast as a bool, regardless of whether or not its left and right hand sides were actually equal. This fixes that. Additionally, if the evaluation is ambiguous (namely, there are free variables), it throws a TypeError.
Brief description of what is fixed or changed
This modifies the class definition of Relational in the symengine python wrapper by overriding bool. If the relation can sucessfully and unambiguously evaluated as a bool (i.e. both sides are constants with no free variables), then it does the simplification using its simplify function.
If the relational's boolean value is ambiguous (i.e. it has free symbols), then it throws the TypeError(f'Relational with free symbols cannot be cast as bool: {self}')
.
This is closer to the expected behavior of the bool function on these types.
Other comments
The TypeError check will subtract one side from the other, and then expand out the expression. If that expansion has no free symbols, then the expression will evaluate normally.
This currently fails test_eval_double2()
in test_eval.py. I feel that this test is incorrect. The test is currently written as this:
def test_eval_double2():
x = Symbol("x")
e = sin(x)**2 + sqrt(2)
raises(RuntimeError, lambda: e.n(real=True))
assert abs(e.n() - x**2 - 1.414) < 1e-3
That final assertion fails, but the statement is not mathematically true. Subtracting x**2
from sin(x)**2
does not cancel out the original sin function. This difference can be arbitrarily big. I believe it should be written (and was intended to be) as such:
def test_eval_double2():
x = Symbol("x")
e = sin(x)**2 + sqrt(2)
raises(RuntimeError, lambda: e.n(real=True))
assert abs(e.n() - sin(x)**2.0 - 1.414) < 1e-3
In fact, I feel that some of those tests should have some "not equal" test counterparts. The test was only passing before because of the bug in Issue #312. This issue would have been caught with the presence of such a test.
I don't like the inexact calculations. If symengine doesn't know that the equality is True or False exactly, then it shouldn't do the simplification.
I don't like the inexact calculations. If symengine doesn't know that the equality is True or False exactly, then it shouldn't do the simplification.
In retrospect, I agree. I will get rid of approximations in a commit tomorrow.
Do you feel it would be appropriate in the event that symengine cannot exactly do such a calculation, to still outsource it sympy (if available)?