sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.3k stars 449 forks source link

Upgrade Maxima to 5.24.0 #12094

Closed orlitzky closed 12 years ago

orlitzky commented 12 years ago

I've successfully replaced the 'src' directory in the current maxima-5.23.2.p3.spkg with the tarball from 5.24.0. All tests pass with one small exception; fix included.

This will fix #11591, and prevent a regression in the fix for #11483.

Package at http://boxen.math.washington.edu/home/jdemeyer/spkg/maxima-5.24.0.p0.spkg

Apply attachment: sage-trac-12094.patch

CC: @benjaminfjones @nexttime @burcin @jhpalmieri

Component: symbolics

Author: Michael Orlitzky

Reviewer: Karl-Dieter Crisman

Merged: sage-5.0.beta2

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

kcrisman commented 12 years ago
comment:1

Attachment: sage-trac-12094.patch.gz

Hmm, but isn't the latest Maxima 5.25.1? Just wondering.


The procedure for upgrading is as follows.

orlitzky commented 12 years ago
comment:2

Replying to @kcrisman:

Hmm, but isn't the latest Maxima 5.25.1? Just wondering.

Yep, sorry, I just mentioned it on sage-devel: there is an unrelated regression in 5.25.1 that I didn't want to deal with at the same time. I'll try to make a proper spkg for this.

kcrisman commented 12 years ago
comment:3

Replying to @orlitzky:

Replying to @kcrisman:

Hmm, but isn't the latest Maxima 5.25.1? Just wondering.

Yep, sorry, I just mentioned it on sage-devel: there is an unrelated regression in 5.25.1 that I didn't want to deal with at the same time. I'll try to make a proper spkg for this.

Just be sure to link to a reference for that regression so that other readers of this ticket know why you didn't go all the way up.

orlitzky commented 12 years ago
comment:4

Replying to @kcrisman:

Just be sure to link to a reference for that regression so that other readers of this ticket know why you didn't go all the way up.

Sure. In sage/calculus/desolvers.py, we have,

sage: y=function('y',x); assume(x>0); assume(y>0)                       
sage: desolve(x*diff(y,x)-x*sqrt(y^2+x^2)-y == 0, y, contrib_ode=True)
[x - arcsinh(y(x)/x) == c]

Instead of that nice (confirmed correct) result, I was getting back a much larger expression. I tested in the Maxima console, and it was returning the correct answer, as expected above. However, that wasn't coming back out to sage.

I didn't test with 5.25.0. There was some other general shuffling about of terms in doctest results, and I thought it best to compartmentalize the damage since this is my first time touching any of this code.

orlitzky commented 12 years ago
comment:5

Here's the spkg. Let me know if I can do anything better:

http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg

I looked around on the spkg-upload Google code page, but I can't for the life of me figure out what I'm supposed to do there.

kcrisman commented 12 years ago
comment:6

http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg

Thanks. Hopefully we can at the very least look at this at Sage Days 35.5.

I looked around on the spkg-upload Google code page, but I can't for the life of me figure out what I'm supposed to do there.

If you have your own website, then it's really not an issue :)

kcrisman commented 12 years ago
comment:7

Replying to @orlitzky:

Replying to @kcrisman:

Just be sure to link to a reference for that regression so that other readers of this ticket know why you didn't go all the way up.

Sure. In sage/calculus/desolvers.py, we have,

> sage: y=function('y',x); assume(x>0); assume(y>0)                       
> sage: desolve(x*diff(y,x)-x*sqrt(y^2+x^2)-y == 0, y, contrib_ode=True)
> [x - arcsinh(y(x)/x) == c]

Like this, eh?

