Skip to content

regression: assertion left == right failed for floats #128898

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

Open
BoxyUwU opened this issue Aug 9, 2024 · 8 comments
Open

regression: assertion left == right failed for floats #128898

BoxyUwU opened this issue Aug 9, 2024 · 8 comments
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@BoxyUwU
Copy link
Member

BoxyUwU commented Aug 9, 2024

[INFO] [stdout] thread 'tests::test_get_ordebook_skew_by_range_orderbook' panicked at tests/test_orderbook.rs:645:9:
[INFO] [stdout] assertion `left == right` failed
[INFO] [stdout]   left: 0.8855105548523078
[INFO] [stdout]  right: 0.885510554852308
@BoxyUwU BoxyUwU added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Aug 9, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 9, 2024
@BoxyUwU BoxyUwU removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 9, 2024
@tgross35
Copy link
Contributor

tgross35 commented Aug 9, 2024

The first crate is doing exact comparison on ln from floating point numbers, so the results are pretty order of operations sensitive (result is a difference in rounding). I haven't dug into what exactly could have caused the difference, I don't think the cmath implementation should have changed in this rev.

        let anw = sum_bid.ln() - sum_ask.ln();
        let result = orderbook.get_ordebook_skew_by_range(&range_diff);

        assert_eq!(anw, result);

and https://github.com/TheLastBytebender/rust-workshop/blob/66bb073d11487d3ad4ffa9716676e4f03666afad/src/trading/orderbook.rs#L126

@tgross35
Copy link
Contributor

tgross35 commented Aug 9, 2024

The second one is more surprising, those numbers are pretty different

[INFO] [stdout] thread 'tests::test_back_prop' panicked at tests/matrix.rs:280:9:
[INFO] [stdout] assertion `left == right` failed
[INFO] [stdout]   left: 2.9802323e-9
[INFO] [stdout]  right: 5.9604646e-9

Third one

[INFO] [stdout] thread 'processor::processor_test' panicked at src/processor.rs:173:9:
[INFO] [stdout] assertion `left != right` failed
[INFO] [stdout]   left: 0.0
[INFO] [stdout]  right: 0.0

@zachs18
Copy link
Contributor

zachs18 commented Aug 9, 2024

The lightbird test failure (third one) is not a regression AFAICT; I can reproduce it locally with 1.73.0, 1.76.0, and 1.80.1, after running it a few times.

I'm not entirely sure I understand the test in question, but it appears to be asserting that no processor had 0% usage over the past second, which is not necessarily the case, especially on multi-core systems and especially when running only cargo test and not much else.

@workingjubilee
Copy link
Member

Hmm. Can we exclude a crate from running its tests only but still check if it builds?

@workingjubilee
Copy link
Member

This was a float issue that got classed as a sorting issue (but all the sorting issues seem to actually be hashmap issues): https://crater-reports.s3.amazonaws.com/beta-1.81-2/beta-2024-07-26/gh/tommarek.MPCHomeControl/log.txt

@apiraino
Copy link
Contributor

apiraino commented Oct 7, 2024

This is now a stable regression, but I'm not sure what we can do (or want to do)

@apiraino apiraino added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Oct 7, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Nov 8, 2024

P Triage: status unclear, are these regressions still problematic?

@apiraino
Copy link
Contributor

apiraino commented Nov 12, 2024

I guess we'll keep this issue around in case we can reference it from other reports (Zulip thread)

@rustbot label -I-prioritize

@rustbot rustbot removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants