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: sage-6.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:

Status badges

Change History (38)

comment:1 follow-ups: Changed 7 years ago by kcrisman

Any tix this fixes, btw?

comment:2 in reply to: ↑ 1 Changed 7 years ago by pbruin

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.

comment:4 in reply to: ↑ 1 ; follow-up: Changed 7 years ago by jdemeyer

Replying to kcrisman:

Any tix this fixes, btw?

#14965 claims to be fixed by Maxima 5.34

comment:5 follow-up: Changed 7 years ago by fbissey

How many patches can we drop with this version?

comment:6 Changed 7 years ago by pbruin

  • Authors set to Peter Bruin
  • Branch set to u/pbruin/16908-maxima-5.34.0
  • Commit set to 0fd6543a6f86c8493002641d79c91d05bf28a5bc
  • Status changed from new to needs_review

comment:7 in reply to: ↑ 4 Changed 7 years ago by pbruin

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.]

Last edited 7 years ago by pbruin (previous) (diff)

comment:8 in reply to: ↑ 5 Changed 7 years ago by pbruin

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.]

Last edited 7 years ago by pbruin (previous) (diff)

comment:9 Changed 7 years ago by pbruin

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

comment:10 Changed 7 years ago by jpflori

  • Cc jpflori added

comment:11 Changed 7 years ago by git

  • Commit changed from 0fd6543a6f86c8493002641d79c91d05bf28a5bc to d83032da5bce8c654c9cbcda59da215ce1cb0083

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

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

comment:12 Changed 7 years ago by pbruin

  • 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 pbruin

  • Branch changed from u/pbruin/16908-maxima-5.34.0 to u/pbruin/16908-maxima-5.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 jdemeyer

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...

Last edited 7 years ago by jdemeyer (previous) (diff)

comment:15 follow-up: Changed 7 years ago by 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.

comment:16 in reply to: ↑ 15 Changed 7 years ago by jdemeyer

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 git

  • Commit changed from 44c889e34cb18fadf29e8142b849a11c62a3f2c7 to 2785ccc9b069cb55d804e43a506be7460abd8cd1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2785cccTrac 16908: upgrade Maxima to 5.34.1

comment:18 Changed 7 years ago by pbruin

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): 
    116116            sage: f = function('f')
    117117            sage: print_latex(f(x),x)
    118118            '\\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}
    119121        """
    120122        from sage.misc.latex import latex
    121123        if not is_SymbolicVariable(x):
    class DefiniteIntegral(BuiltinFunction): 
    236238            sage: f = function('f')
    237239            sage: print_latex(f(x),x,0,1)
    238240            '\\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}
    241243        """
    242244        from sage.misc.latex import latex
    243245        if not is_SymbolicVariable(x):

comment:19 Changed 7 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:20 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

Conflicts with #16651 or #16858

comment:21 Changed 7 years ago by git

  • Commit changed from 2785ccc9b069cb55d804e43a506be7460abd8cd1 to b8babffdf9b44a53c1ea4ebfecf0b3cc0e1de0bc

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

comment:22 Changed 7 years ago by pbruin

  • Status changed from needs_work to positive_review

comment:23 Changed 7 years ago by vbraun

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

comment:24 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

comment:25 follow-up: Changed 7 years ago by jdemeyer

I'll have a look at the doctest failures.

comment:26 in reply to: ↑ 25 Changed 7 years ago by pbruin

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.

comment:27 Changed 7 years ago by git

  • Commit changed from b8babffdf9b44a53c1ea4ebfecf0b3cc0e1de0bc to e216b6fd358015ff4d22a11e24ed75d491121245

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

comment:28 Changed 7 years ago by jdemeyer

  • Cc zimmerma added

Another trivial change to a French book test.

comment:29 follow-up: Changed 7 years ago by jdemeyer

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 pbruin

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.

comment:31 Changed 7 years ago by pbruin

  • 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 follow-up: Changed 7 years ago by zimmerma

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

comment:33 in reply to: ↑ 32 Changed 7 years ago by pbruin

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*(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?

comment:34 follow-up: Changed 7 years ago by 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.

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 pbruin

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:: 
    180180
    181181    sage: t, y = var('t, y')
    182182    sage: desolve_rk4(t*y*(2-y), y, ics=[0,1], end_points=[0, 1], step=0.5)
    183     [[0, 1], [0.5, 1.12419127424558], [1.0, 1.4615901622888245]]
     183    [[0, 1], [0.5, 1.12419127424558], [1.0, 1.461590162288825]]
    184184
    185185    Sage example in ./integration.tex, line 861::
    186186

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!

Last edited 7 years ago by pbruin (previous) (diff)

comment:36 follow-up: Changed 7 years ago by zimmerma

  • 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 jdemeyer

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 vbraun

  • Branch changed from u/pbruin/16908-maxima-5.34.1 to e216b6fd358015ff4d22a11e24ed75d491121245
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.