Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Maxima to 5.34.1 #16908

Closed
pjbruin opened this issue Aug 29, 2014 · 44 comments
Closed

Upgrade Maxima to 5.34.1 #16908

pjbruin opened this issue Aug 29, 2014 · 44 comments

Comments

@pjbruin
Copy link
Contributor

pjbruin commented Aug 29, 2014

Tarball: ​http://pub.math.leidenuniv.nl/~bruinpj/sage/maxima-5.34.1.tar.bz2

This appears to fix #14965.

CC: @zimmermann6 @jpflori @williamstein

Component: packages: standard

Keywords: maxima

Author: Peter Bruin

Branch/Commit: e216b6f

Reviewer: Jeroen Demeyer

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

@pjbruin pjbruin added this to the sage-6.4 milestone Aug 29, 2014
@kcrisman
Copy link
Member

comment:1

Any tix this fixes, btw?

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 3, 2014

comment:2

Replying to @kcrisman:

Any tix this fixes, btw?

I haven't had the time to investigate this in detail. It doesn't fix any of the tickets listed in #13973 that are still open, but I haven't looked at other open Maxima-related tickets. The update shouldn't be too difficult, but there are some problems, such as my fix for the matrix exponentation bug (see #13973) not working anymore.

@rwst
Copy link

rwst commented Sep 4, 2014

comment:3

Changelog is here: https://sourceforge.net/p/maxima/code/ci/master/tree/ChangeLog-5.34

@jdemeyer
Copy link

jdemeyer commented Sep 4, 2014

comment:4

Replying to @kcrisman:

Any tix this fixes, btw?

#14965 claims to be fixed by Maxima 5.34

@kiwifb
Copy link
Member

kiwifb commented Sep 4, 2014

comment:5

How many patches can we drop with this version?

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 5, 2014

Branch: u/pbruin/16908-maxima-5.34.0

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 5, 2014

Author: Peter Bruin

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 5, 2014

Commit: 0fd6543

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 5, 2014

comment:7

Replying to @jdemeyer:

Replying to @kcrisman:

Any tix this fixes, btw?

#14965 claims to be fixed by Maxima 5.34

It does appear to be fixed; I am not getting an error, but haven't checked if the answer is correct.

