On Mon, Apr 18, 2011 at 10:24:17PM -0700, William Stein wrote:
> So the main issue I pointed out above, namely that SageObject has a
> __hash__ defined, is really a non-issue, since that's the standard
> default convention in Python anyways. The real issue to consider is
> whether using the string representation as a default is a good idea.
> The only real drawback is efficiency, since computing the string
> representation of an object can be costly. So...
Two other drawbacks I have met on a regular basis:
- In quite a few situations, the repr contains very little
information about the object, giving a lot of collisions:
sage: DiGraph([(1, 2, 0)])
Digraph on 2 vertices
sage: DiGraph([(4, 5, 6)])
Digraph on 2 vertices
Ok, that example is not great since graphs can't yet be made
mutable. Besides, one could argue that this is just a question of
optimization and should be solved by implementing a proper hash
function for the relevant objects.
- Much worst: the Python assumption that the hash value of an object
does not change can easily be broken inadvertently, even by a total
beginner, by using the rename feature:
sage: K = QQ['x']
sage: hash(K)
-764788796815899192
sage: K.rename("K")
sage: hash(K)
9600028874
> Project: comment out the two lines in sage_object.pyx, run the Sage
> test suite, observe failures [1], choose one, and write a better
> (faster; more intelligent) __hash__ method for the objects that lead
> to the failure.
Altogether, I would be quite favorable to such a change.
Since Parents are supposed to have unique representation, an
intermediate step that should be fairly safe would be to first move
the definition of __hash__ from SageObject to Element.
For the record: for classes that derive from UniqueRepresentation, the
hash function is reinstated back to id (currently using a Python
method, which is much slower than Python's default implementation).
> [1] The files in devel/sage/sage that fail are:
> ...
> devel/sage-main/sage/sets/disjoint_union_enumerated_sets.py # 10 doctests
> failed
For the record, I reduced the failure above to:
sage: sage: x = {1: FiniteEnumeratedSet([1,2,3]), 2:
FiniteEnumeratedSet([4,5,6])}
sage: y = {1: FiniteEnumeratedSet([1,2,3]), 2:
FiniteEnumeratedSet([4,5,6])}
sage: hash(Family(x))
99386144
sage: hash(Family(y))
99386384
but did not manage to reduce it much further ...
Cheers,
Nicolas
--
Nicolas M. Thiéry "Isil" <[email protected]>
http://Nicolas.Thiery.name/
--
To post to this group, send an email to [email protected]
To unsubscribe from this group, send an email to
[email protected]
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org
