Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ Contributors (0.9.0):

Changes (0.9.0):

- [gh-323](https://github.com/flintlib/python-flint/pull/323),
Faster conversion of `fmpz` to `float`. (RO)
- [gh-322](https://github.com/flintlib/python-flint/pull/322),
Add `mul_low` and `pow_trunc` methods to `fmpz_poly`, `fmpq_poly` and
`nmod_poly`. (RO)
Expand Down
11 changes: 11 additions & 0 deletions src/flint/test/test_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,17 @@ def test_fmpz():
assert math.ceil(f) == 2
assert type(math.ceil(f)) is flint.fmpz

# Float conversion should follow Python rounding conventions
for mant in range(2**56 - 64, 2**56 + 64):
for shift in range(0, 900, 20):
for eps in range(-5, 5):
f = (mant << shift) + eps
assert float(f) == float(flint.fmpz(f))
assert float(-f) == float(flint.fmpz(-f))
# Test float conversion overflow
assert raises(lambda: float(flint.fmpz(2**1024)), OverflowError)
assert raises(lambda: float(flint.fmpz(-2**1024)), OverflowError)

assert flint.fmpz(2) != []
assert +flint.fmpz(0) == 0
assert +flint.fmpz(1) == 1
Expand Down
36 changes: 36 additions & 0 deletions src/flint/types/fmpz.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ from flint.utils.typecheck cimport typecheck
from flint.utils.conversion cimport chars_from_str
from flint.utils.conversion cimport str_from_chars, _str_trunc
cimport libc.stdlib
from libc.math cimport nextafter

from flint.flintlib.types.flint cimport FMPZ_REF, FMPZ_TMP, FMPZ_UNKNOWN, COEFF_IS_MPZ
from flint.flintlib.functions.flint cimport flint_free
Expand Down Expand Up @@ -105,6 +106,41 @@ cdef class fmpz(flint_scalar):
return fmpz_get_intlong(self.val)

def __float__(self):
if not COEFF_IS_MPZ(self.val[0]):
return <double>(<slong>self.val[0])
Comment on lines 108 to +110
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the C23 standard text:
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf

In 6.3.1.4 it says:

When a value of integer type is converted to a standard floating type, if the value being converted
can be represented exactly in the new type, it is unchanged. If the value being converted is in the
range of values that can be represented but cannot be represented exactly, the result is either the
nearest higher or nearest lower representable value, chosen in an implementation-defined manner.
If the value being converted is outside the range of values that can be represented, the behavior is
undefined.

In our case here I think that in a 32 bit build this is fine since every 32 bit integer can go directly to a double. For a 64 bit slong values greater than 2^53 here cannot be represented exactly and then the result would be implementation-defined so it's kind of unsafe even if it seems to work under testing right now.

I think it is best to do here what FLINT's fmpz_get_d does which is to check if the result is in the range [-2^53, 2^53] before casting to double and otherwise fall back on the other path.

That check is unnecessary in the 32-bit case though. We probably should define a function for this using preprocessor defines somewhere near here:

cdef extern from *:
"""
/* FLINT_BITS is not known until C compile time. We need to check if long
* or long long matches FLINT_BITS to know which CPython function to call.
*/
#if FLINT_BITS == 32 && LONG_MAX == 2147483647
#define pylong_as_slong PyLong_AsLongAndOverflow
#elif FLINT_BITS == 64 && LLONG_MAX == 9223372036854775807
#define pylong_as_slong PyLong_AsLongLongAndOverflow
#else
#error FLINT_BITS does not match width of long or long long.
#endif
"""
slong pylong_as_slong(PyObject *pylong, int *overflow)

I think we want a macro something like

#if sizeof(slong) == 4
   #define _fmpz_can_cast_double(x) MPZ_IS_COEFF(x)
#elif sizeof(slong) == 8
   /* x in [-2^53, 2^53] */
   #define _fmpz_can_cast_double(x) MPZ_IS_COEFF(x) \
        && -9007199254740992 <= (slong)(x) \
        && (slong)(x) <= 9007199254740992
#endif

I guess checking sizeof(slong) is what should really be used for pylong_as_slong as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somewhere near here:

Actually it probably makes more sense to put the macro in fmpz.pyx where it is used. If other code wants to use it in future then we can consider putting it somewhere more central.

cdef slong bits = fmpz_bits(self.val)
cdef long tz
cdef double d
cdef int roundup = 0
if bits <= 1023:
# Known to be representable by a IEEE-754 double
d = fmpz_get_d(self.val)
# fmpz_get_d always rounds towards zero
# Sometimes we need to round away from zero:
# - if the 54-th most significant bit is 1
# - if further bits are not zero
# - or if all further bits are zero but the 53-th bit is 1 (round-to-even convention)
if fmpz_sgn(self.val) == -1:
# tstbit behaves as if self was represented as 2's complement
# We look for patterns 0.10...0 or x.0xxxxxx
tz = fmpz_val2(self.val)
if bits >= 54:
if fmpz_tstbit(self.val, bits - 54) == 0:
if tz < bits - 54:
roundup = 1
else:
if tz == bits - 54 and fmpz_tstbit(self.val, bits - 53) == 0:
roundup = 1
else:
if bits >= 54 and fmpz_tstbit(self.val, bits - 54) == 1:
tz = fmpz_val2(self.val)
if tz < bits - 54 or (tz == bits - 54 and fmpz_tstbit(self.val, bits - 53) == 1):
roundup = 1
if roundup:
# increase the mantissa of d by 1
d = nextafter(d, 2 * d)
return d

return float(fmpz_get_intlong(self.val))

def __floor__(self):
Expand Down
Loading