Hi!

Currently I try to make hash and comparison of UniqueRepresentation
faster. When my attempts broke some tests, I found that
UniqueRepresentation is used improperly in some locations.

For example, it is used on "named" groups.
  sage: G = SymmetricGroup(6)
  sage: type(G)
  sage.groups.perm_gps.permgroup_named.SymmetricGroup_with_category
  sage: isinstance(G, UniqueRepresentation)
  True
  sage: G3 = G.subgroup([G((1,2,3,4,5,6)),G((1,2))])
  sage: hash(G)
  110093248
  sage: hash(G3)
  -2734260432119589081
  sage: G == G3
  True

This is because "named groups" override the __eq__ method of
UniqueRepresentation, but do not override the __hash__ method. So,
that's broken. Similar things happen a lot in sage/graphs.

UniqueRepresentation is not just about "caching of an object by means of the
arguments used to create it"! In addition, it is about objects that compare
equal if and only if they are identical. Hence, overriding __eq__ simply
makes no sense for sub-classes of UniqueRepresentation.

What shall we do about it?

To the very least, UniqueRepresentation should not be used as a base
class for named groups. One could still use ClasscallMetaclass and use a
weak_cached_function decorator on __classcall__, in order to cache named
groups.

But in addition, the hash is broken in the example above. Do you have a
good suggestion for a hash that is compatible with comparison of these
named groups? Similarly for graphs.

Best regards,
Simon


-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-devel?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to