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

The minimal example for st.floats(1.1, 1.6) should be 1.5, as if there were no bounds #3357

Closed
Zac-HD opened this issue May 24, 2022 · 4 comments
Labels
test-case-reduction about efficiently finding smaller failing examples

Comments

@Zac-HD
Copy link
Member

Zac-HD commented May 24, 2022

#3327 made enormous improvements to generation of bounded floats, but shrinking still has issues if integer values are disallowed. From #3327 (comment):

Well! I didn't figure it out. (1.1, 1.9) DOES get to 1.5, and I've added that test. But all is not well. (1.1, 1.6) yields 1.501953125. Here is my best attempt at explaining what happens, but I'm not confident about my understanding:

There is a one bit difference between 1.501953125 and 1.5; in the lex form, it's the last bit in the next-to-last byte. The shrinker successfully clears the earlier mantissa bytes. We do attempt to clear that bit, but at the same time, we roll over the final byte to 255. When bit-reversed, those are the high bits of the mantissa, and we run afoul of the clamper upper bound. And that's why increasing the upper bound to 1.9 makes it work.

I've noticed some float-specific shrinking logic (but have not yet spent more than a moment trying to grok it). Even when attempting to disable that logic, I'm seeing approximately the same behavior on this test, so I suspect it's not relevant here. But I don't fully trust myself on that assertion either.

Seems plausible to me that we need to change the clamper to be more shrink-friendly, since there are probably other cases where this has an undesired effect? Not confident in any diagnosis yet though; this is a tracking issue and the next step should probably be investigation rather than jumping directly to a fix.

@Zac-HD Zac-HD added the test-case-reduction about efficiently finding smaller failing examples label May 24, 2022
@jobh
Copy link
Contributor

jobh commented Jul 26, 2023

I don't think the clamper is to blame here, it only exposes the fundamental issue with shrinking floats: They want to be shrunk from both sides (magnitude and precision). Additionally, maybe as a consequence of this dual goal?, it appears to shrink discontinuously in float-space (when a lex-space byte rolls over).

When the clamp is wide enough, that's fine. The float jumps to a relatively far-away intermediate value, but keeps shrinking towards the origin. In contrast, the narrow clamp maps this intermediate value to another with a more complex lex, and stops. But the thing is, the same would happen without clamping, with a narrow bug region! Hence, it's just a symptom of the underlying issue.

So, I think the fix should probably be in the float-specific shrinker: to generate nearby shrink candidates by dropping most-significant precision bits - it already does for the special case of dropping the fractional part completely. I can create a draft PR for further discussion later if the "analysis" above seems reasonable.

@Zac-HD
Copy link
Member Author

Zac-HD commented Aug 3, 2023

I'm back from a wilderness camping trip, and that analysis sounds great! I'll review your PR in the next few days 🤩

@jobh
Copy link
Contributor

jobh commented Aug 3, 2023

Thanks! I hope you return happy and mentally rested. To celebrate, I upgraded the draft to a proper PR.

@Zac-HD
Copy link
Member Author

Zac-HD commented Aug 6, 2023

Fixed by #3706 🎉

@Zac-HD Zac-HD closed this as completed Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-case-reduction about efficiently finding smaller failing examples
Projects
None yet
Development

No branches or pull requests

2 participants