Opened 7 years ago
Closed 7 years ago
#16908 closed task (fixed)
Upgrade Maxima to 5.34.1
Reported by:  pbruin  Owned by:  

Priority:  minor  Milestone:  sage6.4 
Component:  packages: standard  Keywords:  maxima 
Cc:  zimmerma, jpflori, was  Merged in:  
Authors:  Peter Bruin  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  e216b6f (Commits, GitHub, GitLab)  Commit:  e216b6fd358015ff4d22a11e24ed75d491121245 
Dependencies:  Stopgaps: 
Description (last modified by )
Tarball: http://pub.math.leidenuniv.nl/~bruinpj/sage/maxima5.34.1.tar.bz2
This appears to fix #14965.
Change History (38)
comment:1 followups: ↓ 2 ↓ 4 Changed 7 years ago by
comment:2 in reply to: ↑ 1 Changed 7 years ago by
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 Maximarelated 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.
comment:3 Changed 7 years ago by
Changelog is here: https://sourceforge.net/p/maxima/code/ci/master/tree/ChangeLog5.34
comment:4 in reply to: ↑ 1 ; followup: ↓ 7 Changed 7 years ago by
comment:5 followup: ↓ 8 Changed 7 years ago by
How many patches can we drop with this version?
comment:6 Changed 7 years ago by
 Branch set to u/pbruin/16908maxima5.34.0
 Commit set to 0fd6543a6f86c8493002641d79c91d05bf28a5bc
 Status changed from new to needs_review
comment:7 in reply to: ↑ 4 Changed 7 years ago by
comment:8 in reply to: ↑ 5 Changed 7 years ago by
Replying to fbissey:
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.]
comment:9 Changed 7 years ago by
I have tested this (and all tests pass) on x86_64 and ARM 32bit.
comment:10 Changed 7 years ago by
 Cc jpflori added
comment:11 Changed 7 years ago by
 Commit changed from 0fd6543a6f86c8493002641d79c91d05bf28a5bc to d83032da5bce8c654c9cbcda59da215ce1cb0083
Branch pushed to git repo; I updated commit sha1. New commits:
d83032d  Trac 16908: replace compileverbose.patch by suitable fixes in Sage code

comment:12 Changed 7 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
 Summary changed from Upgrade Maxima to 5.34.0 to Upgrade Maxima to 5.34.1
Maxima 5.34.1 has been released, an updated branch is coming soon.
comment:13 Changed 7 years ago by
 Branch changed from u/pbruin/16908maxima5.34.0 to u/pbruin/16908maxima5.34.1
 Commit changed from d83032da5bce8c654c9cbcda59da215ce1cb0083 to 44c889e34cb18fadf29e8142b849a11c62a3f2c7
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:14 Changed 7 years ago by
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...
comment:15 followup: ↓ 16 Changed 7 years ago by
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.
comment:16 in reply to: ↑ 15 Changed 7 years ago by
Replying to pbruin:
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.
comment:17 Changed 7 years ago by
 Commit changed from 44c889e34cb18fadf29e8142b849a11c62a3f2c7 to 2785ccc9b069cb55d804e43a506be7460abd8cd1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2785ccc  Trac 16908: upgrade Maxima to 5.34.1

comment:18 Changed 7 years ago by
I amended the existing commit instead of adding a new one, because the commit message mentioned the wrong Maxima version. I added the following doctests:

src/sage/symbolic/integration/integral.py
a b class IndefiniteIntegral(BuiltinFunction): 116 116 sage: f = function('f') 117 117 sage: print_latex(f(x),x) 118 118 '\\int f\\left(x\\right)\\,{d x}' 119 sage: latex(integrate(tan(x)/x, x)) 120 \int \frac{\tan\left(x\right)}{x}\,{d x} 119 121 """ 120 122 from sage.misc.latex import latex 121 123 if not is_SymbolicVariable(x): … … class DefiniteIntegral(BuiltinFunction): 236 238 sage: f = function('f') 237 239 sage: print_latex(f(x),x,0,1) 238 240 '\\int_{0}^{1} f\\left(x\\right)\\,{d x}' 239 sage: latex(integrate( 1/(1+sqrt(x)),x,0,1))240 \int_{0}^{1} \frac{ 1}{\sqrt{x} + 1}\,{d x}241 sage: latex(integrate(tan(x)/x, x, 0, 1)) 242 \int_{0}^{1} \frac{\tan\left(x\right)}{x}\,{d x} 241 243 """ 242 244 from sage.misc.latex import latex 243 245 if not is_SymbolicVariable(x):
comment:19 Changed 7 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:20 Changed 7 years ago by
 Status changed from positive_review to needs_work
