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.

Sign up for GitHub

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

Jump to bottom

Wrong hashes for some Rationals #50628

Closed
sostock opened this issue Jul 21, 2023 · 2 comments · Fixed by #50655
Closed

Wrong hashes for some Rationals #50628

sostock opened this issue Jul 21, 2023 · 2 comments · Fixed by #50655
Assignees
@oscardssmith
Labels
domain:hashing kind:bug Indicates an unexpected problem or unintended behavior kind:regression Regression in behavior compared to a previous version
Milestone
1.10

Comments

@sostock
Copy link
Contributor

sostock commented Jul 21, 2023

On Julia v1.10.0-alpha1 and master, some Rationals which are equal have different hashes:

julia> hash(5//2) # same hash as on older Julia versions (≥ v1.6)
0x16b7f40fdbb2c57d

julia> hash(Int128(5)//2) # the same hash is also obtained for UInt128(5)//2 and BigInt(5)//2
0xb795033f6f2a0674
@sostock sostock added kind:bug Indicates an unexpected problem or unintended behavior kind:regression Regression in behavior compared to a previous version domain:hashing labels Jul 21, 2023
@oscardssmith oscardssmith self-assigned this Jul 21, 2023
@oscardssmith
Copy link
Member

oscardssmith commented Jul 21, 2023 •
edited
Loading

caused by #50041. The change in hash is intentional but the inconsistency isn't. There are a few bugs that were added that I just found. Thanks for the test case!

@giordano giordano added this to the 1.10 milestone Jul 21, 2023
@oscardssmith
Copy link
Member

My initial guess was partially wrong. The actual problem was caused by both #50041 missing hashing odd denominators combined with the fact that #49996 was incorrectly computing the position of the right most bit. PR incoming.

@oscardssmith oscardssmith mentioned this issue Jul 24, 2023
Merged
oscardssmith added a commit that referenced this issue Jul 26, 2023
@oscardssmith
fix hashing regression. (#50655)
c777c71 
This fixes 2 bugs introduced by #49996 and #50041.
Closes #50628.
KristofferC pushed a commit that referenced this issue Aug 10, 2023
@oscardssmith @KristofferC
fix hashing regression. (#50655)
3e133c9 
This fixes 2 bugs introduced by #49996 and #50041.
Closes #50628.

(cherry picked from commit c777c71)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oscardssmith oscardssmith

Labels
domain:hashing kind:bug Indicates an unexpected problem or unintended behavior kind:regression Regression in behavior compared to a previous version
Projects
None yet
Milestone
1.10
Development

Successfully merging a pull request may close this issue.

fix hashing regression. oscardssmith/julia
3 participants
@giordano @oscardssmith @sostock