Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash function of libgap objects #30498

Open
videlec opened this issue Sep 3, 2020 · 12 comments
Open

Hash function of libgap objects #30498

videlec opened this issue Sep 3, 2020 · 12 comments

Comments

@videlec
Copy link
Contributor

videlec commented Sep 3, 2020

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.

Component: packages: standard

Issue created by migration from https://trac.sagemath.org/ticket/30498

@videlec videlec added this to the sage-9.2 milestone Sep 3, 2020
@videlec

This comment has been minimized.

@dimpase

This comment has been minimized.

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Sep 3, 2020

comment:4

Is libgap an exception?

sage: hash(singular(2))                                                                                                                                                                                                                
-3887916961774677074
sage: hash(pari(2))                                                                                                                                                                                                                    
-5620492333743569407
sage: hash(maxima_calculus(2))                                                                                                                                                                                                         
-7166118788106343663

@videlec
Copy link
Contributor Author

videlec commented Sep 3, 2020

comment:5

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.

@dimpase
Copy link
Member

dimpase commented Sep 3, 2020

comment:6

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?

@videlec
Copy link
Contributor Author

videlec commented Sep 3, 2020

comment:7

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 [...]

@dimpase
Copy link
Member

dimpase commented Sep 3, 2020

comment:8

I am not sure whether this only applies to objects of the same class/type.

@nbruin
Copy link
Contributor

nbruin commented Sep 3, 2020

comment:9

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!

@videlec
Copy link
Contributor Author

videlec commented Sep 4, 2020

comment:10

Replying to @nbruin:

It's important to keep hashes cheap too!

Regarding that point, hash(str(self)) is slow!

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Member

mkoeppe commented Feb 13, 2021

comment:12

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@embray
Copy link
Contributor

embray commented Feb 16, 2021

comment:13

I opened an issue for this against gappy as well: embray/gappy#11

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants