Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#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:

Description (last modified by jdemeyer)

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)

9667.patch (1.2 KB) - added by jdemeyer 10 years ago.
Patch to be applied on top of #9343

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by jdemeyer

  • Description modified (diff)

Changed 10 years ago by jdemeyer

Patch to be applied on top of #9343

comment:2 Changed 10 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-4.6 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

comment:3 Changed 10 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Ignore this ticket, see #9764 instead.

comment:4 Changed 10 years ago by mpatel

  • Resolution set to duplicate
  • Status changed from positive_review to closed

comment:5 follow-ups: Changed 9 years ago by was

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 9 years ago by jdemeyer

Replying to was:

For the record, this change introduced a major bug into Sage, because PARI's hash_GEN is itself buggy.

See #11611, I have not tracked it down precisely.

comment:7 in reply to: ↑ 5 Changed 9 years ago by jdemeyer

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.

Note: See TracTickets for help on using tickets.