Opened 12 years ago
Closed 12 years ago
#4957 closed defect (fixed)
[with patch, positive review] inconsistent integer hashing
Reported by: | robertwb | Owned by: | craigcitro |
---|---|---|---|
Priority: | critical | Milestone: | sage-3.3 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
sage: z = 18446462603027742720 sage: hash(z) 66912258 sage: hash(int(z)) -131071 sage: hash(long(z)) -131071
This causes problems with looking up values in hashtables...
Attachments (1)
Change History (9)
comment:1 Changed 12 years ago by
- Milestone changed from sage-3.4.1 to sage-3.3
- Owner changed from somebody to craigcitro
- Status changed from new to assigned
- Summary changed from inconsistent integer hashing to [with patch, needs review] inconsistent integer hashing
comment:2 Changed 12 years ago by
- Summary changed from [with patch, needs review] inconsistent integer hashing to [with patch, positive review] inconsistent integer hashing
Excellent. I haven't been able to break it, and the code (and comment) look good.
comment:3 Changed 12 years ago by
- Summary changed from [with patch, positive review] inconsistent integer hashing to [with patch, needs work] inconsistent integer hashing
This is broken on 64 bit linux:
sage -t -long "devel/sage/sage/rings/integer.pyx" ********************************************************************** File "/scratch/mabshoff/sage-3.3.alpha2/devel/sage/sage/rings/integer.pyx", line 2085: sage: n = -920390823904823094890238490238484; n.__hash__() Expected: 6874330978542788722 Got: 6917515397235318898 ********************************************************************** File "/scratch/mabshoff/sage-3.3.alpha2/devel/sage/sage/rings/integer.pyx", line 2101: sage: hash(n) Expected: -9223372036854767616 Got: 8192 ********************************************************************** File "/scratch/mabshoff/sage-3.3.alpha2/devel/sage/sage/rings/integer.pyx", line 2104: sage: hash(n) == hash(int(n)) Expected: True Got: False **********************************************************************
comment:4 Changed 12 years ago by
Darn :(. The first two may be OK (we need to see what hash(int(n)) is, but the second one is a problem.
Changed 12 years ago by
comment:5 Changed 12 years ago by
- Summary changed from [with patch, needs work] inconsistent integer hashing to [with patch, needs review] inconsistent integer hashing
Ok, I fixed it. It turns out it was some sloppy C coding on my part: I really wanted sizeof(mp_limb_t)
instead of sizeof(int)
.
comment:6 Changed 12 years ago by
I bet this is the right fix, could you re-run the tests on a 64 bit machine?
comment:7 Changed 12 years ago by
- Summary changed from [with patch, needs review] inconsistent integer hashing to [with patch, positive review] inconsistent integer hashing
That does the trick on sage.math
comment:8 Changed 12 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 3.3.alpha3.
Cheers,
Michael
Note: See
TracTickets for help on using
tickets.
This was ugly. It turns out that we were shifting an
int
to the right by 45 bits on a 32 bit machine, which one might think would result in zero, but in fact shifts to the right by(45%32) = 13
bits.The attached patch fixes this, and adds some doctests.