Maxima 5.25.0 http://maxima.sourceforge.net
using Lisp SBCL 1.0.24
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('contrib_ode);
(%o1) /Users/karl-dietercrisman/Downloads/maxima-5.25.0/share/contrib/diffequa\
tions/contrib_ode.mac
(%i8) display2d:false;

(%o8) false
(%i9) assume(y>0);

(%o9) [redundant]
(%i10) (TEMP:contrib_ode(x*('diff(y,x,1))-x*sqrt(y^2+x^2)-y,y,x), if TEMP=false then TEMP else substitute(y=y(x),TEMP)); 

(%o10) [x-asinh(y(x)/abs(x)) = %c]

If you run it without the assumption, you get

(%i13) (TEMP:contrib_ode(x*('diff(y,x,1))-x*sqrt(y^2+x^2)-y,y,x), if TEMP=false then TEMP else substitute(y=y(x),TEMP)); 

Is  y  zero or nonzero?

nonzero;
(%o13) [-(y(x)*asinh(abs(y(x))/abs(x))-x*abs(y(x)))/abs(y(x)) = %c]

But I assume this isn't what Sage gave you, either. Strange that this would change.

orlitzky commented 12 years ago
comment:8

Replying to @kcrisman:

But I assume this isn't what Sage gave you, either. Strange that this would change.

Yeah. I think you just missed an 'assume(x > 0)' in the first one. This is correct:

sage: maxima.version()
'5.25.1'
sage: maxima.console()
;;; Loading #P"/home/mjo/src/sage-4.8.alpha2/local/lib/ecl/sb-bsd-sockets.fas"
;;; Loading #P"/home/mjo/src/sage-4.8.alpha2/local/lib/ecl/sockets.fas"
;;; Loading #P"/home/mjo/src/sage-4.8.alpha2/local/lib/ecl/defsystem.fas"
;;; Loading #P"/home/mjo/src/sage-4.8.alpha2/local/lib/ecl/cmp.fas"
Maxima 5.25.1 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('contrib_ode);
(%o1) /home/mjo/src/sage-4.8.alpha2/local/share/maxima/5.25.1/share/contrib/di\
ffequations/contrib_ode.mac
(%i2) display2d: false;
(%o2) false
(%i3) assume(x > 0);
(%o3) [x > 0]
(%i4) assume(y > 0);
(%o4) [y > 0]
(%i5) eqn: x * 'diff(y,x) - x*sqrt(x^2 + y^2) - y = 0;
(%o5) x*'diff(y,x,1)-x*sqrt(y^2+x^2)-y = 0
(%i6) contrib_ode(eqn,y,x);
(%o6) [x-asinh(y/x) = %c]

and in the same sage session,

sage: y = function('y',x)
sage: assume(x > 0)
sage: assume(y > 0)
sage: desolve(x*diff(y,x)-x*sqrt(y^2+x^2)-y == 0, y, contrib_ode=True)
[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]

Boom. Not sure what the deal is. It doesn't simplify with full_simplify, and I'm not man enough to try it by hand. That was my motivation to give 5.24.0 a try =)

kcrisman commented 12 years ago
comment:9

and in the same sage session,

> sage: y = function('y',x)
> sage: assume(x > 0)
> sage: assume(y > 0)
> sage: desolve(x*diff(y,x)-x*sqrt(y^2+x^2)-y == 0, y, contrib_ode=True)
> [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]

Wow! That is really baffling.

Here's the best I could do.

sage: F.expand_log().simplify_full().expand()
x + 1/2*log(x^2 + 2*y(x)^2 + 2*sqrt(x^2 + y(x)^2)*y(x)) + log(2/x) - arcsinh(abs(y(x))/x) - arcsinh(y(x)/x)

Probably this just differs by a constant - though this is after assuming positive things and still there is the abs in there.

kiwifb commented 12 years ago
comment:10

I documented a number of things that changed or were wrong with maxima-5.25.x some time ago on sage-devel. I wouldn't think there is anything to do in the spkg apart from updating the sources. Are you sure that you got all the doctests that need fixing with 5.24.0? I thought there was another one but I could be wrong.

orlitzky commented 12 years ago
comment:11

Replying to @kiwifb:

I documented a number of things that changed or were wrong with maxima-5.25.x some time ago on sage-devel. I wouldn't think there is anything to do in the spkg apart from updating the sources. Are you sure that you got all the doctests that need fixing with 5.24.0? I thought there was another one but I could be wrong.

I hope so. This is how I'm testing. Did I miss anything?

cd spkg/standard
sage -f maxima-5.24.0.p0.spkg
(works)

cd ../..
sage -b new_maxima
(works)

sage -tp 4 --long devel/sage-new_maxima/
(all pass except three tests in cmdline.py that always fail)

cd devel/sage-new_maxima/
sage -hg status
(nothing)

sage -hg log | most
(shows the last commit on top of the alpha is my patch)
kcrisman commented 12 years ago
comment:12

Just fyi - not that we have to act on this:

From: Robert Dodier <robert.dodier@gmail.com>
To: maxima@math.utexas.edu
Subject: [Maxima] 5.26 release branch planned for Dec 6
Message-ID:
       <CAAsY_sRjQEWUjc83VkXG-oMBtPcMve9T_cC2W=MoGb-_aOvMmA@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1

Hi, I am planning to make a release branch for Maxima 5.26
around 2011-12-06 00:00 UTC.

As ever, I'll just make the branch with whatever is
current on the git master or whatever the appropriate
terminology is.

best

Robert Dodier
kcrisman commented 12 years ago
comment:13

Replying to @kiwifb:

I documented a number of things that changed or were wrong with maxima-5.25.x some time ago on sage-devel. I wouldn't think there is anything to do in the spkg apart from updating the sources. Are you sure that you got all the doctests that need fixing with 5.24.0? I thought there was another one but I could be wrong.

See this sage-devel thread. I think this would be worth checking and making sure is right. I'd rather upgrade as much as possible and allow a (correct) ungood simplification go by.

orlitzky commented 12 years ago
comment:14

Replying to @kcrisman:

See this sage-devel thread. I think this would be worth checking and making sure is right. I'd rather upgrade as much as possible and allow a (correct) ungood simplification go by.

I ran into all of those, too. One of them is fixed with a simplify(), but the desolve thing is still a problem.

My thinking was that it would be easier to review and test 5.24, which is a relatively unintrusive improvement, and then bump to 5.25.1 after closing some bugs and adding doctests for fixes introduced in 5.24.

kcrisman commented 12 years ago
comment:15

My thinking was that it would be easier to review and test 5.24, which is a relatively unintrusive improvement, and then bump to 5.25.1 after closing some bugs and adding doctests for fixes introduced in 5.24.

There will be a number of people needing easy bugfixes at Sage Days 35.5, but I don't know how many of the many symbolics tickets with "fixed upstream" are in fixed in 5.24, how many in 5.25, etc.

But I guess in the end it depends on who does the work :) So I guess if you have a working spkg, maybe we should go with that. Is it based on the spkg at #11966?

orlitzky commented 12 years ago
comment:16

Replying to @kcrisman:

There will be a number of people needing easy bugfixes at Sage Days 35.5, but I don't know how many of the many symbolics tickets with "fixed upstream" are in fixed in 5.24, how many in 5.25, etc.

But I guess in the end it depends on who does the work :) So I guess if you have a working spkg, maybe we should go with that. Is it based on the spkg at #11966?

Yes, it is. I'm going through some of the symbolics tickets now. Many of them are fixed by the abs_integrate package, whose inclusion initially prompted me to bump maxima. Not all of them are fixed, of course, but I don't see any regressions, either.

kcrisman commented 12 years ago
comment:17

I wouldn't expect any regressions. Be sure to check out symbolics, and feel free to add any tickets to that list that aren't already on it.

kcrisman commented 12 years ago
comment:18

Let's be sure to fix the double negative in the message on OS X reported here.

kcrisman commented 12 years ago
comment:19

Just FYI for when this ticket is finished, the latest Maxima is 5.26.0, officially released today.

kcrisman commented 12 years ago
comment:22

Thanks for this - I'll try to look at it in the near future.

Just FYI for when this ticket is finished, the latest Maxima is 5.26.0, officially released today.

Though apparently it was also officially released today. Weird.

kcrisman commented 12 years ago

Description changed:

--- 
+++ 
@@ -3,3 +3,5 @@
 I've successfully replaced the 'src' directory in the current maxima-5.23.2.p2.spkg with the tarball from 5.24.0. All tests pass with one small exception; fix included.

 This will fix #11591, and prevent a regression in the fix for #11483.
+
+Package at [http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg](http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg)
kcrisman commented 12 years ago
comment:24

Okay, here is something we should do.


 * TODO:
   Is the `spkg-dist` script still needed, i.e., does anybody run
   this after upgrading the upstream source tree?
   (It deletes the stuff in some language directories, `src/doc/info/*/`,
   and creates dummy `Makefile.in`s, but apparently nobody has run it on
   the recent Maxima spkgs.)

This was added by Leif to SPKG.txt, and I think it's a good point. To wit:

$ du -h -c
<snip>
110M    total
$ ./spkg-dist 
rm -rf "src/doc/info/es/"*
rm -rf "src/doc/info/es.utf8/"*
rm -rf "src/doc/info/pt/"*
rm -rf "src/doc/info/pt.utf8/"*
rm -rf "src/doc/info/pt_BR/"*
rm -rf "src/doc/info/pt_BR.utf8/"*
$ du -h -c
<snip>
 64M    total

Also, we can add to

for X in ['es', 'es.utf8', 'pt', 'pt.utf8', 'pt_BR', 'pt_BR.utf8']:

by doing

for X in ['de', 'de.utf8', 'es', 'es.utf8', 'pt', 'pt.utf8', 'pt_BR', 'pt_BR.utf8']:

What do people think?

Otherwise, I don't foresee any problems here; it works, and I'm running tests on OS X 10.6 currently. If that goes well, I'll try it on sage.math and perhaps call it a day?

kcrisman commented 12 years ago

Author: Michael Orlitzky

kcrisman commented 12 years ago

Reviewer: Karl-Dieter Crisman

kcrisman commented 12 years ago
comment:25

Yeah, since we aren't really committed to providing upstream documentation other than English (or even at all, see #10823 for trying to put those docs someplace accessible, which could be done at least with the html here).

And we'd need to make it MUCH clearer in SPKG.txt that this should be done every time.

Oh, and we need to fix the double negative in spkg-install

as it occurs.  Don't worry, the build process doesn't 
not hang." 

as pointed out in comment:18.

kcrisman commented 12 years ago

Work Issues: typo, remove some docs, update SPKG.txt

kcrisman commented 12 years ago
comment:26

Tests pass, as expected. Let's fix these minor things and get it in.

orlitzky commented 12 years ago
comment:27

Replying to @kcrisman:

Tests pass, as expected. Let's fix these minor things and get it in.

Thanks for looking at this. I'm trying to get the local docs to install, then I'll post an updated spkg.

kcrisman commented 12 years ago
comment:28

Tests pass, as expected. Let's fix these minor things and get it in.

Thanks for looking at this. I'm trying to get the local docs to install, then I'll post an updated spkg.

? Do you mean you are trying to apply #10823 ideas here? I guess that's ok, but don't spend tons of time on it.

Do be sure to update that script spkg-dist and apply it (and commit it in hg), then test again to make sure it still builds; that will save at least a few MB compressed, I suspect.

orlitzky commented 12 years ago
comment:29

New spkg, same location, as of a few minutes ago.

Replying to @kcrisman:

Do you mean you are trying to apply #10823 ideas here? I guess that's ok, but don't spend tons of time on it.

Yes, the docs are already built, so it wasn't a big deal to add. I installed the new spkg twice -- once with the doc variable set and once without -- it seems to work well enough. I checked the few pages that link to figures and they do show up.

Do be sure to update that script spkg-dist and apply it (and commit it in hg), then test again to make sure it still builds; that will save at least a few MB compressed, I suspect.

Updated, committed, and applied. I added the extra directories that you suggested, and removed an unused import (time).

I also fixed the double negative, and updated SPKG.txt to be more clear about running spkg-dist.

kcrisman commented 12 years ago
comment:30

Wow, this is 11 MB instead of 21 MB! Good to know.

Okay, I'll check out the rest. Looks good so far.

kcrisman commented 12 years ago

for review only

kcrisman commented 12 years ago

Attachment: 12094.diff.gz

Attachment: 12094-review.diff.gz

for review only

kcrisman commented 12 years ago
comment:31

By the way, using spkg-upload or getting an account on sage.math might lead to much faster download speeds for this spkg :) It does take a while.

I think positive review to the code changes etc. Just making sure things work on Linux and then testing the doc install. Very nice work.

kcrisman commented 12 years ago
comment:32

Docs look good. Not anticipating any problems on Linux. Modulo testing there (in progress), good work.

kcrisman commented 12 years ago

Changed work issues from typo, remove some docs, update SPKG.txt to none

orlitzky commented 12 years ago
comment:33

Thanks, I'll start putting together patches for the symbolics tickets this fixes.

Regarding the speed, I'm screaming about it daily. I will continue to do so at gradually increasing volume until it gets fixed.

benjaminfjones commented 12 years ago
comment:34

The updates look good to me too. I am also running tests on Fedora linux, also not expecting any problems. I doctested (most of) the symbolics by hand and had no problems there.

kcrisman commented 12 years ago
comment:35

Yeah, this is good. Also, now it should be more ready for the next update, and you (or others) can start reviewing those tickets which depend on this.

jdemeyer commented 12 years ago

Description changed:

--- 
+++ 
@@ -5,3 +5,5 @@
 This will fix #11591, and prevent a regression in the fix for #11483.

 Package at [http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg](http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg)
+
+Apply [attachment: sage-trac-12094.patch](https://github.com/sagemath/sage-prod/files/10654124/sage-trac-12094.patch.gz)
jdemeyer commented 12 years ago
comment:37

This should be rebased to the maxima spkg from sage-4.8 (see #12131).

orlitzky commented 12 years ago
comment:38

I've updated the spkg again; hopefully I moved my changes over cleanly.

Since I got to do it over again, I cleaned up the trailing whitespace in spkg-install this time.

orlitzky commented 12 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,4 @@
-(What's the procedure for upgrading these?)
-
-I've successfully replaced the 'src' directory in the current maxima-5.23.2.p2.spkg with the tarball from 5.24.0. All tests pass with one small exception; fix included.
+I've successfully replaced the 'src' directory in the current maxima-5.23.2.p3.spkg with the tarball from 5.24.0. All tests pass with one small exception; fix included.

 This will fix #11591, and prevent a regression in the fix for #11483.
kcrisman commented 12 years ago
comment:39

Terribly sorry for missing that, Jeroen. Good thing we have many eyes on all of this...

I've updated the spkg again; hopefully I moved my changes over cleanly.

Looks like it.

Since I got to do it over again, I cleaned up the trailing whitespace in spkg-install this time.

Eventually I figured out how to check this in my editor.

Hopefully this time positive review for real!

jdemeyer commented 12 years ago

Merged: sage-5.0.beta2

jdemeyer commented 12 years ago

Description changed:

--- 
+++ 
@@ -2,6 +2,6 @@

 This will fix #11591, and prevent a regression in the fix for #11483.

-Package at [http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg](http://michael.orlitzky.com/sage/maxima-5.24.0.p0.spkg)
+Package at [http://boxen.math.washington.edu/home/jdemeyer/spkg/maxima-5.24.0.p0.spkg](http://boxen.math.washington.edu/home/jdemeyer/spkg/maxima-5.24.0.p0.spkg)

 Apply [attachment: sage-trac-12094.patch](https://github.com/sagemath/sage-prod/files/10654124/sage-trac-12094.patch.gz)