Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
generate floats using lexographically ordered encoding #49
base: main
Are you sure you want to change the base?
generate floats using lexographically ordered encoding #49
Changes from 4 commits
08a170d
1df83c0
e58ac23
4ecb381
822574e
fb8907c
fb2b833
d1ece54
9bfb8f5
c207dc9
c8462e4
54b1c8d
3d0e211
c1555cb
9f39ce8
add36a1
e694269
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What is
bias
?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.
bias
is the IEEE754 bias for the exponent. I'll add a docstring.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.
We always check numbers
>= 0.0
below, is thisassume!
necessary? What's the corresponding test in hypothesis?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.
I'll have to double check but I believe it's excluding -0 which makes things a bit screwy. This isn't present in the hypothesis test.
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.
Don't splat
Vector
s, it leads to unnecessary specialization of the called function since it needs to compile a new version per length. Also requires dynamic dispatch, since the length of theVector
is not known at compile time (it's nice to keep CI times lower if we can).One problem with this approach though is that it's not guaranteed that all of these examples are actually run 🤔 Maybe looping over the array and doing
@check max_examples=1 roundtrip_encoding(Data.Just(x))
would be better? If theassume!
is removed, this could also just be a@testset for
.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.
We can do
nextfloat(zero(T))
andprevfloat(one(T))
here too and skip theassume!
, I think.