Opened 4 years ago

Closed 3 years ago

#28074 closed defect (fixed)

Fix caching of Macaulay2 polynomial rings

Reported by: gh-mwageringel Owned by:
Priority: minor Milestone: sage-9.0
Component: interfaces: optional Keywords: Macaulay2
Cc: dimpase, saliola Merged in:
Authors: Markus Wageringel Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 5709e61 (Commits, GitHub, GitLab) Commit: 5709e615c731313dedbaa23ec09d07a9b7bf5cf7
Dependencies: #27979 Stopgaps:

GitHub link to the corresponding issue

Description

This ticket:

  • adds caching of univariate Macaulay2 polynomial rings;
  • fixes an issue where, upon conversion of an ideal from Sage to Macaulay2, the Macaulay2 ring that is already cached does not get re-used, but is reconstructed instead;
sage: R.<x,y> = QQ[]
sage: R1 = macaulay2(R)
sage: I = R.ideal(y^2 - x)
sage: R2 = macaulay2(I).ring()
sage: R1._operator('===', R2)   # should be true
false
  • moves is_exact() from MPolynomialRing_macaulay2_repr to MPolynomialRing_base where it seems to belong.

Change History (13)

comment:1 Changed 4 years ago by gh-mwageringel

Authors: Markus Wageringel
Branch: u/gh-mwageringel/28074
Commit: c1464de8799cb4c0ecb9bdab7bc1c00e1b66863b
Dependencies: #27979
Status: newneeds_review

This is accomplished by refactoring and deduplicating some code related to the conversion. In some places, I replaced _macaulay2_() methods by implementations of _macaulay2_init_() which returns a string to be used as input for the Macaulay2 interpreter. This has the advantage that the generic interface mechanism in sage.structure.sage_object then takes care of the caching. The methods _macaulay2_set_ring() and _macaulay2_base_str() are removed.

(For the future – not in this ticket – it would also be nice to have caching when converting in the other direction, from Macaulay2 to Sage, so that the new Sage object remembers the Macaulay2 element it was constructed from. However, none of the interfaces seem to be doing this currently, as far as I can tell, so this could be more complicated to deal with.)

This branch is based on #27979 to avoid merge conflicts. Only the last commit is new. I tested this with Macaulay2 version 1.12 and Python 2 and 3.


New commits:

a4036cdFixed further Macaulay2-related doctests
c0321d3fix ascii art for matrices in Macaulay2 1.13+
7fe0483deprecate Macaulay2Element.to_sage
68b5715deprecate Macaulay2Element.structure_sheaf
e0ead46py3 fixes
2eba33ctrac 27979 several enhancements to M2 interface
c1464de28074: refactor caching of Macaulay2 polynomial rings

comment:2 Changed 4 years ago by git

Commit: c1464de8799cb4c0ecb9bdab7bc1c00e1b66863bef43417171d2bf7ccc68e8ade7b02df9f945ea3a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ef4341728074: refactor caching of Macaulay2 polynomial rings

comment:3 Changed 4 years ago by gh-mwageringel

I have just fixed 2 pyflakes warnings. The remaining one is about long which is dealt with in #27826.

comment:4 Changed 4 years ago by git

Commit: ef43417171d2bf7ccc68e8ade7b02df9f945ea3a5e606b61b210c23b15a89c27094214f2c800c457

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

46da1b628074: refactor caching of Macaulay2 polynomial rings
5e606b6use cpdef for is_exact as in super class CommutativeRing

comment:5 Changed 4 years ago by gh-mwageringel

I rebased on 8.9.beta4 and changed the signature of is_exact to resolve a Cython warning.

comment:6 Changed 4 years ago by dimpase

Cc: dimpase saliola added

comment:7 Changed 3 years ago by git

Commit: 5e606b61b210c23b15a89c27094214f2c800c4572cbfbb342448065e96a01a658ea00ffcddce60ac

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2cbfbb328074: refactor caching of Macaulay2 polynomial rings

comment:8 Changed 3 years ago by gh-mwageringel

Rebased.

comment:9 Changed 3 years ago by chapoton

Status: needs_reviewneeds_work

red branch

comment:10 Changed 3 years ago by dimpase

Branch: u/gh-mwageringel/28074public/interfaces/t28074
Commit: 2cbfbb342448065e96a01a658ea00ffcddce60ac5709e615c731313dedbaa23ec09d07a9b7bf5cf7
Status: needs_workneeds_review

New commits:

5709e6128074: refactor caching of Macaulay2 polynomial rings

comment:11 Changed 3 years ago by dimpase

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

looks good to me. Sorry for sitting on it for so long.

comment:12 Changed 3 years ago by chapoton

Milestone: sage-8.9sage-9.0

moving milestone to 9.0 (after release of 8.9)

comment:13 Changed 3 years ago by vbraun

Branch: public/interfaces/t280745709e615c731313dedbaa23ec09d07a9b7bf5cf7
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.