operations between sage and gmpy2 numbers

[vklein]

Currently, operations involving Sage and gmpy2 numbers are broken (see 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: ​https://github.com/aleaxit/gmpy/issues/214

[jdemeyer]

Replying to vklein:

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?

[vklein]

You're right, description updated

[jdemeyer]

Thanks for the clarification.

[vdelecroix]

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

[vdelecroix]

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

[jdemeyer]

Replying to vdelecroix:

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.

[vdelecroix]

Let me update the ticket description then.

[vklein]

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 ?

[vdelecroix]

Replying to vklein:

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

This is a great news. Thanks for the work!

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

Do we have a schedule for the next gmpy2 release? Since gmpy2 becomes a standard package for the next Sage release it would be nice if it would happen before.

Otherwise, I am not a big fan of using patches that are integrated upstream but not released yet.

[vdelecroix]

Replying to vdelecroix:

Replying to vklein:

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

Do we have a schedule for the next gmpy2 release? Since gmpy2 becomes a standard package for the next Sage release it would be nice if it would happen before.

Somehow ​#199 is a bit of an answer.

[vklein]

After some talk with vdelecroix we agree to make a gmpy2 patch with ​#217.

---

New commits:

​edfe875

Trac #23052: patch gmpy2 with PR #217 ...

[vdelecroix]

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]              ^

[git]

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

​b60be6d

Trac #23052: Fix thousands lines of warnings.

[vklein]

Warnings fixed.

[vdelecroix]

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

[git]

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

​5dff83b

Trac #23052: arith.misc.crt ensure this function

[vklein]

function sage.arith.misc.crt fixed

[vdelecroix]

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.

[git]

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

​b7fbb9a

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

[vklein]

Patch renamed and links added.