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

Generate special values for f32 and f64 #183

Closed
wants to merge 7 commits into from

Conversation

bekh6ex
Copy link
Contributor

@bekh6ex bekh6ex commented Sep 26, 2017

Issue #27

bekh6ex and others added 7 commits September 20, 2017 10:07
As soon as they are exactly the same
The reason to make it more probable is:
Imagine a user would test function accepting two float arguments.
By default quickcheck runs each test 100 times.
Probability of special value generation (lets say 0.0) after this change
is 1 of 100.
Then probability of two 0.0 generation is 1 of 10000 which (simplifying)
means that tests should be run 100 times. If we run tests on per commit
basis then 100 commits should be made - pretty realistic number.

If we leave it as it was before with probability 1 of 1000, it would mean
that would "require" to commit 10000 times (again, simplifying) to make
two 0.0 appear.

PS: Doesn't totally solve the problem. Function accepting 3 floats will
still require to run tests 10000 times to get three 0.0
for _ in 0..10000 {
let f = f32::arbitrary(&mut gen);
if !f.is_finite() {
non_finite_was_found = true;
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 test can theoretically create a false negative, but probability of that is (as far as I've managed to calculate) 1 of 500.
Maybe number of iterations should be increased, should it?

Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to respond to this. I looked into merging this PR, but it's making a lot of changes and adding a new config knob. I would like to move discussion of how we approach this back to #27 --- Could you please sketch out an implementation path and motivate it? Adding new knobs for this isn't ideal. It'd be nice if we just did the right thing.

@BurntSushi
Copy link
Owner

Closing due to inactivity.

@BurntSushi BurntSushi closed this Aug 25, 2018
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 this pull request may close these issues.

2 participants