This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 13886b5c45 fix(arrow-buffer): round i256 to f64 correctly at f64
midpoints (#11315)
13886b5c45 is described below
commit 13886b5c4536654f4ac7211ef2d42bb1ac44d115
Author: Miguel Cordero Pamphile <[email protected]>
AuthorDate: Tue Oct 6 03:32:10 2026 +0000
fix(arrow-buffer): round i256 to f64 correctly at f64 midpoints (#11315)
# Which issue does this PR close?
- Closes #11314.
# Rationale for this change
`i256_to_f64` normalizes the input, keeps the top 64 bits and discards
the lower 192, so the final `i64 as f64` rounds a value truncated at 64
bits. When the discarded bits put the exact value just past a binary64
midpoint, the result can round to the wrong neighbor: `(1_i128 << 63) +
1024 + 1` returns `0x43e0000000000000` instead of `0x43e0000000000001`.
# What changes are included in this PR?
Set a sticky bit on the retained value when any discarded bit is
non-zero, so midpoint ties round to nearest (ties to even only apply
when the exact value is the midpoint). No API changes. A naive `top + 1`
variant was also checked and is incorrect, since it can move a value
that sits just below a midpoint up to the midpoint; setting the low bit
preserves the side of the midpoint.
# Are these changes tested?
- The reported value plus midpoint neighborhoods for every binary64
exponent in `i256` range, both signs, checked against
`to_string().parse::<f64>()`.
- `MIN`, `MAX`, `-1`, `0`, `1`, and 1000 deterministic pseudo-random
full-range values against the same oracle.
- `cargo test -p arrow-buffer` (358 passed) and `cargo test -p
arrow-cast --lib test_cast_decimal256` (12 passed); `cargo fmt --check`
and `cargo clippy -p arrow-buffer --all-targets -- -D warnings` clean.
# Are there any user-facing changes?
No API changes. `i256::to_f64` now returns the correctly rounded `f64`,
which also fixes `Decimal256 -> Float64` casts that use it.
# AI usage disclosure
Per CONTRIBUTING: the sticky-bit approach was suggested in the issue. I
reproduced the failure, validated the approach at scale against
correctly rounded conversions (midpoint sweeps and randomized
comparisons, including ruling out the `+1` variant), and wrote the
tests. AI assistance was used for analysis and drafting; I have reviewed
and verified every change and own the final implementation.
---------
Signed-off-by: Miguel Cordero Pamphile <[email protected]>
---
arrow-buffer/src/bigint/mod.rs | 57 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 55 insertions(+), 2 deletions(-)
diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs
index eb1fad0abb..db996acfdb 100644
--- a/arrow-buffer/src/bigint/mod.rs
+++ b/arrow-buffer/src/bigint/mod.rs
@@ -677,7 +677,10 @@ impl i256 {
fn i256_to_f64(input: i256) -> f64 {
let k = i256::redundant_leading_sign_bits_i256(input);
let n = input << k; // left-justify (no redundant sign bits)
- let n = (n.high >> 64) as i64; // throw away the lower 192 bits
+ // If the retained bits land exactly on a midpoint, set the low bit so
+ // non-zero discarded bits push it just past and it rounds to nearest.
+ let sticky = n.low != 0 || (n.high as u64) != 0;
+ let n = ((n.high >> 64) as i64) | i64::from(sticky);
(n as f64) * f64::powi(2.0, 192 - (k as i32)) // convert to f64 and
scale it, as we left-shift k bit previous, so we need to scale it by 2^(192-k)
}
@@ -1342,7 +1345,7 @@ impl Not for i256 {
mod tests {
use super::*;
use num_traits::Signed;
- use rand::{RngExt, rng};
+ use rand::{RngExt, SeedableRng, rng, rngs::StdRng};
#[test]
fn test_signed_cmp() {
@@ -1877,6 +1880,56 @@ mod tests {
}
}
+ #[test]
+ fn test_i256_to_f64_midpoint_rounding() {
+ // Regression test for #11314: the value is one above the binary64
midpoint.
+ let integer = (1_i128 << 63) + 1024 + 1;
+ assert_eq!(
+ i256::from_i128(integer).to_f64().unwrap().to_bits(),
+ (integer as f64).to_bits()
+ );
+
+ let two = i256::from(2);
+ // Binary64 has a 53-bit significand: inside [2^e, 2^(e+1)) adjacent
values
+ // are 2^(e-52) apart, so the exact tie is 2^(e-53) above 2^e. Ties
are where
+ // #11314 rounded the wrong way. Start at 53 (below that the midpoint
is not
+ // an integer) and stop at 254 (2^255 overflows i256).
+ for exponent in 53..=254 {
+ let midpoint = two.wrapping_pow(exponent) +
two.wrapping_pow(exponent - 53);
+ for delta in -2_i64..=2 {
+ for value in [
+ midpoint + i256::from(delta),
+ -(midpoint + i256::from(delta)),
+ ] {
+ let expected: f64 = value.to_string().parse().unwrap();
+ assert_eq!(
+ value.to_f64().unwrap().to_bits(),
+ expected.to_bits(),
+ "i256 {value} should round to nearest"
+ );
+ }
+ }
+ }
+ }
+
+ #[test]
+ fn test_i256_to_f64_fuzz() {
+ // `to_string().parse::<f64>()` is a correctly rounded
decimal-to-binary oracle.
+ for value in [i256::MIN, i256::MAX, i256::MINUS_ONE, i256::ZERO,
i256::ONE] {
+ let expected: f64 = value.to_string().parse().unwrap();
+ assert_eq!(value.to_f64().unwrap().to_bits(), expected.to_bits());
+ }
+
+ let mut rng = StdRng::seed_from_u64(42);
+ for _ in 0..1_000 {
+ let low = u128::from(rng.random::<u64>()) |
(u128::from(rng.random::<u64>()) << 64);
+ let high = u128::from(rng.random::<u64>()) |
(u128::from(rng.random::<u64>()) << 64);
+ let value = i256::from_parts(low, high as i128);
+ let expected: f64 = value.to_string().parse().unwrap();
+ assert_eq!(value.to_f64().unwrap().to_bits(), expected.to_bits());
+ }
+ }
+
#[test]
fn test_decimal256_to_f64_typical_values() {
let v = i256::from_i128(42_i128);
