Be more careful about setting the Maxima 'domain'

[orlitzky]

Ultimately, we should provide the user a nice way to set this. In the meantime, I'd like to clean up a few places where we play fast and loose with it:

• simplify_radical() and simplify_log() set the domain to 'real' before the round trip through Maxima and back. This has no effect on any doctest (radcan ignores domain anyway).

• Expression.expand_log() sets the domain to 'real' when it's called, and 'complex' when it returns. We should make a note of the previous value rather than assuming it is 'complex' when the method is called.

Depends on #12845

CC: @zimmermann6

Component: symbolics

Author: Michael Orlitzky

Reviewer: Burcin Erocal

Merged: sage-5.6.beta0

Issue created by migration from https://trac.sagemath.org/ticket/12780

[orlitzky]

comment:1

Whoops, there was one failing doctest that I missed (I got a timeout somewhere in ptestlong, I think). Anyway, the doctest was wrong: simplify_radical() was doing something it shouldn't have, and the correct fix is to assume some variables are real. Then all we need is simplify().

This patch could potentially help with fixing simplify_full(), but I think it has merit on its own: we shouldn't switch between real/complex behind the user's back. Sometime soon I'll propose a way for the user to set the domain.

[orlitzky]

Same patch with the functional.py doctest removed

[orlitzky]

Dependencies: #12845

[orlitzky]

comment:3

Attachment: sage-trac_12780.patch.gz

I've refreshed the patch without the fixed doctest. That's now #12845.

[kcrisman]

comment:4

I like the concept of allowing the switch. So you're sure that domain is ignored for radcan and for whatever is going on with the log simplify? Just asking.

[orlitzky]

comment:5

Replying to @kcrisman:

I like the concept of allowing the switch. So you're sure that domain is ignored for radcan and for whatever is going on with the log simplify? Just asking.

Dr. Fateman recently (March 14) mentioned on the Maxima list that radcan was written before Maxima's assumptions framework, and that all simplification takes place outside of radcan.

I think we can allow the switch where it makes sense. I left the domain: real; call in expand_log() alone because it only makes sense to call expand_log() on a real argument.

With simplify_log(), it's less clear. Right now, if I do,

sage: f = sqrt(x**2)
sage: f
sqrt(x^2)
sage: f.simplify_log()
abs(x)

we silently convert the expression to the reals. This isn't a result of the log simplification algorithm; it's a side effect of setting the domain to real (which provides no other tested benefits).

Obviously assuming that you're working over the reals can allow some simplifications, but we should just make that available to the user rather than doing it arbitrarily in some simplification functions but not others.

[kcrisman]

comment:6

With simplify_log(), it's less clear. Right now, if I do,

> sage: f = sqrt(x**2)
> sage: f
> sqrt(x^2)
> sage: f.simplify_log()
> abs(x)

we silently convert the expression to the reals. This isn't a result of the log simplification algorithm; it's a side effect of setting the domain to real (which provides no other tested benefits).

Hmm, yeah, that seems bad. So all the "usual" results of simplify_log are still obtained if we remove the domain business (as you are implying)? Then this sounds good.

[orlitzky]

comment:7

Replying to @kcrisman:

With simplify_log(), it's less clear. Right now, if I do,

> sage: f = sqrt(x**2)
> sage: f
> sqrt(x^2)
> sage: f.simplify_log()
> abs(x)

we silently convert the expression to the reals. This isn't a result of the log simplification algorithm; it's a side effect of setting the domain to real (which provides no other tested benefits).

Hmm, yeah, that seems bad. So all the "usual" results of simplify_log are still obtained if we remove the domain business (as you are implying)? Then this sounds good.

Yep, and modulo #12845, the same goes for simplify_radical().

[jdemeyer]

comment:8

Please fill in your real name as Author.

[orlitzky]

Author: Michael Orlitzky

[burcin]

comment:10

Looks good to me.

[burcin]

Reviewer: Burcin Erocal

[jdemeyer]

Merged: sage-5.6.beta0

[kcrisman]

comment:12

See #14305. Apparently radcan does not ignore domain after all.

(%i1) radcan(sqrt(x^2));
(%o1)                               abs(x)
(%i2) domain:complex;
(%o2)                               complex
(%i3) radcan(sqrt(x^2));
(%o3)                                  x

This is true in the current Maxima (5.29.1) as well as the older 5.26 series in Sage 5.1/5.2.

[kcrisman]

comment:14

Dr. Fateman recently (March 14) mentioned on the Maxima list that radcan was written before Maxima's assumptions framework, and that all simplification takes place outside of radcan.

I think that what happened is that this is not part of the assumption framework! There is no assuming going on here, as far as Maxima is concerned. See for instance Fateman's answer here.

[zimmermann6]

comment:15

if x is assumed to be real the correct answer for sqrt(x^2) is abs(x).

However if x is assumed to be complex, the correct answer is either x or -x,
more precisely the one with a positive real part (or a nonnegative imaginary part if the real part is zero). Then if x is non-real the answer abs(x) is wrong, since this is the norm of x, and the norm is real. Consider for example x = -3+4*I, whose norm is 5, but whose square root is 3-4*I.

Maple 15 gives:

> assume(x,real);
> simplify(sqrt(x^2));
                                    | x~ |

