Update to maxima 5.48

[antonio-rojas]

Mostly output format changes, but it is not trivial to make test pass with both old and new versions

[antonio-rojas]

@tobiasdiez looks like this is still not available in conda, any idea who to ping to make it happen?

[github-actions]

Documentation preview for this PR (built with commit a56965b; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[tobiasdiez]

Only conda-forge/maxima-feedstock#40 needs to be merged by either @isuruf or @saraedum.

[antonio-rojas]

Reported this regression upstream at https://sourceforge.net/p/maxima/bugs/4601/

[orlitzky]

You may want to base this on #40574 in case some of the new output changes. I got started with the 5.48.x upgrade but gave up when I realized that not all of the output changes are improvements. (Thinking: we may want to support 5.47.x simultaneously rather than force people to upgrade to a version with regressions, but this is a lot of work).

[orlitzky]

IMO anything in a TESTS:: block is a good candidate to be moved to pytest, and to be checked with assert actual == expected. It's pretty dumb that we have to go through and update this output every time a new version of maxima is released. If the answer is right, the output is OK.

[antonio-rojas]

You may want to base this on #40574 in case some of the new output changes. I got started with the 5.48.x upgrade but gave up when I realized that not all of the output changes are improvements. (Thinking: we may want to support 5.47.x simultaneously rather than force people to upgrade to a version with regressions, but this is a lot of work).

Since that is already positively reviewed and this will take some time, I will rebase it after the next beta. Besides the non-trivial work of rewriting the doctests, supporting both versions also requires dealing with the pcprntd → *pcprntd* rename, which I have no idea how to do (I don't speak lisp).

[tobiasdiez]

supporting both versions also requires dealing with the pcprntd → *pcprntd* rename, which I have no idea how to do (I don't speak lisp).

A bit of dreaming: After #39030, we could have meson write the version of maxima (or actually all dependencies) into config.py. Then you should be able to conditionally use the correct ecl_eval call based on the version (and also make the doctests work for different maxima versions).

[antonio-rojas]

Actually, it looks like we may just drop that principal redefinition. It was originally added in #11987 to fix integrate(sin(x)*sin(x/3)/x^2, x, 0, oo), but that seems to work just fine nowadays without that redefinition. I don't completely understand the code though.

CORRECTION: That redefinition is there since the introduction of the maxima lib interface in 970a833, not clear why. Running CI now without it to see if anything breaks.

[antonio-rojas]

Seems to be needed:

**********************************************************************
Error: Failed example:: Got: 4/9
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        
    integrate(1/x^3,x,-1,3)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Integral is divergent.
Got:
    4/9
**********************************************************************

[antonio-rojas]

After #40574 it looks like #20467 is back:

**********************************************************************
File "/usr/lib/python3.13/site-packages/sage/symbolic/integration/integral.py", line 1096, in sage.symbolic.integration.integral.integrate
Failed example:
    integral(sin(k*x)/x*erf(x^2), x, 0, oo, algorithm='maxima')
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.13/site-packages/sage/doctest/forker.py", line 733, in _run
        self.compile_and_execute(example, compiler, test.globs)
        ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.13/site-packages/sage/doctest/forker.py", line 1157, in compile_and_execute
        exec(compiled, globs)
        ~~~~^^^^^^^^^^^^^^^^^
      File "<doctest sage.symbolic.integration.integral.integrate[152]>", line 1, in <module>
        integral(sin(k*x)/x*erf(x**Integer(2)), x, Integer(0), oo, algorithm='maxima')
        ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.13/site-packages/sage/misc/functional.py", line 787, in integral
        return x.integral(*args, **kwds)
               ~~~~~~~~~~^^^^^^^^^^^^^^^
      File "sage/symbolic/expression.pyx", line 13181, in sage.symbolic.expression.Expression.integral
        return integral(self, *args, **kwds)
      File "/usr/lib/python3.13/site-packages/sage/symbolic/integration/integral.py", line 1104, in integrate
        return integrator(expression, v, a, b)
      File "/usr/lib/python3.13/site-packages/sage/symbolic/integration/external.py", line 41, in maxima_integrator
        result = maxima.sr_integral(expression, v, a, b)
      File "/usr/lib/python3.13/site-packages/sage/interfaces/maxima_lib.py", line 802, in sr_integral
        return max_to_sr(maxima_eval(([max_integrate],
                         ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
                                      [sr_to_max(SR(a)) for a in args])))
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "sage/libs/ecl.pyx", line 828, in sage.libs.ecl.EclObject.__call__
        return ecl_wrap(ecl_safe_apply(self.obj, (<EclObject>lispargs).obj))
      File "sage/libs/ecl.pyx", line 356, in sage.libs.ecl.ecl_safe_apply
        raise RuntimeError("ECL says: {}".format(message))
    RuntimeError: ECL says: BINDING-STACK overflow at size 10240. Stack can probably be resized.
    Proceed with caution.
**********************************************************************

Reproducible in maxima itself (but only in 5.48):

(%i1) load(abs_integrate);
(%o1) /usr/share/maxima/5.48.1/share/contrib/integration/abs_integrate.mac
(%i2) integrate(sin(k*x)/x*erf(x^2),x,0,inf);

Maxima encountered a Lisp error:

 BINDING-STACK overflow at size 10240. Stack can probably be resized.
Proceed with caution.

Automatically continuing.
To enable the Lisp debugger set *debugger-hook* to nil.

[orlitzky]

After #40574 it looks like #20467 is back:

https://sourceforge.net/p/maxima/bugs/4603/

[nbruin]

CORRECTION: That redefinition is there since the introduction of the maxima lib interface in 970a833, not clear why. Running CI now without it to see if anything breaks.

That's because the original: https://sourceforge.net/p/maxima/code/ci/master/tree/src/defint.lisp#l1162 has a conditional to either throw a divergent error or to print a message about using a principal value definition for an integral once. In maxima_lib, that printed message is lost for sure. That's why the printing of the message is changed to the raising of an error.

Maxima was not designed to be used as a library. So sometimes it tries to make progress by asking questions interactively. We need to kill that and raise an error instead, since the library use doesn't allow for asking the user for clarification. Similarly (like here), maxima will sometimes compute a specific interpretation of a value that is otherwise undefined and print a message to clarify it did that. We don't have room to support that, so the safest way is to then just raise an error instead. That's why maxima_lib patches the (defun principal ...). The ticket #11987 refined the behaviour already. Removing the patch to (defun principal ...) entirely would probably lead to sage quietly give principal values for divergent integrals. So be careful with doing so!

In particular, integrate(1/x^3,x,-1,3) should return an error for us and doesn't in maxima proper:

(%i1) integrate(1/x^3,x,-1,3);
Principal Value
(%o1)  4/9

Removing the patch to defun principal could well change that one.

[antonio-rojas]

In particular, integrate(1/x^3,x,-1,3) should return an error for us and doesn't in maxima proper:

(%i1) integrate(1/x^3,x,-1,3);
Principal Value
(%o1)  4/9

Removing the patch to defun principal could well change that one.

Yes, see the comment following the one you replied to.

[antonio-rojas]

So how do we proceed? Judging from maxima release history i'm afraid we probably won't see a release with the regression fixes for months/years (until 5.49 is released). My suggestion is to add the patches for https://sourceforge.net/p/maxima/bugs/4601/ and https://sourceforge.net/p/maxima/bugs/4603/ to sage-the-distro (and conda if possible). The respective tests will fail in distributions using unpatched 5.48 system packages, but that could serve as a reminder that they are shipping a buggy maxima package.

Note that 5.48 also fixes some regressions, cf. https://groups.google.com/g/sage-support/c/pCYoD6x2Y8M

[nbruin]

Since that is already positively reviewed and this will take some time, I will rebase it after the next beta. Besides the non-trivial work of rewriting the doctests, supporting both versions also requires dealing with the pcprntd → *pcprntd* rename, which I have no idea how to do (I don't speak lisp).

Oh I do speak it a bit. We can just retrieve the maxima version at runtime and act upon that:

maxima_version = [int(a) for a in ecl_eval("(maxima-version1)").python().strip('"').split("_")]
if maxima_version >= [5,48,0]:
    ecl_eval('(defun principal nil (cond ($noprincipal (diverg)) ((not *pcprntd*) (merror "Divergent Integral"))))')
else:
    ecl_eval('(defun principal nil (cond ($noprincipal (diverg)) ((not pcprntd) (merror "Divergent Integral"))))')

I just saw the question about why the override was necessary; I didn't see why it was considered. This should at least be an easy fix. We're monkey-patching maxima at runtime, so it's not so crazy to do the version check at runtime as well, and do something version-dependent.

[nbruin]

IMO anything in a TESTS:: block is a good candidate to be moved to pytest, and to be checked with assert actual == expected. It's pretty dumb that we have to go through and update this output every time a new version of maxima is released. If the answer is right, the output is OK.

How about just rewriting the test to do that assert? You can even do that in the examples. If you want to display the output even when it varies, you can do so and mark it #random to prevent doctest failures. Although perhaps we want to keep the documented output in line with what the user will actually get back.

There is sage --fixdoctests which should make updating doctest output way easier, but you'd obviously have to carefully review the changes to make sure they're all safe. Going with #random would be the only option if you want to support versions that give different outputs.

[orlitzky]

IMO anything in a TESTS:: block is a good candidate to be moved to pytest, and to be checked with assert actual == expected. It's pretty dumb that we have to go through and update this output every time a new version of maxima is released. If the answer is right, the output is OK.

How about just rewriting the test to do that assert? You can even do that in the examples. If you want to display the output even when it varies, you can do so and mark it #random to prevent doctest failures. Although perhaps we want to keep the documented output in line with what the user will actually get back.

This is certainly doable, but I worry that the quality of the documentation suffers as the workarounds get more elaborate. If a new user sees,

EXAMPLES:

Computing trigonometric integrals works::

    sage: integrate(sin(x), x, 0, pi/2)
    1

then the purpose is clear, and the user can re-run the example himself to learn how things work. This is obviously contrived, but if instead we have an example like,

Computing trigonometric integrals works::

    sage: maxima_547_expected = "1"
    sage: maxima_548_expected = "cos(0)"
    sage: actual = integrate(sin(x), x, 0, pi/2)
    sage: actual  # random
    1
    sage: str(actual) in [maxima_547_expected, maxima_548_expected]
    True

then he's going to be completely confused as to what most of that has to do with trigonometric integrals. You can leave a comment explaining the boilerplate, but I don't think that necessarily improves the situation. And if the user tries to experiment with this, the results he gets may not be what he's expecting based on the example output.

So ultimately, I think "ugly" tests like that are better off being done in pytest, but it's more of a "would be nice" than a "has to be done this way."

[nbruin]

In such a situation, perhaps replace the educational example with one that isn't so dependent on the version and move the other test (which is now interesting because different versions of maxima treat it differently) move it to the TESTS section?

[tobiasdiez]

Another big problem with

sage: str(actual) in [maxima_547_expected, maxima_548_expected]
    True

in doctests is that in case of a failure, you only get "expected true, got false". No information what was actual the result. The same code in pytest will, in case of failure, actually print out a helpful error message, including the value of actual.

[orlitzky]

In such a situation, perhaps replace the educational example with one that isn't so dependent on the version and move the other test (which is now interesting because different versions of maxima treat it differently) move it to the TESTS section?

That would be fine too. At that point I think there are a lot of small benefits to moving the test to pytest:

1. The doctest runner requires a lot of custom code
2. Doctests run in a sage shell with preparsing/imports that are slower than not doing them
3. String comparisons of the results is usually a lot slower than just comparing them
4. Doctests implicitly are context-dependent on the earlier tests of the same method
5. Partially related to that, doctests can't be run selectively / in parallel

But we are splitting hairs. I care more about getting rid of the string comparisons that fail every time there's a new version of maxima than I do about the moral purity of the replacement.

[nbruin]

in doctests is that in case of a failure, you only get "expected true, got false". No information what was actual the result. The same code in pytest will, in case of failure, actually print out a helpful error message, including the value of actual.

I think you can improve that by writing assert ... in the doctest instead. The resulting error trace should be a little more informative.

The argument for doctests was that the tests live in the same spot as the code, increasing the chance the tests get updated when the code changes. For documentation (because of docstrings ...) this will remain the case. If we also use pytest then there is an extra thing for people to keep track of: we'll still have our docstring examples (and test blocks), but no in addition there are the pytest files/functions/classes.

So we'd need to have some significant advantages for pytest in the face of the higher cognitive load (note that the people advocating for pytest, by necessity, are already familiar with and used to pytest, so they'll underestimate the load it inflicts on community members that are not). A minor example I can already see is that the test code will not live in a string, so it'll be a little easier to edit/lint tests, so it's quite possible an argument can be made for it and perhaps even a convincing one.

[nbruin]

5. Partially related to that, doctests can't be run selectively / in parallel

sage -pt ?

[orlitzky]

5. Partially related to that, doctests can't be run selectively / in parallel

sage -pt ?

This tests files in parallel, but if there are thousands of tests in one file, it runs them sequentially. It doesn't matter too much if you're testing the entire sage library, but if you're working on a single file and testing it frequently, you can waste a lot of time. For an extreme example, I have 64 very slow cores -- being able to test a single file in parallel can reduce the edit-test-edit cycle from ten minutes to ten seconds.

With a lot of work, we could probably make the doctest runner more parallel; but with pytest, someone else has already done it, and (as a bonus) we don't have to maintain the code. Pytest also does not have the limitation that tests for the same method must share an environment. It's not wrong to compute a ton of integrals in the docstring for integrate(), but most of those integrals are independent, and could be parallelized if they weren't doctests.