Incorrect Eq reduction that leads to wrong expression

[will133]

It seems like the boolean And/Or function do not work properly with functions (like ITE) with a equality test. For instance:

>>> import sympy
>>> sympy.sympify('Or(x, ITE(a, b, c) == 1)')
x
>>> sympy.sympify('Or(x, ITE(a, b, c) == 1, y)')
x | y
>>> sympy.sympify('Or(x, Eq(ITE(a, b, c), 1), y)')
x | y

It seems like it would drop the function call, which would result in a serious logical error.

The behavior is somewhat erratic imho:

>>> sympy.sympify('Or(x, Eq(func(a), 1), y)')
x | y | Eq(func(a), 1)
>>> sympy.sympify('Or(x, Eq(func(a, b, c), 1), y)')
x | y | Eq(func(a, b, c), 1)
>>> sympy.sympify('Or(x, Eq(where(a, b, c), 1), y)')
x | y | Eq(where(a, b, c), 1)
>>> sympy.sympify('Or(x, Eq(where(a, b, c), true), y)')
x | y

(NOte that the last case is necessary sometimes when b and c are actually boolean column and (where/ITE) and the new boolean checking would make just having where(a, b, c) (without the Eq() failed) due to type checking.

I'm upgrading from the 0.7.* series and this behavior used to work fine. I'm using sympy 1.2 on Python 2.7/Linux.

[will133]

It looks like the problem has to do with Eq. For instance, if you compare an ITE with a constant, it would reduce to the constant:

In [17]: sympy.sympify('Eq(foo(a), 1)')
Eq(foo(a), 1)

In [18]: sympy.sympify('Eq(ITE(a, b, c), 1)')
False

[will133]

I think it boils down to this code in the sympy/core/relational.py where the Equality __new__ function would try to evaluate the expression:

            if lhs == rhs:
                return S.true  # e.g. True == True
            elif all(isinstance(i, BooleanAtom) for i in (rhs, lhs)):
                return S.false  # True != False
            elif not (lhs.is_Symbol or rhs.is_Symbol) and (
                    isinstance(lhs, Boolean) !=
                    isinstance(rhs, Boolean)):
                return S.false  # only Booleans can equal Booleans

It looks like ITE is actually a boolean function. This seems somewhat odd to me as the result of the if-then-else ITE(a, b, c) can have b and c that is a non-boolean. Futhermore, this is somewhat contradictory as the numpy ITE code printer seems to generate code like Piecewise. The ITE(a, b, c) would look like this in numpy:

    (numpy.select([a,True], [b,c], default=numpy.nan))

(Note the default value is a numpy.nan, not a bool)

In any case, if you have a boolean anything and compare to a non-boolean anything, it would return False.

Do you really want to do this reduction though? I think it'd probably be safer NOT to do it if you are comparing unknown things. Otherwise, should this really raise an exception instead? I think it will definitely lead to mistakes like this:

In [6]: sympy.sympify('Eq(ITE(a, b, c), 30)')
False

Which IMHO mean if a is True, compare b to 30, else compare c to 30. The False would just completely ignore the values in a, b, c.

[asmeurer]

Some notes here:

• Booleans in SymPy are not numbers. The are True or False. Strictly speaking if Eq(boolean, 1) always gave False it would be correct.

• If you want to represent a boolean equality, use Equivalent. However, it should also be noted that Equivalent(x, True) is the same thing as just x (and it will automatically reduce as such).

• ITE is a boolean function. Having arguments of ITE that aren't boolean isn't supported (it might work, but that would just be garbage in-garbage out). If you want an expression, use Piecewise. The NumPy printer defaulting to nan is a bug, although a minor one since it's impossible to actually get nan from that expression since True is always included in the conditions.

Regarding this issue, perhaps Eq should just raise an exception on booleans. I don't know, but it seems that there aren't any valid cases where you would use Eq on a boolean and not really want Equivalent.

[asmeurer]

Strictly speaking if Eq(boolean, 1) always gave False it would be correct.

Looking closer at the code you posted, I guess this is the case. See #13445. @smichr do you think it makes more sense for Eq to raise an exception pointing people to Equivalent for booleans?

[will133]

Thanks for the explanation. I always thought that ITE was like numpy.where but it seems like it's not the case. Is there a equivalent of numpy.where (like where(a, b, c) and b and c are not booleans)?

I think the problem most of the time is that a sometimes non-boolean function is used in mixture of a boolean test. Right now, if you use a non-BooleanFunction in a And/Or/Not clause you'll get an exception. However, if you have a function (say would return 0/1) and you want to convert to a boolean you would have to do something like Eq(func(x), 1). It's easy that sometimes people may do Eq(func(x), True) by mistake. In such case, the func(x) is actually not executed at all and the whole thing would be reduced to False.

IMHO it would be MUCH better if an exception is raised in that case since you are comparing two things that are not really comparable by sympy's definition. It's very easy to make this mistake and have part of equation silently wrong and you may not notice it.

[will133]

Sorry I missed the Piecewise part. I guess you'll use Piecewise to support numpy.where.

[smichr]

@smichr do you think it makes more sense for Eq to raise an exception pointing people to Equivalent for booleans?

Perhaps. So an error would be raised whenever one tried to compare Boolean with non-Boolean?

[smichr]

an error would be raised whenever one tried to compare Boolean with non-Boolean?

If you use Eq instead of Equivalent you will get:

>>> Equivalent(ITE(a,b,c),1)
ITE(a, b, c)
>>> Equivalent(ITE(a,b,c),0)
~ITE(a, b, c)

If that looks like, an error directing users to use Equivalent for Booleans rather than Eq would make sense.