erts: Round bignum to float conversion correctly - #11391
Conversation
|
|
CT Test Results 3 files 136 suites 50m 52s ⏱️ Results for commit aae60d0. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
There was a problem hiding this comment.
Pull request overview
This PR fixes big_to_double/2 in the BEAM emulator to perform correctly rounded bignum→double conversion (nearest representable IEEE-754 double, ties-to-even), and adds regression tests to ensure float/1 agrees with binary_to_float/1 for large integers.
Changes:
- Update
big_to_double/2to avoid per-digit rounding error accumulation by extracting the top bits + sticky/round bits and rounding exactly once. - Add
big_float_3/1test coverage for known reproducers, randomized bignums, and power-of-two boundary cases. - Register
big_float_3/1inbig_SUITEexports and group list.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
erts/emulator/beam/big.c |
Implements single-rounding bignum→double conversion with guard/sticky rounding to satisfy IEEE-754 requirements. |
erts/emulator/test/big_SUITE.erl |
Adds big_float_3/1 regression tests for correctly rounded integer→float conversion and updates test grouping/exports. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@yaglo Any objections on my little optimization to reverse the mantissa loop and break when |
Absolutely no objections |
31ffdf3 to
9a171d8
Compare
|
Squashed and rebased on |
big_to_double/2 accumulated the result one digit at a time:
while (xl--) {
d = d * dbase + *--s;
}
Each iteration rounds, so the errors compound and the result can be the
second-nearest double rather than the nearest. IEEE 754 requires the
nearest representable value, ties to even. A value that fits in a single
digit rounds only once and was already correct, so the defect appears
from two digits up: 16% of 65-bit integers converted to the wrong double,
and never to the nearer one.
For example, float(428654966685883400000) returned 4.2865496668588343e20
where the nearest double is 4.286549666858834e20. The affected value is
also what binary_to_float/1 returns for the same digits, since that
routes through the platform's correctly rounded decimal parser.
Compute the bit length instead, and for values wider than the 53-bit
mantissa take the top 54 bits — 53 of mantissa plus one round bit.
Rounding is then determined by reading as few of the lower digits
as possible.
Values of 53 bits or fewer keep the digit-by-digit accumulation,
which cannot round there.
Integers wider than 1024 bits return -1 up front: they are at least 2^1024 and cannot be
finite. The previous loop stopped as soon as the accumulator went
infinite, and without this check a conversion would become O(size) for
bignums that reach tens of thousands of digits. Exactly 1024 bits can
still be finite and takes the rounding path, where the overflow check
after ldexp() catches a mantissa that carries up to 2^1024.
The defect surfaced while implementing RFC 8785 (JSON Canonicalization
Scheme), which serializes numbers as ECMAScript does and therefore has
to decide whether an integer's decimal digits survive a round trip
through a double. Comparing float/1 against binary_to_float/1 over
random integers disagreed often enough to be reproducible, and the
disagreement was always in binary_to_float/1's favour.
big_float_3 covers the reproducers, random values from 50 to 300 bits,
and every power of two up to 2^1023; it fails on the previous
implementation.
Co-authored-by: Sverker Eriksson <sverker@erlang.org>
9a171d8 to
aae60d0
Compare
|
I guess the Windows build failure is unrelated to the changes? |
|
The Windows error seems unrelated. What do you think about this as release note?
|
|
Maybe it can be simplified: integers up to 64 bits were actually fine on 32-bit systems too (two digits only round once), so it could just say "larger than 64 bits" and drop the last sentence. Could also mention it's not just float/1, 1.0 * N takes the same path. And maybe it's worth showing the correct value in the example:
|
big_to_double/2accumulated the result one digit at a time:Every iteration rounds, so the errors compound and the result can be the second-nearest double rather than the nearest. IEEE 754 requires the nearest representable value, ties to even.
A value that fits in a single digit rounds only once and was already correct, so the defect starts at two digits. On a 64-bit build that means integers above 2^64:
Both call sites describe the same real number, so they should agree.
binary_to_float/1routes through the platform's correctly rounded decimal parser and returns the nearest double;float/1does not. Around 16% of 65-bit integers converted to the wrong double, and never to the nearer one — the error was systematic, not a tie-breaking difference.Fix
Compute the bit length, and for values wider than the 53-bit mantissa take the top 54 bits (53 of mantissa plus one round bit) while recording whether any lower bit is set, then round exactly once. Values of 53 bits or fewer keep the digit-by-digit accumulation, which cannot round there.
Collecting the sticky bit visits every digit, so integers wider than 1024 bits return -1 up front: they are at least 2^1024 and cannot be finite. The previous loop stopped as soon as the accumulator went infinite, and without this check a conversion would become O(size) for bignums that reach tens of thousands of digits. Exactly 1024 bits can still be finite and takes the rounding path, where the existing overflow check after
ldexp()catches a mantissa that carries up to 2^1024.Converting
(1 bsl 4000000) - 1takes 24 us with the short-circuit and 1917 us without it, measured on the 32-bit build described below.Testing
big_float_3covers the reproducers, random values from 50 to 300 bits, and every power of two up to 2^1023. It fails on the previous implementation.big_SUITE(23/23) andfloat_SUITE(15/15) pass on this branch.The rounding boundary was checked explicitly, using the fact that the largest finite double is 2^1024 - 2^971:
trunc(max double)1.7976931348623157e308max + 2^970(tie, rounds up)max + 2^970 - 11.7976931348623157e3082^1024 - 1(1024 bits)2^10238.98846567431158e3072^1024,2^1025,2^2000The tie case confirms that 1024-bit values still go through the rounding path rather than being rejected by the new short-circuit.
The digit-straddling arithmetic is the only part that depends on
D_EXP, so it was checked two ways.First, the algorithm was extracted into a standalone harness and instantiated at both 32-bit and 64-bit digit widths, then run over 94,116 vectors checked against a correctly rounded oracle: every bit width from 1 to 300, all powers of two to 2^1023 with their neighbours, and exact ties with their neighbours. Both widths were correct on every vector and agreed with each other bit for bit; 1,143 of those vectors are mis-rounded by the previous implementation.
Second, it was built and run on a 32-bit target (
i686-pc-linux-gnu, wordsize 4, soD_EXPis 32). The reproducers all convert correctly; 220,000 random values spanning 53 to 500 bits and a further 300,000 80-bit values show no divergence frombinary_to_float/1; negation stays symmetric over 50,000 values; and1 bsl 2000still raisesbadarith.Provenance
The defect surfaced while implementing RFC 8785 (JSON Canonicalization Scheme). Canonical JSON numbers are IEEE 754 doubles, so a canonicalizer handed a large integer must decide whether it is exactly representable — convert it to the nearest double, and check whether the decimal digits come back unchanged.
That made the conversion itself worth testing.
float/1andbinary_to_float/1are given the same real number by construction, so they should return the same double; over random integers they disagreed often enough to be reproducible. Measuring which result was nearer the integer settled which one was wrong: it was neverfloat/1, and there were no ties.