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

use u64 instead of u32 for max_shrink_iters #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kali
Copy link

@kali kali commented Apr 7, 2022

Use u64 instead of u32 for max_shrink_iters. I have high cardinality problems (tensors...) and hitting this all the time. I can't see any realistic drawback.

Fixes #254

@cameron1024
Copy link
Member

Hi, thanks for the PR ☺️

This seems reasonable, the main drawback I'm thinking about is the breaking change. This would require proptest 2.0, and I'm not sure if there are any plans for that any time soon.

That said, I can't imagine this would break many people's code in practice. I've only ever used this with an int literal 🤷

@kali
Copy link
Author

kali commented Nov 16, 2022

Yeah, same, I would expect most people to use this with the envir or through annotations. But I see the the point, it's strictly a breaking API change.

I've seen there is a merged PR improving the strategy for vectors. It is very possible this PR actually helps also my tensor use case...

@cameron1024
Copy link
Member

Hopefully that should improve things. That said, I think there's potentially general usefulness in being able to have more than 2^32 iters. If/when we do release a breaking change, this seems like something that should definitely be included. Other than the breaking nature, I can't see any downsides either

@rexmas rexmas added the 2.0-wishlist This issue proposes breaking changes we'd like in a 2.0 release label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0-wishlist This issue proposes breaking changes we'd like in a 2.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PROPTEST_MAX_SHRINK_ITERS as u32 is too small
3 participants