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

Infinite Repetition/Never Ending Test with f32 and f64. #295

Open
Alexhuszagh opened this issue Aug 9, 2021 · 17 comments · May be fixed by #296
Open

Infinite Repetition/Never Ending Test with f32 and f64. #295

Alexhuszagh opened this issue Aug 9, 2021 · 17 comments · May be fixed by #296

Comments

@Alexhuszagh
Copy link

Alexhuszagh commented Aug 9, 2021

Issue

I've got a test that uses the following structure:

quickcheck! {
    fn f32_quickcheck(f: f32) -> bool {
        if f.is_special() {
            true
        } else {
            let string = f.to_string();
            f == string.parse::<f32>()
        }
    }

The minimum failing input is:

f32::from_bits(0b11001111000000000000000000000000);   // -2147483600.0

Adding print debugging with --nocapture shows that this value, -2147483600.0, is called repeatedly, without ever stopping, if the checks pass. The actual implementation uses parsing for an arbitrary radix (using big-integer arithmetic), and a float formatter, but pressing enter in the terminal has printed values rapidly being produced afterwards, showing that the quickcheck is completing each iteration faster than the newline can show (and therefore that the test is stalling out indefinitely due to quickcheck, not the actual test code).

A similar result also occurs for f64 with the following structure:

quickcheck! {
    fn f64_quickcheck(f: f64) -> bool {
        if f.is_special() {
            true
        } else {
            let string = f.to_string();
            f == string.parse::<f64>()
        }
    }

The minimal failing input is:

f64::from_bits(0b1100001111100000000000000000000000000000000000000000000000000000) // f=-9223372036854776000.0

It's worth noting that these have a practically identical bit pattern, which may help debugging this issue.

The currently rustc/QuickCheck version is:

$ rustc --version
rustc 1.55.0-nightly (b41936b92 2021-07-20)
[[package]]
name = "quickcheck"
version = "1.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6"
dependencies = [
 "env_logger",
 "log",
 "rand 0.8.4",
]

Debugging/Solution

Is there any way to print the seed so I can more reliably debug these issues? Although the same infinite loop is reproducing on each subsequent run, this is somewhat difficult to debug, given that it uses a random seed and never exits, making it very hard to submit a PR.

@Alexhuszagh Alexhuszagh changed the title Infinite Repetition/Never Ending Test with f32 Infinite Repetition/Never Ending Test with f32 and f64. Aug 9, 2021
Alexhuszagh added a commit to Alexhuszagh/rust-lexical-experimental that referenced this issue Aug 9, 2021
- Added more tests for binary floats.
- Added support for mandatory signs in integer writers.
- Added support for generic radix floats.
- Added extensive unittests for generic radix floats.
- Implemented the write float ToLexical API.
- Added the WriteFloat trait.

Disabled some QuickCheck tests due to the following bug:
BurntSushi/quickcheck#295
@neithernut
Copy link

Also reproduces with i32::MIN but not with e.g. i32::MIN + 1.

Reproducer:

use quickcheck::Arbitrary;
i32::MIN.shrink().for_each(|v| println!("{}", v));

Looking at the SignedShrinker, it's quite obvious that the shrinker is quaranteed to yield a result if it was initialized from i32::MIN:

if self.x == <$ty>::MIN

Which results in the endless iteration.

I'm honestly surprised this didn't surface any earlier.

@neithernut neithernut linked a pull request Aug 9, 2021 that will close this issue
@neithernut
Copy link

Can you verify that #296 solves your problem?

@Alexhuszagh
Copy link
Author

Can you verify that #296 solves your problem?

Yep, it does. Wonderful, thank you.

@Alexhuszagh
Copy link
Author

Alexhuszagh commented Aug 25, 2021

Weirdly, I'm getting another issue with a stack overflow for cases failing with the bit pattern 0b11001110111111111111111111110000 or 0b11001111000000000000000000000000 for a signed 32-bit integer type. Both these cases fail, and I'm unsure specifically which one causes the indefinite repetition.

It then attempts to shrink as follows (I believe, this is just the data passed to my test function being printed out as bits):

0b11001110111111111111111111111000
0b11001110111111111111111111111100
0b11001110111111111111111111111110
0b11001110111111111111111111111111
0b11001111000000000000000000000000
0b10000000000000000000000000000000
0b11001110100000000000000000000000
0b11001110110000000000000000000000
0b11001110111000000000000000000000
0b11001110111100000000000000000000
0b11001110111110000000000000000000
0b11001110111111000000000000000000
0b11001110111111100000000000000000
0b11001110111111110000000000000000
0b11001110111111111000000000000000
0b11001110111111111100000000000000
0b11001110111111111110000000000000
0b11001110111111111111000000000000
0b11001110111111111111100000000000
0b11001110111111111111110000000000
0b11001110111111111111111000000000
0b11001110111111111111111100000000
0b11001110111111111111111110000000
0b11001110111111111111111111000000
0b11001110111111111111111111100000

This repeats indefinitely until there's a fatal error due to a stack overflow. I've been using your branch of proptest until the changes are merged, but this also reproduces on the main quickcheck repository. This also reproduces on v1.0.3, so it should be another issue with the shrinker.

@neithernut
Copy link

Can you be more specific about what types you use? i32? u32?

Also, I this may warrant a new issue, though it definitely looks related.

@Alexhuszagh
Copy link
Author

Can you be more specific about what types you use? i32? u32?

Also, I this may warrant a new issue, though it definitely looks related.

I'm using f32, so it should be using a signed shrinker. I'll see if I can create a minimal failing case.

@neithernut
Copy link

Looks like I failed to fix the issue properly.

The shrinker is bounded but it does yield the original value, which results in an endless recursion as quickcheck executes the test with the exact same value again and again. As I can't reproduce the issue with a i32 I suspect some information loss in the conversion to f32. I'll have another look later.

@neithernut
Copy link

I just applied some additional band-aids. Could you check whether you can reproduce the issue with 0c279d6 (part of #296)?

@Alexhuszagh
Copy link
Author

Alexhuszagh commented Aug 30, 2021

@neithernut Seems like it has, both #285 and this bug seem to be fixed by the changes. I can't reproduce it exactly with the cases, since running 50 or so repeats never had had those specific bit patterns occur, however, all the failing cases in the shrinker before had a bit pattern of 0b1100111XXXXXXXXXXXXXXXXXXXXXXXXX, so I created the following test:

#[cfg(test)]
mod tests {
    use quickcheck::quickcheck;

    quickcheck! {
        fn case_failure(a: f32) -> bool {
            let mask = 0b11001110000000000000000000000000;
            a.to_bits() & mask != mask
        }
    }
}

The minimum failing case, as expected was 0b11001110000000000000000000000000, or -536870900.0, which is fixed in your branch, but overflows the stack in the master branch. This now seems to be a further extension of #285.

0b11001000000000000000000000000000 does not cause a stack overflow on either, while 0b11001100000000000000000000000000 does produce a stack overflow for the master branch, so I believe this is a useful metric that the issue is fixed.

@KamilaBorowska
Copy link

KamilaBorowska commented Jan 27, 2022

I get an infinite loop with the following test case:

quickcheck! {
    fn something(x: f64) -> bool {
        let x = x.abs() % 355.0;
        x.cosh().acosh() - x < 1e-10
    }
}

@neithernut
Copy link

I have trouble reproducing the issue (both on master and with #296). Does this happen with some specific initial values of x?

@KamilaBorowska
Copy link

It's random for me :(, may necessitate running the test multiple times.

@neithernut
Copy link

To be clear: it does reproduce on master. That much is expected since no fix was merged since the original issue report. However, I dedicated quite a bit of CPU time in an attempt to reproduce the issue with the changes in #296 applied, without any success. So... can you actually reproduce your issue with the fix(es) in #296 or "only" on current master (aka tag 1.0.3 aka defde6f)?

@KamilaBorowska
Copy link

Yeah, I only reproduced this with 1.0.3.

@fosskers
Copy link

It seems this might still be an issue?

@neithernut
Copy link

#296 was supposed to fix this, but it was never merged.

...and that's not the only issue still open with an existing fix. quickcheck appears to be practically abandoned. Depending on your situation, you may want to consider alternatives such as proptest or qcheck (which is a fork).

@fosskers
Copy link

Related: image-rs/imageproc#522

dead-claudia added a commit to dead-claudia/journald-exporter that referenced this issue Jul 6, 2024
...and switch the 32-bit integer parser to just exhaustive checking.
(More on that later.)

Why move away from QuickCheck?

1. The maintainer appears to have little interest in actually
   maintaining it. BurntSushi/quickcheck#315

2. Its API is incredibly inefficient, especially on failure, and it's
   far too rigid for my needs. For one, I need something looser than
   `Arbitrary: Clone` so things like `std::io::Error` can be generated
   more easily. Also, with larger structures, efficiency will directly
   correlate to faster test runs. Also, I've run into the limitations
   of not being able to access the underlying random number generator
   far too many times to count, as I frequently need to generate random
   values within ranges, among other things.
   - BurntSushi/quickcheck#279
   - BurntSushi/quickcheck#312
   - BurntSushi/quickcheck#320
   - BurntSushi/quickcheck#267

3. It correctly limits generated `Vec` and `String` length, but it
   doesn't similarly enforce limits on test length.

4. There's numerous open issues in it that I've addressed, in some
   cases by better core design. To name a few particularly bad ones:
   - Misuse of runtime bounds in `Duration` generation, `SystemTime`
     generation able to panic for unrelated reasons:
     BurntSushi/quickcheck#321
   - Incorrect generation of `SystemTime`:
     BurntSushi/quickcheck#321
   - Unbounded float shrinkers:
     BurntSushi/quickcheck#295
   - Avoiding pointless debug string building:
     BurntSushi/quickcheck#303
   - Signed shrinker shrinks to the most negative value, leading to
     occasional internal panics:
     BurntSushi/quickcheck#301

There's still some room for improvement, like switching away from a
recursive loop: BurntSushi/quickcheck#285.
But, this is good enough for my use cases right now. And this code
base is structured such that such a change is *much* easier to do.
(It's also considerably simpler.)

As for the integer parser change, I found a way to re-structure it so
I could perform true exhaustive testing on it. Every code path has
every combination of inputs tested, except for memory space as a whole.
This gives me enough confidence that I can ditch the randomized
property checking for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants