Opened 21 months ago
Closed 17 months ago
#28074 closed defect (fixed)
Fix caching of Macaulay2 polynomial rings
Reported by:  ghmwageringel  Owned by:  

Priority:  minor  Milestone:  sage9.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: 
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 reused, 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()
fromMPolynomialRing_macaulay2_repr
toMPolynomialRing_base
where it seems to belong.
Change History (13)
comment:1 Changed 21 months ago by
 Branch set to u/ghmwageringel/28074
 Commit set to c1464de8799cb4c0ecb9bdab7bc1c00e1b66863b
 Dependencies set to #27979
 Status changed from new to needs_review
comment:2 Changed 21 months ago by
 Commit changed from c1464de8799cb4c0ecb9bdab7bc1c00e1b66863b to ef43417171d2bf7ccc68e8ade7b02df9f945ea3a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ef43417  28074: refactor caching of Macaulay2 polynomial rings

comment:3 Changed 21 months ago by
I have just fixed 2 pyflakes warnings. The remaining one is about long
which is dealt with in #27826.
comment:4 Changed 20 months ago by
 Commit changed from ef43417171d2bf7ccc68e8ade7b02df9f945ea3a to 5e606b61b210c23b15a89c27094214f2c800c457
comment:5 Changed 20 months ago by
I rebased on 8.9.beta4 and changed the signature of is_exact
to resolve a Cython warning.
comment:6 Changed 19 months ago by
 Cc dimpase saliola added
comment:7 Changed 19 months ago by
 Commit changed from 5e606b61b210c23b15a89c27094214f2c800c457 to 2cbfbb342448065e96a01a658ea00ffcddce60ac
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2cbfbb3  28074: refactor caching of Macaulay2 polynomial rings

comment:8 Changed 19 months ago by
Rebased.
comment:10 Changed 18 months ago by
 Branch changed from u/ghmwageringel/28074 to public/interfaces/t28074
 Commit changed from 2cbfbb342448065e96a01a658ea00ffcddce60ac to 5709e615c731313dedbaa23ec09d07a9b7bf5cf7
 Status changed from needs_work to needs_review
New commits:
5709e61  28074: refactor caching of Macaulay2 polynomial rings

comment:11 Changed 18 months ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
looks good to me. Sorry for sitting on it for so long.
comment:12 Changed 17 months ago by
 Milestone changed from sage8.9 to sage9.0
moving milestone to 9.0 (after release of 8.9)
comment:13 Changed 17 months ago by
 Branch changed from public/interfaces/t28074 to 5709e615c731313dedbaa23ec09d07a9b7bf5cf7
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
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 insage.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:
Fixed further Macaulay2related doctests
fix ascii art for matrices in Macaulay2 1.13+
deprecate Macaulay2Element.to_sage
deprecate Macaulay2Element.structure_sheaf
py3 fixes
trac 27979 several enhancements to M2 interface
28074: refactor caching of Macaulay2 polynomial rings