FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

More efficient conversion of fmpz to float by remyoudompheng · Pull Request #323 · flintlib/python-flint · GitHub

  1. flintlib
  2. python-flint

Repository navigation

More efficient conversion of fmpz to float #323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
remyoudompheng wants to merge 6 commits into
base: main
Choose a base branch
Could not load branches
Branch not found: {{ refName }}

Are you sure you want to change the base?

Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
from
Conversation 13 Commits 6 Checks 81 Files changed
Open

More efficient conversion of fmpz to float #323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4da6b11
More efficient conversion of fmpz to float
remyoudompheng Sep 7, 2025
b3656ca
Add fast path for float conversion of numbers below 1023 bits
remyoudompheng Sep 7, 2025
9382a42
Implement round-to-even compliant rounding
remyoudompheng Sep 7, 2025
91dc079
Additional fmpz to float conversion tests
remyoudompheng Sep 7, 2025
5896ada
Improve conversion of small or negative fmpz to float
remyoudompheng Sep 7, 2025
6a5370c
Update release notes
remyoudompheng Sep 7, 2025
File filter

Filter by extension

Filter by extension .md  (1) .py  (1) .pyx  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
2 changes: 2 additions & 0 deletions README.md
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

oscarbenjamin Sep 7, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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:

python-flint/src/flint/flintlib/types/flint.pxd

Lines 95 to 108 in 0a59f53

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.

oscarbenjamin Sep 7, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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:

skirpichev Dec 23, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Shouldn't you allow maximal exponent (1024) too?

BTW, you might rely on sys.float_info parameters, instead of assuming IEEE doubles.
# 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
Back | FazBrowse Home | New Git URL