Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Magma interface cannot convert multivariate polynomials back to Sage #11580

Closed
nbruin opened this issue Jul 7, 2011 · 24 comments
Closed

Magma interface cannot convert multivariate polynomials back to Sage #11580

nbruin opened this issue Jul 7, 2011 · 24 comments

Comments

@nbruin
Copy link
Contributor

nbruin commented Jul 7, 2011

Evidently:

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

Apply attachment: trac_11580-magma.patch to the extcode repository.

Apply attachment: trac_11580-doctests.patch to the Sage library.

Depends on #11456

Component: interfaces

Author: Nils Bruin

Reviewer: William Stein, Marco Streng

Merged: sage-4.7.2.alpha3

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

@nbruin
Copy link
Contributor Author

nbruin commented Jul 7, 2011

Author: Nils Bruin

@nbruin

This comment has been minimized.

@nbruin
Copy link
Contributor Author

nbruin commented Jul 9, 2011

Attachment: trac_11580-magma_mpols.patch.gz

@nbruin
Copy link
Contributor Author

nbruin commented Jul 9, 2011

comment:2

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

@williamstein
Copy link
Contributor

comment:3

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?

@nbruin
Copy link
Contributor Author

nbruin commented Jul 15, 2011

Attachment: trac_11580-doctest.patch.gz

@nbruin
Copy link
Contributor Author

nbruin commented Jul 15, 2011

comment:4

Thanks William! Done.

@nbruin

This comment has been minimized.

@jdemeyer
Copy link

Reviewer: William Stein

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

Work Issues: Coordinate with #11456

@mstreng
Copy link

mstreng commented Jul 29, 2011

Changed reviewer from William Stein to William Stein, Marco Streng

@mstreng
Copy link

mstreng commented Jul 29, 2011

Changed work issues from Coordinate with #11456 to rebase on top of #11456

@mstreng
Copy link

mstreng commented Jul 29, 2011

comment:9

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.

@mstreng
Copy link

mstreng commented Jul 29, 2011

Attachment: trac_11580-doctests.patch.gz

@mstreng
Copy link

mstreng commented Jul 29, 2011

Changed work issues from rebase on top of #11456 to none

@mstreng

This comment has been minimized.

@mstreng
Copy link

mstreng commented Jul 29, 2011

comment:10

Attachment: trac_11580-magma.patch.gz

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.

@jdemeyer
Copy link

jdemeyer commented Aug 1, 2011

Dependencies: #11456

@mstreng
Copy link

mstreng commented Sep 2, 2011

comment:12

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...

@williamstein
Copy link
Contributor

comment:13

Positive review of new version. Thanks for the rebase.

@nexttime

This comment has been minimized.

@nexttime nexttime mannequin changed the title Magma interface cannot convert multivariate polynomials back to sage Magma interface cannot convert multivariate polynomials back to Sage Sep 8, 2011
@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2011

Merged: sage-4.7.2.alpha3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants