TypeError in gmpy2.iroot

[Smit-create]

gmpy2 produces correct result upto n=2**63-1 but with n=2**63 it leads to TypeError
Related to the sympy issue.

>>> gmpy2.iroot(4, 2**63-1)
(mpz(1), False)
>>> gmpy2.iroot(4, 2**63)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: iroot() requires 'mpz','int' arguments

Converting the arguments to mpz also doesn't work as:

>>> gmpy2.iroot(gmpy.mpz(4), gmpy.mpz(2**63))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: iroot() requires 'mpz','int' arguments

[casevh]

The exception should really have been ValueError.

By default, gmpy2 just mimics the behavior of the underlying GMP functions. The GMP functions expect the root to be pass as an unsigned long int. (Note: this could be different of 32 bit platforms or Windows.) With the size constraints of the mpz type, the result of iroot() will always be mpz(1) for any root large enough to trigger the overflow.

If you think it is helpful, I can change iroot() to always return mpz(1) instead of raising an exception.

[skirpichev]

Now:

>>> import gmpy2
>>> gmpy2.iroot(4, 2**63)
(mpz(1), False)

@casevh, probably you can close this issue.

[asmeurer]

Do gmpy operations generally overflow? I would rather have an error than an overflow.

[skirpichev]

@asmeurer, CPython operations on int's overflow...

>>> (256).to_bytes(1, 'big', signed=False)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: int too big to convert

[asmeurer]

That's not an overflow, that's an error.

[casevh]

I believe it should raise an OverflowError. GMP's mpz_root function requires and "unsigned long" argument. CPython raises an OverflowError exception when an argument can't be converted to a native C type. I was using my conversion code that returned ValueError but I'm now using Python's conversion code.

Are you ok with raising OverflowError?

[asmeurer]

OverflowError seems like the correct exception type to use.

[isuruf]

Unless the first operand is really large (>=2*(2**63) which takes > 1 exabyte to store) isn't the answer supposed to be mpz(1), False)?

[casevh]

@isuruf You are correct. I'll leave it alone and close the ticket. I'll verify the Windows semantics, too.

[tornaria]

Now:

>>> import gmpy2
>>> gmpy2.iroot(4, 2**63)
(mpz(1), False)

However:

>>> import gmpy2
>>> gmpy2.iroot(4, 2**64-1)
(mpz(1), False)
>>> gmpy2.iroot(4, 2**64)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: n must be > 0

And on 32 bit:

>>> import gmpy2
>>> gmpy2.iroot(4, 2**32-1)
(mpz(1), False)
>>> gmpy2.iroot(4, 2**32)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: n must be > 0

[casevh]

I have a patch that returns OverflowError when a positive integer cannot be converted to an unsigned long. Is that okay with everyone? Otherwise, it wouldn't be too difficult to return (mpz(1), False).

[tornaria]

I have a patch that returns OverflowError when a positive integer cannot be converted to an unsigned long. Is that okay with everyone?

An OverflowError seems better suited than the current message.

Otherwise, it wouldn't be too difficult to return (mpz(1), False).

In principle a=2**(2**32) fits in 512M and iroot(a, 2**32) should be (2, True), etc, so it's a bit more tricky than that (at least for 32 bit).

Would it make sense to export ULONG_MAX or something like that and document that n must be <= ULONG_MAX ?

[oscarbenjamin]

If y = iroot(x, n) then x >= y**n so if y > 1 then x.bit_length() >= n. It should be straight-forward to pick out all cases where x.bit_length() < n. Otherwise an OverflowError seems reasonable.

[casevh]

What should we do on 64-bit Windows?

GMP defines mpbitcnt_t as an unsigned long. MPIR defines mpbitcnt_t as an unsigned long long. I have a patch for GMP that changes the definition of mpbintcnt_t to unsigned long long.

I used to statically link gmpy2 so I didn't mind using the patched version of gmpy2. gmpy2 now exports a C-API that is used by Cython so I've started to provide DLLs for GMP, MPFR, and MPC. (See issue #320.) Should I distribute DLLs and header files that strictly follow GMP's definition or use the patched version?

I personally prefer the patched version of GMP since it makes 64-bit Windows, Linux, and MacOS behave the same.