Skip to content

Miri: non-deterministic floating point operations in foreign_items #143906

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Miri: non-deterministic floating point operations in foreign_items #143906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a5027a
Move float non determinism helpers to math.rs
LorrensP-2158466 Jul 7, 2025
f391acb
Implement nondet behaviour and change/add tests.
LorrensP-2158466 Jul 7, 2025
ba59bff
Change stdlib float tests to account for miri nondet floats.
LorrensP-2158466 Jul 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions library/core/src/num/f32.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1879,7 +1879,7 @@ pub mod math {
///
/// let x = 2.0_f32;
/// let abs_difference = (f32::math::powi(x, 2) - (x * x)).abs();
/// assert!(abs_difference <= 1e-5);
/// assert!(abs_difference <= 1e-4);
///
/// assert_eq!(f32::math::powi(f32::NAN, 0), 1.0);
/// ```
Expand Down Expand Up @@ -1942,8 +1942,8 @@ pub mod math {
/// let abs_difference_x = (f32::math::abs_sub(x, 1.0) - 2.0).abs();
/// let abs_difference_y = (f32::math::abs_sub(y, 1.0) - 0.0).abs();
///
/// assert!(abs_difference_x <= f32::EPSILON);
/// assert!(abs_difference_y <= f32::EPSILON);
/// assert!(abs_difference_x <= 1e-4);
/// assert!(abs_difference_y <= 1e-4);
/// ```
///
/// _This standalone function is for testing only.
Expand Down Expand Up @@ -1988,7 +1988,7 @@ pub mod math {
/// // x^(1/3) - 2 == 0
/// let abs_difference = (f32::math::cbrt(x) - 2.0).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
///
/// _This standalone function is for testing only.
Expand Down
62 changes: 31 additions & 31 deletions library/std/src/num/f32.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

1e-5 didn't work for f32, right? Is there just a single test that is the culprit, or is it a bunch of them?

Copy link
Contributor Author

@LorrensP-2158466 LorrensP-2158466 Jul 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

asinh failed, but it's getting confusing having 3 different constants for these tests, so I wanted to unify them to one. Side note: a single f32 test has 1e-3.

Maybe if we change that test as well, I can change them back to their limits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know what you and @tgross35 prefer. Having a single constant that works for them all, or individual limits, or something else.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense that operations which are direct wrappers around a libm functions, like sin, expect higher precision than things like asinh. So I'd say optimize for minimizing the diff here.

And yeah, please change all the asin tests to use FRAC_PI_4 to avoid instability around asin(1).

Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl f32 {
/// ```
/// let x = 2.0_f32;
/// let abs_difference = (x.powi(2) - (x * x)).abs();
/// assert!(abs_difference <= 1e-5);
/// assert!(abs_difference <= 1e-4);
///
/// assert_eq!(f32::powi(f32::NAN, 0), 1.0);
/// ```
Expand All @@ -328,7 +328,7 @@ impl f32 {
/// ```
/// let x = 2.0_f32;
/// let abs_difference = (x.powf(2.0) - (x * x)).abs();
/// assert!(abs_difference <= 1e-5);
/// assert!(abs_difference <= 1e-4);
///
/// assert_eq!(f32::powf(1.0, f32::NAN), 1.0);
/// assert_eq!(f32::powf(f32::NAN, 0.0), 1.0);
Expand Down Expand Up @@ -388,7 +388,7 @@ impl f32 {
/// // ln(e) - 1 == 0
/// let abs_difference = (e.ln() - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand All @@ -413,7 +413,7 @@ impl f32 {
/// // 2^2 - 4 == 0
/// let abs_difference = (f.exp2() - 4.0).abs();
///
/// assert!(abs_difference <= 1e-5);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -442,7 +442,7 @@ impl f32 {
/// // ln(e) - 1 == 0
/// let abs_difference = (e.ln() - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
///
/// Non-positive values:
Expand Down Expand Up @@ -479,7 +479,7 @@ impl f32 {
/// // log5(5) - 1 == 0
/// let abs_difference = (five.log(5.0) - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
///
/// Non-positive values:
Expand Down Expand Up @@ -512,7 +512,7 @@ impl f32 {
/// // log2(2) - 1 == 0
/// let abs_difference = (two.log2() - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
///
/// Non-positive values:
Expand Down Expand Up @@ -545,7 +545,7 @@ impl f32 {
/// // log10(10) - 1 == 0
/// let abs_difference = (ten.log10() - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
///
/// Non-positive values:
Expand Down Expand Up @@ -582,8 +582,8 @@ impl f32 {
/// let abs_difference_x = (x.abs_sub(1.0) - 2.0).abs();
/// let abs_difference_y = (y.abs_sub(1.0) - 0.0).abs();
///
/// assert!(abs_difference_x <= f32::EPSILON);
/// assert!(abs_difference_y <= f32::EPSILON);
/// assert!(abs_difference_x <= 1e-4);
/// assert!(abs_difference_y <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -621,7 +621,7 @@ impl f32 {
/// // x^(1/3) - 2 == 0
/// let abs_difference = (x.cbrt() - 2.0).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -652,7 +652,7 @@ impl f32 {
/// // sqrt(x^2 + y^2)
/// let abs_difference = (x.hypot(y) - (x.powi(2) + y.powi(2)).sqrt()).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand All @@ -676,7 +676,7 @@ impl f32 {
///
/// let abs_difference = (x.sin() - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand All @@ -700,7 +700,7 @@ impl f32 {
///
/// let abs_difference = (x.cos() - 1.0).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand All @@ -725,7 +725,7 @@ impl f32 {
/// let x = std::f32::consts::FRAC_PI_4;
/// let abs_difference = (x.tan() - 1.0).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -784,7 +784,7 @@ impl f32 {
/// // acos(cos(pi/4))
/// let abs_difference = (f.cos().acos() - std::f32::consts::FRAC_PI_4).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
#[doc(alias = "arccos")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -813,7 +813,7 @@ impl f32 {
/// // atan(tan(1))
/// let abs_difference = (f.tan().atan() - 1.0).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[doc(alias = "arctan")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -854,8 +854,8 @@ impl f32 {
/// let abs_difference_1 = (y1.atan2(x1) - (-std::f32::consts::FRAC_PI_4)).abs();
/// let abs_difference_2 = (y2.atan2(x2) - (3.0 * std::f32::consts::FRAC_PI_4)).abs();
///
/// assert!(abs_difference_1 <= f32::EPSILON);
/// assert!(abs_difference_2 <= f32::EPSILON);
/// assert!(abs_difference_1 <= 1e-4);
/// assert!(abs_difference_2 <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -884,8 +884,8 @@ impl f32 {
/// let abs_difference_0 = (f.0 - x.sin()).abs();
/// let abs_difference_1 = (f.1 - x.cos()).abs();
///
/// assert!(abs_difference_0 <= 1e-6);
/// assert!(abs_difference_1 <= 1e-6);
/// assert!(abs_difference_0 <= 1e-4);
/// assert!(abs_difference_1 <= 1e-4);
/// ```
#[doc(alias = "sincos")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -914,7 +914,7 @@ impl f32 {
/// let approx = x + x * x / 2.0;
/// let abs_difference = (x.exp_m1() - approx).abs();
///
/// assert!(abs_difference < 1e-10);
/// assert!(abs_difference < 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -945,7 +945,7 @@ impl f32 {
/// let approx = x - x * x / 2.0;
/// let abs_difference = (x.ln_1p() - approx).abs();
///
/// assert!(abs_difference < 1e-10);
/// assert!(abs_difference < 1e-4);
/// ```
///
/// Out-of-range values:
Expand Down Expand Up @@ -982,7 +982,7 @@ impl f32 {
/// let g = ((e * e) - 1.0) / (2.0 * e);
/// let abs_difference = (f - g).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -1012,7 +1012,7 @@ impl f32 {
/// let abs_difference = (f - g).abs();
///
/// // Same result
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -1042,7 +1042,7 @@ impl f32 {
/// let g = (1.0 - e.powi(-2)) / (1.0 + e.powi(-2));
/// let abs_difference = (f - g).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand All @@ -1067,7 +1067,7 @@ impl f32 {
///
/// let abs_difference = (f - x).abs();
///
/// assert!(abs_difference <= 1e-7);
/// assert!(abs_difference <= 1e-4);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's this one that failed, not the one I commented here https://github.com/rust-lang/rust/pull/143906/files#r2225770166

Maybe we should change the test to an area where asin is more stable -- like pi/4, which the other 2 tests are already using?

Agreed.

Copy link
Member

Choose a reason for hiding this comment

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

This one might be amplifying an underlying error introduced in hypot or ln_1p due to how it is implemented.

But 1000 tests should still be enough for all practical purposes... strange.

/// ```
#[doc(alias = "arcsinh")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -1095,7 +1095,7 @@ impl f32 {
///
/// let abs_difference = (f - x).abs();
///
/// assert!(abs_difference <= 1e-6);
/// assert!(abs_difference <= 1e-4);
/// ```
#[doc(alias = "arccosh")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -1125,7 +1125,7 @@ impl f32 {
///
/// let abs_difference = (f - e).abs();
///
/// assert!(abs_difference <= 1e-5);
/// assert!(abs_difference <= 1e-4);
/// ```
#[doc(alias = "arctanh")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -1153,7 +1153,7 @@ impl f32 {
///
/// let abs_difference = (x.gamma() - 24.0).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -1248,7 +1248,7 @@ impl f32 {
/// let one = x.erf() + x.erfc();
/// let abs_difference = (one - 1.0).abs();
///
/// assert!(abs_difference <= f32::EPSILON);
/// assert!(abs_difference <= 1e-4);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down
16 changes: 8 additions & 8 deletions library/std/src/num/f64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl f64 {
/// ```
/// let x = 2.0_f64;
/// let abs_difference = (x.powi(2) - (x * x)).abs();
/// assert!(abs_difference <= 1e-14);
/// assert!(abs_difference <= 1e-10);
///
/// assert_eq!(f64::powi(f64::NAN, 0), 1.0);
/// ```
Expand All @@ -328,7 +328,7 @@ impl f64 {
/// ```
/// let x = 2.0_f64;
/// let abs_difference = (x.powf(2.0) - (x * x)).abs();
/// assert!(abs_difference <= 1e-14);
/// assert!(abs_difference <= 1e-10);
///
/// assert_eq!(f64::powf(1.0, f64::NAN), 1.0);
/// assert_eq!(f64::powf(f64::NAN, 0.0), 1.0);
Expand Down Expand Up @@ -725,7 +725,7 @@ impl f64 {
/// let x = std::f64::consts::FRAC_PI_4;
/// let abs_difference = (x.tan() - 1.0).abs();
///
/// assert!(abs_difference < 1e-14);
/// assert!(abs_difference < 1e-10);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -754,7 +754,7 @@ impl f64 {
/// // asin(sin(pi/2))
/// let abs_difference = (f.sin().asin() - std::f64::consts::FRAC_PI_2).abs();
///
/// assert!(abs_difference < 1e-7);
/// assert!(abs_difference < 1e-5);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This failed because of the double operations, but it's weird this one is so unprecise when running under miri, while others like cos and tan are not.

Copy link
Member

Choose a reason for hiding this comment

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

I tried 1000 seeds and 1e-7 always worked. Why did you reduce to 1e-5?

(1e-8 failed quickly.)

Copy link
Member

Choose a reason for hiding this comment

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

it's weird this one is so unprecise when running under miri, while others like cos and tan are not.

asin seems to be quite sensitive around input 1, which is what we are using here.
And yeah, looking at WA, its derivative explodes towards 1: https://www.wolframalpha.com/input?i2d=true&i=D%5Basin%5C%2840%29x%5C%2841%29%2Cx%5D.

In contrast, we are evaluating acos at around 0.7, where its derivative is much lower.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should change the test to an area where asin is more stable -- like pi/4, which the other 2 tests are already using?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I commented this on the wrong test, but you're last 2 replies apply to that test as well.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, but you did change this one to 1e-5... why?

Copy link
Contributor Author

@LorrensP-2158466 LorrensP-2158466 Jul 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I tried 1000 seeds and 1e-7 always worked

This is not a good indicator; I also did this check, but it still failed in CI. Thus, I changed it based on the assumption that this one would fail similarly in CI.

Copy link
Member

Choose a reason for hiding this comment

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

If we hit a 1/1000 event in CI the first try, that is worth further investigation as it indicates something rather unlikely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, I was a little fast in my reply. The test that failed in CI was asinh, but that one was also battle-tested with 1000 seeds. That CI failure just prompted me to change this test as well.

/// ```
#[doc(alias = "arcsin")]
#[rustc_allow_incoherent_impl]
Expand Down Expand Up @@ -914,7 +914,7 @@ impl f64 {
/// let approx = x + x * x / 2.0;
/// let abs_difference = (x.exp_m1() - approx).abs();
///
/// assert!(abs_difference < 1e-20);
/// assert!(abs_difference < 1e-10);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -945,7 +945,7 @@ impl f64 {
/// let approx = x - x * x / 2.0;
/// let abs_difference = (x.ln_1p() - approx).abs();
///
/// assert!(abs_difference < 1e-20);
/// assert!(abs_difference < 1e-10);
/// ```
///
/// Out-of-range values:
Expand Down Expand Up @@ -1153,7 +1153,7 @@ impl f64 {
///
/// let abs_difference = (x.gamma() - 24.0).abs();
///
/// assert!(abs_difference <= f64::EPSILON);
/// assert!(abs_difference <= 1e-10);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down Expand Up @@ -1248,7 +1248,7 @@ impl f64 {
/// let one = x.erf() + x.erfc();
/// let abs_difference = (one - 1.0).abs();
///
/// assert!(abs_difference <= f64::EPSILON);
/// assert!(abs_difference <= 1e-10);
/// ```
#[rustc_allow_incoherent_impl]
#[must_use = "method returns a new number and does not mutate the original value"]
Expand Down
Loading
Loading