> assume(x,complex);  
> simplify(sqrt(x^2));
                                  csgn(x~) x~

If this ticket did change the default domain of symbolic variables from real to complex, this is a MAJOR change.

Paul

[kcrisman]

comment:16

if x is assumed to be real the correct answer for sqrt(x^2) is abs(x).

Again, according to symbolics experts (as opposed to functions experts) like Fateman, there is no such thing as abs(x), only x or -x, and one then chooses a branch - arbitrarily, but consistently. At least, so I understand that argument.

However if x is assumed to be complex, the correct answer is either x or -x,

Which is what is given here, since one doesn't know a priori whether x has + or - real part, etc. So Maxima picks x.

more precisely the one with a positive real part (or a nonnegative imaginary part if the real part is zero). Then if x is non-real the answer abs(x) is wrong, since this is the norm of x, and the norm is real. Consider for example x = -3+4*I, whose norm is 5, but whose square root is 3-4*I.

Yes, that's presumably why domain:complex does not allow abs(x) in Maxima either. Maybe we need a csgn function too, but I don't know whether Ginac supports this, though apparently it does.

If this ticket did change the default domain of symbolic variables from real to complex, this is a MAJOR change.

The default domain has been complex for a long, long time. It was just exposed here that we didn't do that in simplify_radical - presumably to avoid the very behavior you are noticing at #14305, but we must have forgotten that.

Note also that I am not advocating for a particular resolution here, just trying to summarize the arguments and previous behavior.

[zimmermann6]

comment:17

and one then chooses a branch - arbitrarily, but consistently

I believe when one chooses x for sqrt(x^2), one can always find some inconsistency. For example:

sage: e=sqrt(x^2)-sqrt((x+2)^2)
sage: e.simplify_radical()     
-2
sage: e(x=-1)                  
0

Paul

[kcrisman]

comment:18

Replying to @zimmermann6:

and one then chooses a branch - arbitrarily, but consistently

I believe when one chooses x for sqrt(x^2), one can always find some inconsistency. For example:

sage: e=sqrt(x^2)-sqrt((x+2)^2)
sage: e.simplify_radical()     
-2
sage: e(x=-1)                  
0

Paul

You are probably right. I would encourage you to take this up with the Maxima developers on their list, because I personally want to know whether it's even worth thinking about this, or whether mjo is really right and we should just can simplify_radical, or perhaps relegate it to the deepest recesses of Tartarus.

[orlitzky]

comment:19

Prior to this patch, simplify_radical() was doing two unrelated things:

1. Setting the maxima simplification domain to 'real'.
2. Calling radcan().

There was one doctest within sage that incorrectly relied on this, #12845. You can see that I fixed it by making the assumption explicit. It looks like the example in #14305 is doing the same thing. If you expect,

sage: sqrt(x^2).simplify_radical()
abs(x)

then you're relying on the implicit conversion to domain:real; which this ticket changed. You have no reason to expect sqrt(x^2) == abs(x) unless you assume that x is real, and we don't. The simplification domain in sage has always been 'complex', except where these sneaky functions twiddled it behind your back. Without the assumption that we're dealing with real numbers, sqrt(x^2) should simply be left alone. As noted above, you can't "simplify" it without screwing something up.

The real problem in #14305 is that without said assumption, radcan() will do something ridiculous. That's what radcan() does. If you want correct answers, don't use radcan(). The radcan() function does something very specific, and it works as documented. What it doesn't do is "simplification," and it has no business in sage under the name simplify_foo(). Please help me kill it: #12737.

[zimmermann6]

comment:20

Michael,

ok, we will modify our book, taking into account that by default symbolic variables are considered complex.

However I'm not happy with this ticket (#12780). Before we had (say in 5.1):

sage: assume(x,'real')
sage: sqrt(x^2).simplify_radical()
abs(x)

This was correct. And now (say in 5.8):

sage: assume(x,'real')
sage: sqrt(x^2).simplify_radical()
x

This is wrong, thus we have a regression with this ticket.

Paul

Note: I didn't call radcan, but simplify_radical...

[orlitzky]

comment:21

Replying to @zimmermann6:

Note: I didn't call radcan, but simplify_radical...

The only thing that simplify_radical() does is call Maxima's radcan(), and radcan doesn't do simplification. Instead, it (usually) mangles your expression. That's why I'm so vocally opposed to it being called "simplify."

I agree 100% that the current answer is wrong.. nothing with "simplify" in the name should convert sqrt(x^2) to x. But the previous behavior was also wrong. You can leave off the assumption that x is real, and this will still happen:

sage: sqrt(x^2).simplify_radical()
abs(x)

It's less wrong, maybe. But still wrong. In fact, the underlying call to radcan() wasn't doing anything here. The "simplification" is actually due to the silent switch to the reals. To see this, you can set the Maxima domain, and send your expression for a round trip through Maxima and back. This is in a current version of sage:

sage: maxima_lib.eval('domain: real;')
'real'
sage: maxima_lib(sqrt(x^2))
abs(x)

Now that we've fixed that bug (in this ticket), the expression sqrt(x^2) is passed verbatim to radcan(). Now, it has something to mangle. And it does. It gives you x back. So ultimately, the previous, more-correct behavior was the result of a lesser bug preventing radcan() from doing more damage.