-
Notifications
You must be signed in to change notification settings - Fork 587
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
Fold integer endpoint upweighting into weights=
#4138
Conversation
not writing the idx case to the bytestream could result in the idx != 0 case being much bytestream-simpler but not ir-simpler
ac57b11
to
116bbd3
Compare
I got tired of seeing this flake (which, as I discovered, happens because the number of bits of the two integers were not equal, failing a check in redistribute_block_pairs), so I rewrote it into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - thanks again @tybug!
Oooh, that would be very nice. I suspect this will be trivial after we drop the bytestring representation and pretty annoying until then, so probably we just have to work through all the other cleanups first 😿 |
This makes
st.integers
map to a singledraw_integer
call in our IR, improving efficiency for shrinking, and possibly generation due to e.g. example copying (generate_mutations_from
). Previously discussed at #4101 (comment) and other intermittent places.The new interface is:
I've made several practical compromises here to ease implementation:
sum(p)
must be strictly less than 1. In other words, weights cannot be a total interval. This allows the forcing implementation to force into the remaining probability mass, which is not possible if it is 0.1 - sum(p)
- gets distributed over all values, not just the unmapped ones. I think this is fine for current usages, but something to improve in the future.This approach is actually not my preferred api interface. I would like to have
weights: dict[IntervalSet, float]
which maps intervals to p. This would make it possible to express things like "downweight everything below zero". Unfortunately, I had difficulty getting this approach to work with shrinking. My WIP branch is at https://github.com/HypothesisWorks/hypothesis/compare/master...tybug:hypothesis:integer-weights?expand=1, but I don't expect to come back to it in the near future.@pschanely I think you ignore
weights
in hypothesis-crosshair, so I don't expect this to change anything for you, but I thought you'd appreciate a heads up for interface changes regardless 🙂