simplify(Piecewise) doesn't simplify the conditionals enough

[asmeurer]

>>> Piecewise((x*y, And(x >= y, Eq(y, 0))), (x, Eq(x, 0)))
⎧x⋅y  for y = 0 ∧ x ≥ y
⎨
⎩ x       for x = 0
>>> simplify(Piecewise((x*y, And(x >= y, Eq(y, 0))), (x, Eq(x, 0))))
{0  for (x = 0 ∨ y = 0) ∧ (x = 0 ∨ x ≥ 0)
>>> simplify_logic(Piecewise((x*y, And(x >= y, Eq(y, 0))), (x, Eq(x, 0))).args[0].args[1])
y = 0 ∧ x ≥ y
``

It could actually be simplified even further to y = 0 ∧ x ≥ 0.

[JamZYu]

Hi, I'm a student new to open source project can I look into this issue and work on it?
As far as I have explored, after calling simplify() on Piecewise functions, the conditionals can still be simplified with simplify_logic()

>>> simplify(Piecewise((x*y, And(x >= y, Eq(y, 0))), (x, Eq(x, 0))))
Piecewise((0, (Eq(x, 0) | Eq(y, 0)) & (Eq(x, 0) | (x >= 0))))
>>> simplify_logic(simplify(Piecewise((x*y, And(x >= y, Eq(y, 0))), (x, Eq(x, 0)))).args[0].args[1])
Eq(x, 0) | (Eq(y, 0) & (x >= 0))

I would considering to add simplify_logic again to the conditionals in Piecewise()._eval_simplified() after expressions have been merged.

For y = 0 ∧ x ≥ y to be further simplified to y = 0 ∧ x ≥ 0, it is beyond the scope of logic_simplify, but simplify() looks like already taken care of this:

>>> simplify(Piecewise((x*y, Eq(y, 0) & (x >= y))))
Piecewise((0, Eq(y, 0) & (x >= 0)))

[asmeurer]

I think that should be a reasonable fix, though you should try it and see how it affects the tests.

Where does the simplify(And(Eq(y, 0), x >= y)) occur? Maybe we should be putting the logic simplification there.

[oscargus]

I beg to differ. It cannot simplify to y = 0 ∧ x ≥ 0. Consider y=1, x=0.

[oscargus]

I revisited this one now. First, my comment above is incorrect. It can indeed be simplified to that.

I believe that the things is that the PoS form is selected as it is simpler than the SoP form before any relational simplifications. Then, there are no relational simplifications to be done once the PoS form is selected.

(Btw, running simplify on the Piecewise again leads to 0.)

[oscarbenjamin]

The condition shouldn't simplify away completely because this should give nan for some possible values:

In [6]: p = Piecewise((x*y, And(x >= y, Eq(y, 0))), (x, Eq(x, 0)))

In [7]: p
Out[7]: 
⎧x⋅y  for y = 0 ∧ x ≥ y
⎨                      
⎩ x       for x = 0    

In [8]: simplify(p)
Out[8]: {0  for (x = 0 ∨ y = 0) ∧ (x = 0 ∨ x ≥ 0)

In [9]: simplify(_)
Out[9]: 0

In [10]: p.subs(y, 1)
Out[10]: {x  for x = 0

In [11]: p.subs(y, 1).subs(x, 1)
Out[11]: nan

[oscargus]

I was also surprised. I wonder if one should add a (NaN, True) piece at the end if no default value is there in the construction of the Piecewise? Will help to avoid these things plus that it is quite pedagogical...

Doing that by hand will now give:
Piecewise((0, (x >= 0) & (Eq(x, 0) | Eq(y, 0))), (nan, True))

[oscarbenjamin]

I was also surprised. I wonder if one should add a (NaN, True) piece at the end if no default value is there in the construction of the Piecewise?

There is always an implicit nan, True at the end:

In [15]: Piecewise((1, False))
Out[15]: nan

We need to make sure that conditions don't simplify away if they are not known to be True though.

[oscargus]

Sure, but it is not visible and clearly some of the simplification routines does not consider segments not explicitly there. So although not fixing the underlying problem, it would help, both to make people understand what happens and solve the erroneous behavior.

[asmeurer]

Maybe it would indeed help to explicitly add that to the args, although I don't think the printers should print it.

[oscarbenjamin]

Maybe it would indeed help to explicitly add that to the args, although I don't think the printers should print it.

It might help but really there's just a lot of buggy code in Piecewise. The fundamental tension is between Piecewise as a purely logical construct A if Acond else B if Bcond ... and Piecewise as a piecewise function of a real variable like f(x) if x < a else g(x) if a <= x < b else .... Formally Piecewise is the former but pieces of the simplification code assume the latter. All usage of as_set/as_relational needs to be removed because the conditions can not be assumed to represent ranges of a single real variable.

[oscargus]

I just remind you of #17443

I may have a bit of time to work on this over the weekend and think it would be quite nice to get it in, even in the current, probably suboptimal condition (the ordering of simplifications is really a problem and I do not think it can be solved rather than heuristically). At least I do not add any as_set/as_relational as far as I can remember... Maybe I'll give it a go on Saturday to rebase it etc. I think it will also benefit significantly from #17330, so maybe I'll just start there (for which it would be nice to get #21854 in as they affect the same code segments).

Another way would of course be to support multi-variate expressions for as_set/as_relational which I think is the major limitation in the current approach.

There is also the aspect of being able to do something "perfect", but not have time to do it, and to make something that improves the current state in reasonable time. Always adding NaN and not printing it would be one(?) line in Piecewise and a few in the printing routine (maybe a switch that is off by default, I'm sure there will be questions about why their NaN segment doesn't show up at one stage or another...).

[oscarbenjamin]

If you can work on refactoring Piecewise then that would be great. I think that the first thing to do is ignore inequalities and focus on equalities and unequalities. Those are actually a more common use of Piecewise anyway and simplifying them is much easier and should not be messed up by logic designed for inequalities. See also #21360

[oscarbenjamin]

I don't think that as_set/as_relational is a good approach at all because we should aim to preserve the form of the conditions as much as possible. Obviously if the condition (or part of it) can be shown to be True or False then that's a good simplification but otherwise replacing equalities/unequalities with inequalities is not a simplification. The key strength of unequalities and equalities is that they make it possible to use substitutions in different parts of the Piecewise which is the most important way to simplify things and collapse the cases.

[oscargus]

There are two different aspects here: 1) use knowledge of equalities/unequalities to reduce the number of segments, 2) use general simplification techniques to simplify the conditions. I am quite sure that the only was using inequalities is used to simplify the number of segments is e.g. Piecewise((1, x >= 2), (2, x >= 4)), but that is more a consequence of the second condition being a subset of the first.

Btw, using #17443 on this gives {0 for x ≥ 0 ∧ (x = 0 ∨ y = 0) so no solution there. (In fact, probably the removal of the second segment results in this condition that then cannot be simplified.)