Refactor hash float #129
Merged
Refactor hash float #129
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hi!
I noticed that for hashing ordered floats, we normalize NaN to a unique representation, just to later check for .is_nan() in raw_double_bits anyway.
Also, for NotNan, we have this .is_nan() check in raw_double_bits, even though it can never be true due to the NotNan invariant.
So I decided to try to restructure the hashing code a bit. The special cases are now all handled uniformly inside the hash implementations, and raw_double_bits just does the raw bit conversion, without any special handling.
I took care to not change the actual resulting hashes, in case that's relevant for backwards compatibility.
Also, I added a test case for mapping 0 and -0 for NotNan to the same hash, and one for mapping different representations of NaN for OrderedFloat to the same hash.