operations between sage and gmpy2 numbers #23052

vinklein · 2017-05-22T10:18:18Z

Currently, operations involving Sage and gmpy2 numbers are broken (see [comment:7]). Though, it works fine with gmpy2 and Python numbers. The reason is that the various binary operations implemented in gmpy2 just check for Python integer types. It should be possible to modify these functions in order that an operation involving a gmpy2 number and a type implementing one of the __mpz__/__mpq__/__mpfr__/__mpc__ method should work.

Note: #22928 implemented the conversion Sage type -> gmpy2 type via the implementation of the __mpz__, __mpq__, __mpfr__ and __mpc__ methods.

Upstream issue: aleaxit/gmpy#214

Upstream: Fixed upstream, in a later stable release.

CC: @videlec @jdemeyer

Component: packages: standard

Author: Vincent Klein

Branch/Commit: b7fbb9a

Reviewer: Vincent Delecroix

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

The text was updated successfully, but these errors were encountered:

jdemeyer · 2017-05-22T10:26:39Z

Replying to @vinklein:

Provide coercion for gmpy2 numbers for operations -, +, * and /

This makes no sense to me. What does it mean to provide coercion for certain operations only?

vinklein · 2017-05-22T11:22:26Z

comment:3

You're right, description updated

jdemeyer · 2017-05-22T11:44:02Z

Changed dependencies from 22928 to #22928

jdemeyer · 2017-05-22T11:44:02Z

comment:4

Thanks for the clarification.

videlec · 2018-02-07T08:45:06Z

Changed dependencies from #22928 to #22928, #23024

videlec · 2019-03-03T10:19:23Z

comment:7

I don't think that this feature is desirable. The behavior should be the same as with cypari2

sage: import cypari2
sage: pari = cypari2.Pari()
sage: type(pari(1) + 1)
<type 'cypari2.gen.Gen'>
sage: type(1 + pari(1))
<type 'cypari2.gen.Gen'>

Mixing gmpy2 numbers with sage numbers currently raise errors

sage: import gmpy2
sage: type(gmpy2.mpz(1) + 1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-eeaeca1c122b> in <module>()
----> 1 type(gmpy2.mpz(Integer(1)) + Integer(1))

/opt/sage/local/lib/python2.7/site-packages/sage/rings/integer.pyx in sage.rings.integer.Integer.__add__ (build/cythonized/sage/rings/integer.c:11460)()
   1683             return y
   1684 
-> 1685         return coercion_model.bin_op(left, right, operator.add)
   1686 
   1687     cpdef _add_(self, right):

/opt/sage/local/lib/python2.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:11110)()
   1208         # We should really include the underlying error.
   1209         # This causes so much headache.
-> 1210         raise bin_op_exception(op, x, y)
   1211 
   1212     cpdef canonical_coercion(self, x, y):

TypeError: unsupported operand parent(s) for +: '<type 'mpz'>' and 'Integer Ring'
sage: type(1 + gmpy2.mpz(1))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-ea356a02d58b> in <module>()
----> 1 type(Integer(1) + gmpy2.mpz(Integer(1)))

/opt/sage/local/lib/python2.7/site-packages/sage/rings/integer.pyx in sage.rings.integer.Integer.__add__ (build/cythonized/sage/rings/integer.c:11460)()
   1683             return y
   1684 
-> 1685         return coercion_model.bin_op(left, right, operator.add)
   1686 
   1687     cpdef _add_(self, right):

/opt/sage/local/lib/python2.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:11110)()
   1208         # We should really include the underlying error.
   1209         # This causes so much headache.
-> 1210         raise bin_op_exception(op, x, y)
   1211 
   1212     cpdef canonical_coercion(self, x, y):

TypeError: unsupported operand parent(s) for +: 'Integer Ring' and '<type 'mpz'>'

videlec · 2019-03-03T10:22:30Z

comment:8

To make this work, one has to modify the various binary operators in gmpy2 (e.g. GMPy_Integer_Add).

jdemeyer · 2019-03-03T10:53:04Z

comment:9

Replying to @videlec:

To make this work, one has to modify the various binary operators in gmpy2 (e.g. GMPy_Integer_Add).

This is obviously the right thing to do. Given that

sage: from gmpy2 import mpz
sage: mpz(1) + int(1)
mpz(2)

works, it's actually strange that

sage: from gmpy2 import mpz
sage: mpz(1) + Integer(1)
Traceback (most recent call last):
...
TypeError: unsupported operand parent(s) for +: '<type 'mpz'>' and 'Integer Ring'

doesn't work.

videlec · 2019-03-03T10:54:26Z

comment:10

Let me update the ticket description then.

videlec · 2019-03-03T10:59:13Z

Upstream: Not yet reported upstream; Will do shortly.

vinklein · 2019-03-04T17:10:40Z

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

vinklein · 2019-03-07T07:30:54Z

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.

vinklein · 2019-03-07T09:47:21Z

comment:14

The gmpy2 pull request #217 has been merged this morning.

