erts: Round bignum to float conversion correctly - #11391
Open
yaglo wants to merge 2 commits into
Open
Conversation
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 —
while recording whether any lower bit is set, then round 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 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.
Signed-off-by: Stanislav Yaglo <yaglo@me.com>
Contributor
CT Test Results 3 files 136 suites 51m 31s ⏱️ Results for commit c2a61ce. ♻️ 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 |
* Use erts_fit_in_bits_uint() to count significant bits * Reverse mantissa loop and end it when sticky is set and extended tests.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.