You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by gi...@apache.org on 2026/10/06 03:32:36 UTC
(arrow-rs) branch gh-readonly-queue/main/pr-11315-6e359083973ced84b77e04ac23c26433378cd91d created (now 13886b5c45)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-11315-6e359083973ced84b77e04ac23c26433378cd91d
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
at 13886b5c45 fix(arrow-buffer): round i256 to f64 correctly at f64 midpoints (#11315)
This branch includes the following new commits:
new 13886b5c45 fix(arrow-buffer): round i256 to f64 correctly at f64 midpoints (#11315)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(arrow-rs) 01/01: fix(arrow-buffer): round i256 to f64 correctly at f64 midpoints (#11315)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11315-6e359083973ced84b77e04ac23c26433378cd91d
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 13886b5c4536654f4ac7211ef2d42bb1ac44d115
Author: Miguel Cordero Pamphile <mi...@gmail.com>
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 <mi...@gmail.com>
---
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);