-
-
Notifications
You must be signed in to change notification settings - Fork 648
Update to maxima-5.48.1 and re-enable abs_integrate #40574
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
base: develop
Are you sure you want to change the base?
Conversation
We are still describing a bunch of patches that don't exist.
Documentation preview for this PR (built with commit 16a04e2; changes) is ready! 🎉 |
one can load Maxima's |
The problem was that it causes some (new) incorrect results, and you can't unload it. Several of these tests now work without abs_integrate. The
The last two may be unattractive, but you do get an answer, so it's only the first two where we completely lose the ability to integrate them. And the first is covered by the maxima documentation: https://maxima.sourceforge.io/docs/manual/de/maxima_181.html. What do we gain by testing them and then throwing away the result? |
At least, leave a comment somewhere on the use of abs_integrate |
There are some tests in this file that still mention abs_integrate, though it was disabled a few years ago. We remove all mention of it, and clean up a bit: * Drop a test repeated in src/sage/symbolic/integration/integral.py * Remove "# known bug" from any examples that now pass without abs_integrate * Delete any remaining "# known bug" tests. They are not really bugs -- you just don't get a nice answer -- and it is a waste of time to run them and then throw away the answer.
Replace two examples that can only be solved when abs_integrate is loaded. These examples are now tested to NOT produce a meaningful result, so that if in the future they can be solved without abs_integrate, we will know. The surrounding text explains why the abs_integrate package is not loaded by default, and how to load it manually.
03e2d9c
to
4d02f7f
Compare
Ok, the new commit adds back the two examples where maxima is helpless without |
It actually looks like many of the |
All of the issues we encountered (and reported) with abs_integrate have been fixed. We re-enable it with doctests to, 1. Ensure that none of those errors have returned 2. Test that we can solve some new integrals The list of problems comes from, sagemath#12731 With the upstream fixes from, https://sourceforge.net/p/maxima/code/ci/3ca4235b
c454002
to
1a69eea
Compare
Added an experimental commit to re-enable |
are you on maxima 5.47, or 5.48 ?
|
5.48 |
we should get sage to 5.48 too |
The wrong answer is returned unless you have Maxima 5.48, which is not widespread enough for us to require it yet.
We can solve one additional integral in this file now that abs_integrate is back.
We can solve one additional integral in this file now that abs_integrate is back.
The tests pass, so maybe let's keep it enabled? I upgraded maxima as well. |
I just check the website 5.48.1 is out. I think we can directly bump to 5.48.1 which has more bug fixes @orlitzky |
9ac0c0a
to
f7a1a90
Compare
And you need to change the title @orlitzky |
The output from one of these examples has changed slightly after a Maxima upgrade. We add ".expand()" on the end of it to ensure that the answer is consistent.
The output from two examples has changed slightly after a Maxima upgrade. We add a "full_simplify()" to both to ensure that the output is consistent.
We used to have Maxima's
abs_integrate
package loaded by default, but it caused more problems than it solved, and was disabled in #12731. All known issues have been resolved, however:This PR cleans up some existing tests, debt left over from when
abs_integrate
was disabled, and then re-enables it with a whole new collection of tests for the old problems. We do not gain as much as we used to (vanilla maxima has improved), but we do get some extra integrals for "free" now that the problems appear to be resolved.