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

Float check #1069

Merged
merged 3 commits into from
Feb 10, 2021
Merged

Float check #1069

merged 3 commits into from
Feb 10, 2021

Conversation

robdockins
Copy link
Contributor

Implement :exhaust for floating point values, and improve the random generator to produce more representative samples.

by enumerating all bitpatterns.

Note, this overcounts somewhat the number of distinct floating point
values there are, since all NaNs are considered identical
as `Float` values.  We could be more careful here to avoid
generating more NaN values than required, but I'm not sure it's worth
the effort.

Fixes #1051
This generator will will generate "special" values, and should
also provide better coverage across the range of values the type
can represent, instead of being strongly biased toward values near
1, as the old generator was.
Copy link
Contributor

@brianhuffman brianhuffman left a comment

Choose a reason for hiding this comment

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

I agree that having :exhaust test multiple NaN values is not a problem: If the float type is small enough to be exhaustively tested, then you're going to be spending a relatively small amount of time checking NaN cases anyway.

For the random test generation, there seems to be a lot of arbitrary choices in there, and I'm wondering whether simply using genBinary by itself would suffice (with a small modification to replace NaN values with infinities half the time). In any case, the extra heuristics shouldn't hurt, so it probably doesn't matter much.

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