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

**Note:**See TracTickets for help on using tickets.

Is libgap an exception?