Should we make a patch based on this or wait for the next gmpy2 release ?

vinklein · 2019-03-08T09:41:01Z

Changed dependencies from #22928, #23024 to none

videlec · 2019-03-08T18:46:20Z

comment:21

You create a lot of warnings with the macros. This should be fixed.

gmpy2-2.1.0a4.p1]     src/gmpy2_cache.c: In function 'GMPy_MPZ_NewInit':
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:44:60: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
[gmpy2-2.1.0a4.p1]      #define HAS_STRICT_MPZ_CONVERSION(x) HAS_MPZ_CONVERSION(x) && \
[gmpy2-2.1.0a4.p1]                                                                 ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:52:25: note: in expansion of macro 'HAS_STRICT_MPZ_CONVERSION'
[gmpy2-2.1.0a4.p1]                              HAS_STRICT_MPZ_CONVERSION(x))
[gmpy2-2.1.0a4.p1]                              ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:61:25: note: in expansion of macro 'IS_INTEGER'
[gmpy2-2.1.0a4.p1]      #define IS_RATIONAL(x) (IS_INTEGER(x) || IS_RATIONAL_ONLY(x))
[gmpy2-2.1.0a4.p1]                              ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:64:21: note: in expansion of macro 'IS_RATIONAL'
[gmpy2-2.1.0a4.p1]      #define IS_REAL(x) (IS_RATIONAL(x) || IS_REAL_ONLY(x))
[gmpy2-2.1.0a4.p1]                          ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_cache.c:191:9: note: in expansion of macro 'IS_REAL'
[gmpy2-2.1.0a4.p1]          if (IS_REAL(n)) {
[gmpy2-2.1.0a4.p1]              ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:46:62: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
[gmpy2-2.1.0a4.p1]      #define HAS_STRICT_MPFR_CONVERSION(x) HAS_MPFR_CONVERSION(x) && \
[gmpy2-2.1.0a4.p1]                                                                   ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:63:63: note: in expansion of macro 'HAS_STRICT_MPFR_CONVERSION'
[gmpy2-2.1.0a4.p1]      #define IS_REAL_ONLY(x) (MPFR_Check(x) || PyFloat_Check(x) || HAS_STRICT_MPFR_CONVERSION(x))
[gmpy2-2.1.0a4.p1]                                                                    ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_convert.h:64:39: note: in expansion of macro 'IS_REAL_ONLY'
[gmpy2-2.1.0a4.p1]      #define IS_REAL(x) (IS_RATIONAL(x) || IS_REAL_ONLY(x))
[gmpy2-2.1.0a4.p1]                                            ^
[gmpy2-2.1.0a4.p1]     src/gmpy2_cache.c:191:9: note: in expansion of macro 'IS_REAL'
[gmpy2-2.1.0a4.p1]          if (IS_REAL(n)) {
[gmpy2-2.1.0a4.p1]              ^

sagetrac-git · 2019-03-11T08:35:15Z

Branch pushed to git repo; I updated commit sha1. New commits:

`b60be6d`	`Trac #23052: Fix thousands lines of warnings.`

sagetrac-git · 2019-03-11T08:35:15Z

Changed commit from edfe875 to b60be6d

vinklein · 2019-03-11T08:35:54Z

comment:23

Warnings fixed.

videlec · 2019-03-11T15:18:44Z

comment:24

some failures...

Doctesting 3797 files using 14 threads.
sage -t --long src/sage/arith/misc.py
**********************************************************************
File "src/sage/arith/misc.py", line 3120, in sage.arith.misc.crt
Failed example:
    crt(mpz(2), mpz(3), mpz(7), mpz(11))
Expected:
    58
Got:
    mpz(58)
**********************************************************************
File "src/sage/arith/misc.py", line 3122, in sage.arith.misc.crt
Failed example:
    crt(mpz(2), 3, mpz(7), numpy.int8(11))
Expected:
    58
Got:
    mpz(58)
**********************************************************************
1 item had failures:
   2 of  37 in sage.arith.misc.crt
    [1056 tests, 2 failures, 30.02 s]
----------------------------------------------------------------------
sage -t --long src/sage/arith/misc.py  # 2 doctests failed
----------------------------------------------------------------------

sagetrac-git · 2019-03-11T17:30:34Z

Branch pushed to git repo; I updated commit sha1. New commits:

`5dff83b`	`Trac #23052: arith.misc.crt ensure this function`

sagetrac-git · 2019-03-11T17:30:34Z

Changed commit from b60be6d to 5dff83b

vinklein · 2019-03-11T17:32:16Z

comment:26

function sage.arith.misc.crt fixed

videlec · 2019-03-11T19:00:02Z

comment:27

The patch that you called PR217.patch comes from two pull requests (#217 and #218). You would better change the name of the patch and mention very explicitely in the header of the patch the urls of the actual pull requests.

sagetrac-git · 2019-03-12T07:31:39Z

Changed commit from 5dff83b to b7fbb9a

sagetrac-git · 2019-03-12T07:31:39Z

Branch pushed to git repo; I updated commit sha1. New commits:

`b7fbb9a`	`Trac #23052: Rename patch and add links to the...`

vinklein · 2019-03-12T07:32:27Z

comment:29

Patch renamed and links added.

videlec · 2019-03-12T08:47:24Z

Reviewer: Vincent Delecroix

vbraun · 2019-03-14T18:13:58Z

Changed branch from u/vklein/23052 to b7fbb9a

vinklein mannequin added this to the sage-8.0 milestone May 22, 2017

vinklein mannequin added c: packages: optional labels May 22, 2017

This comment has been minimized.

Sign in to view

vinklein mannequin self-assigned this May 22, 2017

This comment has been minimized.

Sign in to view

videlec changed the title ~~Provide coercion for gmpy2 numbers~~ coercion for gmpy2 numbers May 22, 2017

This comment has been minimized.

Sign in to view

videlec changed the title ~~coercion for gmpy2 numbers~~ operations between sage and gmpy2 numbers Mar 3, 2019

videlec added c: packages: standard and removed c: packages: optional labels Mar 3, 2019

This comment has been minimized.

Sign in to view

vinklein mannequin added the s: needs review label Mar 8, 2019

vinklein mannequin modified the milestones: sage-8.0, sage-8.7 Mar 8, 2019

videlec added s: needs work and removed s: needs review labels Mar 8, 2019

vinklein mannequin added s: needs review and removed s: needs work labels Mar 11, 2019

videlec added s: needs work and removed s: needs review labels Mar 11, 2019

vinklein mannequin added s: needs review and removed s: needs work labels Mar 11, 2019

videlec added s: positive review and removed s: needs review labels Mar 12, 2019

vbraun removed the s: positive review label Mar 14, 2019

vbraun closed this as completed in ae38e0e Mar 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

operations between sage and gmpy2 numbers #23052

operations between sage and gmpy2 numbers #23052

vinklein mannequin commented May 22, 2017

This comment has been minimized.

jdemeyer commented May 22, 2017

This comment has been minimized.

vinklein mannequin commented May 22, 2017

jdemeyer commented May 22, 2017

jdemeyer commented May 22, 2017

This comment has been minimized.

videlec commented Feb 7, 2018

This comment has been minimized.

videlec commented Mar 3, 2019

videlec commented Mar 3, 2019

jdemeyer commented Mar 3, 2019

videlec commented Mar 3, 2019

videlec commented Mar 3, 2019

This comment has been minimized.

This comment has been minimized.

vinklein mannequin commented Mar 4, 2019

vinklein mannequin commented Mar 7, 2019

vinklein mannequin commented Mar 7, 2019

vinklein mannequin commented Mar 8, 2019

videlec commented Mar 8, 2019

sagetrac-git mannequin commented Mar 11, 2019

sagetrac-git mannequin commented Mar 11, 2019

vinklein mannequin commented Mar 11, 2019

videlec commented Mar 11, 2019

sagetrac-git mannequin commented Mar 11, 2019

sagetrac-git mannequin commented Mar 11, 2019

vinklein mannequin commented Mar 11, 2019

videlec commented Mar 11, 2019

sagetrac-git mannequin commented Mar 12, 2019

sagetrac-git mannequin commented Mar 12, 2019

vinklein mannequin commented Mar 12, 2019

videlec commented Mar 12, 2019

vbraun commented Mar 14, 2019

operations between sage and gmpy2 numbers #23052

operations between sage and gmpy2 numbers #23052

Comments

vinklein mannequin commented May 22, 2017

This comment has been minimized.

jdemeyer commented May 22, 2017

This comment has been minimized.

vinklein mannequin commented May 22, 2017

jdemeyer commented May 22, 2017

jdemeyer commented May 22, 2017

This comment has been minimized.

videlec commented Feb 7, 2018

This comment has been minimized.

videlec commented Mar 3, 2019

videlec commented Mar 3, 2019

jdemeyer commented Mar 3, 2019

videlec commented Mar 3, 2019

videlec commented Mar 3, 2019

This comment has been minimized.

This comment has been minimized.

vinklein mannequin commented Mar 4, 2019

vinklein mannequin commented Mar 7, 2019

vinklein mannequin commented Mar 7, 2019

vinklein mannequin commented Mar 8, 2019

videlec commented Mar 8, 2019

sagetrac-git mannequin commented Mar 11, 2019

sagetrac-git mannequin commented Mar 11, 2019

vinklein mannequin commented Mar 11, 2019

videlec commented Mar 11, 2019

sagetrac-git mannequin commented Mar 11, 2019

sagetrac-git mannequin commented Mar 11, 2019

vinklein mannequin commented Mar 11, 2019

videlec commented Mar 11, 2019

sagetrac-git mannequin commented Mar 12, 2019

sagetrac-git mannequin commented Mar 12, 2019

vinklein mannequin commented Mar 12, 2019

videlec commented Mar 12, 2019

vbraun commented Mar 14, 2019