Make Sage work with maxima 5.47#35707
Conversation
|
Just FTR, doctests pass for me with 5.47.0 (normal and long) with the current PR. My suggestions are merely cosmetic, and I wouldn't mind keeping it as it is (all these testing is horrible anyway). Once we settle on something, I could try to run tests on 5.47.0 / 5.46.0 / 5.45.1, just to make sure it's ok. |
|
If the appending of Another solution would be subtracting the know answer and simplify it to 0. That can only work for exact results of course. |
| chebyshevu(x, x) | ||
| sage: maxima(chebyshev_U(2,x, hold=True)) | ||
| 3*((-(8*(1-_SAGE_VAR_x))/3)+(4*(1-_SAGE_VAR_x)^2)/3+1) | ||
| 3*(...-...(8*(1-_SAGE_VAR_x))/3)+(4*(1-_SAGE_VAR_x)^2)/3+1) |
There was a problem hiding this comment.
sage: maxima(chebyshev_U(2,x, hold=True)).sage()
4*(x - 1)^2 + 8*x - 5
I don't know what this is trying to acomplish... I guess that maxima knows how to handle chebyshev_U(2, x).
There was a problem hiding this comment.
I guess it's testing conversion to maxima, so I'm not sure converting back to sage the result makes much sense
There was a problem hiding this comment.
Yeah, it's supposed to be an example of the __init__ method. Why send it to maxima at all?
|
Summary: don't waste too much time in this... Maybe just put the "new correct" answer in the |
| sage: A.eigenvectors() | ||
| [[[0,4],[3,1]],[[[1,0,0,-4],[0,1,0,-2],[0,0,1,-4/3]],[[1,2,3,4]]]] | ||
| sage: A.eigenvectors().sage() | ||
| [[[0, 4], [3, 1]], [[[1, 0, 0, -4], [0, 1, 0, -2], [0, 0, 1, -4/3]], [[1, 2, 3, 4]]]] |
There was a problem hiding this comment.
This is difficult to test correctly because the eigenvalues, eigenvectors, and multiplicities are split across three lists and need to be matched up. The eigenvectors I would not expect to be unique anyway.
Lazy fix: switch it to a matrix with only nonnegative eigenvalues and a set of eigenvectors whose entries can be chosen integral and nonnegative.
| sage: A = maxima("matrix ([1, -4], [1, -1])") | ||
| sage: eig = A.eigenvectors() | ||
| sage: eig | ||
| [[[-sqrt(3)*%i,sqrt(3)*%i],[1,1]], [[[1,(sqrt(3)*%i+1)/4]],[[1,-(sqrt(3)*%i-1)/4]]]] |
There was a problem hiding this comment.
I thought I commented on this one already, but don't see it. I don't know why we're telling people to use the low-level maxima interface to compute the eigenvalues of a 2x2 matrix in a linear algebra tutorial. Personally I would just delete it (and the surrounding explanation).
There was a problem hiding this comment.
Then the gap section immediately after that should be dropped too. I think we should leave any further doc cleanup for a follow up ticket.
orlitzky
left a comment
There was a problem hiding this comment.
Ok, we shouldn't hold this up any longer. The first commit is pretty important. Thanks.
src/sage/interfaces/maxima_lib.py
Outdated
| ecl_eval("(require 'maxima \"{}\")".format(MAXIMA_FAS)) | ||
| else: | ||
| ecl_eval("(require 'maxima)") | ||
| ecl_eval("(maxima::initialize-runtime-globals)") |
There was a problem hiding this comment.
It seems this is unfixing #26968, as seen here: https://github.com/void-linux/void-packages/actions/runs/5366826557/jobs/9736459766?pr=44624#step:7:43488
Presumably, calling initialize-runtime-globals will run set-pathnames and fail without the workaround that is in the try block... Can this be moved after that?
There was a problem hiding this comment.
My suggestion:
--- a/src/sage/interfaces/maxima_lib.py
+++ b/src/sage/interfaces/maxima_lib.py
@@ -133,12 +133,11 @@ if MAXIMA_FAS:
ecl_eval("(require 'maxima \"{}\")".format(MAXIMA_FAS))
else:
ecl_eval("(require 'maxima)")
-ecl_eval("(maxima::initialize-runtime-globals)")
ecl_eval("(in-package :maxima)")
-ecl_eval("(setq $nolabels t))")
-ecl_eval("(defvar *MAXIMA-LANG-SUBDIR* NIL)")
ecl_eval("(set-locale-subdir)")
+# this has to happen early so the workaround works
+# do not add other initialization before this block
try:
ecl_eval("(set-pathnames)")
except RuntimeError:
@@ -155,6 +154,8 @@ except RuntimeError:
# Call `(set-pathnames)` again to complete its job.
ecl_eval("(set-pathnames)")
+ecl_eval("(initialize-runtime-globals)")
+ecl_eval("(setq $nolabels t))")
ecl_eval("(defun add-lineinfo (x) x)")
ecl_eval('(defun principal nil (cond ($noprincipal (diverg)) ((not pcprntd) (merror "Divergent Integral"))))')
ecl_eval("(remprop 'mfactorial 'grind)") # don't use ! for factorials (#11539)Calling initialize-runtime-globals will run set-pathnames and be subject to the issue described in sagemath#26968. Thus the workaround introduced in sagemath#35195 has to be done before anything that may call set-pathnames (e.g. initialize-runtime-globals).
|
Please merge antonio-rojas#2 into this PR before merging this PR in main. |
Make sure sagemath#26968 is not unfixed after sagemath#35707.
|
Documentation preview for this PR (built with commit 808ab47; changes) is ready! 🎉 |
followup to sagemath#35707 (make sage maxima 5.47-compatible) fixes sagemath#38599
followup to sagemath#35707 (make sage maxima 5.47-compatible) fixes sagemath#38599
sagemathgh-38601: upgrade maxima to 5.47.0 followup to sagemath#35707 (make sage maxima 5.47-compatible) fixes sagemath#38599 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38601 Reported by: Dima Pasechnik Reviewer(s): Kwankyu Lee
sagemathgh-38601: upgrade maxima to 5.47.0 followup to sagemath#35707 (make sage maxima 5.47-compatible) fixes sagemath#38599 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38601 Reported by: Dima Pasechnik Reviewer(s): Kwankyu Lee
Maxima 5.47 has been released. The main change that affects Sage is https://sourceforge.net/p/maxima/code/ci/f5ca00582c24cfedca53a664335f6c13e1e40cf9, which makes the maxima ecl library crash because the
e-valvariable is unassigned. We fix this by running theinitialize-runtime-globalsfunction on load.The remaining issues are a few harmless test failures caused by numerical noise and output format changes (such as additional parentheses near minus signs)