Sage: Ticket #10682: Upgrade maxima to 5.26
https://trac.sagemath.org/ticket/10682
<p>
I test
</p>
<pre class="wiki">sage: var ('n,k')
sage: sum (binomial(n,k)*k^2, k, 0, n) # is right
sage: sum (binomial(n,k)*k^2, k, 1, n) # is right n(n+1)2^(n2)
sage: sum (binomial(n,k)*k^2, k, 2, n) # is false : I get 0
</pre><p>
This works correctly on Maxima 5.26  we need to upgrade! The new spkg is at
</p>
<p>
<a class="extlink" href="http://perso.telecomparistech.fr/~flori/sage/maxima5.26.0.p0.spkg"><span class="icon"></span>http://perso.telecomparistech.fr/~flori/sage/maxima5.26.0.p0.spkg</a>
</p>
<p>
Install the spkg and apply:
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10682/trac10682_1.patch" title="Attachment 'trac10682_1.patch' in Ticket #10682">trac10682_1.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/10682/trac10682_1.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10682/trac_10682reviewerlocal.patch" title="Attachment 'trac_10682reviewerlocal.patch' in Ticket #10682">trac_10682reviewerlocal.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/10682/trac_10682reviewerlocal.patch" title="Download"></a>
</li></ul>enusSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/10682
Trac 1.1.6mboratkoThu, 17 Nov 2011 07:33:42 GMTdescription changed
https://trac.sagemath.org/ticket/10682#comment:1
https://trac.sagemath.org/ticket/10682#comment:1
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=1">diff</a>)
</li>
</ul>
TicketwasSun, 26 Feb 2012 19:55:06 GMTpriority changed
https://trac.sagemath.org/ticket/10682#comment:2
https://trac.sagemath.org/ticket/10682#comment:2
<ul>
<li><strong>priority</strong>
changed from <em>major</em> to <em>critical</em>
</li>
</ul>
<p>
This is not some general issue where sum fails on all inputs with lower bound != 0, 1. For example, this is fine:
</p>
<pre class="wiki">sage: var('n,k')
(n, k)
sage: sum(k, k, 1, n)
1/2*n^2 + 1/2*n
sage: sum(k, k, 2, n)
1/2*n^2 + 1/2*n  1
</pre><p>
Starting at 2, I guess this should be the answer:
</p>
<pre class="wiki">sage: (sum (binomial(n,k)*k^2, k, 1, n)  sum (binomial(n,k)*k^2, k, 1, 1))
(n^2 + n)*2^(n  2)  n
</pre><p>
The answer seems fine for starting at 3,4, by the way:
</p>
<pre class="wiki">sage: a = (sum (binomial(n,k)*k^2, k, 1, n)  sum (binomial(n,k)*k^2, k, 1, 2))
sage: b = sum (binomial(n,k)*k^2, k, 3, n)
sage: bool(a==b)
True
sage: a = (sum (binomial(n,k)*k^2, k, 1, n)  sum (binomial(n,k)*k^2, k, 1, 3))
sage: b = sum (binomial(n,k)*k^2, k, 4, n)
sage: bool(a==b)
True
</pre><p>
This is probably a serious bug in Maxima, since the real work is done by the function
</p>
<pre class="wiki">sage.calculus.calculus.maxima.sr_sum
</pre><p>
which just calls maxima in some complicated way.
</p>
<p>
I am raising the priority since this is a subtle and very serious mathematically incorrect result. Also, somebody needs to isolate this to a puremaxima session exhibiting the bug (if it is in maxima!) and report upstream. I tried for a minute and failed.
</p>
TicketnbruinSun, 26 Feb 2012 20:37:05 GMT
https://trac.sagemath.org/ticket/10682#comment:3
https://trac.sagemath.org/ticket/10682#comment:3
<pre class="wiki">Maxima 5.23.2 http://maxima.sourceforge.net
using Lisp ECL 11.1.1
Distributed under the GNU Public License. See the file COPYING.
Dedicated to the memory of William Schelter.
The function bug_report() provides bug reporting information.
(%i1) load(simplify_sum);
(%o1) /usr/local/sage/4.7.1/local/share/maxima/5.23.2/share/contrib/solve_rec/simplify_sum.mac
(%i2) simplify_sum(sum(binomial(n,k)*k^2,k,2,n));
(%o2) 0
</pre>
TicketnbruinSun, 26 Feb 2012 21:20:36 GMT
https://trac.sagemath.org/ticket/10682#comment:4
https://trac.sagemath.org/ticket/10682#comment:4
<p>
Possibly the following ticket is relevant:
</p>
<p>
<a class="extlink" href="http://sourceforge.net/tracker/index.php?func=detail&aid=2919296&group_id=4933&atid=104933"><span class="icon"></span>maxima: binomial sums  ID: 2919296</a>
</p>
<p>
If so, it might be fixed. I don't have access to maxima 5.26 to test.
</p>
TicketdimpaseMon, 27 Feb 2012 01:43:30 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:5
https://trac.sagemath.org/ticket/10682#comment:5
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_info</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:4" title="Comment 4">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Possibly the following ticket is relevant:
</p>
<p>
<a class="extlink" href="http://sourceforge.net/tracker/index.php?func=detail&aid=2919296&group_id=4933&atid=104933"><span class="icon"></span>maxima: binomial sums  ID: 2919296</a>
</p>
<p>
If so, it might be fixed. I don't have access to maxima 5.26 to test.
</p>
</blockquote>
<p>
It is fixed. (What's a big deal of having access to maxima 5.26? It compiles with ECL supplied with Sage 4.8):
</p>
<pre class="wiki">$ ./maximalocal
;;; Loading #P"/usr/local/src/sage/sage4.8.alpha4/local/lib/ecl11.1.1/sbbsdsockets.fas"
;;; Loading #P"/usr/local/src/sage/sage4.8.alpha4/local/lib/ecl11.1.1/sockets.fas"
;;; Loading #P"/usr/local/src/sage/sage4.8.alpha4/local/lib/ecl11.1.1/defsystem.fas"
;;; Loading #P"/usr/local/src/sage/sage4.8.alpha4/local/lib/ecl11.1.1/cmp.fas"
Maxima 5.26.0_41_gbc6210e http://maxima.sourceforge.net
using Lisp ECL 11.1.1
Distributed under the GNU Public License. See the file COPYING.
Dedicated to the memory of William Schelter.
The function bug_report() provides bug reporting information.
(%i1) load(simplify_sum);
(%o1) /tmp/maxima/share/contrib/solve_rec/simplify_sum.mac
(%i2) simplify_sum(sum(binomial(n,k)*k^2,k,2,n));
2 n
(n + n) 2  4 n
(%o2) 
4
(%i3)
</pre><p>
Thus, we need to update maxima spkg to 5.26.
</p>
<p>
By the way, I never understood why sum() is hardcoded this way in Sage. Sometimes I'd rather use Python's sum(), but I don't know how to do this.
</p>
TicketdimpaseMon, 27 Feb 2012 01:46:06 GMTdescription, summary changed
https://trac.sagemath.org/ticket/10682#comment:6
https://trac.sagemath.org/ticket/10682#comment:6
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=6">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>sum fails with lower bound != 0 or 1</em> to <em>sum fails with lower bound != 0 or 1 (upgrade maxima to 5.26)</em>
</li>
</ul>
TicketnbruinMon, 27 Feb 2012 03:04:49 GMT
https://trac.sagemath.org/ticket/10682#comment:7
https://trac.sagemath.org/ticket/10682#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:5" title="Comment 5">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Sometimes I'd rather use Python's sum(), but I don't know how to do this.
</p>
</blockquote>
<p>
This is offtopic but a good question. It shows people don't think of checking the <a class="extlink" href="http://docs.python.org/library/__builtin__.html"><span class="icon"></span>python documentation</a> for something like this, so perhaps a more prominent pointer is necessary
</p>
<pre class="wiki">sage: import __builtin__
sage: __builtin__.sum?
...
Docstring:
sum(sequence[, start]) > value
</pre><blockquote class="citation">
<p>
(What's a big deal of having access to maxima 5.26? It compiles with ECL supplied with Sage 4.8)
</p>
</blockquote>
<p>
I tried on sage.math.washington.edu with the 4.8 precompiled tarball there and it didn't work for me. Luckily there are other people more skilled at compiling maxima.
</p>
TicketdimpaseMon, 27 Feb 2012 09:41:08 GMTstatus, description changed
https://trac.sagemath.org/ticket/10682#comment:8
https://trac.sagemath.org/ticket/10682#comment:8
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=8">diff</a>)
</li>
</ul>
TicketdimpaseMon, 27 Feb 2012 09:41:21 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:9
https://trac.sagemath.org/ticket/10682#comment:9
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketdimpaseMon, 27 Feb 2012 22:44:27 GMTdescription changed; work_issues set
https://trac.sagemath.org/ticket/10682#comment:10
https://trac.sagemath.org/ticket/10682#comment:10
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=10">diff</a>)
</li>
<li><strong>work_issues</strong>
set to <em>several doctests need to be patched due to changes in output format/term order</em>
</li>
</ul>
<p>
Several doctests need to be patched due to changes in output format/term order. See
<a class="extlink" href="http://boxen.math.washington.edu/home/dima/tmp/maxima5.26ptestlong.log"><span class="icon"></span>the log</a>.
</p>
TicketdimpaseMon, 27 Feb 2012 23:14:31 GMTcc set
https://trac.sagemath.org/ticket/10682#comment:11
https://trac.sagemath.org/ticket/10682#comment:11
<ul>
<li><strong>cc</strong>
<em>mjo</em> added
</li>
</ul>
TicketdimpaseMon, 27 Feb 2012 23:34:03 GMT
https://trac.sagemath.org/ticket/10682#comment:12
https://trac.sagemath.org/ticket/10682#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:10" title="Comment 10">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Several doctests need to be patched due to changes in output format/term order. See
<a class="extlink" href="http://boxen.math.washington.edu/home/dima/tmp/maxima5.26ptestlong.log"><span class="icon"></span>the log</a>.
</p>
</blockquote>
<p>
The first test failure, in <code>devel/sage/sage/matrix/matrix2.pyx</code>, actually shows that 5.26 has improved in quality of such computations. The error between the exact and .n() answers has decreased by several orders of magnitude.
One more reason for upgrade!
</p>
TicketdimpaseMon, 27 Feb 2012 23:55:12 GMT
https://trac.sagemath.org/ticket/10682#comment:13
https://trac.sagemath.org/ticket/10682#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:12" title="Comment 12">dimpase</a>:
</p>
<blockquote class="citation">
<p>
The first test failure, in <code>devel/sage/sage/matrix/matrix2.pyx</code>, actually shows that 5.26 has improved in quality of such computations. The error between the exact and .n() answers has decreased by several orders of magnitude.
</p>
</blockquote>
<p>
Here is the evidence (<code>a</code> is the matrix from the <code>matrix/matrix2.pyx</code>, line 10369, test, <code>b</code> is the exact matrix). The following is with the new spkg:
</p>
<pre class="wiki">sage: b=matrix([[1,pi],[100,1/100]])
sage: a=matrix(RR,[[1.,pi.n()],[100.,.01]])
sage: (a.exp()b.exp()).norm()
9.32525865157e07
</pre><p>
The same computation on Sage 4.8 gives <code>(a.exp()b.exp()).norm()</code> equal to <code>0.373383533611</code>
</p>
TicketmjoTue, 28 Feb 2012 01:54:13 GMT
https://trac.sagemath.org/ticket/10682#comment:14
https://trac.sagemath.org/ticket/10682#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:10" title="Comment 10">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Several doctests need to be patched due to changes in output format/term order. See
<a class="extlink" href="http://boxen.math.washington.edu/home/dima/tmp/maxima5.26ptestlong.log"><span class="icon"></span>the log</a>.
</p>
</blockquote>
<p>
The failure in calculus/desolvers.py is what kept me from upgrading straight to 5.26 (I settled for 5.24 instead).
</p>
<p>
The issue is somewhere in our maxima interface... if you try the commands in a maxima console, you get the correct (simple) answer. If you try them with <code>maxima._eval_line()</code>, you get the mess from the doctest.
</p>
TicketdimpaseTue, 28 Feb 2012 03:07:51 GMT
https://trac.sagemath.org/ticket/10682#comment:15
https://trac.sagemath.org/ticket/10682#comment:15
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:14" title="Comment 14">mjo</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:10" title="Comment 10">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Several doctests need to be patched due to changes in output format/term order. See
<a class="extlink" href="http://boxen.math.washington.edu/home/dima/tmp/maxima5.26ptestlong.log"><span class="icon"></span>the log</a>.
</p>
</blockquote>
<p>
The failure in calculus/desolvers.py is what kept me from upgrading straight to 5.26 (I settled for 5.24 instead).
</p>
<p>
The issue is somewhere in our maxima interface... if you try the commands in a maxima console, you get the correct (simple) answer. If you try them with <code>maxima._eval_line()</code>, you get the mess from the doctest.
</p>
</blockquote>
<p>
OK, this will need to be sorted out, and few other things. I am preparing the patch for most other, "trivial", failures, and will post it shortly.
</p>
TicketnbruinTue, 28 Feb 2012 05:33:49 GMT
https://trac.sagemath.org/ticket/10682#comment:16
https://trac.sagemath.org/ticket/10682#comment:16
<p>
Thanks for preparing the spkg, Dima! My problem was probably that I didn't rebuild ecl first. It works very well now. I think I found the culprit for the desolve problem:
</p>
<pre class="wiki">Maxima 5.26.0 http://maxima.sourceforge.net
using Lisp ECL 11.1.1
Distributed under the GNU Public License. See the file COPYING.
Dedicated to the memory of William Schelter.
The function bug_report() provides bug reporting information.
(%i1) display2d: false;
(%o1) false
(%i2) load('contrib_ode);
(%o2) "/mnt/usb1/scratch/nbruin/5.0/local/share/maxima/5.26.0/share/contrib/diffequations/contrib_ode.mac"
(%i3) domain : complex;
(%o3) complex
(%i4) assume(x>0);
(%o4) [x > 0]
(%i5) assume(y>0);
(%o5) [y > 0]
(%i6) contrib_ode(x*'diff(y,x,1)x*sqrt(y^2+x^2)y,y,x);
(%o6) [(log((2*sqrt(4/x^2)*x^2*sqrt((4*y^4+4*x^2*y^2)/x^2)+8*y^2+4*x^2)/x^2)
sqrt(4/x^2)*x*asinh(2*y^2/(x*sqrt(4*y^2)))
sqrt(4/x^2)*x*asinh(2*y/sqrt(4*x^2))+sqrt(4/x^2)*x^2)
/(sqrt(4/x^2)*x)
= %c]
(%i7) domain: real;
(%o7) real
(%i8) contrib_ode(x*'diff(y,x,1)x*sqrt(y^2+x^2)y,y,x);
(%o8) [xasinh(y/x) = %c]
</pre><p>
so the problem is that we set "domain: complex" and apparently the behaviour of something there has changed. The assumptions x>0 and y>0 imply that x,y are real (and maxima's assume facility agrees with that), but as we are well aware, assumptions don't get used everywhere. At the very least, it seems that assumptions on variables do not affect all the things that "domain: complex" affects.
</p>
TicketdimpaseTue, 28 Feb 2012 08:36:24 GMT
https://trac.sagemath.org/ticket/10682#comment:17
https://trac.sagemath.org/ticket/10682#comment:17
<p>
there still is a bug in
</p>
<pre class="wiki">sage/functions/special.py", line 1464:
sage: elliptic_e(z, 1)
Expected:
sin(z)
Got:
sin(z) + 2*round(z/pi)
</pre><p>
Note that BOTH expected and the received values are wrong!
The right answer is <code>abs(sin(z)) + 2*round(z/pi)</code>
</p>
<p>
Do you guys know how to report this upstream?
</p>
<p>
Dima
</p>
TicketdimpaseTue, 28 Feb 2012 10:34:27 GMT
https://trac.sagemath.org/ticket/10682#comment:18
https://trac.sagemath.org/ticket/10682#comment:18
<p>
to fix the problem in <code>sage/interfaces/maxima_lib.py</code>, printing extra <code>(false)</code>, I added the patch mentioned
<a class="extlink" href="https://groups.google.com/d/msg/sagedevel/tJwrHO9ipP4/aaHyktqvQesJ"><span class="icon"></span>here</a> to the spkg.
</p>
<p>
It fixes the problem. (I didn't want to mess with the unstable git version of Maxima, so we are applying the patch directly to
the stable Maxima 5.26.0).
</p>
TicketdimpaseTue, 28 Feb 2012 11:50:42 GMT
https://trac.sagemath.org/ticket/10682#comment:19
https://trac.sagemath.org/ticket/10682#comment:19
<p>
with the attached patch, and (updated) spkg, only one failure remains when running <code>make ptestlong</code>:
</p>
<pre class="wiki">sage t long force_lib devel/sage/sage/calculus/desolvers.py
**********************************************************************
File "/mnt/usb1/scratch/dima/sage5.0.beta5/devel/sagemain/sage/calculus/desolvers.py", line 358:
sage: desolve(x*diff(y,x)x*sqrt(y^2+x^2)y == 0, y, contrib_ode=True)
Expected:
[x  arcsinh(y(x)/x) == c]
Got:
[1/2*(2*x^2*sqrt(x^(2))  2*x*sqrt(x^(2))*arcsinh(y(x)/sqrt(x^2))  2*x*sqrt(x^(2))*arcsinh(y(x)^2/(sqrt(y(x)^2)*x)) + log(4*(2*x^2*sqrt((x^2*y(x)^2 + y(x)^4)/x^2)*sqrt(x^(2)) + x^2 + 2*y(x)^2)/x^2))/(x*sqrt(x^(2))) == c]
**********************************************************************
1 items had failures:
1 of 59 in __main__.example_1
***Test Failed*** 1 failures.
</pre><p>
This ought to be fixed by that complex/real domain thing described above by nbruin, but I don't know where this should be done.
</p>
TicketdimpaseWed, 29 Feb 2012 05:18:47 GMT
https://trac.sagemath.org/ticket/10682#comment:20
https://trac.sagemath.org/ticket/10682#comment:20
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:16" title="Comment 16">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Thanks for preparing the spkg, Dima! My problem was probably that I didn't rebuild ecl first. It works very well now. I think I found the culprit for the desolve problem:
</p>
</blockquote>
<blockquote class="citation">
<p>
so the problem is that we set "domain: complex" and apparently the behaviour of something there has changed. The assumptions x>0 and y>0 imply that x,y are real (and maxima's assume facility agrees with that), but as we are well aware, assumptions don't get used everywhere. At the very least, it seems that assumptions on variables do not affect all the things that "domain: complex" affects.
</p>
</blockquote>
<p>
Should we provide an option to do "domain: real" on the fly?
I don't know the details of the maxima interface, but I imagine this is doable...
</p>
<p>
Dima
</p>
TicketdimpaseWed, 29 Feb 2012 10:40:51 GMT
https://trac.sagemath.org/ticket/10682#comment:21
https://trac.sagemath.org/ticket/10682#comment:21
<p>
Here is the patch I propose for <code>calculus/desolvers.py</code>:
</p>
<pre class="wiki">68c68
< def desolve(de, dvar, ics=None, ivar=None, show_method=False, contrib_ode=False, domain='complex'):

> def desolve(de, dvar, ics=None, ivar=None, show_method=False, contrib_ode=False):
113,115d112
<  ``domain``  (optional) specifies the domain to use to solve the ODE.
< By default it is complex numbers; setting domain='real' will make it real numbers.
< It corresponds to Maxima's 'domain : complex;' or, respectively, 'domain : real;'
361,365d357
< sage: desolve(x*diff(y,x)x*sqrt(y^2+x^2)y == 0, y, contrib_ode=True, domain='real')
< [x  arcsinh(y(x)/x) == c]
<
< Trac #10682 updated Maxima to 5.26, and it started to show a different solution in the complex domain for the ODE above::
<
367,368c359,360
< [1/2*(2*x^2*sqrt(x^(2))  2*x*sqrt(x^(2))*arcsinh(y(x)/sqrt(x^2))  2*x*sqrt(x^(2))*arcsinh(y(x)^2/(sqrt(y(x)^2)*x)) + log(4*(2*x^2*sqrt((x^2*y(x)^2 + y(x)^4)/x^2)*sqrt(x^(2)) + x^2 + 2*y(x)^2)/x^2))/(x*sqrt(x^(2))) == c]
<

> [x  arcsinh(y(x)/x) == c]
>
428d419
< P("domain : "+domain)
</pre><p>
It gives deslove() one more optional parameter <code>domain</code>, which takes care of this new discrepancy with this ODE.
I'll update the whole patch shortly, test, and then expect the ticket to be ready for review.
</p>
TicketdimpaseWed, 29 Feb 2012 11:27:39 GMTstatus, description changed
https://trac.sagemath.org/ticket/10682#comment:22
https://trac.sagemath.org/ticket/10682#comment:22
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=22">diff</a>)
</li>
</ul>
TicketjpfloriWed, 29 Feb 2012 12:19:15 GMT
https://trac.sagemath.org/ticket/10682#comment:23
https://trac.sagemath.org/ticket/10682#comment:23
<p>
Could you please post a diff between the new and old spkg's here?
</p>
<p>
Just for review of course.
</p>
TicketjpfloriWed, 29 Feb 2012 12:32:22 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:24
https://trac.sagemath.org/ticket/10682#comment:24
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
As far as the Sage patch is concerned, it would be nice to limit the length of code lines to 79 characters.
</p>
<p>
For example, in sage/symbolic/relation.py you corrected the output but put everything on one line, which is not very readable in 80 char terminals.
</p>
<p>
The error in f.nintegal() should definitely raise an Error. Letting it output of tuple of reals is wrong. I'll try to locate where this should take place if you have no time or no idea.
</p>
TicketdimpaseWed, 29 Feb 2012 12:44:01 GMT
https://trac.sagemath.org/ticket/10682#comment:25
https://trac.sagemath.org/ticket/10682#comment:25
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:23" title="Comment 23">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Could you please post a diff between the new and old spkg's here?
</p>
<p>
Just for review of course.
</p>
</blockquote>
<p>
Here it is (and of course I took the current maxima release 5.26.0source from sf.net and replaced src/ with it, and then
ran the script <code>spkgdist</code>)
</p>
<pre class="wiki">$ hg diff r tip r 35
diff git a/SPKG.txt b/SPKG.txt
 a/SPKG.txt
+++ b/SPKG.txt
@@ 40,11 +40,6 @@
== Changelog ==
=== maxima5.26.0 (Dima Pasechnik, February 28th 2012) ===
 * upgrading to version 5.26.0 to take care of #10682.
 * added patch/comm.patch to fix Maxima bug #3484414.
 (false) precedes the display output if display2d is set to false.

=== maxima5.24.0.p0 (Michael Orlitzky, January 29th 2012) ===
* Trac #12094: Version bump to prevent a regression with the
abs_integrate package.
diff git a/patches/comm.patch b/patches/comm.patch
deleted file mode 100644
 a/patches/comm.patch
+++ /dev/null
@@ 1,12 +0,0 @@
 src/src/comm.lisp
+++ src/src/comm.lisp
@@ 753,7 +753,9 @@
 (setq ans (list '(mequal simp) (disp2 l) ans)))
 (if lablist (nconc lablist (cons (elabel ans) nil)))
 (setq tim (getinternalruntime))
 (displa (list '(mlable) (if lablist linelable) ans))
+ (let ((*displaylabelsp* nil))
+ (declare (special *displaylabelsp*))
+ (displa (list '(mlable) (if lablist linelable) ans)))
 (mterpri)
 (timeorg tim)))
</pre>
TicketdimpaseWed, 29 Feb 2012 12:53:17 GMT
https://trac.sagemath.org/ticket/10682#comment:26
https://trac.sagemath.org/ticket/10682#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:24" title="Comment 24">jpflori</a>:
</p>
<blockquote class="citation">
<p>
As far as the Sage patch is concerned, it would be nice to limit the length of code lines to 79 characters.
</p>
<p>
For example, in sage/symbolic/relation.py you corrected the output but put everything on one line, which is not very readable in 80 char terminals.
</p>
</blockquote>
<p>
oops, OK, I somehow had trouble splitting lines, and thought that the doctests framework doesn't quite allow this.
What is the syntax? Will it just ignore '\n', anywhere ?
</p>
<blockquote class="citation">
<p>
The error in f.nintegal() should definitely raise an Error. Letting it output of tuple of reals is wrong. I'll try to locate where this should take place if you have no time or no idea.
</p>
</blockquote>
<p>
Are you saying that this "error code"=6 is a new addition in Maxima, and is not properly handled by the Sage interface?
</p>
TicketjpfloriWed, 29 Feb 2012 13:18:14 GMT
https://trac.sagemath.org/ticket/10682#comment:27
https://trac.sagemath.org/ticket/10682#comment:27
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:26" title="Comment 26">dimpase</a>:
</p>
<blockquote class="citation">
<p>
oops, OK, I somehow had trouble splitting lines, and thought that the doctests framework doesn't quite allow this. What is the syntax? Will it just ignore '\n', anywhere ?
</p>
</blockquote>
<p>
I think that you can more or less split where there is a space.
</p>
<p>
You can just copy what used to be done for the previous examples.
</p>
<blockquote class="citation">
<p>
Are you saying that this "error code"=6 is a new addition in Maxima, and is not properly handled by the Sage interface?
</p>
</blockquote>
<p>
I'm not sure that this is new, but:
</p>
<ul><li>Sage used to return a <a class="missing wiki">ValueError?</a> so I guess that it used to catch a Maxima error and translate it
</li><li>So it seems that Maxima's behavior has changed even though the "6" case used to be there, maybe a Maxima expert could tell us why this was decided. Anyhow, your fix might be the right one then...
</li></ul><p>
A last comment, in your modification to desolve() you modify the domain var of Maxima.
</p>
<p>
So if I use domain=real with your code, then all of Maxima will use domain=real afterwards.
</p>
<p>
Maybe we'd better set back domain to whatever it was worth before at the end of the computation.
</p>
TicketjpfloriWed, 29 Feb 2012 13:24:52 GMT
https://trac.sagemath.org/ticket/10682#comment:28
https://trac.sagemath.org/ticket/10682#comment:28
<p>
By the way the upstream tarball does not seem to include the latest changelogs.
</p>
TicketkcrismanWed, 29 Feb 2012 13:47:56 GMT
https://trac.sagemath.org/ticket/10682#comment:29
https://trac.sagemath.org/ticket/10682#comment:29
<blockquote class="citation">
<p>
A last comment, in your modification to desolve() you modify the domain var of Maxima.
</p>
<p>
So if I use domain=real with your code, then all of Maxima will use domain=real afterwards.
</p>
<p>
Maybe we'd better set back domain to whatever it was worth before at the end of the computation.
</p>
</blockquote>
<p>
Yes, definitely. I'd be surprised if this didn't cause doctest failures otherwise, since we have <code>domain:complex</code> because it made other things work in the past. Otherwise this is a good solution, I think, and gives the user more access to Maxima's flexibility without too much effort, which is a good thing.
</p>
TicketjpfloriWed, 29 Feb 2012 13:54:21 GMT
https://trac.sagemath.org/ticket/10682#comment:30
https://trac.sagemath.org/ticket/10682#comment:30
<p>
What I told was wrong for the "6" stuff.
</p>
<p>
In fact, Maxima used to return the expression unmodified when the precision was too high, now it behaves correctly, so I'm happy with your fix.
</p>
<p>
However, we could rid of some of the error handling in nintegral and mention (and before that find...) the corresponding commit in maxima git tree.
</p>
TicketmjoWed, 29 Feb 2012 14:31:32 GMT
https://trac.sagemath.org/ticket/10682#comment:31
https://trac.sagemath.org/ticket/10682#comment:31
<p>
I think it would be nice if we handled this automatically rather than requiring the user to know about the internals of solution/simplification. I mean, none of <em>us</em> knew what the problem was until Nils pointed it out.
</p>
<p>
My understanding is that we default to <code>domain: complex</code> because it will make the fewest invalid simplifications and is compatible with pynac. In general, that's the right thing to do. But here? We know that <em>all</em> of the variables in the system are real, from the assumption that x,y>0.
</p>
<p>
Why not just set the domain to real in that case?
</p>
<p>
(I think we should add something like a <code>set_domain_real</code> method to maxima_lib either way to make this cleaner.)
</p>
TicketjpfloriWed, 29 Feb 2012 16:09:03 GMT
https://trac.sagemath.org/ticket/10682#comment:32
https://trac.sagemath.org/ticket/10682#comment:32
<p>
If we just rely on assumptions of the form 'x > 0', it should not be too complicated to check that all variables are supposed to be real.
</p>
<p>
Anyway, isn't there something in Maxima which "knows" that a variable is real after such assumptions ?
</p>
<p>
If that is the case, its current behavior could be considered quite strange.
</p>
TicketkcrismanWed, 29 Feb 2012 17:59:56 GMT
https://trac.sagemath.org/ticket/10682#comment:33
https://trac.sagemath.org/ticket/10682#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:32" title="Comment 32">jpflori</a>:
</p>
<blockquote class="citation">
<p>
If we just rely on assumptions of the form 'x > 0', it should not be too complicated to check that all variables are supposed to be real.
</p>
<p>
Anyway, isn't there something in Maxima which "knows" that a variable is real after such assumptions ?
</p>
</blockquote>
<p>
If I understand correctly, Maxima and assumptions play weakly together. They are ignored in a lot of "dummy variable" contexts like solving and integration. So maybe here too.
</p>
<p>
Interestingly, our GSL numerical ODE solvers seem to only have real domains (see <a class="extlink" href="http://ask.sagemath.org/question/1156/ode_solverunabletoconverttofloat"><span class="icon"></span>this ask.sagemath.org question</a>, I can't verify this independently), so maybe we could instead just <em>always</em> change to <code>domain:real</code> for ODE and then convert back when done... these are all hacks, but some hacks are better than others.
</p>
TicketmjoWed, 29 Feb 2012 19:33:32 GMT
https://trac.sagemath.org/ticket/10682#comment:34
https://trac.sagemath.org/ticket/10682#comment:34
<p>
For the assumptions, we don't necessarily need maxima to recognize them. If the user tells us that <code>x>0</code>, then we should be able to set <code>domain: real</code> whether or not maxima is going to use that information.
</p>
<p>
In simplification, I gather that maxima doesn't really know about real/complex. The docs simply say,
</p>
<pre class="wiki">Option variable: domain
Default value: real
When domain is set to complex, sqrt (x^2) will remain sqrt (x^2) instead of returning abs(x).
</pre><p>
They used to add,
</p>
<pre class="wiki">The notion of a "domain" of simplification is still in its infancy, and controls little more than this at the moment.
</pre><p>
I think we should leave the default <code>complex</code> to be on the safe side. We could strictly improve some things by checking the assumptions, though. There are a few easy cases that we should be able to handle safely:
</p>
<ol><li>assume(<code>x</code>, 'real')
</li><li>assume(<code>x</code> > constant)
</li><li>assume(<code>x</code> > <code>y</code>) (where <code>y</code> is known to be real by similar reasoning)
</li><li>et cetera
</li></ol><p>
If all variables in an expression are determined to be real, we can set <code>domain: real</code> before asking maxima to do anything with it.
</p>
TicketkcrismanWed, 29 Feb 2012 19:51:34 GMT
https://trac.sagemath.org/ticket/10682#comment:35
https://trac.sagemath.org/ticket/10682#comment:35
<p>
Good points. That being said, if the DE is one that is more complicated, and some nasty expression plays the role that <code>x</code> does here, we don't want to try to figure out if that expression is real or not.
</p>
TicketdimpaseWed, 29 Feb 2012 20:06:49 GMT
https://trac.sagemath.org/ticket/10682#comment:36
https://trac.sagemath.org/ticket/10682#comment:36
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:35" title="Comment 35">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Good points. That being said, if the DE is one that is more complicated, and some nasty expression plays the role that <code>x</code> does here, we don't want to try to figure out if that expression is real or not.
</p>
</blockquote>
<p>
I thing we should not try to do our own processing of assume()'s at this point. On another ticket, maybe.
</p>
<p>
Regarding the domain switch, I simply could not find a place to hook it up, as an independent call, instead opting for desolve() to have an extra parameter. If you read desolve() code you see that it digs up the maxima instance to call, as the parent of the expression. I could not find an independent, "global" hook to call maxima, and it seems that it might not even be exposed. It seems that the original design aimed at possibly many independent maxima instances to be run.
</p>
<p>
By the way, the call of desolve with domain='complex', (or just with domain omitted) will reset the domain back to complex. As such, this gives one a way to control the domain, something that was missing in the maxima interface before (although it is a hack).
</p>
<p>
Any thoughts on this?
</p>
TicketkcrismanWed, 29 Feb 2012 20:24:19 GMT
https://trac.sagemath.org/ticket/10682#comment:37
https://trac.sagemath.org/ticket/10682#comment:37
<blockquote class="citation">
<p>
Regarding the domain switch, I simply could not find a place to hook it up, as an independent call, instead opting for desolve() to have an extra parameter. If you read desolve() code you see that it digs up the maxima instance to call, as the parent of the expression. I could not find an independent, "global" hook to call maxima, and it seems that it might not even be exposed. It seems that the original design aimed at possibly many independent maxima instances to be run.
</p>
</blockquote>
<p>
Yes, and still is. But you should be able to import <code>sage.calculus.calculus.maxima</code> and the like.
</p>
<pre class="wiki">sage: sage.calculus.calculus.maxima('domain')
complex
sage: sage.calculus.calculus.maxima('domain:real')
real
sage: sage.calculus.calculus.maxima('domain')
real
sage: sage.calculus.calculus.maxima('domain:complex')
complex
sage: sage.calculus.calculus.maxima('domain')
complex
</pre>
TicketdimpaseWed, 29 Feb 2012 20:29:27 GMT
https://trac.sagemath.org/ticket/10682#comment:38
https://trac.sagemath.org/ticket/10682#comment:38
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:37" title="Comment 37">kcrisman</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Regarding the domain switch, I simply could not find a place to hook it up, as an independent call, instead opting for desolve() to have an extra parameter. If you read desolve() code you see that it digs up the maxima instance to call, as the parent of the expression. I could not find an independent, "global" hook to call maxima, and it seems that it might not even be exposed. It seems that the original design aimed at possibly many independent maxima instances to be run.
</p>
</blockquote>
<p>
Yes, and still is. But you should be able to import <code>sage.calculus.calculus.maxima</code> and the like.
</p>
</blockquote>
<p>
I think I experimented with something like this, and switching domain this way had no effect! But I might be wrong. I'll recheck. We'll be travelling NZSingapore in the coming 16 hours, so it would take time.
</p>
TicketmjoWed, 29 Feb 2012 20:36:50 GMT
https://trac.sagemath.org/ticket/10682#comment:39
https://trac.sagemath.org/ticket/10682#comment:39
<p>
We might as well just add <code>domain</code> as a @property on maxima_lib. That way we can do,
</p>
<pre class="wiki">old_domain = maxima.domain
maxima.domain = 'real'
result = perform_things()
maxima.domain = old_domain
return result
</pre>
TicketnbruinWed, 29 Feb 2012 21:16:14 GMT
https://trac.sagemath.org/ticket/10682#comment:40
https://trac.sagemath.org/ticket/10682#comment:40
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:32" title="Comment 32">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Anyway, isn't there something in Maxima which "knows" that a variable is real after such assumptions ?
</p>
</blockquote>
<p>
It's just a little weak in its deductions:
</p>
<pre class="wiki">(%i1) domain: complex;
(%o1) complex
(%i2) display2d: false;
(%o2) false
(%i3) sqrt(x^2);
(%o3) sqrt(x^2)
(%i4) assume(x>0);
(%o4) [x > 0]
(%i5) sqrt(x^2);
(%o5) x
(%i6) sqrt(4/x^2);
(%o6) sqrt(4/x^2)
(%i7) domain:real;
(%o7) real
(%i8) sqrt(4/x^2);
(%o8) 2/x
</pre><p>
It is ingrained now that the calculus maxima runs with <code>domain: complex</code>, so if you want to temporarily switch to <code>domain: real</code>, you should switch back afterwards. In principle this could play havoc with multiple threads, but maxima is not threadsafe anyway and our signal management isn't compatible with ECL's ideas of dealing with threads either. So, if in <code>desolve</code> (conditional on domain="real") we just do:
</p>
<pre class="wiki">P("load('contrib_ode)")
...
try:
P("domain: real")
soln = P(cmd)
finally:
P("domain: complex")
</pre><p>
we should be fine (the "try ... finally" should guard that we end up with <code>domain: complex</code>). I don't think performance is a concern if we're doing a "load" every time anyway!.
</p>
TicketjpfloriWed, 29 Feb 2012 22:36:52 GMT
https://trac.sagemath.org/ticket/10682#comment:41
https://trac.sagemath.org/ticket/10682#comment:41
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:40" title="Comment 40">nbruin</a>:
</p>
<blockquote class="citation">
<p>
ideas of dealing with threads either. So, if in <code>desolve</code> (conditional on domain="real") we just do: <code> P("load('contrib_ode)") ... try: P("domain: real") soln = P(cmd) finally: P("domain: complex") </code> we should be fine (the "try ... finally" should guard that we end up with <code>domain: complex</code>). I don't think performance is a concern if we're doing a "load" every time anyway!.<sup></sup>
</p>
</blockquote>
<p>
That's the kind of solution I would propose.
</p>
<p>
As far as the Maxima instance is concerned, you get it through the ._maxima_().parent() call in nintegral(...) and it happens to be the unique library instance of Maxima (which is currently used for all the calculus stuff). In particular, a call to nintegral should not modify its settings.
</p>
<p>
To refine Nils answer, Id say that in try we set domain to the option passed and in "finally" we should restore the previous value of domain which could be "real"rather than "complex" if the user decided to change it globally.
</p>
<p>
To get the previous value something like str(P('domain').strip(' ') would do at the Sage level although this is kind of dirty (and surely slow)
</p>
TicketdimpaseThu, 01 Mar 2012 15:51:20 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:42
https://trac.sagemath.org/ticket/10682#comment:42
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
please review the revised patch. I have fixed the formatting problems and removed the <code>domain</code> parameter (which I added in the previous version of the patch) from <code>desolve().</code>
</p>
<p>
Instead, I simply call <code>sage: sage.calculus.calculus.maxima('domain:real')</code>
and <code>sage: sage.calculus.calculus.maxima('domain:complex')</code> in the docstest.
This works just as well.
</p>
TicketjpfloriThu, 01 Mar 2012 16:14:18 GMTstatus, work_issues changed; reviewer, keywords, author set
https://trac.sagemath.org/ticket/10682#comment:43
https://trac.sagemath.org/ticket/10682#comment:43
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>JeanPierre Flori,</em>
</li>
<li><strong>work_issues</strong>
changed from <em>several doctests need to be patched due to changes in output format/term order</em> to <em>domain issue, error handling in nintegral</em>
</li>
<li><strong>keywords</strong>
<em>maxima</em> <em>5.26.0</em> <em>binomial</em> <em>sum</em> added
</li>
<li><strong>author</strong>
set to <em>Dima Pasechnick</em>
</li>
</ul>
<p>
I would rather follow Nils approach and at least include a try/finally statement in cas an error is raised.
</p>
<p>
Doing so, we're sure not to let the domain set to real after leaving the function because of an error.
</p>
<p>
Then, you'd better use "P" already present in the code rather than sage.calculus.calculs.maxima so that you're sure to use the same maxima as the one that will be used for solving the equation. Currently it's equivalent, but if something else changes in the future, such a choice will make everything easier.
</p>
<p>
Finally, on my side, I'd really prefer first to use the option approach you already implemented, and then to bakup the previous value of domain and set it back in the finally statement (rather than setting it to complex all the time).*
</p>
<p>
If you want to do it more cleanly, you can use
</p>
<pre class="wiki">def desolve(..., domain='real')
...
olddomain=P.get('domain')
P.set('domain',domain)
...
try:
...
finally:
P.set('domain',olddomain)
...
</pre><p>
rather than P('domain :'+str)
</p>
<p>
And the error handling at the end of nintegral should definitely be reworked.
</p>
<p>
For example the test "if 'quad_qags' in str(v)" is useless now that Maxima behavior has changed.
</p>
<p>
Moreover, I'm not sure the "ERROR in srt(err)" can ever be reached now (was it before???).
</p>
<p>
I could not locate the commit corresponding to this change although I tried a little hard. Maxima source tree is a little obscure for me. This commit should be mentionned in the doc and would help to answer the two above remarks about error handling. Some expert of Maxima is needed for that.
</p>
<p>
A last question is to know what we do when nintegral returns an error code.
</p>
<p>
Do we let the situation as is? Or do we raise an error. I would agrre that this should be treated in another ticket, but I also don't think there is a better choice to make. Some discussion should happen.
</p>
TicketmjoThu, 01 Mar 2012 16:56:34 GMT
https://trac.sagemath.org/ticket/10682#comment:44
https://trac.sagemath.org/ticket/10682#comment:44
<p>
The whole
</p>
<pre class="wiki">desolve(..., make_it_work=True)
</pre><p>
approach is silly. If we don't get a solution without it, we should use contrib_ode. If we know the variables are real, we should set domain=real.
</p>
<p>
It might require a second ticket and some work on <a class="missing wiki">MaximaLib?</a>/desolve, but we shouldn't just paper over the fact that this doesn't work (and is a regression in that regard). Requiring the user to manually set the domain <strong>is</strong> a regression.
</p>
TicketnbruinThu, 01 Mar 2012 17:16:34 GMT
https://trac.sagemath.org/ticket/10682#comment:45
https://trac.sagemath.org/ticket/10682#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:43" title="Comment 43">jpflori</a>:
</p>
<blockquote class="citation">
<p>
And the error handling at the end of nintegral should definitely be reworked.
</p>
<p>
For example the test "if 'quad_qags' in str(v)" is useless now that Maxima behavior has changed.
</p>
<p>
Moreover, I'm not sure the "ERROR in srt(err)" can ever be reached now (was it before???).
</p>
<p>
I could not locate the commit corresponding to this change although I tried a little hard. Maxima source tree is a little obscure for me. This commit should be mentionned in the doc and would help to answer the two above remarks about error handling. Some expert of Maxima is needed for that.
</p>
</blockquote>
<p>
I did find <a class="extlink" href="http://maxima.git.sourceforge.net/git/gitweb.cgi?p=maxima/maxima;a=commitdiff;h=df7393f0f2433cd60937e42b7cae624add3ca085"><span class="icon"></span>this commit</a> in the history of <code>src/numerical/slatec/quadpack.lisp</code>. It seems to indicate that something about quadpack error reporting has changed and may have become more configurable. Perhaps we can configure it in a way that the reported errors are more suitable and detectable for us?
</p>
<p>
desolve: I think Dima's new approach is acceptable in that it both fixes the doctest and documents the regression. Did anybody ever check if the returned solution with <code>domain: complex</code> is actually correct with some crazy combination of branch cut choices?
</p>
TicketmjoThu, 01 Mar 2012 17:32:47 GMT
https://trac.sagemath.org/ticket/10682#comment:46
https://trac.sagemath.org/ticket/10682#comment:46
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:45" title="Comment 45">nbruin</a>:
</p>
<blockquote class="citation">
<p>
desolve: I think Dima's new approach is acceptable in that it both fixes the doctest and documents the regression. Did anybody ever check if the returned solution with <code>domain: complex</code> is actually correct with some crazy combination of branch cut choices?
</p>
</blockquote>
<p>
It fixes the doctest in the same way that deleting the doctest fixes the doctest.
</p>
<p>
All we have to do to fix <em>this</em> ticket is backport simplify_sum.mac:
</p>
<pre class="wiki">sage: maxima.version()
'5.24.0'
sage: n,k = var('n,k')
sage: sum(binomial(n,k)*k^2, k, 2, n)
1/4*(n^2 + n)*2^n  n
</pre><p>
If we're going to upgrade maxima after that, we should do it properly, i.e. no regressions.
</p>
TicketjpfloriThu, 01 Mar 2012 17:36:10 GMT
https://trac.sagemath.org/ticket/10682#comment:47
https://trac.sagemath.org/ticket/10682#comment:47
<p>
I saw that commit but I must admit I do not understand whether that actually modified the behavior of Maxima and slatec or not.
</p>
<p>
My point here is that we should get rid of the now useless lines of code and if possible document it properly either in the file itself or in SPKG.txt with a pointer to the corresponding Maxima commit and some explanation.
</p>
<p>
Deciding whether the current behavior of letting the user know that an error occurred through the last value of the tuple (as maxima does) which is already document, or whether we should actually look at that value in Sage itself and raise an actual Error accordingly can be done later (or not, I'll personally live with it as it is).
</p>
TicketjpfloriThu, 01 Mar 2012 17:42:07 GMT
https://trac.sagemath.org/ticket/10682#comment:48
https://trac.sagemath.org/ticket/10682#comment:48
<p>
I also quite agree with mjo.
</p>
<p>
If possible, let's just update the 5.25 spkg with an additional patch for the sum (as Dima just did for the false stuff), so that it is quickly reviewed and merged.
</p>
<p>
And let's upgrade maxima in another ticket (and another one for error handling, and potentially another one for a real solution to the domain stuff, my preferred solution for that is the domain option with try/finally and real by default, for mjo I'm not sure we can call it a regression, did Sage (and Maxima?) actually outputed something different for domain equal to real or complex before? or was it always as it is now with domain= real?).
</p>
TicketmjoThu, 01 Mar 2012 18:06:42 GMT
https://trac.sagemath.org/ticket/10682#comment:49
https://trac.sagemath.org/ticket/10682#comment:49
<p>
It always returned,
</p>
<pre class="wiki">[x  arcsinh(y(x)/x) == c]
</pre><p>
regardless of the <code>domain</code>.
</p>
<p>
But, it used to give the correct answer, and now it doesn't.
</p>
<p>
I'm running a ptestlong now with simplify_sum.mac from 5.26. If that (and the maxima tests) pass, I would certainly give a patched 5.24 a positive review.
</p>
TicketdimpaseFri, 02 Mar 2012 00:17:25 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:50
https://trac.sagemath.org/ticket/10682#comment:50
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:49" title="Comment 49">mjo</a>:
</p>
<blockquote class="citation">
<p>
It always returned, <code>[x  arcsinh(y(x)/x) == c]</code>
</p>
<p>
regardless of the <code>domain</code>.
</p>
<p>
But, it used to give the correct answer, and now it doesn't.
</p>
<p>
I'm running a ptestlong now with simplify_sum.mac from 5.26. If that (and the maxima tests) pass, I would certainly give a patched 5.24 a positive review.
</p>
</blockquote>
<p>
Can you demonstrate in some way that the solution it gives in the complex domain is wrong? If yes, you have a point. Still, if it were wrong we would have a good Maxima bug report to file. And we have a workaround for this, which is good enough IMHO.
</p>
<p>
One way or another, I would much prefer have a patch in 5.26 than in 5.24. The latter is an obsolete version, and I see very little value in keeping it (I am sure they fixed a zillion other bugs in 5.26, which were not covered by Sage doctests). Just from the point of view of maintaining good relations with Maxima community it is important to upgrade to 5.26.
</p>
TicketdimpaseFri, 02 Mar 2012 01:35:31 GMT
https://trac.sagemath.org/ticket/10682#comment:51
https://trac.sagemath.org/ticket/10682#comment:51
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:50" title="Comment 50">dimpase</a>:
</p>
<blockquote class="citation">
<blockquote>
<p>
(I am sure they fixed a zillion other bugs in 5.26, which were not covered by Sage doctests).
</p>
</blockquote>
</blockquote>
<p>
E.g. see my comment above about the quality of numerical approximation in
matrix/matrix2.pyx, line 10369, test, as an example of improvement. Actually, the quality of the approximation returned there by the current maxima spkg is so poor that it can be regarded as a bug!
</p>
TicketdimpaseFri, 02 Mar 2012 01:51:40 GMT
https://trac.sagemath.org/ticket/10682#comment:52
https://trac.sagemath.org/ticket/10682#comment:52
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:43" title="Comment 43">jpflori</a>:
</p>
<blockquote class="citation">
<p>
I would rather follow Nils approach and at least include a try/finally statement in cas an error is raised.
</p>
<p>
Doing so, we're sure not to let the domain set to real after leaving the function because of an error.
</p>
<p>
Then, you'd better use "P" already present in the code rather than sage.calculus.calculs.maxima so that you're sure to use the same maxima as the one that will be used for solving the equation. Currently it's equivalent, but if something else changes in the future, such a choice will make everything easier.
</p>
</blockquote>
<p>
I really cannot imagine a scenario when one would run several instances of maxima in a totally uncoordinated way. If such a major change was ever to happen, surely a facility to execute a series of maxima commands in a given maxima instance would be available, and could be used then.
</p>
<p>
</p>
<blockquote class="citation">
<p>
Finally, on my side, I'd really prefer first to use the option approach you already implemented, and then to bakup the previous value of domain and set it back in the finally statement (rather than setting it to complex all the time).*
</p>
</blockquote>
<p>
That's a good point, regardless. As I said, when working on the previous patch I overlooked a way to set domain by another call to maxima.
</p>
<p>
[...]
</p>
<blockquote class="citation">
<p>
A last question is to know what we do when nintegral returns an error code.
</p>
<p>
Do we let the situation as is? Or do we raise an error. I would agrre that this should be treated in another ticket, but I also don't think there is a better choice to make. Some discussion should happen.
</p>
</blockquote>
<p>
It surely can be handled better, but it surely should happen on another ticket.
</p>
TicketmjoFri, 02 Mar 2012 02:50:46 GMT
https://trac.sagemath.org/ticket/10682#comment:53
https://trac.sagemath.org/ticket/10682#comment:53
<p>
I would be happy with the following:
</p>
<ol><li>Add a public method to set the maxima simplification domain. This is useful elsewhere, and is painequivalent to adding a <code>domain</code> parameter to the <code>desolve</code> function. Can be a separate ticket.
</li><li>Open another ticket to figure out the domain automatically in <code>desolve</code> where all the variables are known.
</li><li>Update this doctest to use that method, and add a reference to that ticket.
</li></ol>
TicketnbruinFri, 02 Mar 2012 07:09:15 GMT
https://trac.sagemath.org/ticket/10682#comment:54
https://trac.sagemath.org/ticket/10682#comment:54
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:49" title="Comment 49">mjo</a>:
</p>
<blockquote class="citation">
<p>
But, it used to give the correct answer, and now it doesn't.
</p>
</blockquote>
<p>
As far as I can check, the newly returned solution is valid:
</p>
<pre class="wiki">sage: y=function('y',x)
sage: sol=(log((2*sqrt(4/x^2)*x^2*sqrt((4*y^4+4*x^2*y^2)/x^2)+8*y^2+4*x^2)/x^2)
....: sqrt(4/x^2)*x*asinh(2*y^2/(x*sqrt(4*y^2)))
....: sqrt(4/x^2)*x*asinh(2*y/sqrt(4*x^2))+sqrt(4/x^2)*x^2)/(sqrt(4/x^2)*x)
sage: var('DY')
DY
sage: eqn=sol.diff(x).subs_expr(diff(y,x)==DY)
sage: dydx=eqn.coefficient(DY,0)/eqn.coefficient(DY,1)
sage: assume(x>0)
sage: E=x*dydxx*sqrt(y^2+x^2)y
sage: E.factor()
0
</pre><p>
so I don't think we need to consider maxima 5.26's response a regression in this respect. It is simply returning a different solution than 5.24 did when <code>domain: complex</code> is in effect.
</p>
TicketjpfloriFri, 02 Mar 2012 09:24:58 GMT
https://trac.sagemath.org/ticket/10682#comment:55
https://trac.sagemath.org/ticket/10682#comment:55
<p>
About the multiplie maxima instance, there is indeed only one library instance and I don't think that will change in the near future.
</p>
<p>
But one can instantiate pexpect interfaces (one or multiple one).
</p>
<p>
So if I'm a little bizarre and want to use such an interface, I'd like it to behave as much as possible as the library one (e.g. doing something like de_max = maxima(de) and then calling desolve on de_max, in that case we'll get a pexpect interface by the parent() call)..
</p>
<p>
Of course desole is not really implemented to work in that case and in fact it does not seem to do very well right now (it just returns different kinds of <a class="missing wiki">NotImplementedError?</a> telling that Maxima failed).
</p>
<p>
As a consequence we might also think of getting rid of the _maxima_() and parent() call and explicitely only use sage.calculus.calculus.maxima instead.
</p>
<p>
In that case I'll be completely happy with the use of sage.calculus.calculus.maxima for setting the domain.
</p>
<p>
Setting the domain can be domain with a simple call to set('domain','xxx') as mentioned above.
</p>
<p>
But I just discovered that we (I?) messed up something when building the library interface.
</p>
<p>
With the pexpect interface (be sure to actually start it with maxima.start()), one can use maxima.domain directly. This does not work with the library interface.
</p>
<p>
Of course we can also add a new method on top of that as well.
</p>
TicketkcrismanFri, 02 Mar 2012 13:59:45 GMTauthor changed
https://trac.sagemath.org/ticket/10682#comment:56
https://trac.sagemath.org/ticket/10682#comment:56
<ul>
<li><strong>author</strong>
changed from <em>Dima Pasechnick</em> to <em>Dima Pasechnik</em>
</li>
</ul>
<p>
Disagree with having the other interface instances behaving like the standard one. On the contrary, some people will probably want "vanilla" Maxima in that setting. That said, there should be a nice easy way to say "pexpect interface with the same defaults as the "calculus" copy" or something like that.
</p>
TicketdimpaseFri, 02 Mar 2012 14:16:29 GMT
https://trac.sagemath.org/ticket/10682#comment:57
https://trac.sagemath.org/ticket/10682#comment:57
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:55" title="Comment 55">jpflori</a>:
[...]
</p>
<blockquote class="citation">
<p>
Setting the domain can be domain with a simple call to set('domain','xxx') as mentioned above.
</p>
<p>
But I just discovered that we (I?) messed up something when building the library interface.
</p>
<p>
With the pexpect interface (be sure to actually start it with maxima.start()), one can use maxima.domain directly. This does not work with the library interface.
</p>
<p>
Of course we can also add a new method on top of that as well.
</p>
</blockquote>
<p>
OK, I am confused now  can we have a new public method to do the simplification domain switching in the library, i.e.
the equivalent of <code>sage.calculus.calculus.maxima('domain:real')</code>, etc., or it would not work due to a bug in the library interface? If the latter, it looks like one can give it the positive review now, right?
</p>
<p>
I am relieved to hear that there are "real" regressions  bedankt, Nils!
</p>
TicketdimpaseFri, 02 Mar 2012 14:18:32 GMT
https://trac.sagemath.org/ticket/10682#comment:58
https://trac.sagemath.org/ticket/10682#comment:58
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:57" title="Comment 57">dimpase</a>:
</p>
<blockquote class="citation">
<p>
I am relieved to hear that there are "real" regressions  bedankt, Nils!
</p>
</blockquote>
<p>
I meant to say "there are no 'real' regressions", of course...
</p>
TicketjpfloriFri, 02 Mar 2012 14:34:19 GMT
https://trac.sagemath.org/ticket/10682#comment:59
https://trac.sagemath.org/ticket/10682#comment:59
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:57" title="Comment 57">dimpase</a>:
</p>
<blockquote class="citation">
<p>
OK, I am confused now  can we have a new public method to do the simplification domain switching in the library, i.e. the equivalent of <code>sage.calculus.calculus.maxima('domain:real')</code>, etc., or it would not work due to a bug in the library interface? If the latter, it looks like one can give it the
</p>
</blockquote>
<p>
You're also confusing me now :)
</p>
<p>
I really do not like and understand how trac treats newline stuff, whence my maybe confusing message above. I'll try to format that one better.
</p>
<p>
I'll also says a lot a lot of trivialities, but it's to be sure that we all agree on what we want or do not want.
</p>
<p>
Fisrt, sage.calculus.calculus.maxima('domain: real') does work as intended indeed.
</p>
<p>
If you think this is not public enough we can add a new method "set_domain" in <a class="missing wiki">MaximaAbstract?</a> class which does the same thing:
</p>
<pre class="wiki">def set_domain(self, domain):
r"""
Sets the working domain of this Maxima instance to `domain`.
"""
self('domain : "+domain)
</pre><p>
or equivalently
</p>
<pre class="wiki">def set_domain(self, domain):
r"""
Sets the working domain of this Maxima instance to `domain`.
"""
self.set('domain', domain)
</pre><p>
with some basic doctesting.
</p>
<p>
In that form I doubt this is really much more useful than calling sage.calculus.calculus.maxima(...).
</p>
<p>
Maybe you meant a function declared at the top level of sage.calculus.calculus which does the same ?
</p>
<p>
It's maybe a little more meaningful.
</p>
<p>
What I meant about the Maxima Library interface bug is that we should be able to access the "domain" variable of Maxima directly as an attribute of the Sage object, so that one could replace the above method by
</p>
<pre class="wiki">def set_domain(self, domain):
...
self.domain = domain
</pre><p>
or if its a function in sage.calculus.calculus :
</p>
<pre class="wiki">def set_domain(domain):
r"""
Sets the domain of the Maxima instance used by Sage for calculus to `domain`
""""
maxima.domain = domain
</pre><p>
This works for the pexpect interface but is broken for the library interface.
</p>
TicketjpfloriFri, 02 Mar 2012 14:37:20 GMT
https://trac.sagemath.org/ticket/10682#comment:60
https://trac.sagemath.org/ticket/10682#comment:60
<p>
And I do not think that this should be positively reviewed yet: the error handling changes of Maxima in nintegral should be documented and the now useless lines removed.
</p>
TicketnbruinFri, 02 Mar 2012 20:24:58 GMT
https://trac.sagemath.org/ticket/10682#comment:61
https://trac.sagemath.org/ticket/10682#comment:61
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:60" title="Comment 60">jpflori</a>:
</p>
<blockquote class="citation">
<p>
And I do not think that this should be positively reviewed yet: the error handling changes of Maxima in nintegral should be documented and the now useless lines removed.
</p>
</blockquote>
<p>
Indeed, if you read <a class="extlink" href="http://www.math.utexas.edu/pipermail/maxima/2008/012975.html"><span class="icon"></span>this thread</a> it seems that in maxima 5.16.2 they decided to return the expression in case of error in evaluation and that this decision was reversed, so now they're just reporting the error codes as computed by quadpack. Those error codes are documented in the <code>sage.calculus.calculus.nintegral</code> docstring, so probably the lines
</p>
<pre class="wiki"> #This is just a work around until there is a response to
#http://www.math.utexas.edu/pipermail/maxima/2008/012975.html
if 'quad_qags' in str(v):
raise ValueError, "Maxima (via quadpack) cannot compute the integral to that precision"
</pre><p>
could just be removed. A workaround is not necessary anymore. If we prefer to raise errors rather than rely on checking error codes, we could do
</p>
<pre class="wiki"> errorcode_dict = {
1 : 'too many subintervals were done',
2 : 'excessive roundoff error detected',
3 : 'extremely bad integrand behavior occurs',
4 : 'failed to converge',
5 : 'integral is probably divergent or slowly convergent',
6 : 'invalid input'} #this def can go outside function
cond=int(v[3])
if cond in errorcode_dict:
raise RuntimeError("Maxima (via quadpack) reports an error:'%s'"%error_code_dict(cond)
else:
return float(v[0]), float(v[1]), Integer(v[2]), Integer(v[3])
</pre><p>
However, once you start changing things like that, we should perhaps change the return value format (or make it a parameter 'raise_errors_rather_than_return_error_codes').
</p>
TicketnbruinFri, 02 Mar 2012 21:49:10 GMT
https://trac.sagemath.org/ticket/10682#comment:62
https://trac.sagemath.org/ticket/10682#comment:62
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:61" title="Comment 61">nbruin</a>:
</p>
<pre class="wiki"> #This is just a work around until there is a response to
#http://www.math.utexas.edu/pipermail/maxima/2008/012975.html
if 'quad_qags' in str(v):
raise ValueError, "Maxima (via quadpack) cannot compute the integral to that precision"
</pre><p>
My apologies! I've looked at the <a class="extlink" href="http://maxima.git.sourceforge.net/git/gitweb.cgi?p=maxima/maxima;a=blob;f=src/numerical/slatec/quadpack.lisp;h=c07efea3ce3b57e7797c692aa25dc7dee5feb8b5;hb=HEAD#l48"><span class="icon"></span>quadpack.lisp source</a> and as you might notice, there's a <code>handlercase ... error</code> construct there, which is a lisp version of <code>try ... except</code>. The error clause constructs a return value which would be an unevaluated <code>quad_qags</code> call in maxima. So the raise above could in principle still be triggered (e.g., give an integrand that triggers an error when evaluated in a way that's not caught otherwise). We need to leave this in. One could leave out the try...except around the maxima call if the error messages produced by maxima are sufficiently informative.
</p>
<p>
The patch that triggered Mike Hansen's request and Dodier's answer is probably <a class="extlink" href="http://maxima.git.sourceforge.net/git/gitweb.cgi?p=maxima/maxima;a=commitdiff;h=9f156949a789dab37bf548acc20a08f4fd9c6086"><span class="icon"></span>this one</a>. That code is still there as far as I can see. I think we're best off leaving both error checks in there. The change in behaviour between 5.24 to 5.26 is likely caused somewhere else. The 5.26 behaviour is still as documented (provided we consider 1e14 as 'invalid input').
</p>
<p>
Whether we want to raise an error rather than return an error code is an independent question.
</p>
TicketdimpaseSat, 03 Mar 2012 08:56:51 GMT
https://trac.sagemath.org/ticket/10682#comment:63
https://trac.sagemath.org/ticket/10682#comment:63
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:60" title="Comment 60">jpflori</a>:
</p>
<blockquote class="citation">
<p>
And I do not think that this should be positively reviewed yet: the error handling changes of Maxima in nintegral should be documented and the now useless lines removed.
</p>
</blockquote>
<p>
I read in Nils' latest comment that there are no useless lines to remove, right? Regarding the documentation, how about adding
the following to the patch:
</p>
<pre class="wiki">diff git a/sage/calculus/calculus.py b/sage/calculus/calculus.py
 a/sage/calculus/calculus.py
+++ b/sage/calculus/calculus.py
@@ 632,7 +632,8 @@
 ``5``  integral is probably divergent or slowly
convergent
  ``6``  the input is invalid
+  ``6``  the input is invalid; this includes the case of desired_relative_error
+ being too small to be achieved
ALIAS: nintegrate is the same as nintegral
</pre><p>
would this be sufficient?
</p>
TicketjpfloriSat, 03 Mar 2012 10:20:44 GMT
https://trac.sagemath.org/ticket/10682#comment:64
https://trac.sagemath.org/ticket/10682#comment:64
<p>
Im currently looking at Maxima's code and am not yet convinced of what changed in the last version.
</p>
<p>
There' indeed the possibility that an error get raised by Maxima (handler ... error stuff) if some values cannot be converted to floats floatorlose stuff).
</p>
<p>
In this case Maxima returns the partially evaluated expression.
</p>
<p>
But that does not explain why Maxima now returns (0,0,0,6) rather than the old unevaluated expression for integrating x with a crazy expression. I'm currently looking at dqags.lisp but did not find anything meaningful yet.
</p>
<p>
In particular, it also makes me wonder whether the message of the Sage error (for the "workaround", but also for the earlier try/catch stuff) is meaningful.
</p>
<p>
Will it ever be related to precision problems ? because Maxima seems not to complain any more.
</p>
<p>
and as I said earlier, returning Error ourselves when when detect that the last value of the tuple and as Nils proposes to do, seems a good idea to me, but should be done later. I'd just like to actually reflect what Maxima tells at the Sage level even though the end user still has to check this last value for a moment.
</p>
TicketnbruinSat, 03 Mar 2012 22:39:28 GMT
https://trac.sagemath.org/ticket/10682#comment:65
https://trac.sagemath.org/ticket/10682#comment:65
<p>
OK, this poking around in maxima's source is really quite unproductive, but I think I have located the source of the change in behaviour. With maxima 5.26, observe:
</p>
<pre class="wiki">sage: x.nintegrate(x,0,1,1e14)
(0.0, 0.0, 0, 6)
sage: from sage.libs.ecl import *
sage: ecl_eval("(setf f2cllib::*stopsignalserrorp* T)")
<ECL: T>
sage: x.nintegrate(x,0,1,1e14)
ValueError: Maxima (via quadpack) cannot compute the integral to that precision
</pre><p>
whereas with maxima 5.23.2 we have the opposite:
</p>
<pre class="wiki">sage: x.nintegrate(x,0,1,1e14)
ValueError: Maxima (via quadpack) cannot compute the integral to that precision
sage: from sage.libs.ecl import *
sage: ecl_eval("(setf f2cllib::*stopsignalserrorp* T)")
<ECL: T>
sage: x.nintegrate(x,0,1,1e14)
(0.0, 0.0, 0, 6)
</pre><p>
I think <a class="extlink" href="http://maxima.git.sourceforge.net/git/gitweb.cgi?p=maxima/maxima;a=commitdiff;h=9067349233cc217786983b07c23a2b78cf7d27bc"><span class="icon"></span>this commit</a> is responsible]:
</p>
<pre class="wiki"> (defun stop (&optional arg)
(when arg
(format cl::*erroroutput* "~A~%" arg))
 (unless *stopsignalserrorp*
+ (when *stopsignalserrorp*
(cerror "Continue anyway" "STOP reached")))
</pre><p>
In later commits, the following is added:
</p>
<pre class="wiki">+;;; Revision 1.116 2011/02/20 20:51:04 rtoy
+;;; Oops. STOP should signal an error if *STOPSIGNALSERRORP* is
+;;; nonNIL.
</pre><p>
so I think the maxima 5.26 behaviour is as intended. I think you can give Dima's patch a positive review now. Everything is explained and any improvements should be handled on future tickets.
</p>
TicketjpfloriSat, 03 Mar 2012 22:56:44 GMT
https://trac.sagemath.org/ticket/10682#comment:66
https://trac.sagemath.org/ticket/10682#comment:66
<p>
Thanx a lot for locating this! It would have taken me ages to do so. But you did not quite answered one of my questions: we won't get the last error because of precision stuff, so this can get removed? My point is that those last lines were explicitely made to catch that specific error that is not reported as such anymore, so I think it's useless, or if it can then it won't be becase of precision provlems anymore so the stuff about precision should be removed (and the reference to the maxima mailing list should be removed as well).
</p>
<p>
I also agree it's not really important from a usefulness point of view, but having a good understanding of what we use in Sage is and our code should reflect that.
</p>
TicketnbruinSat, 03 Mar 2012 23:02:32 GMT
https://trac.sagemath.org/ticket/10682#comment:67
https://trac.sagemath.org/ticket/10682#comment:67
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:66" title="Comment 66">jpflori</a>:
</p>
<blockquote class="citation">
<p>
we won't get the last error because of precision stuff, so this can get removed? My point is that those last lines were explicitely made to catch that specific error that is not reported as such anymore, so I think it's useless, or if it can then it won't be becase of precision provlems anymore so the stuff about precision should be removed
</p>
</blockquote>
<p>
No, you already noted yourself that a failure to convert to float can trigger this:
</p>
<pre class="wiki">sage: var('n')
sage: x.nintegrate(x,0,n)
...
> 712 raise ValueError, "Maxima (via quadpack) cannot compute the integral to that precision"
...
ValueError: Maxima (via quadpack) cannot compute the integral to that precision
</pre><p>
that's why I said before that this check is necessary.
</p>
TicketjpfloriSat, 03 Mar 2012 23:04:36 GMT
https://trac.sagemath.org/ticket/10682#comment:68
https://trac.sagemath.org/ticket/10682#comment:68
<p>
Yup but that won't be because of too high precision and that's what our error message says...
</p>
<p>
So let's just remove the "to that precision" part.
</p>
TicketjpfloriSat, 03 Mar 2012 23:05:11 GMT
https://trac.sagemath.org/ticket/10682#comment:69
https://trac.sagemath.org/ticket/10682#comment:69
<p>
As soon as we agree on this last point, I'll be happy to provide a reviewer patch fixing some formatting problems (spaces at end of lines and stuff like that) and give the ticket a positive review.
</p>
<p>
To be clear about the domain stuff, I don't think we really need an additional "public" method to set it, fixing the maxima library interface so that sage.calculus.calculus.maxima.domain works as intended (ie can be read and assigned directly) seems enough for me.<br />
</p>
<p>
And sorry about the typos above, it's late here...<br />
</p>
TicketnbruinSat, 03 Mar 2012 23:42:05 GMT
https://trac.sagemath.org/ticket/10682#comment:70
https://trac.sagemath.org/ticket/10682#comment:70
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:68" title="Comment 68">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Yup but that won't be because of too high precision and that's what our error message says...
</p>
<p>
So let's just remove the "to that precision" part.
</p>
</blockquote>
<p>
Yes, you're right. Agreed. In fact, I expect that any precision problems will always be reported via the error code, so they will *never* come out as an error or an unevaluated call. This only happened before because fortan's "STOP" raised an error, which it doesn't do now anymore.
</p>
<p>
That also makes me think that the body should be
</p>
<pre class="wiki"> v = ex._maxima_().quad_qags(x, a, b,
epsrel=desired_relative_error,
limit=maximum_num_subintervals)
#If an error gets raised during evaluation, maxima just returns the unevaluated expression
#we raise an error instead
if 'quad_qags' in str(v):
raise ValueError, "Maxima (via quadpack) cannot compute the integral"
return float(v[0]), float(v[1]), Integer(v[2]), Integer(v[3])
</pre><p>
i.e., just let <code>quad_qags</code> raise its own errors instead of masking it with a blanket error message. Presently we are not aware of any error that would get raised under normal operation in there anyway, so we would want to know what went wrong.
</p>
TicketdimpaseSun, 04 Mar 2012 06:23:12 GMT
https://trac.sagemath.org/ticket/10682#comment:71
https://trac.sagemath.org/ticket/10682#comment:71
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:70" title="Comment 70">nbruin</a>:
OK, I'll update the patch as follows:
</p>
<pre class="wiki">diff git a/sage/calculus/calculus.py b/sage/calculus/calculus.py
 a/sage/calculus/calculus.py
+++ b/sage/calculus/calculus.py
@@ 632,7 +632,8 @@
 ``5``  integral is probably divergent or slowly
convergent
  ``6``  the input is invalid
+  ``6``  the input is invalid; this includes the case of desired_relative_error
+ being too small to be achieved
ALIAS: nintegrate is the same as nintegral
@@ 641,9 +642,6 @@
integration using the GSL C library. It is potentially much faster
and applies to arbitrary user defined functions.
 Also, there are limits to the precision to which Maxima can compute
 the integral: here the error code "6" means "the input is invalid".

::
sage: f = x
@@ 714,14 +712,14 @@
limit=maximum_num_subintervals)
except TypeError, err:
if "ERROR" in str(err):
 raise ValueError, "Maxima (via quadpack) cannot compute the integral to that precision"
+ raise ValueError, "Maxima (via quadpack) cannot compute the integral"
else:
raise TypeError, err
#This is just a work around until there is a response to
#http://www.math.utexas.edu/pipermail/maxima/2008/012975.html
if 'quad_qags' in str(v):
 raise ValueError, "Maxima (via quadpack) cannot compute the integral to that precision"
+ raise ValueError, "Maxima (via quadpack) cannot compute the integral"
return float(v[0]), float(v[1]), Integer(v[2]), Integer(v[3])
</pre>
TicketnbruinMon, 05 Mar 2012 17:29:34 GMT
https://trac.sagemath.org/ticket/10682#comment:72
https://trac.sagemath.org/ticket/10682#comment:72
<p>
In case anyone is waiting for my response:
</p>
<ul><li>Dima's proposed patch update is fine with me
</li><li>I can't find Dima's updated patch on the ticket: Dima, in case you forgot to actually upload the amended patch, please do!
</li><li>After that, JP can fix some of the formatting that bothered him and then, for the first time since ages, we can ship an uptodate maxima with the new sage release!
</li></ul>
TicketdimpaseMon, 05 Mar 2012 23:51:06 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:73
https://trac.sagemath.org/ticket/10682#comment:73
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:72" title="Comment 72">nbruin</a>:
</p>
<blockquote class="citation">
<p>
In case anyone is waiting for my response:
</p>
<ul><li>Dima's proposed patch update is fine with me
</li><li>I can't find Dima's updated patch on the ticket: Dima, in case you forgot to actually upload the amended patch, please do!
</li></ul></blockquote>
<p>
I haven't forgotten, I was just waiting for OK...
It's updated now.
</p>
<blockquote class="citation">
<ul><li>After that, JP can fix some of the formatting that bothered him and then, for the first time since ages, we can ship an uptodate maxima with the new sage release!
</li></ul></blockquote>
TicketjpfloriTue, 06 Mar 2012 13:32:10 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:74
https://trac.sagemath.org/ticket/10682#comment:74
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
I'm in the process of fixing some formatting issues (from my point of view) in Sage source tree and in the SPKG itself.
</p>
<p>
Two remarks:
</p>
<ul><li>I bumped the version number to 5.26.0.p0 because we use a patch on top of 5.26.0
</li></ul><ul><li>there is no spkcheck although maxima provides a test suite.
</li></ul><p>
I quickly looked for a good reason why there is no such file and could not yet locate one.
</p>
<p>
Therefore, I'm currently testing "make check" and I got one failure in rtest8 related to some quad_qawf not getting evaluated (this smells like an integration issue like above).
</p>
<p>
This gives me 1 test failed out of 9,121 in 360 secs (and it does not return an error at the console level...)
</p>
<p>
Can someone reproduce that failure with SPKG_CHECK=yes and the updated spkg at <a class="extlink" href="http://perso.telecomparistech.fr/~flori/maxima5.26.0.p0.spkg"><span class="icon"></span>http://perso.telecomparistech.fr/~flori/maxima5.26.0.p0.spkg</a>
</p>
TicketjpfloriTue, 06 Mar 2012 13:41:59 GMT
https://trac.sagemath.org/ticket/10682#comment:75
https://trac.sagemath.org/ticket/10682#comment:75
<p>
Sorry, there is a "sage" missing between flori and maxima...
</p>
<p>
So it is <a class="extlink" href="http://perso.telecomparistech.fr/~flori/sage/maxima5.26.0.p0.spkg"><span class="icon"></span>http://perso.telecomparistech.fr/~flori/sage/maxima5.26.0.p0.spkg</a>
</p>
<p>
And this issue is already reported here
</p>
<p>
<a class="extlink" href="http://sourceforge.net/mailarchive/message.php?msg_id=28697907"><span class="icon"></span>http://sourceforge.net/mailarchive/message.php?msg_id=28697907</a>
</p>
<p>
although I'm not on OpenBSD/i386 but on a Ubuntu x86_64 system which surely is less important that the underlying lisp.<br />
</p>
TicketjpfloriTue, 06 Mar 2012 13:44:47 GMTreviewer, work_issues changed
https://trac.sagemath.org/ticket/10682#comment:76
https://trac.sagemath.org/ticket/10682#comment:76
<ul>
<li><strong>reviewer</strong>
changed from <em>JeanPierre Flori,</em> to <em>JeanPierre Flori, Nils Bruin</em>
</li>
<li><strong>work_issues</strong>
changed from <em>domain issue, error handling in nintegral</em> to <em>spkgcheck</em>
</li>
</ul>
<p>
Here is the address on the byg tracker:
</p>
<p>
<a class="extlink" href="http://sourceforge.net/tracker/?func=detail&atid=104933&aid=3475996&group_id=4933"><span class="icon"></span>http://sourceforge.net/tracker/?func=detail&atid=104933&aid=3475996&group_id=4933</a>
</p>
TicketkcrismanTue, 06 Mar 2012 14:13:43 GMT
https://trac.sagemath.org/ticket/10682#comment:77
https://trac.sagemath.org/ticket/10682#comment:77
<blockquote class="citation">
<p>
although I'm not on OpenBSD/i386 but on a Ubuntu x86_64 system which surely is less important that the underlying lisp.<br />
</p>
</blockquote>
<p>
Correct; the Maxima list is always full of minor failures in their (very large, I believe) test suite. This shouldn't necessarily hold up positive review, because it is quite dependent on the Lisp, if I recall from reading those messages. Also, adding the test suite should in theory be another ticket; no need to hold up having an actual uptodate Maxima!
</p>
TicketjpfloriTue, 06 Mar 2012 16:50:41 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/10682#comment:78
https://trac.sagemath.org/ticket/10682#comment:78
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>spkgcheck</em> deleted
</li>
</ul>
<p>
Ok, I'll split the addition of the spkgcheck.
</p>
<p>
Nils, could you have a look at my "reviewer" patch (that I'll upload in a minute for the sage tree, you can already look at the mercurial log for the spkg, ignoring the last commit for the spkgcheck).
</p>
<p>
I got rid of a lot of "useless" spaces, jsut want to be sure every one is happy with that.
</p>
TicketjpfloriTue, 06 Mar 2012 16:51:27 GMTattachment set
https://trac.sagemath.org/ticket/10682
https://trac.sagemath.org/ticket/10682
<ul>
<li><strong>attachment</strong>
set to <em>trac_10682reviewer.patch</em>
</li>
</ul>
<p>
Reviewer patch, formatting issues
</p>
TicketjpfloriTue, 06 Mar 2012 17:14:19 GMTattachment set
https://trac.sagemath.org/ticket/10682
https://trac.sagemath.org/ticket/10682
<ul>
<li><strong>attachment</strong>
set to <em>trac_10682spkgdp.patch</em>
</li>
</ul>
<p>
spkg diff from author, for review only
</p>
TicketnbruinTue, 06 Mar 2012 18:44:19 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:79
https://trac.sagemath.org/ticket/10682#comment:79
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:78" title="Comment 78">jpflori</a>:
</p>
<blockquote class="citation">
<p>
I got rid of a lot of "useless" spaces, jsut want to be sure every one is happy with that.
</p>
</blockquote>
<p>
Appreciated! I happen to believe that while it's nice to fix whitespace, increasing the footprint of patches by fixing it in lines that are otherwise not affected is not worth it, so I'm not too happy about it. But I don't feel sufficiently strongly about it to enter into a discussion about whether you should put the whitespace back. So I won't hold up a positive review on this. You can make up your own mind on whether you want to fix whitespace like this in the future.
</p>
TicketjpfloriTue, 06 Mar 2012 18:58:26 GMTdescription changed
https://trac.sagemath.org/ticket/10682#comment:80
https://trac.sagemath.org/ticket/10682#comment:80
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=80">diff</a>)
</li>
</ul>
<p>
I usually do not do it, especially so many lines at a time, but my reviewer patch was all about removing some white spaces reintroduced by Dima, and split some overlongish lines (that's another tricky choice, I can leave with long lines, but it's much more readable in a terminal without them and according to our coding guidelines we should stick to 79 chars at max...).
</p>
<p>
So I chose to suppress them this time.
</p>
<p>
IIRC there was a discussion some time ago on saedevel about make a one time huge patch for suppressing spaces but some people, especially among sagecombinat, raised the problem that it would break patch queues and that we should rather ensure that no such new lines where added and fix for previous spaces made locally.
</p>
<p>
I hope my choice won't break anything.
</p>
<p>
Anyway, good that this ticket is finally positively reviewed.
</p>
TicketjdemeyerTue, 06 Mar 2012 19:54:22 GMTdescription, summary changed
https://trac.sagemath.org/ticket/10682#comment:81
https://trac.sagemath.org/ticket/10682#comment:81
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=81">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>sum fails with lower bound != 0 or 1 (upgrade maxima to 5.26)</em> to <em>Upgrade maxima to 5.26</em>
</li>
</ul>
TicketjdemeyerTue, 06 Mar 2012 19:59:37 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:82
https://trac.sagemath.org/ticket/10682#comment:82
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:80" title="Comment 80">jpflori</a>:
</p>
<blockquote class="citation">
<p>
IIRC there was a discussion some time ago on saedevel about make a one time huge patch for suppressing spaces but some people, especially among sagecombinat, raised the problem that it would break patch queues and that we should rather ensure that no such new lines where added and fix for previous spaces made locally.
</p>
</blockquote>
<p>
For precisely this reason I would prefer not to just remove all spaces everywhere. In code which is changed by the patch, it can be done. But not just everywhere like this.
</p>
TicketdimpaseWed, 07 Mar 2012 04:59:43 GMTstatus, description changed
https://trac.sagemath.org/ticket/10682#comment:83
https://trac.sagemath.org/ticket/10682#comment:83
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=83">diff</a>)
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:82" title="Comment 82">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:80" title="Comment 80">jpflori</a>:
</p>
<blockquote class="citation">
<p>
IIRC there was a discussion some time ago on saedevel about make a one time huge patch for suppressing spaces but some people, especially among sagecombinat, raised the problem that it would break patch queues and that we should rather ensure that no such new lines where added and fix for previous spaces made locally.
</p>
</blockquote>
<p>
For precisely this reason I would prefer not to just remove all spaces everywhere. In code which is changed by the patch, it can be done. But not just everywhere like this.
</p>
</blockquote>
<p>
would it be OK if we just not apply the reviewer's patch?
</p>
TicketjpfloriWed, 07 Mar 2012 06:41:01 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:84
https://trac.sagemath.org/ticket/10682#comment:84
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:83" title="Comment 83">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:82" title="Comment 82">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:80" title="Comment 80">jpflori</a>:
</p>
<blockquote class="citation">
<p>
IIRC there was a discussion some time ago on saedevel about make a one time huge patch for suppressing spaces but some people, especially among sagecombinat, raised the problem that it would break patch queues and that we should rather ensure that no such new lines where added and fix for previous spaces made locally.
</p>
</blockquote>
<p>
For precisely this reason I would prefer not to just remove all spaces everywhere. In code which is changed by the patch, it can be done. But not just everywhere like this.
</p>
</blockquote>
<p>
would it be OK if we just not apply the reviewer's patch?
</p>
</blockquote>
<p>
I don't think so. My patch is overkill, but it's no reason to reintroduce additional formatting issues inside Sage code. If we go on like this, its going to be a real mess.
</p>
TicketdimpaseWed, 07 Mar 2012 06:54:06 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:85
https://trac.sagemath.org/ticket/10682#comment:85
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:84" title="Comment 84">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:83" title="Comment 83">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:82" title="Comment 82">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:80" title="Comment 80">jpflori</a>:
</p>
<blockquote class="citation">
<p>
IIRC there was a discussion some time ago on saedevel about make a one time huge patch for suppressing spaces but some people, especially among sagecombinat, raised the problem that it would break patch queues and that we should rather ensure that no such new lines where added and fix for previous spaces made locally.
</p>
</blockquote>
<p>
For precisely this reason I would prefer not to just remove all spaces everywhere. In code which is changed by the patch, it can be done. But not just everywhere like this.
</p>
</blockquote>
<p>
would it be OK if we just not apply the reviewer's patch?
</p>
</blockquote>
<p>
I don't think so. My patch is overkill, but it's no reason to reintroduce additional formatting issues inside Sage code. If we go on like this, its going to be a real mess.
</p>
</blockquote>
<p>
I haven't noticed that I have introduced any formatting issues. Could you point me out to this, or even better, of course, create a patch that fixes my faults?
Thanks!
</p>
TicketjpfloriWed, 07 Mar 2012 07:55:23 GMT
https://trac.sagemath.org/ticket/10682#comment:86
https://trac.sagemath.org/ticket/10682#comment:86
<p>
I'll provide a patch asap of course, that's basically trailing whitespaces (e.g. last lines of your patch) in your lines and too long lines (e.g. first line of your patch) and wrong indentation for some doc (e.g. first line of your patch).
</p>
<p>
It's just that the current patch do not apply on 4.8 and as compiling atlas on my computer takes ages and installing properly atlas on my debian seems broken...
</p>
<p>
More seriously, you also removed the lines
</p>
<p>
"Also, there are limits to the precision to which Maxima can compute..."
</p>
<p>
I agree that we do not raise an error anymore, but Maxima can still not compute up to that precision.
</p>
<p>
You also left the stuff about waiting for an answer on Maxima's list, but IIRC there has been an answer there, so what is done is not really a workaround, but rather a decision from Sage developpers.
</p>
TicketjpfloriWed, 07 Mar 2012 08:43:59 GMT
https://trac.sagemath.org/ticket/10682#comment:87
https://trac.sagemath.org/ticket/10682#comment:87
<p>
Last questions (I hope):
</p>
<p>
About the lines patched in wester.py, why did you add a semicolon after "n=var('n')" ?
</p>
<p>
And what do you mean exactly by "Maxima 5.26 does not do "just" simpify() here." (sic)?
</p>
<p>
That now Maxima cannot simplify this expression when it is only told that Re(x) >0 and Re(y)>0 and that we have to call simplify_exp() instead ?
</p>
<p>
I just check that this is indeed the case now and was not with Maxima's 5.24.0.p0 (ok this could be simply deduced from the doctest changes).
</p>
<p>
Is this a decision from Maxima's developers or a bug introduced recently? if the former holds a pointer to where the change was made would be welcome; if the latter holds, this should be reported upstream.
</p>
<p>
I guess that this is related to the domain stuff, because setting domain back to real answer right away.
</p>
<p>
And it's strange that the assumptions is not needed anymore.
</p>
<p>
In fact looking into the simplify_exp code shows that maxima.eval('domain: real$') is called before calling the radcan method which actually simplifies the expression.
</p>
<p>
In particular, this might actually be a Maxima regression.
</p>
TicketjpfloriWed, 07 Mar 2012 10:05:32 GMTstatus, description changed
https://trac.sagemath.org/ticket/10682#comment:88
https://trac.sagemath.org/ticket/10682#comment:88
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10682?action=diff&version=88">diff</a>)
</li>
</ul>
TicketnbruinWed, 07 Mar 2012 17:41:00 GMT
https://trac.sagemath.org/ticket/10682#comment:89
https://trac.sagemath.org/ticket/10682#comment:89
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:87" title="Comment 87">jpflori</a>:
</p>
<p>
Thanks for updating the patch! It's OK with me. I'll leave the specific questions for Dima to answer.
</p>
<blockquote class="citation">
<p>
That now Maxima cannot simplify this expression when it is only told that Re(x) >0 and Re(y)>0 and that we have to call simplify_exp() instead ?
In particular, this might actually be a Maxima regression.
</p>
</blockquote>
<p>
I agree that this is a Maxima regression. First maxima could verify an equality given sufficient information, and now it can't. Assuming everything in sight is real is not the same as assuming some real parts are positive (although who knows what awful bug had as a sideeffect this particular equality got verified).
</p>
<p>
If you look in the history of <code>simp.lisp</code> you'll see there is some recent probably relevant work there, so I think reporting this issue might have a chance of getting it fixed (and will certainly be appreciated).
</p>
<p>
We are at the mercy of Maxima for these issues. Our alternatives are:
</p>
<ul><li>not upgrade and live with existing bugs
</li><li>live with the regression
</li><li>fix and patch maxima
</li></ul><p>
I think in this case, Option 2 is the practical one. Option 3 is obviously the best (both for Maxima and for Sage) but doesn't solve anything _now_.
</p>
<p>
As soon as Dima has commented, back to positive review!
</p>
TicketdimpaseThu, 08 Mar 2012 05:27:34 GMT
https://trac.sagemath.org/ticket/10682#comment:90
https://trac.sagemath.org/ticket/10682#comment:90
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:86" title="Comment 86">jpflori</a>:
</p>
<blockquote class="citation">
<p>
More seriously, you also removed the lines
</p>
<p>
"Also, there are limits to the precision to which Maxima can compute..."
</p>
<p>
I agree that we do not raise an error anymore, but Maxima can still not compute up to that precision.
</p>
</blockquote>
<p>
these lines essentially duplicated the description of return code 6 several lines above these.
</p>
<blockquote class="citation">
<p>
You also left the stuff about waiting for an answer on Maxima's list, but IIRC there has been an answer there, so what is done is not really a workaround, but rather a decision from Sage developpers.
</p>
</blockquote>
<p>
well, that's the stuff I had no knowledge about, so I left it stand, obviously.
</p>
<p>
Dima
</p>
TicketdimpaseThu, 08 Mar 2012 05:30:03 GMT
https://trac.sagemath.org/ticket/10682#comment:91
https://trac.sagemath.org/ticket/10682#comment:91
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:87" title="Comment 87">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Last questions (I hope):
</p>
<p>
About the lines patched in wester.py, why did you add a semicolon after "n=var('n')" ?
</p>
</blockquote>
<p>
hmm, perhaps a result of a copy/paste from Sage prompt...
</p>
<blockquote class="citation">
<p>
And what do you mean exactly by "Maxima 5.26 does not do "just" simpify() here." (sic)?
</p>
<p>
That now Maxima cannot simplify this expression when it is only told that Re(x) >0 and Re(y)>0 and that we have to call simplify_exp() instead ?
</p>
</blockquote>
<p>
Indeed.
</p>
TicketjpfloriThu, 08 Mar 2012 06:34:32 GMT
https://trac.sagemath.org/ticket/10682#comment:92
https://trac.sagemath.org/ticket/10682#comment:92
<p>
Ok, I understand. Nonetheless I think its better to be more verbose as was the case before, especially that no error is now explicitely returned.
</p>
<p>
If someone does not pay attention, one might think that Sage returns 0 for the integral of x between 0 an 1.
</p>
TicketdimpaseThu, 08 Mar 2012 10:37:28 GMT
https://trac.sagemath.org/ticket/10682#comment:93
https://trac.sagemath.org/ticket/10682#comment:93
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:92" title="Comment 92">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Ok, I understand. Nonetheless I think its better to be more verbose as was the case before, especially that no error is now explicitely returned.
</p>
<p>
If someone does not pay attention, one might think that Sage returns 0 for the integral of x between 0 an 1.
</p>
</blockquote>
<p>
OK, I updated the patch as follows, to reflect this and a couple of other minor points raised:
</p>
<pre class="wiki">
diff git a/sage/calculus/calculus.py b/sage/calculus/calculus.py
 a/sage/calculus/calculus.py
+++ b/sage/calculus/calculus.py
@@ 641,6 +641,8 @@
``numerical_integral`` that implements numerical
integration using the GSL C library. It is potentially much faster
and applies to arbitrary user defined functions.
+ Also, there are limits to the precision to which Maxima can compute
+ the integral to due to limitations in quadpack.
::
diff git a/sage/calculus/wester.py b/sage/calculus/wester.py
 a/sage/calculus/wester.py
+++ b/sage/calculus/wester.py
@@ 383,9 +383,10 @@
::
sage: # (YES) Assuming Re(x)>0, Re(y)>0, deduce x^(1/n)*y^(1/n)(x*y)^(1/n)=0.
 sage: # Maxima 5.26 does not do "just" simpify() here. Thus simplify_exp() used.
+ sage: # Maxima 5.26 cannot do "just" simpify() here. Thus simplify_exp() is used.
+ sage: # This is a regression from 5.24
sage: # assume(real(x) > 0, real(y) > 0) # (not needed for simplify_exp())
 sage: n = var('n');
+ sage: n = var('n')
sage: f = x^(1/n)*y^(1/n)(x*y)^(1/n)
sage: f.simplify_exp()
0
</pre>
TicketjpfloriThu, 08 Mar 2012 10:55:16 GMT
https://trac.sagemath.org/ticket/10682#comment:94
https://trac.sagemath.org/ticket/10682#comment:94
<p>
In fact, I already corrected these issues in my patch, so if you're happy with my corrections, you'd better reupload the previous version of your patch and let the ticket have positive review.
</p>
<p>
Otherwise, the reviewer patch will have to be rebased.
</p>
TicketdimpaseThu, 08 Mar 2012 11:21:59 GMTattachment set
https://trac.sagemath.org/ticket/10682
https://trac.sagemath.org/ticket/10682
<ul>
<li><strong>attachment</strong>
set to <em>trac10682_1.patch</em>
</li>
</ul>
<p>
Sage library patch to go with the new spkg
</p>
TicketdimpaseThu, 08 Mar 2012 11:29:21 GMT
https://trac.sagemath.org/ticket/10682#comment:95
https://trac.sagemath.org/ticket/10682#comment:95
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:94" title="Comment 94">jpflori</a>:
</p>
<blockquote class="citation">
<p>
In fact, I already corrected these issues in my patch, so if you're happy with my corrections, you'd better reupload the previous version of your patch and let the ticket have positive review.
</p>
</blockquote>
<p>
reuploaded the previous version.
Used Apple's Time Machine backup fot the 1st time ever :)
</p>
TicketkcrismanThu, 08 Mar 2012 18:08:09 GMT
https://trac.sagemath.org/ticket/10682#comment:96
https://trac.sagemath.org/ticket/10682#comment:96
<pre class="wiki">"just" simpify()
</pre><p>
I assume that, since you are changing that line anyway, it would be trivial to edit "by hand" the text .patch file so that this reads
</p>
<pre class="wiki">"just" simplify()
</pre><p>
It seems to be the sort of thing we should fix :)
</p>
TicketjpfloriThu, 08 Mar 2012 21:56:30 GMT
https://trac.sagemath.org/ticket/10682#comment:97
https://trac.sagemath.org/ticket/10682#comment:97
<p>
These lines are in fact changed in the reviewer patch.
</p>
TicketkcrismanThu, 08 Mar 2012 22:21:36 GMT
https://trac.sagemath.org/ticket/10682#comment:98
https://trac.sagemath.org/ticket/10682#comment:98
<p>
Sorry, I just saw what you pasted above. Sorry for the noise.
</p>
TicketnbruinFri, 09 Mar 2012 08:03:17 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:99
https://trac.sagemath.org/ticket/10682#comment:99
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
I'm sorry to report that the new spkg seems to cause problems for the expect interface for maxima. I don't think this problem arose with Dima's original spkg. Of course, I'd be very happy to hear that other people cannot reproduce this problem and that I'm not building ecl/maxima correctly, but currently I'm experiencing:
</p>
<pre class="wiki">sage: maxima(1)
[HANGS]
</pre><p>
This happens to me on two different builds where I've applied the recipe as outlined in the ticket description. For one, see <a class="extlink" href="http://sage.math.washington.edu/home/nbruin/sage5.0/"><span class="icon"></span>sage.math</a>.
As it stands, I don't think this ticket will pass doctests (it doesn't for me).
</p>
TicketdimpaseFri, 09 Mar 2012 08:06:45 GMT
https://trac.sagemath.org/ticket/10682#comment:100
https://trac.sagemath.org/ticket/10682#comment:100
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:99" title="Comment 99">nbruin</a>:
</p>
<blockquote class="citation">
<p>
I'm sorry to report that the new spkg seems to cause problems for the expect interface for maxima. I don't think this problem arose with Dima's original spkg. Of course, I'd be very happy to hear that other people cannot reproduce this problem and that I'm not building ecl/maxima correctly, but currently I'm experiencing:
</p>
<pre class="wiki">sage: maxima(1)
[HANGS]
</pre><p>
This happens to me on two different builds where I've applied the recipe as outlined in the ticket description. For one, see <a class="extlink" href="http://sage.math.washington.edu/home/nbruin/sage5.0/"><span class="icon"></span>sage.math</a>.
As it stands, I don't think this ticket will pass doctests (it doesn't for me).
</p>
</blockquote>
<p>
I also see weirdness: e.g. some doctests ran out of time with the present spkg/patches pair, while they worked fine with my original spkg/patches pair...
</p>
TicketjpfloriFri, 09 Mar 2012 08:08:08 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:101
https://trac.sagemath.org/ticket/10682#comment:101
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
<p>
It's strange that you report that it did not happen with Dima original package because the only changes I've made to the spkg are purely typographical (see <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10682/trac_10682spkg.patch" title="Attachment 'trac_10682spkg.patch' in Ticket #10682">trac_10682spkg.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/10682/trac_10682spkg.patch" title="Download"></a> Download).
I'll recheck on my computer.
</p>
TicketjpfloriFri, 09 Mar 2012 08:33:17 GMT
https://trac.sagemath.org/ticket/10682#comment:102
https://trac.sagemath.org/ticket/10682#comment:102
<p>
I confirm that something went wrong with the last spkg I uploaded, maybe I downloaded a wrong base spkg, I'll check that now.
</p>
TicketjpfloriFri, 09 Mar 2012 09:33:44 GMTattachment set
https://trac.sagemath.org/ticket/10682
https://trac.sagemath.org/ticket/10682
<ul>
<li><strong>attachment</strong>
set to <em>trac_10682spkg.patch</em>
</li>
</ul>
<p>
spkg diff from reviewer, for review only
</p>
TicketjpfloriFri, 09 Mar 2012 09:55:13 GMT
https://trac.sagemath.org/ticket/10682#comment:103
https://trac.sagemath.org/ticket/10682#comment:103
<p>
Ok that was an error from my part, some TABS were actually needed in spkginstall.
This is fixed now.
I've also swpped two lines in the sage tree patch.
I'll upload a fixed version in a moment.
</p>
TicketjpfloriFri, 09 Mar 2012 10:20:56 GMTattachment set
https://trac.sagemath.org/ticket/10682
https://trac.sagemath.org/ticket/10682
<ul>
<li><strong>attachment</strong>
set to <em>trac_10682reviewerlocal.patch</em>
</li>
</ul>
<p>
Reviewer patch
</p>
TicketjpfloriFri, 09 Mar 2012 10:59:22 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:104
https://trac.sagemath.org/ticket/10682#comment:104
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
make ptest is fine on my computer now (ubuntu/amd64).
</p>
TicketdimpaseFri, 09 Mar 2012 13:51:57 GMT
https://trac.sagemath.org/ticket/10682#comment:105
https://trac.sagemath.org/ticket/10682#comment:105
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10682#comment:104" title="Comment 104">jpflori</a>:
</p>
<blockquote class="citation">
<p>
make ptest is fine on my computer now (ubuntu/amd64).
</p>
</blockquote>
<p>
make ptestlong went well on MacOSX 10.6.8.
looks like it's good to go!
</p>
TicketnbruinFri, 09 Mar 2012 19:13:36 GMTstatus changed
https://trac.sagemath.org/ticket/10682#comment:106
https://trac.sagemath.org/ticket/10682#comment:106
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
no complaints here.
</p>
TicketjdemeyerTue, 13 Mar 2012 08:22:00 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/10682#comment:107
https://trac.sagemath.org/ticket/10682#comment:107
<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>sage5.0.beta8</em>
</li>
</ul>
TicketjdemeyerTue, 20 Mar 2012 07:36:23 GMTauthor changed
https://trac.sagemath.org/ticket/10682#comment:108
https://trac.sagemath.org/ticket/10682#comment:108
<ul>
<li><strong>author</strong>
changed from <em>Dima Pasechnik</em> to <em>Dmitrii Pasechnik</em>
</li>
</ul>
Ticket