sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 479 forks source link

Fixes for outdated Macaulay2 interface #25885

Closed mwageringel closed 5 years ago

mwageringel commented 6 years ago

This ticket implements support for polynomial rings with indexed variables such as macaulay2("QQ[x_0..x_2]").to_sage() in the conversion routine of the Macaulay2 interface. Only integer indices will work, as sage has no mechanism to handle indices that are tuples or lists.

In the process, this also fixes all the failing doctests due to switching to the most recent version of Macaulay2 1.11. Note that this was mostly a matter of small differences in the output of Macaulay2, except for the _tab_completion which was actually not working.

CC: @jplab @eulerreich @slel @tom111

Component: interfaces

Keywords: Macaulay2, interface

Author: Markus Wageringel

Branch/Commit: ea0a691

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/25885

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

Commit: d90528e

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

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

d90528eallow indexed variables in Macaulay2 interface
edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago
comment:3

This is awesome that someone takes care about macaulay2 interface, thanks !

Aside question: there used to be an old-style spkg some 10+ years ago, see http://old.files.sagemath.org/spkg/archive/, do you plan to package it again or is it beyond your knowledge of Sage packaging system (otherwise i could give a try) ?

mwageringel commented 6 years ago
comment:4

I don't currently have plans for that. I am not so much concerned about the Sage packaging system, but building Macaulay2 seems like a complicated endeavor, as I am not familiar with its build process at all. The current approach of calling the system-wide installation of Macaulay2 seems to be quite simple to set up, though, so I am not sure about the concrete advantages of creating a package again.

In any case, it would be good to know the reasons why exactly support for the old package was dropped.

slel commented 6 years ago

Changed keywords from none to Macaulay2, interface

slel commented 6 years ago
comment:5

See also:

mwageringel commented 6 years ago
comment:6

I should update the macaulay2.ring function, as well.

simon-king-jena commented 6 years ago
comment:7

Can you explain to me how to install Macaulay 2 on Ubuntu? I tried installation from the upstream sources, but first of all the list of dependencies given in upstream's INSTALL file is not complete (e.g., givaro is needed but not named), and when I tried to install the upstream sources in a Sage shell (as Sage does have givaro), I got other compile errors.

mwageringel commented 6 years ago
comment:9

Replying to @simon-king-jena:

Can you explain to me how to install Macaulay 2 on Ubuntu? I tried installation from the upstream sources, but first of all the list of dependencies given in upstream's INSTALL file is not complete (e.g., givaro is needed but not named), and when I tried to install the upstream sources in a Sage shell (as Sage does have givaro), I got other compile errors.

I'm afraid I can't help with this, as I don't have any experience with installing Macaulay2 from source.

dimpase commented 5 years ago
comment:10

I think I know how to fix tab completion:

diff --git a/src/sage/interfaces/macaulay2.py b/src/sage/interfaces/macaulay2.py
index 75a8ff62f0..0aae900ace 100644
--- a/src/sage/interfaces/macaulay2.py
+++ b/src/sage/interfaces/macaulay2.py
@@ -908,7 +908,7 @@ class Macaulay2Element(ExtraTabCompletion, ExpectElement):
                 if parent currentClass === currentClass then break;
                 currentClass = parent currentClass;
                 )
-            toString total""" % self.name())
+            print toString total""" % self.name())
         r = sorted(r[1:-1].split(", "))
         return r
dimpase commented 5 years ago

Reviewer: Dima Pasechnik

dimpase commented 5 years ago
comment:11

OK, in fact this branch already fixes the tab completion, sorry for noise. I am testing this with M2 version 1.12.0.1, the latest available in source, and I get errors in the 2 places where you changed

load -- read Macaulay2 commands
*******************************

to

load
****

(and, naturally, at the place where the version is checked, but that's OK) I'm willing to give this a positive review, provided that load ---... stuff is fixed, which is easy to make generic and working for both versions:

@@ -425,7 +425,7 @@ class Macaulay2(ExtraTabCompletion, Expect):
         EXAMPLES::

             sage: macaulay2.version() # optional - macaulay2
-            (1, 11)
+            (1, 1...
         """
         s = self.eval("version")
         r = re.compile("VERSION => (.*?)\n")
diff --git a/src/sage/interfaces/macaulay2.py b/src/sage/interfaces/macaulay2.py
index f6199ba220..d1ef414025 100644
--- a/src/sage/interfaces/macaulay2.py
+++ b/src/sage/interfaces/macaulay2.py
@@ -519,8 +519,8 @@ class Macaulay2(ExtraTabCompletion, Expect):
         EXAMPLES::

             sage: macaulay2.help("load")  # optional - macaulay2
-            load
-            ****
+            load ...
+            ****...
             ...
               * "input" -- read Macaulay2 commands and echo
               * "notify" -- whether to notify the user when a file is loaded
@@ -1179,8 +1179,8 @@ class Macaulay2Function(ExpectFunction):
         EXAMPLES::

             sage: print(macaulay2.load.__doc__)  # optional - macaulay2
-            load
-            ****
+            load ...
+            ****...
             ...
               * "input" -- read Macaulay2 commands and echo
               * "notify" -- whether to notify the user when a file is loaded
dimpase commented 5 years ago

Changed branch from u/gh-mwageringel/macaulay2 to public/interfaces/M2v1.11plus

dimpase commented 5 years ago

Changed commit from d90528e to ea0a691

dimpase commented 5 years ago

New commits:

e0f0bf8Merge branch 'u/gh-mwageringel/macaulay2' of trac.sagemath.org:sage into m2b
ea0a691made tests work for M2 versions 1.11 and 1.12
dimpase commented 5 years ago
comment:13

to make the story short, I've just added these changes. Good to go.

edd8e884-f507-429a-b577-5d554626c0fe commented 5 years ago
comment:14

@dimpase : apparently you already installed M2, do you think you could package it for Sage ?

dimpase commented 5 years ago
comment:15

All we need is to have M2 executable in the PATH. So while it's desirable, it's strictly speaking not too urgent. (I'm at ICERM now, with several M2 developers, and many users, around, and they also want such packaging, so there is demand for this).

Given that M2 is a mini-distribution, akin to Sage (well, much smaller, but still) it is a nontrivial task to have it built in Sage (many, but not all, libraries needed by M2 are in Sage, so these need to be dealt with too).

Fortunately M2 now (almost) got rid of the need to patch its libraries for bdwgc (aka Boehm GC, aka gc), this means won't need changes to Sage's libs to accommodate them.

mwageringel commented 5 years ago
comment:16

@dimpase: Thanks for the updates. I still had a small change related to this lying around that I would like to commit, once I manage to compile the current version of Sage.

vbraun commented 5 years ago

Changed branch from public/interfaces/M2v1.11plus to ea0a691