#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: |
GitHub link to the corresponding issue
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 13 years ago by
| Description: | modified (diff) |
|---|
Changed 13 years ago by
| Attachment: | 9667.patch added |
|---|
comment:2 Changed 12 years ago by
| Description: | modified (diff) |
|---|---|
| Milestone: | sage-4.6 → sage-duplicate/invalid/wontfix |
| Status: | new → needs_review |
comment:3 Changed 12 years ago by
| Status: | needs_review → positive_review |
|---|
Ignore this ticket, see #9764 instead.
comment:4 Changed 12 years ago by
| Resolution: | → duplicate |
|---|---|
| Status: | positive_review → closed |
comment:5 follow-ups: 6 7 Changed 12 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 Changed 12 years ago by
comment:7 Changed 12 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