Changes between Version 1 and Version 5 of Ticket #25393


Ignore:
Timestamp:
05/18/18 16:48:45 (3 years ago)
Author:
tscrim
Comment:

Replying to embray:

I think you must have misread me or something. I didn't mean that if two instances have the same hash it implies they are ==. I mean that in order for hashing to be implemented correctly, then as you said if they are == they must have the same hash.

Paraphrasing what you wrote: if x.__class__ == y.__class__ and hash(x) == hash(y), then x == y. It is a bad smell for x == y with hash(x) == hash(y). We're on the same page with what needs to be done, but I just saying the ticket description needed an update, which I have just done.

IMO, it is not a code smell to have hash(x) == hash(y) but x != y. However, I do agree that it is better to have a more perfect hash function, but there are only so many hash values since hashes are limited to 64 bit integers (unless I am misunderstanding something in Python).

It actually wasn't immediately obvious to me that this class is not immutable. Where can it be mutated? All I noticed is that you define two ContinuousMaps that are the same except that one is constructed with is_isomorphism=True and the other with is_isomorphism=False. In this case they are == but have different hashes.

See add_expr and set_expr.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #25393

    • Property Cc ​egourgoulhon added
    • Property Status changed from new to needs_info
  • Ticket #25393 – Description

    v1 v5  
    33This is one of the classes impacted by the issue raised [https://trac.sagemath.org/ticket/25387#comment:3 in this comment].
    44
    5 I'm not sure what the best solution is.  Right now I feel like it's somewhat ill-defined, because it's possible to have two `ContinuousMaps` that compare as equal, but have different hashes.  That's not necessarily wrong, but it isn't obviously right either.  If two instances of the same class have the same hash, they must also be equal to each other with `==`.  Technically I believe you can get away with the inverse--having different hashes but equivalent via `==`--but it's not advisable and is a bad smell.
     5I'm not sure what the best solution is.  Right now I feel like it's somewhat ill-defined, because it's possible to have two `ContinuousMaps` that compare as equal, but have different hashes.  That's not necessarily wrong, but it isn't obviously right either.  If `x == y`, then `hash(x) == hash(y)` for a correct implementation.  Technically I believe you can get away with the inverse--having `hash(x) == hash(y)` but `x != y`--but it is advisable for performance to avoid this.