Opened 2 years ago
Last modified 10 days ago
#30498 new defect
Hash function of libgap objects
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | packages: standard | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Libgap object hash function is not compatible with Python/Sage
sage: hash(2) 2 sage: hash(libgap(2)) 4749963527729243378
As a consequence
sage: set([libgap(2)]) == set([2]) False
Note that the implementation of __hash__
is not very subtle hash(str(self))
and could easily be modified to handle properly
integers and rationals.
Change History (18)
comment:1 Changed 2 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 2 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 2 years ago by
Description: | modified (diff) |
---|
comment:4 follow-up: 5 Changed 2 years ago by
comment:5 Changed 2 years ago by
Replying to dimpase:
Is libgap an exception?
sage: hash(singular(2)) -3887916961774677074 sage: hash(pari(2)) -5620492333743569407 sage: hash(maxima_calculus(2)) -7166118788106343663
Good point! But I would prefer to fix the various interfaces with various tickets. In particular for pari, the hash comes from cypari2
and is implemented using a C function from the PARI libaray. It is not on Sage side. The specificity of the libgap interface is that it is using hash(str(self))
which can easily adapted to match Sage on integers/rationals. Feel free to open further tickets for singular and maxima.
comment:6 Changed 2 years ago by
I am not convinced that this is a good idea. Why would a hash of differently typed things be equal? To me, the fact that ZZ(2) and int(2) both hash to 2 is more of a hassle, i.e., a hash collision! But I might be wrong - should we discuss this on sage-devel?
comment:7 Changed 2 years ago by
https://docs.python.org/3/reference/datamodel.html#object.__hash
[...] The only required property is that objects which compare equal have the same hash value [...]
comment:8 follow-up: 9 Changed 2 years ago by
I am not sure whether this only applies to objects of the same class/type.
comment:9 follow-up: 10 Changed 2 years ago by
Replying to dimpase:
I am not sure whether this only applies to objects of the same class/type.
The python spec has no such requirement, which means that generally, objects for different class/type should compare unequal.
In sage, our equality is too liberal to combine useful hashes with keeping to this strict rule:
sage: 2 == GF(3)(2) == 5 True
so we're already forced to be pragmatic about it. Within the same parent, I think we do want it to hold -- otherwise you're better off making the objects not hashable. Outside of that, we can do a reasonable effort, but things will break eventually. People will have to learn that in Sage, you need to normalize your keys prior to putting them in a dictionary, and what that means depends on your application. It will generally mean forcing all the keys into the same parent. It's important to keep hashes cheap too!
comment:10 Changed 2 years ago by
Replying to nbruin:
It's important to keep hashes cheap too!
Regarding that point, hash(str(self))
is slow!
comment:11 Changed 2 years ago by
Milestone: | sage-9.2 → sage-9.3 |
---|
comment:12 Changed 2 years ago by
Milestone: | sage-9.3 → sage-9.4 |
---|
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:13 Changed 2 years ago by
I opened an issue for this against gappy as well: https://github.com/embray/gappy/issues/11
comment:14 Changed 19 months ago by
Milestone: | sage-9.4 → sage-9.5 |
---|
comment:15 Changed 14 months ago by
Milestone: | sage-9.5 → sage-9.6 |
---|
comment:16 Changed 9 months ago by
Milestone: | sage-9.6 → sage-9.7 |
---|
comment:17 Changed 5 months ago by
Milestone: | sage-9.7 → sage-9.8 |
---|
comment:18 Changed 10 days ago by
Milestone: | sage-9.8 |
---|
Is libgap an exception?