[GitHub] [arrow-datafusion] metesynnada opened a new pull request, #6048: Float support on interval artihmetics

["metesynnada (via GitHub)" <gi...@apache.org>]

metesynnada opened a new pull request, #6048:
URL: https://github.com/apache/arrow-datafusion/pull/6048

   # Which issue does this PR close?
   
   Closes #5974.
   
   # Rationale for this change
   
   This PR adds support for `Float32` and `Float64` interval arithmetic, ensuring reliability and accuracy in computations by controlling rounding modes as per the IEEE 754 floating-point standard.
   
   # What changes are included in this PR?
   
   - Implemented support for `Float32` and `Float64` interval arithmetic
   - Utilized the libc library for controlling rounding modes temporarily, until Rust gets fenv support
   - Implemented custom `next_up` and `next_down` functions for `f32` and `f64`, which will be replaced by Rust's built-in functions once the unstable feature becomes stable and the tracking issue for the unstable Rust feature (https://github.com/rust-lang/rust/issues/91399)
   
   # Are these changes tested?
   
   - Added new tests to cover `Float32` and `Float64` interval arithmetic operations, including custom next_up and next_down functions
   - Ensured existing tests pass with the new changes
   
   # Are there any user-facing changes?
   
   Users will now be able to use `Float32` and `Float64` in interval arithmetic, meaning that executors using the interval library now support float operations.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6048: Float support on interval artihmetics

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #6048:
URL: https://github.com/apache/arrow-datafusion/pull/6048#discussion_r1172532072


