fix Piecewise assumptions

[smichr]

fixes #10258

[smichr]

note to self: perhaps better to return None (if possible) keep old code but instead of return when_multiple, return None (and keep the Expr header test).

[asmeurer]

Yeah, I've wondered about that commutative thing before. Do tests break if you change that (I vaguely remember something depends on it)?

Maybe it would be helpful at the very least to fix that after #6494, so that people can easily write Function('f', commutative=False).

[asmeurer]

What is this? You shouldn't be able to put lists in SymPy objects.

[smichr]

I was surprised, too, but this is necessary as long as anything in SymPy can emit a list, e.g. solve. A Piecewise is an appropriate container for handling any objects (Basic or otherwise) whose selection depends on some condition. The only way to represent a solution to an equation with piecewise conditions is with a Piecewise of lists (or dicts).

For a univariate Piecewise we can give this:

>> solve(Piecewise((x**2-4,x>-4),(x+6,True)), x)
[-6, -2, 2]

But for a multivariate one we need to give a piecewise result:

>> solve(Piecewise((x**2-4,y>0),(x+6,True)),x)
Piecewise(([-2, 2], y > 0), (-6, True))

[asmeurer]

This shouldn't be allowed. The result should be a list of Piecewise, not the other way around. No SymPy object should have args that is a list, since list isn't Basic (and it's mutable, which is double bad).

[smichr]

I was puzzled by this when I encountered it. What object will you return so that when you substitute y=1 you get the list [-2. 2]? The syntax is solve(function) -> list of solutions not [list of solutions], so [-2, 2] not [[-2, 2]] Piecewise allows us to say "this list under this condition otherwise this list."

[smichr]

We could just hack around this by making Piecewise return its solve results in a Tuple when necessary and documenting accordingly.

[smichr]

So we yield a number of items equal to the longest solution and accept that there will be repeats?

[asmeurer]

There are only repeats for y < 0. I see this as similar to solve(x**2 - a, x) -> [sqrt(a), -sqrt(a)], which contains the same solution twice for a=0.

[smichr]

Leave a comment

I see this as similar to

hmm. That makes sense -- when not given enough information to filter the results, duplicates may be returned.

[smichr]

I removed the mods to Function.is_commutative (sans typo). I vaguely recall that the case with a non-Expr arg will now fail because assumptions are worked out based on commutativity being False instead of None. We'll see.

[asmeurer]

You could be right about that. Might end up being a harder thing to fix than expected.

[asmeurer]

How does removing this flag fix the issue?

[smichr]

when_multiple was used to make it emit something other than None when a multivalued expressions existed, e.g. Piecewise((0, x< 0), (1, True)).is_zero -> False but it should give None: the conditions do not allow us to say definitievely that the piecewise is zero. When we have Piecewise((2, x< 0), (1, True)).is_zero then we can say False.

[asmeurer]

Is it doing anything smart enough to check if x.is_negative, or is that outside the scope of this PR?

[smichr]

One would only inspect the condition if the expression were a function of a common variable. And no, that is not included and cannot be until I get to the point of having the robust condition checking in place -- it is currently very easy to trip up the condition checking. Once that is in place, any condition sharing a single variable with the expression could be solved and substituted into the expression when doing the checking so Piecewise arg

(a + b - 3, a + b < 2)` -> solve cond for a and subst using positive symbol `pos` to indicate that we are using "<"
(2 - b - pos) + b - 3 ->
-1 - pos -> a negative

could be inferred.

[smichr]

Leave a comment

Do tests break if you change that

I modified the function to return None if there is a mix of commutative and noncommutative args. Nothing broke (which is not too surprising). What did you have in mind to change, returning None if any arg is noncommutative?

[asmeurer]

I went ahead and implemented undefined function assumptions #12945. So I think with that PR we should definitely make Function('f')(x).is_commutative unrelated to x.is_commutative, since you can easily set Function('f', commutative=False).

[asmeurer]

Can the is_commutative stuff be moved to a separate PR?

[smichr]

moved to a separate PR?

yes -- done.