Maxima from the OS #35615
Closed
Maxima from the OS #35615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes sagemath#12345", use "Add a new method to multiply two integers" --> ### 📚 Description Fixing the linter workflow <!-- Describe your changes here in detail. --> <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
This should work automatically, and the previous behavior is incorrect if we intend to use the system copy of maxima.
This new spkg-configure.m4 checks for both a "maxima" executable, and the usability of a "maxima" package by ECL. (The latter requires a patched maxima until a new release with commit a0d7a43e523 is made.) Notably absent for the moment is a version check, but that should be okay: at the moment, the only system copies of maxima that support being used as an ECL library are those that are patched, and the only reason to patch them is if you intend to use them with SageMath. So in short, if a distro patches their maxima to use it with sage, we expect it to work with sage. This commit also moves the SAGE_MAXIMA_FAS variable handling from ECL's spkg-configure.m4 to maxima's (where it would have belonged in the first place, if maxima had an spkg-configure.m4 at the time.)
This patch won't be present in a system copy of maxima (at least for now), so we disable these tests to avoid spurious failures once we start accepting maxima from the system.
We have some old code and an old test for a memory limit that was present in ECL 12 years ago, worked around in ticket 6772. The memory limit is no longer in place, and would not have applied to a non-ECL maxima from the system anyway. Since the test itself is purposely wasteful (to ensure that we can use lots of RAM), we take this opportunity to remove it and speed up the test suite a bit.
… directory. This just fixes a test. The problem was noticed on Arch where, due to some other circumstances, maxima was executed from within /usr/sbin. But all we know for sure is that maxima will live in PATH. So we modify the test to look only for "maxima", and not its parent directory.
The ECL library is now contained in a separate maxima-fas package that will pull in maxima itself.
|
As mentioned in #32867 (comment) we should deal with the 5.46 update first. The release is over a year old and many distros are shipping it by now, this will make a few tests fail in those |
|
On Gentoo with system maxima 5.46.0 I get few test errors. One of them is "noise" from 5.46 apparently converting floats in integrals to rationals: similarly, one can't use floats in limit computations: But it's actually a regression of sorts, as On the other hand, some limits done better than before: some output formats got better: some outputs are equivalent, but swapped around: as above, but in another file: A format change: (can just test the numerical value) |
|
Should we just merge this, and would on updating Sage's maxima to 5.46.0 ? |
is a regression in Sage interface to maxima - computing this limit at maxima console works just fine. |
|
@nbruin - do you have any idea why with maxima 5.46.0 the following breaks (this is with prints() added: --- a/src/sage/interfaces/maxima_lib.py
+++ b/src/sage/interfaces/maxima_lib.py
@@ -993,8 +993,12 @@ class MaximaLib(MaximaAbstract):
L.append(max_plus)
elif dir == "minus":
L.append(max_minus)
+ print("L=", L)
+ eva=maxima_eval(([max_limit], L))
+ print(" eval result: ", eva,"\n")
return max_to_sr(maxima_eval(([max_limit], L)))
except RuntimeError as error:
+ print("oops, error:", str(error))
s = str(error)
if "Is" in s: # Maxima asked for a condition
self._missing_assumption(s)for integer values the limit is computed just fine as well as in maxima console it's all just fine: |
|
By the looks of it, the expression that sage passes to maxima is the same as what should ressult from entering it directly into maxima. Hence, any differences probably arise from the different options that maxima_calculus sets. Hence, in order to identify the regression in maxima, you should run the example with the environment options that maxima_calculus uses. It's very conceivable that maxima development introduced a regression in the behaviour with those options activated, since their testing mainly takes place with the standard options in force. |
|
ah, right, it's that kind of thing again. Reported upstream : https://sourceforge.net/p/maxima/bugs/4138/ |
|
@thierry-FreeBSD - is ed729a9 OK? |
|
@dimpase Is no version check needed at all? |
Yes it is! |
ATM FreeBSD has 5.46.0. It is usually kept up-to-date. |
|
Documentation preview for this PR is ready! 🎉 |
|
let's review this as a part of #35619 and close after it's merged. |
|
this will be completed in #35619 |
vbraun
pushed a commit
that referenced
this pull request
May 28, 2023
### 📚 upgrade Maxima to 5.46.0 Our 5.45.0 is getting old. This will fix #32898 as well. We have one mild regression found while working on #35615 (already reported upstream), but number of improvements in a number of places. As #35615 is a part of this PR branch, I propose to review them all here. We also promote `info` spkg to standard, and make it install `makeinfo/texi2any`, to deal with Maxima docs. This allows us to simplify configuration/installation of Maxima and ECL. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies #35615 - provide `spkg-configure.m4` for Maxima <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #35619 Reported by: Dima Pasechnik Reviewer(s): Dima Pasechnik, François Bissey, Gonzalo Tornaría, Matthias Köppe, Mauricio Collares, Michael Orlitzky
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will take care of #32867 (the rebased branch is from there)