Ticket #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: | Work issues: | ||
| Report Upstream: | Reviewers: | ||
| Authors: | Merged in: | ||
| 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
Change History
comment:1 Changed 4 years ago by craigcitro
- 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
- Milestone changed from sage-3.4.1 to sage-3.3
comment:2 Changed 4 years ago by robertwb
- 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 4 years ago by mabshoff
- 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 4 years ago by robertwb
Darn :(. The first two may be OK (we need to see what hash(int(n)) is, but the second one is a problem.
comment:5 Changed 4 years ago by craigcitro
- 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 4 years ago by robertwb
I bet this is the right fix, could you re-run the tests on a 64 bit machine?
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.