Sage: Ticket #12780: Be more careful about setting the Maxima 'domain'
https://trac.sagemath.org/ticket/12780
<p>
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:
</p>
<ul><li><code>simplify_radical()</code> and <code>simplify_log()</code> set the domain to 'real' before the round trip through Maxima and back. This has no effect on any doctest (radcan ignores <code>domain</code> anyway).
</li></ul><ul><li>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.
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/12780
Trac 1.1.6mjoThu, 29 Mar 2012 17:31:07 GMT
https://trac.sagemath.org/ticket/12780#comment:1
https://trac.sagemath.org/ticket/12780#comment:1
<p>
Whoops, there was one failing doctest that I missed (I got a timeout somewhere in ptestlong, I think). Anyway, the doctest was wrong: <code>simplify_radical()</code> was doing something it shouldn't have, and the correct fix is to assume some variables are real. Then all we need is <code>simplify()</code>.
</p>
<p>
This patch could potentially help with fixing <code>simplify_full()</code>, 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.
</p>
TicketmjoThu, 29 Mar 2012 17:31:15 GMTstatus changed
https://trac.sagemath.org/ticket/12780#comment:2
https://trac.sagemath.org/ticket/12780#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketmjoSun, 15 Apr 2012 18:37:37 GMTattachment set
https://trac.sagemath.org/ticket/12780
https://trac.sagemath.org/ticket/12780
<ul>
<li><strong>attachment</strong>
set to <em>sage-trac_12780.patch</em>
</li>
</ul>
<p>
Same patch with the functional.py doctest removed
</p>
TicketmjoSun, 15 Apr 2012 18:38:24 GMTdependencies set
https://trac.sagemath.org/ticket/12780#comment:3
https://trac.sagemath.org/ticket/12780#comment:3
<ul>
<li><strong>dependencies</strong>
set to <em>#12845</em>
</li>
</ul>
<p>
I've refreshed the patch without the fixed doctest. That's now <a class="closed ticket" href="https://trac.sagemath.org/ticket/12845" title="defect: Incorrect doctest in sage/misc/functional.py (closed: fixed)">#12845</a>.
</p>
TicketkcrismanSat, 26 May 2012 07:08:05 GMT
https://trac.sagemath.org/ticket/12780#comment:4
https://trac.sagemath.org/ticket/12780#comment:4
<p>
I like the concept of allowing the switch. So you're sure that <code>domain</code> is ignored for <code>radcan</code> and for whatever is going on with the log simplify? Just asking.
</p>
TicketmjoSat, 26 May 2012 16:27:15 GMT
https://trac.sagemath.org/ticket/12780#comment:5
https://trac.sagemath.org/ticket/12780#comment:5
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12780#comment:4" title="Comment 4">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
I like the concept of allowing the switch. So you're sure that <code>domain</code> is ignored for <code>radcan</code> and for whatever is going on with the log simplify? Just asking.
</p>
</blockquote>
<p>
Dr. Fateman recently (March 14) mentioned on the Maxima list that <code>radcan</code> was written before Maxima's assumptions framework, and that all simplification takes place outside of <code>radcan</code>.
</p>
<p>
I think we can allow the switch where it makes sense. I left the <code>domain: real;</code> call in <code>expand_log()</code> alone because it only makes sense to call <code>expand_log()</code> on a real argument.
</p>
<p>
With <code>simplify_log()</code>, it's less clear. Right now, if I do,
</p>
<pre class="wiki">sage: f = sqrt(x**2)
sage: f
sqrt(x^2)
sage: f.simplify_log()
abs(x)
</pre><p>
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 <em>tested</em> benefits).
</p>
<p>
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.
</p>
TicketkcrismanSat, 26 May 2012 16:48:19 GMT
https://trac.sagemath.org/ticket/12780#comment:6
https://trac.sagemath.org/ticket/12780#comment:6
<blockquote class="citation">
<p>
With <code>simplify_log()</code>, it's less clear. Right now, if I do,
</p>
</blockquote>
<pre class="wiki">> sage: f = sqrt(x**2)
> sage: f
> sqrt(x^2)
> sage: f.simplify_log()
> abs(x)
</pre><blockquote class="citation">
<p>
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 <em>tested</em> benefits).
</p>
</blockquote>
<p>
Hmm, yeah, that seems bad. So all the "usual" results of <code>simplify_log</code> are still obtained if we remove the domain business (as you are implying)? Then this sounds good.
</p>
TicketmjoSat, 26 May 2012 17:07:22 GMT
https://trac.sagemath.org/ticket/12780#comment:7
https://trac.sagemath.org/ticket/12780#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12780#comment:6" title="Comment 6">kcrisman</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
With <code>simplify_log()</code>, it's less clear. Right now, if I do,
</p>
</blockquote>
<pre class="wiki">> sage: f = sqrt(x**2)
> sage: f
> sqrt(x^2)
> sage: f.simplify_log()
> abs(x)
</pre><blockquote class="citation">
<p>
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 <em>tested</em> benefits).
</p>
</blockquote>
<p>
Hmm, yeah, that seems bad. So all the "usual" results of <code>simplify_log</code> are still obtained if we remove the domain business (as you are implying)? Then this sounds good.
</p>
</blockquote>
<p>
Yep, and modulo <a class="closed ticket" href="https://trac.sagemath.org/ticket/12845" title="defect: Incorrect doctest in sage/misc/functional.py (closed: fixed)">#12845</a>, the same goes for <code>simplify_radical()</code>.
</p>
TicketjdemeyerFri, 27 Jul 2012 20:40:20 GMT
https://trac.sagemath.org/ticket/12780#comment:8
https://trac.sagemath.org/ticket/12780#comment:8
<p>
Please fill in your real name as Author.
</p>
TicketmjoMon, 30 Jul 2012 15:15:28 GMTauthor set
https://trac.sagemath.org/ticket/12780#comment:9
https://trac.sagemath.org/ticket/12780#comment:9
<ul>
<li><strong>author</strong>
set to <em>Michael Orlitzky</em>
</li>
</ul>
TicketburcinMon, 19 Nov 2012 14:03:17 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/12780#comment:10
https://trac.sagemath.org/ticket/12780#comment:10
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Burcin Erocal</em>
</li>
</ul>
<p>
Looks good to me.
</p>
TicketjdemeyerTue, 18 Dec 2012 11:16:30 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/12780#comment:11
https://trac.sagemath.org/ticket/12780#comment:11
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.6.beta0</em>
</li>
</ul>
TicketkcrismanTue, 19 Mar 2013 13:36:07 GMT
https://trac.sagemath.org/ticket/12780#comment:12
https://trac.sagemath.org/ticket/12780#comment:12
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/14305" title="defect: Doctest: Immediate simplifications of symbolic powers (closed: fixed)">#14305</a>. Apparently radcan does <em>not</em> ignore domain after all.
</p>
<pre class="wiki">(%i1) radcan(sqrt(x^2));
(%o1) abs(x)
(%i2) domain:complex;
(%o2) complex
(%i3) radcan(sqrt(x^2));
(%o3) x
</pre><p>
This is true in the current Maxima (5.29.1) as well as the older 5.26 series in Sage 5.1/5.2.
</p>
TicketzimmermaTue, 19 Mar 2013 13:45:20 GMTcc set
https://trac.sagemath.org/ticket/12780#comment:13
https://trac.sagemath.org/ticket/12780#comment:13
<ul>
<li><strong>cc</strong>
<em>zimmerma</em> added
</li>
</ul>
TicketkcrismanTue, 19 Mar 2013 13:50:59 GMT
https://trac.sagemath.org/ticket/12780#comment:14
https://trac.sagemath.org/ticket/12780#comment:14
<blockquote class="citation">
<p>
Dr. Fateman recently (March 14) mentioned on the Maxima list that <code>radcan</code> was written before Maxima's assumptions framework, and that all simplification takes place outside of <code>radcan</code>.
</p>
</blockquote>
<p>
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 <a class="ext-link" href="http://ask.sagemath.org/question/767/simplification-errors-in-simple-expressions"><span class="icon"></span>here</a>.
</p>
TicketzimmermaTue, 19 Mar 2013 14:10:20 GMT
https://trac.sagemath.org/ticket/12780#comment:15
https://trac.sagemath.org/ticket/12780#comment:15
<p>
if <code>x</code> is assumed to be real the correct answer for <code>sqrt(x^2)</code> is <code>abs(x)</code>.
</p>
<p>
However if <code>x</code> is assumed to be complex, the correct answer is either <code>x</code> or <code>-x</code>,
more precisely the one with a positive real part (or a nonnegative imaginary part if the real part is zero). Then if <code>x</code> is non-real the answer <code>abs(x)</code> is wrong, since this is the norm of <code>x</code>, and the norm is real. Consider for example <code>x = -3+4*I</code>, whose norm is 5, but whose square root is <code>3-4*I</code>.
</p>
<p>
Maple 15 gives:
</p>
<pre class="wiki">> assume(x,real);
> simplify(sqrt(x^2));
| x~ |
> assume(x,complex);
> simplify(sqrt(x^2));
csgn(x~) x~
</pre><p>
If this ticket did change the default domain of symbolic variables from real to complex, this is a <strong>MAJOR</strong> change.
</p>
<p>
Paul
</p>
TicketkcrismanTue, 19 Mar 2013 15:12:14 GMT
https://trac.sagemath.org/ticket/12780#comment:16
https://trac.sagemath.org/ticket/12780#comment:16
<blockquote class="citation">
<p>
if <code>x</code> is assumed to be real the correct answer for <code>sqrt(x^2)</code> is <code>abs(x)</code>.
</p>
</blockquote>
<p>
Again, according to symbolics experts (as opposed to functions experts) like Fateman, there is no such thing as <code>abs(x)</code>, only <code>x</code> or <code>-x</code>, and one then chooses a branch - arbitrarily, but consistently. At least, so I understand that argument.
</p>
<blockquote class="citation">
<p>
However if <code>x</code> is assumed to be complex, the correct answer is either <code>x</code> or <code>-x</code>,
</p>
</blockquote>
<p>
Which is what is given here, since one doesn't know a priori whether <code>x</code> has + or - real part, etc. So Maxima picks <code>x</code>.
</p>
<blockquote class="citation">
<p>
more precisely the one with a positive real part (or a nonnegative imaginary part if the real part is zero). Then if <code>x</code> is non-real the answer <code>abs(x)</code> is wrong, since this is the norm of <code>x</code>, and the norm is real. Consider for example <code>x = -3+4*I</code>, whose norm is 5, but whose square root is <code>3-4*I</code>.
</p>
</blockquote>
<p>
Yes, that's presumably why <code>domain:complex</code> does not allow <code>abs(x)</code> in Maxima either. Maybe we need a <code>csgn</code> function too, but I don't know whether Ginac supports this, though <a class="ext-link" href="http://www.ginac.de/reference/classGiNaC_1_1numeric.html#a83d936877f6090cfcac15e59350f4241"><span class="icon"></span>apparently it does</a>.
</p>
<blockquote class="citation">
<p>
If this ticket did change the default domain of symbolic variables from real to complex, this is a <strong>MAJOR</strong> change.
</p>
</blockquote>
<p>
The default domain has been complex for a long, long time. It was just exposed here that we didn't do that in <code>simplify_radical</code> - presumably to avoid the very behavior you are noticing at <a class="closed ticket" href="https://trac.sagemath.org/ticket/14305" title="defect: Doctest: Immediate simplifications of symbolic powers (closed: fixed)">#14305</a>, but we must have forgotten that.
</p>
<p>
Note also that I am not advocating for a particular resolution here, just trying to summarize the arguments and previous behavior.
</p>
TicketzimmermaTue, 19 Mar 2013 15:26:56 GMT
https://trac.sagemath.org/ticket/12780#comment:17
https://trac.sagemath.org/ticket/12780#comment:17
<blockquote class="citation">
<p>
and one then chooses a branch - arbitrarily, but consistently
</p>
</blockquote>
<p>
I believe when one chooses <code>x</code> for <code>sqrt(x^2)</code>, one can always find some inconsistency. For example:
</p>
<pre class="wiki">sage: e=sqrt(x^2)-sqrt((x+2)^2)
sage: e.simplify_radical()
-2
sage: e(x=-1)
0
</pre><p>
Paul
</p>
TicketkcrismanTue, 19 Mar 2013 15:37:10 GMT
https://trac.sagemath.org/ticket/12780#comment:18
https://trac.sagemath.org/ticket/12780#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12780#comment:17" title="Comment 17">zimmerma</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
and one then chooses a branch - arbitrarily, but consistently
</p>
</blockquote>
<p>
I believe when one chooses <code>x</code> for <code>sqrt(x^2)</code>, one can always find some inconsistency. For example:
</p>
<pre class="wiki">sage: e=sqrt(x^2)-sqrt((x+2)^2)
sage: e.simplify_radical()
-2
sage: e(x=-1)
0
</pre><p>
Paul
</p>
</blockquote>
<p>
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 <code>simplify_radical</code>, or perhaps relegate it to the deepest recesses of Tartarus.
</p>
TicketmjoTue, 19 Mar 2013 16:35:02 GMT
https://trac.sagemath.org/ticket/12780#comment:19
https://trac.sagemath.org/ticket/12780#comment:19
<p>
Prior to this patch, <code>simplify_radical()</code> was doing two unrelated things:
</p>
<ol><li>Setting the maxima simplification domain to 'real'.
</li><li>Calling radcan().
</li></ol><p>
There was one doctest within sage that incorrectly relied on this, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12845" title="defect: Incorrect doctest in sage/misc/functional.py (closed: fixed)">#12845</a>. You can see that I fixed it by making the assumption explicit. It looks like the example in <a class="closed ticket" href="https://trac.sagemath.org/ticket/14305" title="defect: Doctest: Immediate simplifications of symbolic powers (closed: fixed)">#14305</a> is doing the same thing. If you expect,
</p>
<pre class="wiki">sage: sqrt(x^2).simplify_radical()
abs(x)
</pre><p>
then you're relying on the implicit conversion to <code>domain:real;</code> which this ticket changed. You have no reason to expect <code>sqrt(x^2) == abs(x)</code> unless you assume that <code>x</code> 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, <code>sqrt(x^2)</code> should simply be left alone. As noted above, you can't "simplify" it without screwing something up.
</p>
<p>
The real problem in <a class="closed ticket" href="https://trac.sagemath.org/ticket/14305" title="defect: Doctest: Immediate simplifications of symbolic powers (closed: fixed)">#14305</a> is that without said assumption, <code>radcan()</code> will do something ridiculous. That's what <code>radcan()</code> does. <strong>If you want correct answers, don't use <code>radcan()</code></strong>. The <code>radcan()</code> 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 <code>simplify_foo()</code>. Please help me kill it: <a class="closed ticket" href="https://trac.sagemath.org/ticket/12737" title="enhancement: Remove simplify_radical() from simplify_full() (closed: fixed)">#12737</a>.
</p>
TicketzimmermaTue, 19 Mar 2013 20:15:11 GMT
https://trac.sagemath.org/ticket/12780#comment:20
https://trac.sagemath.org/ticket/12780#comment:20
<p>
Michael,
</p>
<p>
ok, we will modify our book, taking into account that by default symbolic variables are considered complex.
</p>
<p>
However I'm not happy with this ticket (<a class="closed ticket" href="https://trac.sagemath.org/ticket/12780" title="enhancement: Be more careful about setting the Maxima 'domain' (closed: fixed)">#12780</a>). Before we had (say in 5.1):
</p>
<pre class="wiki">sage: assume(x,'real')
sage: sqrt(x^2).simplify_radical()
abs(x)
</pre><p>
This was correct. And now (say in 5.8):
</p>
<pre class="wiki">sage: assume(x,'real')
sage: sqrt(x^2).simplify_radical()
x
</pre><p>
This is wrong, thus we have a regression with this ticket.
</p>
<p>
Paul
</p>
<p>
Note: I didn't call <code>radcan</code>, but <code>simplify_radical</code>...
</p>
TicketmjoTue, 19 Mar 2013 21:30:09 GMT
https://trac.sagemath.org/ticket/12780#comment:21
https://trac.sagemath.org/ticket/12780#comment:21
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12780#comment:20" title="Comment 20">zimmerma</a>:
</p>
<blockquote class="citation">
<p>
Note: I didn't call <code>radcan</code>, but <code>simplify_radical</code>...
</p>
</blockquote>
<p>
<br />
</p>
<p>
The only thing that <code>simplify_radical()</code> does is call Maxima's <code>radcan()</code>, and <code>radcan</code> doesn't do simplification. Instead, it (usually) mangles your expression. That's why I'm so vocally opposed to it being called "simplify."
</p>
<p>
I agree 100% that the current answer is wrong.. nothing with "simplify" in the name should convert <code>sqrt(x^2)</code> to <code>x</code>. But the previous behavior was also wrong. You can leave off the assumption that <code>x</code> is real, and this will still happen:
</p>
<pre class="wiki">sage: sqrt(x^2).simplify_radical()
abs(x)
</pre><p>
It's less wrong, maybe. But still wrong. In fact, the underlying call to <code>radcan()</code> 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:
</p>
<pre class="wiki">sage: maxima_lib.eval('domain: real;')
'real'
sage: maxima_lib(sqrt(x^2))
abs(x)
</pre><p>
Now that we've fixed <em>that</em> bug (in this ticket), the expression <code>sqrt(x^2)</code> is passed verbatim to <code>radcan()</code>. <em>Now</em>, it has something to mangle. And it does. It gives you <code>x</code> back. So ultimately, the previous, more-correct behavior was the result of a lesser bug preventing <code>radcan()</code> from doing more damage.
</p>
Ticket