#9667 closed enhancement (duplicate)
Use PARI's hash_GEN() for gen.__hash__
| Reported by: | jdemeyer | Owned by: | was |
|---|---|---|---|
| Priority: | minor | Milestone: | sage-duplicate/invalid/wontfix |
| Component: | interfaces | Keywords: | |
| Cc: | Merged in: | ||
| Authors: | Jeroen Demeyer | Reviewers: | |
| Report Upstream: | N/A | Work issues: | |
| Branch: | Commit: | ||
| Dependencies: | Stopgaps: |
Status badges
Description (last modified by )
The latest version of PARI has a function hash_GEN which hashes a PARI GEN. Since this is very likely faster than hashing the string representation of a GEN, we should use this for the gen class in sage/libs/pari/gen.pyx
This patch has been included in #9764.
Attachments (1)
Change History (8)
comment:1 Changed 11 years ago by
- Description modified (diff)
Changed 11 years ago by
comment:2 Changed 11 years ago by
- Description modified (diff)
- Milestone changed from sage-4.6 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
comment:3 Changed 11 years ago by
- Status changed from needs_review to positive_review
Ignore this ticket, see #9764 instead.
comment:4 Changed 11 years ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
comment:5 follow-ups: ↓ 6 ↓ 7 Changed 10 years ago by
Hi,
For the record, this change introduced a major bug into Sage, because PARI's hash_GEN is itself buggy. For example, by playing with ideals in Sage (code is complicated though...), I quickly got into this situation:
sage: n0 [11, 3; 0, 1] sage: n1 [11, 3; 0, 1] sage: hash(n0) -7493989779944505307 sage: hash(n1) -6341068275337658331
comment:6 in reply to: ↑ 5 Changed 10 years ago by
comment:7 in reply to: ↑ 5 Changed 10 years ago by
Replying to was:
Hi,
For the record, this change introduced a major bug into Sage, because PARI's hash_GEN is itself buggy.
Don't blaim PARI when the fault is the Sage->PARI interface. The issue is not hash_GEN(), it is a problem with how integers are converted from Sage to PARI. I have a patch for this issue at #11611.
Patch to be applied on top of #9343