##########
datafusion/physical-expr/src/intervals/cp_solver.rs:
##########
@@ -564,31 +564,19 @@ mod tests {
     fn experiment(
         expr: Arc<dyn PhysicalExpr>,
         exprs_with_interval: (Arc<dyn PhysicalExpr>, Arc<dyn PhysicalExpr>),
-        left_interval: (Option<i32>, Option<i32>),
-        right_interval: (Option<i32>, Option<i32>),
-        left_expected: (Option<i32>, Option<i32>),
-        right_expected: (Option<i32>, Option<i32>),
+        left_interval: Interval,

Review Comment:
   👍 



##########
datafusion/physical-expr/src/intervals/rounding.rs:
##########
@@ -0,0 +1,401 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Floating point rounding mode utility library
+//! TODO: Remove this custom implementation and the "libc" dependency when
+//!       floating-point rounding mode manipulation functions become available
+//!       in Rust.
+
+use std::ops::{Add, BitAnd, Sub};
+
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+
+// Define constants for ARM
+#[cfg(all(target_arch = "aarch64", not(target_os = "windows")))]
+const FE_UPWARD: i32 = 0x00400000;
+#[cfg(all(target_arch = "aarch64", not(target_os = "windows")))]
+const FE_DOWNWARD: i32 = 0x00800000;
+
+// Define constants for x86_64
+#[cfg(all(target_arch = "x86_64", not(target_os = "windows")))]
+const FE_UPWARD: i32 = 0x0800;
+#[cfg(all(target_arch = "x86_64", not(target_os = "windows")))]
+const FE_DOWNWARD: i32 = 0x0400;
+
+#[cfg(all(
+    any(target_arch = "x86_64", target_arch = "aarch64"),
+    not(target_os = "windows")
+))]
+extern crate libc;
+
+#[cfg(all(
+    any(target_arch = "x86_64", target_arch = "aarch64"),
+    not(target_os = "windows")
+))]
+extern "C" {
+    fn fesetround(round: i32);
+    fn fegetround() -> i32;
+}
+
+/// A trait to manipulate floating-point types with bitwise operations.
+/// Provides functions to convert a floating-point value to/from its bitwise
+/// representation as well as utility methods to handle special values.
+pub trait FloatBits {
+    /// The integer type used for bitwise operations.
+    type Item: Copy
+        + PartialEq
+        + BitAnd<Output = Self::Item>
+        + Add<Output = Self::Item>
+        + Sub<Output = Self::Item>;
+
+    /// The smallest positive floating-point value representable by this type.
+    const TINY_BITS: Self::Item;
+
+    /// The smallest (in magnitude) negative floating-point value representable by this type.
+    const NEG_TINY_BITS: Self::Item;
+
+    /// A mask to clear the sign bit of the floating-point value's bitwise representation.
+    const CLEAR_SIGN_MASK: Self::Item;
+
+    /// The integer value 1, used in bitwise operations.
+    const ONE: Self::Item;
+
+    /// The integer value 0, used in bitwise operations.
+    const ZERO: Self::Item;
+
+    /// Converts the floating-point value to its bitwise representation.
+    fn to_bits(self) -> Self::Item;
+
+    /// Converts the bitwise representation to the corresponding floating-point value.
+    fn from_bits(bits: Self::Item) -> Self;
+
+    /// Returns true if the floating-point value is NaN (not a number).
+    fn float_is_nan(self) -> bool;
+
+    /// Returns the positive infinity value for this floating-point type.
+    fn infinity() -> Self;
+
+    /// Returns the negative infinity value for this floating-point type.
+    fn neg_infinity() -> Self;
+}
+
+impl FloatBits for f32 {
+    type Item = u32;
+    const TINY_BITS: u32 = 0x1; // Smallest positive f32.
+    const NEG_TINY_BITS: u32 = 0x8000_0001; // Smallest (in magnitude) negative f32.
+    const CLEAR_SIGN_MASK: u32 = 0x7fff_ffff;
+    const ONE: Self::Item = 1;
+    const ZERO: Self::Item = 0;
+
+    fn to_bits(self) -> Self::Item {
+        self.to_bits()
+    }
+
+    fn from_bits(bits: Self::Item) -> Self {
+        f32::from_bits(bits)
+    }
+
+    fn float_is_nan(self) -> bool {
+        self.is_nan()
+    }
+
+    fn infinity() -> Self {
+        f32::INFINITY
+    }
+
+    fn neg_infinity() -> Self {
+        f32::NEG_INFINITY
+    }
+}
+
+impl FloatBits for f64 {
+    type Item = u64;
+    const TINY_BITS: u64 = 0x1; // Smallest positive f64.
+    const NEG_TINY_BITS: u64 = 0x8000_0000_0000_0001; // Smallest (in magnitude) negative f64.
+    const CLEAR_SIGN_MASK: u64 = 0x7fff_ffff_ffff_ffff;
+    const ONE: Self::Item = 1;
+    const ZERO: Self::Item = 0;
+
+    fn to_bits(self) -> Self::Item {
+        self.to_bits()
+    }
+
+    fn from_bits(bits: Self::Item) -> Self {
+        f64::from_bits(bits)
+    }
+
+    fn float_is_nan(self) -> bool {
+        self.is_nan()
+    }
+
+    fn infinity() -> Self {
+        f64::INFINITY
+    }
+
+    fn neg_infinity() -> Self {
+        f64::NEG_INFINITY
+    }
+}
+
+/// Returns the next representable floating-point value greater than the input value.
+///
+/// This function takes a floating-point value that implements the FloatBits trait,
+/// calculates the next representable value greater than the input, and returns it.
+///
+/// If the input value is NaN or positive infinity, the function returns the input value.
+///
+/// # Examples
+///
+/// ```
+/// use datafusion_physical_expr::intervals::rounding::next_up;
+///
+/// let f: f32 = 1.0;
+/// let next_f = next_up(f);
+/// assert_eq!(next_f, 1.0000001);
+/// ```
+#[allow(dead_code)]

Review Comment:
   It is strange the "dead code" annotation is still needed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] alamb merged pull request #6048: Float support on interval artihmetics

["alamb (via GitHub)" <gi...@apache.org>]

alamb merged PR #6048:
URL: https://github.com/apache/arrow-datafusion/pull/6048


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org