Opened 10 years ago

Closed 10 years ago

#11580 closed defect (fixed)

Magma interface cannot convert multivariate polynomials back to Sage

Reported by: nbruin Owned by: was
Priority: minor Milestone: sage-4.7.2
Component: interfaces Keywords:
Cc: Merged in: sage-4.7.2.alpha3
Authors: Nils Bruin Reviewers: William Stein, Marco Streng
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11456 Stopgaps:

Status badges

Description (last modified by leif)

Evidently:

sage: R.<x,y,z>=QQ[]
sage: f=x^2+3*y
sage: magma(f).sage()
NameError: name 'x' is not defined

Apply trac_11580-magma.patch to the extcode repository.

Apply trac_11580-doctests.patch to the Sage library.

Attachments (4)

trac_11580-magma_mpols.patch (2.6 KB) - added by nbruin 10 years ago.
trac_11580-doctest.patch (1.9 KB) - added by nbruin 10 years ago.
trac_11580-doctests.patch (1.8 KB) - added by mstreng 10 years ago.
trac_11580-magma.patch (1.6 KB) - added by mstreng 10 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 10 years ago by nbruin

  • Authors set to Nils Bruin
  • Description modified (diff)
  • Status changed from new to needs_review

Changed 10 years ago by nbruin

comment:2 Changed 10 years ago by nbruin

Updated patch so that magma tuples of length 1 become python tuples (doesn't affect the polynomial bit of the patch)

comment:3 Changed 10 years ago by was

  • Status changed from needs_review to needs_work

Hi Nils,

Can you add

sage: R.<x,y,z>=QQ[]
sage: f=x^2+3*y
sage: magma(f).sage()        # optional - magma
x^2 + 3*y  # or whatever

as a doctest in interfaces/magma.py or somewhere?

Changed 10 years ago by nbruin

comment:4 Changed 10 years ago by nbruin

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Thanks William! Done.

comment:5 Changed 10 years ago by was

  • Status changed from needs_review to positive_review

comment:6 Changed 10 years ago by jdemeyer

  • Description modified (diff)
  • Reviewers set to William Stein

comment:7 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 10 years ago by jdemeyer

  • Status changed from positive_review to needs_work
  • Work issues set to Coordinate with #11456

comment:9 Changed 10 years ago by mstreng

  • Reviewers changed from William Stein to William Stein, Marco Streng
  • Work issues changed from Coordinate with #11456 to rebase on top of #11456

This patch breaks conversion of finite field elements from Magma to Sage.

Without patch:

sage: magma(GF(2)(1)).sage()
1

With patch:

sage: magma(GF(2)(1)).sage()
...
Segmentation fault
...
Magma crashed executing _sage_[3], _sage_[4] := Sage(_sage_[2]);

With patch:

sage -t -only-optional=magma devel/sage/sage/rings/finite_rings/element_givaro.pyx
(same crash)

When converting finite field elements from Magma to Sage, they are first converted to a SeqEnum of finite field elments. In particular, a proper implementation of conversions of SeqEnum to Sage sets of an infinite loop. This was fixed in #11456, so we should rebase this patch on top of #11456.

Changed 10 years ago by mstreng

Changed 10 years ago by mstreng

comment:10 Changed 10 years ago by mstreng

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues rebase on top of #11456 deleted

Rebased on top of #11456.

Removed the function "intrinsic Sage(X::SeqEnum) -> MonStgElt, BoolElt", because it was equal (modulo documentation) to the one in #11456.

comment:11 Changed 10 years ago by jdemeyer

  • Dependencies set to #11456

comment:12 Changed 10 years ago by mstreng

I think the original author or reviewer could quickly review my patch. I only removed some stuff from the previous version, which already had a positive review...

comment:13 Changed 10 years ago by was

  • Status changed from needs_review to positive_review

Positive review of new version. Thanks for the rebase.

comment:14 Changed 10 years ago by leif

  • Description modified (diff)
  • Summary changed from Magma interface cannot convert multivariate polynomials back to sage to Magma interface cannot convert multivariate polynomials back to Sage

comment:15 Changed 10 years ago by leif

  • Merged in set to sage-4.7.2.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.