comment:21 Changed 7 years ago by
 Commit changed from 2785ccc9b069cb55d804e43a506be7460abd8cd1 to b8babffdf9b44a53c1ea4ebfecf0b3cc0e1de0bc
Branch pushed to git repo; I updated commit sha1. New commits:
38a52cf  Make the to_poly_solve option work

2573b80  fix two trivial doctest failures

6fb6bee  Use for loop instead of while

c534e55  Merge branch 'develop' into t/16651/buggy_to_poly_solve

a801e64  16651: reviewer's patch: language refinements

1fa0110  Docstring fixes

b8babff  Merge branch 'u/vbraun/buggy_to_poly_solve' of ssh://trac.sagemath.org/sage into ticket/16908maxima5.34.1

comment:22 Changed 7 years ago by
 Status changed from needs_work to positive_review
comment:23 Changed 7 years ago by
I get various doctest failures on top of 6.4.beta3:
sage t long warnlong 40.1 src/sage/calculus/desolvers.py # 2 doctests failed sage t long warnlong 40.1 src/sage/rings/number_field/number_field_element.pyx # 1 doctest failed sage t long warnlong 40.1 src/sage/tests/french_book/integration_doctest.py # 1 doctest failed sage t long warnlong 40.1 src/sage/modules/free_module_element.pyx # 1 doctest failed sage t long warnlong 40.1 src/sage/symbolic/relation.py # 1 doctest failed sage t long warnlong 40.1 src/sage/functions/other.py # 1 doctest failed sage t long warnlong 40.1 src/sage/functions/special.py # 2 doctests failed
Looks like it is all due to changed precision of floats
comment:24 Changed 7 years ago by
 Status changed from positive_review to needs_work
comment:25 followup: ↓ 26 Changed 7 years ago by
I'll have a look at the doctest failures.
comment:26 in reply to: ↑ 25 Changed 7 years ago by
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 32bit.
comment:27 Changed 7 years ago by
 Commit changed from b8babffdf9b44a53c1ea4ebfecf0b3cc0e1de0bc to e216b6fd358015ff4d22a11e24ed75d491121245
comment:29 followup: ↓ 30 Changed 7 years ago by
Do you understand why floats from Maxima are now printed with one digit less of precision???
comment:30 in reply to: ↑ 29 Changed 7 years ago by
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 explodenformatfloat
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.
comment:31 Changed 7 years ago by
 Status changed from needs_work to positive_review
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...
comment:32 followup: ↓ 33 Changed 7 years ago by
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*(2y), 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
comment:33 in reply to: ↑ 32 Changed 7 years ago by
Replying to zimmerma:
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*(2y), 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?
comment:34 followup: ↓ 35 Changed 7 years ago by
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 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
comment:35 in reply to: ↑ 34 Changed 7 years ago by
Replying to zimmerma:
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 27.
The only change made by that changeset in the french_book
directory is

src/sage/tests/french_book/integration_doctest.py
a b Sage example in ./integration.tex, line 838:: 180 180 181 181 sage: t, y = var('t, y') 182 182 sage: desolve_rk4(t*y*(2y), y, ics=[0,1], end_points=[0, 1], step=0.5) 183 [[0, 1], [0.5, 1.12419127424558], [1.0, 1.46159016228882 45]]183 [[0, 1], [0.5, 1.12419127424558], [1.0, 1.461590162288825]] 184 184 185 185 Sage example in ./integration.tex, line 861:: 186 186
The (earlier) commit that added the extra digits was 7cb1dd50 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!
comment:36 followup: ↓ 37 Changed 7 years ago by
 Cc was added
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
comment:37 in reply to: ↑ 36 Changed 7 years ago by
Replying to zimmerma:
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.
comment:38 Changed 7 years ago by
 Branch changed from u/pbruin/16908maxima5.34.1 to e216b6fd358015ff4d22a11e24ed75d491121245
 Resolution set to fixed
 Status changed from positive_review to closed
Any tix this fixes, btw?