[Edit: actually I couldn't reproduce the error with Maxima 5.33.0.]

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 5, 2014

comment:8

Replying to @kiwifb:

How many patches can we drop with this version?

This branch removes two patches and updates another one.

[Edit: the one added patch was not necessary after all, there was a better solution.]

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 5, 2014

comment:9

I have tested this (and all tests pass) on x86_64 and ARM 32-bit.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 6, 2014

Changed commit from 0fd6543 to d83032d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 6, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

d83032dTrac 16908: replace compile-verbose.patch by suitable fixes in Sage code

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 10, 2014

comment:12

Maxima 5.34.1 has been released, an updated branch is coming soon.

@pjbruin

This comment has been minimized.

@pjbruin pjbruin changed the title Upgrade Maxima to 5.34.0 Upgrade Maxima to 5.34.1 Sep 10, 2014
@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 10, 2014

Changed commit from d83032d to 44c889e

@pjbruin

This comment has been minimized.

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 10, 2014

Changed branch from u/pbruin/16908-maxima-5.34.0 to u/pbruin/16908-maxima-5.34.1

@jdemeyer
Copy link

comment:14

Could you justify this change?

- sage: latex(integrate(1/(1+sqrt(x)),x,0,1))
- \int_{0}^{1} \frac{1}{\sqrt{x} + 1}\,{d x}

I think the purpose of this test is to show an integral that Maxima cannot compute. You could replace this by a different integral...

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 10, 2014

comment:15

Maxima is now able to compute the integral (it equals 2 - 2 log(2)), so the doctest did not test the _print_latex_() method anymore. Of course we could put in another doctest with an integral that Maxima isn't able to evaluate, but I thought this _print_latex_() method was simple enough that it wasn't necessary to keep a second doctest.

@jdemeyer
Copy link

comment:16

Replying to @pjbruin:

Maxima is now able to compute the integral (it equals 2 - 2 log(2)), so the doctest did not test the _print_latex_() method anymore. Of course we could put in another doctest with an integral that Maxima isn't able to evaluate, but I thought this _print_latex_() method was simple enough that it wasn't necessary to keep a second doctest.

Well, the 2 doctests are somewhat different, one calls directly print_latex while the removed one uses latex(). So yes, please but back a second doctests.

Apart from this trivial thing, this ticket looks good to me.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2014

Changed commit from 2785ccc to b8babff

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

38a52cfMake the to_poly_solve option work
2573b80fix two trivial doctest failures
6fb6beeUse for loop instead of while
c534e55Merge branch 'develop' into t/16651/buggy_to_poly_solve
a801e6416651: reviewer's patch: language refinements
1fa0110Docstring fixes
b8babffMerge branch 'u/vbraun/buggy_to_poly_solve' of ssh://trac.sagemath.org/sage into ticket/16908-maxima-5.34.1

@vbraun
Copy link
Member

vbraun commented Sep 11, 2014

comment:23

I get various doctest failures on top of 6.4.beta3:

sage -t --long --warn-long 40.1 src/sage/calculus/desolvers.py  # 2 doctests failed
sage -t --long --warn-long 40.1 src/sage/rings/number_field/number_field_element.pyx  # 1 doctest failed
sage -t --long --warn-long 40.1 src/sage/tests/french_book/integration_doctest.py  # 1 doctest failed
sage -t --long --warn-long 40.1 src/sage/modules/free_module_element.pyx  # 1 doctest failed
sage -t --long --warn-long 40.1 src/sage/symbolic/relation.py  # 1 doctest failed
sage -t --long --warn-long 40.1 src/sage/functions/other.py  # 1 doctest failed
sage -t --long --warn-long 40.1 src/sage/functions/special.py  # 2 doctests failed

Looks like it is all due to changed precision of floats

@jdemeyer
Copy link

comment:25

I'll have a look at the doctest failures.

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 12, 2014

comment:26

Replying to @jdemeyer:

I'll have a look at the doctest failures.

There is no need to, I just fixed them and only have to test them on 32-bit.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2014

Changed commit from b8babff to e216b6f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

5dc9973Merge branch 'develop' into ticket/16908-maxima-5.34.1
e216b6fTrac 16908: correct precision in doctests

@jdemeyer
Copy link

comment:28

Another trivial change to a French book test.

@jdemeyer
Copy link

comment:29

Do you understand why floats from Maxima are now printed with one digit less of precision???

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 12, 2014

comment:30

Replying to @jdemeyer:

Do you understand why floats from Maxima are now printed with one digit less of precision???

This is because of recent changes (between 5.33.0 and 5.34.0) to Maxima's exploden-format-float function. In fact, on certain Lisp implementations and platforms (including ECL on x86_64), Maxima used to often print one digit too many; see #15921 for details.

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 12, 2014

comment:31

All relevant doctests (the failed ones above, and everything in sage/interfaces/maxima_*, sage/calculus, sage/functions, sage/symbolic) pass on x86_64 and ARM. Since I ran all long doctests before, I now leave it to the buildbot to catch any unlikely remaining failures...

@zimmermann6
Copy link

comment:32

I'm not happy with the changes in the French book tests. In some places you use ...
which should make tests pass both with the previous versions of Sage (this should be checked), in some places more digits are printed, which will fail with previous versions,
and might fail with future versions, for example:

     sage: t, y = var('t, y')
     sage: desolve_rk4(t*y*(2-y), y, ics=[0,1], end_points=[0, 1], step=0.5)
-    [[0, 1], [0.5, 1.12419127425], [1.0, 1.46159016229]]
+    [[0, 1], [0.5, 1.12419127424558], [1.0, 1.4615901622888245]]

Paul

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 12, 2014

comment:33

Replying to @zimmermann6:

I'm not happy with the changes in the French book tests.

I assume you are mostly referring to the changes made by #16858; this ticket only removes one digit.

In some places you use ...
which should make tests pass both with the previous versions of Sage (this should be checked), in some places more digits are printed, which will fail with previous versions,
and might fail with future versions, for example:

     sage: t, y = var('t, y')
     sage: desolve_rk4(t*y*(2-y), y, ics=[0,1], end_points=[0, 1], step=0.5)
-    [[0, 1], [0.5, 1.12419127425], [1.0, 1.46159016229]]
+    [[0, 1], [0.5, 1.12419127424558], [1.0, 1.4615901622888245]]

Do I understand correctly that you propose to always use ... and only keep the minimum among the various numbers of digits that are output by all existing versions of Sage?

Could you explain more precisely why you object to showing the output of the doctests with the (increased) precision used by the current Sage version? Does it cause more work for you to keep them updated, or do you expect it may cause confusion among users with different versions of Sage?

@zimmermann6
Copy link

comment:34

I assume you are mostly referring to the changes made by #16858; this ticket only removes one digit.

I refer to changeset e216b6f in comment [comment:27].

Yes it would be better to only keep the minimum number of digits output by all versions of Sage (since the one we use in our book, i.e., 5.9).

The intent of those doctests is to only to check the examples that were printed in the book still work in versions of Sage after 5.9. Thus it makes no sense to add more digits than those that were actually printed on http://sagebook.gforge.inria.fr/. Any doctests with a different intent should go elsewhere in my opinion.

Paul

@pjbruin
Copy link
Contributor Author

pjbruin commented Sep 12, 2014

comment:35

Replying to @zimmermann6:

I assume you are mostly referring to the changes made by #16858; this ticket only removes one digit.

I refer to changeset e216b6f in comment [comment:27].

The only change made by that changeset in the french_book directory is

--- a/src/sage/tests/french_book/integration_doctest.py
+++ b/src/sage/tests/french_book/integration_doctest.py
@@ -180,7 +180,7 @@ Sage example in ./integration.tex, line 838::
 
     sage: t, y = var('t, y')
     sage: desolve_rk4(t*y*(2-y), y, ics=[0,1], end_points=[0, 1], step=0.5)
-    [[0, 1], [0.5, 1.12419127424558], [1.0, 1.4615901622888245]]
+    [[0, 1], [0.5, 1.12419127424558], [1.0, 1.461590162288825]]
 
     Sage example in ./integration.tex, line 861::
 

The (earlier) commit that added the extra digits was 7cb1dd5 in #16858.

Yes it would be better to only keep the minimum number of digits output by all versions of Sage (since the one we use in our book, i.e., 5.9).

The intent of those doctests is to only to check the examples that were printed in the book still work in versions of Sage after 5.9.

OK, but surely a doctest where the only change is an increased precision should be classified as "still working"? In any case, a user who types the doctest in a newer version of Sage will also see the extra digits.

Thus it makes no sense to add more digits than those that were actually printed on http://sagebook.gforge.inria.fr/. Any doctests with a different intent should go elsewhere in my opinion.

I don't disagree, but given that the doctests need to be updated in any case, I don't understand why adding the extra digits is bad and adding ... is better. Feel free to open a ticket for it, though!

@zimmermann6
Copy link

comment:36

I won't spend more time on this, please remove the tests corresponding to examples in our book that you break, it makes no sense to maintain them since the original goal is lost.

Paul

@jdemeyer
Copy link

comment:37

Replying to @zimmermann6:

I won't spend more time on this, please remove the tests corresponding to examples in our book that you break, it makes no sense to maintain them since the original goal is lost.

I agree with Peter Bruin that printing a few less or more digits doesn't really "break" the test. Even if the goal of reproducing the exact output of the book cannot be achieved, at least the goal of having output reasonably close to the book can be maintained.

@vbraun
Copy link
Member

vbraun commented Sep 16, 2014

Changed branch from u/pbruin/16908-maxima-5.34.1 to e216b6f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants