Opened 5 years ago

Closed 3 years ago

#18266 closed enhancement (fixed)

Conversion from Sage polynomial to gap/libgap

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-7.4
Component: interfaces Keywords:
Cc: vbraun, jipilab Merged in:
Authors: Vincent Delecroix Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: f1b8bcd (Commits) Commit: f1b8bcd98ffadede706ec244975fab07fa3bd942
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

It is not possible to use (in a straightforward way) univariate polynomial in libgap

sage: R.<x> = ZZ[]
sage: libgap(x^5+2*x+1)
Traceback (most recent call last):
...
ValueError: libGAP: Error, Variable: 'x' must have a value

And multivariate polynomials with either gap or libgap

sage: R.<x,y> = ZZ[]
sage: gap(x+y)
Traceback (most recent call last):
...
TypeError: Gap produced error output
sage: libgap(x+y)
Traceback (most recent call last):
...
ValueError: libGAP: Error, Variable: 'x' must have a value

The above ticket (partially based on #2420) implement the above conversions.

Note: The conversion from gap/libgap to Sage does not work as well and will be dealt with in #21020.

Change History (10)

comment:1 Changed 3 years ago by vdelecroix

  • Branch set to u/vdelecroix/18266
  • Commit set to f1b8bcd98ffadede706ec244975fab07fa3bd942
  • Milestone changed from sage-6.7 to sage-7.3

New commits:

f1b8bcdTrac 18266: Sage polynomial -> gap/libgap polynomial

comment:2 Changed 3 years ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Conversion between Sage and libgap polynomials to Conversion from Sage polynomial to gap/libgap

comment:3 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:4 Changed 3 years ago by tscrim

It looks good except for _coerce_map_from_. I don't think it is a good idea to have libgap have a coerce map from everything; there are many algebras that (lib)gap almost certainly has no idea about (e.g., Jordan algebras).

comment:5 Changed 3 years ago by vdelecroix

  • coercion between interfaces objects and Sage objects should be one way. So if for a given ring there is a coercion Sage -> GAP then it should be the case for every ring that GAP understands.
  • The solution I used is precisely what is done with PARI/GP (see for instance the PariInstance class in /sage/libs/pari/pari_instance.pyx)
  • Implementing the function _coerce_map_from_ would be a useless pain
  • "interface objects" are only intended for people having a knowledge of the underlying software

Now, if for some specific GAP object you want to lift it back to Sage then you can either use conversion (i.e. MyParent(my_interface_object)) or use the .sage() method

sage: pari(13).sage()
13
sage: parent(_)
Integer Ring
sage: gap(13).sage()
13
sage: parent(_)
Integer Ring
sage: libgap(13).sage()
13
sage: parent(_)
Integer Ring

comment:6 Changed 3 years ago by tscrim

The problem with unlimited coercion maps is that we get a bad (IMO incorrect) error message (this is on develop):

sage: cat = Algebras(QQ).WithBasis().FiniteDimensional()
sage: C = CombinatorialFreeModule(QQ, ['x','y','z'], category=cat)
sage: J1 = JordanAlgebra(C, names=['a','b','c'])
sage: J1.an_element() + gap(2)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: unsupported operand parent(s) for '+': 'Jordan algebra of Free module generated by {'x', 'y', 'z'} over Rational Field' and 'Gap'
sage: J1.an_element() + pari(2)
---------------------------------------------------------------------------
PariError                                 Traceback (most recent call last)
...
PariError: incorrect type in gtos [integer expected] (t_POL)

There should be a conversion, but I think a coercion is far too strong. Perhaps we should ask sage-devel?

comment:7 Changed 3 years ago by vdelecroix

It is a good idea to discuss what should we do concerning coercion and external softwares/libraries.

Concerning this ticket, it is very handy that the coercion always go from Sage to third party interfaces

sage: x = polygen(ZZ)
sage: p = x^3 + 2*x + 1
sage: p(pari(2))
13
sage: type(_)
<type 'sage.libs.pari.gen.gen'>

Making explicit the subset of Sage parents that can actually be represented in a given interface might indeed be much better. But certainly harder to write and maintain. The way I choose is the lazy one. The error messages you mentioned are wrong but should never appear if you restrict to "pure Sage functions". If desirable for pari/gap, I guess that implementing it belongs to another ticket.

comment:8 Changed 3 years ago by vdelecroix

And this is not the only wrong thing with interfaces. The default implementation of conversion between Sage and the interfaces etc relies on string representations and global variables... which is of course unreliable

sage: R = PolynomialRing(ZZ,('x','y','z'))
sage: p = 3*R.0*R.1 + R.2^3
sage: pari(p).sage()
Traceback (most recent call last):
...
NameError: name 'y' is not defined

or

sage: pari(ZZ).sage()
<built-in function IntegerRing>
sage: pari(RR)
RealFieldwith53bitsofprecision
sage: pari(RR).sage()
Traceback (most recent call last):
...
NameError: name 'RealFieldwith53bitsofprecision' is not defined
Last edited 3 years ago by vdelecroix (previous) (diff)

comment:9 Changed 3 years ago by tscrim

  • Milestone changed from sage-7.3 to sage-7.4
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Well, since we already do a catch-all coercion for pari and we got no response from sage-devel, I can tolerate the current state of the coercion.

comment:10 Changed 3 years ago by vbraun

  • Branch changed from u/vdelecroix/18266 to f1b8bcd98ffadede706ec244975fab07fa3bd942
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.