Skip to content

Fix discard ratio and maxSuccess interacting poorly#371

Merged
MaximilianAlgehed merged 7 commits intomasterfrom
PR-fix-discard-ratio-maxSuccess
Mar 21, 2024
Merged

Fix discard ratio and maxSuccess interacting poorly#371
MaximilianAlgehed merged 7 commits intomasterfrom
PR-fix-discard-ratio-maxSuccess

Conversation

@MaximilianAlgehed
Copy link
Collaborator

Fixes #338

small + fix `withMaxSuccess` not updating result on discard
@MaximilianAlgehed
Copy link
Collaborator Author

@Rewbert thoughts?

@Rewbert
Copy link
Collaborator

Rewbert commented Mar 20, 2024

This whole discard thing is such a thorn in my side. The choice to compute sizes in this way seems to have been arbitrarily chosen initially (Koen looked guilty when I asked him about this!), so in my mind this is "equally arbitrary".

I have thought a lot about this, and have even tried out a few different approaches altogether, but have not landed in anything that is obviously superior. I think the question of sizes might be a more complicated one than it appears initially.

Regarding the eventual future merge of the parallel branch, this will not impact me negatively.

MaximilianAlgehed and others added 3 commits March 20, 2024 23:01
regardless of if we are using `withMaxSuccess` and `withDiscardRatio` or
`stdArgs{maxSuccess=...,maxDiscardRatio=...}`
@MaximilianAlgehed
Copy link
Collaborator Author

For posterity, I had to factor out computeSize from State because the computeSize definition was set too early based on the Args arguments to quickCheckWith and thereby would ignore withMaxSuccess and withDiscardRatio.

@MaximilianAlgehed MaximilianAlgehed merged commit d88f5f6 into master Mar 21, 2024
@MaximilianAlgehed MaximilianAlgehed deleted the PR-fix-discard-ratio-maxSuccess branch March 21, 2024 08:55
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.

quickCheckWithResult with low maxSuccess fails unexpectedly

3 participants