Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Float support on interval artihmetics #6048

Merged
merged 19 commits into from
Apr 20, 2023
Merged

Float support on interval artihmetics #6048

merged 19 commits into from
Apr 20, 2023

Conversation

metesynnada
Copy link
Contributor

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 (Tracking Issue for float_next_up_down rust-lang/rust#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.

@github-actions github-actions bot added core Core DataFusion crate physical-expr Physical Expressions labels Apr 18, 2023
Copy link
Contributor

@mustafasrepo mustafasrepo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is passed from our internal review process. It is LGTM!. By implementing test cases as macro, coverage of the tests in the cp_solver.rs is increased. Also once standart rust supports next up, next down routines for floating point numbers, rounding.rs file can be removed.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me - Today I learned about how to make a floating point value "just slightly larger" than another 👍

Thanks @metesynnada and @mustafasrepo -- I am on a break this week so I apologize for the delay in reviewing / merging. I also really appreciate the work to unify the interval logic

right_interval: (Option<i32>, Option<i32>),
left_expected: (Option<i32>, Option<i32>),
right_expected: (Option<i32>, Option<i32>),
left_interval: Interval,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/// let next_f = next_up(f);
/// assert_eq!(next_f, 1.0000001);
/// ```
#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is strange the "dead code" annotation is still needed

@alamb alamb merged commit 92bea99 into apache:main Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate physical-expr Physical Expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Float support on interval arithmetics
5 participants