Skip to content

Commit

Permalink
Trac #19016: Better hash for Element
Browse files Browse the repository at this point in the history
As reported on sage-devel [1], `Element` implements its hash based on
its string representation. This causes a lot of troubles

- it breaks the {{{equality => same hash}}} assumption for finitely
presented groups
{{{
    sage: G = groups.presentation.Cyclic(4)
    sage: G.cayley_graph().vertices()
    [1, a, a^2, a^-2, a^3, a^-1]
}}}
  and symbolic expressions
{{{
sage: f=sin(x)^2
sage: g=1-cos(x)^2
sage: bool(f == g)
True
sage: hash(f) == hash(g)
False
}}}
   and possibly many others

- it is highly incompatible with the `rename` feature (see also #8119)
{{{
sage: from sage.structure.element import Element
sage: class E(Element):
....:     def __init__(self):
....:         Element.__init__(self, Parent())
sage: e = E()
sage: hash(e)
-4965357552728411610
sage: e.rename('hey')
sage: hash(e)
-6429308858210906323
}}}
  and similarly, hashing should not be available on any mutable object.

- it might be very bad for performance: see #18215 and #18239 for
examples

There are several possibilities that are currently being discussed:
- make it return `0` by default (original proposition of the ticket)
- make it raise a `TypeError` by default (as it the case for
`SageObject`, see #18246)
- let it as it is but raise a Warning

[1] https://groups.google.com/d/topic/sage-devel/6rXKkF87Gtc/discussion

See also: #19302, #19321, #19331

URL: http://trac.sagemath.org/19016
Reported by: ncohen
Ticket author(s): Nils Bruin, Vincent Delecroix
Reviewer(s): Volker Braun
  • Loading branch information
Release Manager authored and vbraun committed Oct 28, 2015
2 parents 1018f94 + c7b4f0e commit 01bc1e4
Show file tree
Hide file tree
Showing 20 changed files with 397 additions and 79 deletions.
12 changes: 11 additions & 1 deletion src/sage/algebras/quatalg/quaternion_algebra.py
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,17 @@ def ternary_quadratic_form(self, include_basis=False):
return Q

class QuaternionFractionalIdeal(Ideal_fractional):
pass
def __hash__(self):
r"""
Stupid constant hash function!
TESTS::
sage: R = QuaternionAlgebra(-11,-1).maximal_order()
sage: hash(R.right_ideal(R.basis()))
0
"""
return 0

class QuaternionFractionalIdeal_rational(QuaternionFractionalIdeal):
"""
Expand Down
8 changes: 5 additions & 3 deletions src/sage/categories/additive_magmas.py
Original file line number Diff line number Diff line change
Expand Up @@ -711,9 +711,11 @@ def _test_zero(self, **options):
tester.assert_(self.is_parent_of(zero))
for x in tester.some_elements():
tester.assert_(x + zero == x)
# Check that zero is immutable by asking its hash:
tester.assertEqual(type(zero.__hash__()), int)
tester.assertEqual(zero.__hash__(), zero.__hash__())
# Check that zero is immutable if it looks like we can:
if hasattr(zero,"is_immutable"):
tester.assertEqual(zero.is_immutable(),True)
if hasattr(zero,"is_mutable"):
tester.assertEqual(zero.is_mutable(),False)
# Check that bool behave consistently on zero
tester.assertFalse(bool(self.zero()))

Expand Down
8 changes: 5 additions & 3 deletions src/sage/categories/magmas.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,11 @@ def _test_one(self, **options):
for x in tester.some_elements():
tester.assert_(x * one == x)
tester.assert_(one * x == x)
# Check that one is immutable by asking its hash;
tester.assertEqual(type(one.__hash__()), int)
tester.assertEqual(one.__hash__(), one.__hash__())
# Check that one is immutable if it looks like we can test this
if hasattr(one,"is_immutable"):
tester.assertEqual(one.is_immutable(),True)
if hasattr(one,"is_mutable"):
tester.assertEqual(one.is_mutable(),False)

def is_empty(self):
r"""
Expand Down
2 changes: 1 addition & 1 deletion src/sage/categories/regular_crystals.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ def demazure_operator_simple(self, i, ring = None):
sage: K = crystals.KirillovReshetikhin(['A',2,1],2,1)
sage: t = K(rows=[[3],[2]])
sage: t.demazure_operator_simple(0)
B[[[2, 3]]] + B[[[1, 2]]]
B[[[1, 2]]] + B[[[2, 3]]]
TESTS::
Expand Down
12 changes: 12 additions & 0 deletions src/sage/combinat/colored_permutations.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ def __init__(self, parent, colors, perm):
self._perm = perm
MultiplicativeGroupElement.__init__(self, parent=parent)

def __hash__(self):
r"""
TESTS::
sage: C = ColoredPermutations(4, 3)
sage: s1,s2,t = C.gens()
sage: hash(s1), hash(s2), hash(t)
(2666658751600856334, 3639282354432100950, 3639281107336048003) # 64-bit
(-1973744370, 88459862, -1467077245) # 32-bit
"""
return hash(self._perm) ^ hash(self._colors)

def _repr_(self):
"""
Return a string representation of ``self``.
Expand Down
106 changes: 102 additions & 4 deletions src/sage/combinat/crystals/affine.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from sage.misc.abstract_method import abstract_method
from sage.categories.regular_crystals import RegularCrystals
from sage.categories.finite_crystals import FiniteCrystals
from sage.structure.element import parent
from sage.structure.parent import Parent
from sage.structure.unique_representation import UniqueRepresentation
from sage.structure.element_wrapper import ElementWrapper
Expand Down Expand Up @@ -460,13 +461,45 @@ def phi(self, i):
else:
return self.lift().phi(i)

def __lt__(self, other):
def __eq__(self, other):
"""
Non elements of the crystal are incomparable with elements of the
crystal (or should it return ``NotImplemented``?). Elements of this
crystal are compared using the comparison in the underlying
classical crystal.
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
sage: b = K(rows=[[1]])
sage: c = K(rows=[[2]])
sage: b==c
False
sage: b==b
True
sage: b==1
False
"""
return parent(self) is parent(other) and self.value == other.value

def __ne__(self, other):
""""
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
sage: b = K(rows=[[1]])
sage: c = K(rows=[[2]])
sage: b!=c
True
sage: b!=b
False
sage: b!=1
True
"""
return not self == other

def __lt__(self, other):
""""
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
Expand All @@ -479,9 +512,74 @@ def __lt__(self, other):
sage: b<c
True
"""
if self.parent() is not other.parent():
return False
return self.lift() < other.lift()
return parent(self) is parent(other) and self.value < other.value

def __gt__(self, other):
""""
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
sage: b = K(rows=[[1]])
sage: c = K(rows=[[2]])
sage: b>c
False
sage: b>b
False
sage: c>b
True
"""
return parent(self) is parent(other) and self.value > other.value

def __le__(self, other):
""""
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
sage: b = K(rows=[[1]])
sage: c = K(rows=[[2]])
sage: b<=c
True
sage: b<=b
True
sage: c<=b
False
"""
return parent(self) is parent(other) and self.value <= other.value

def __ge__(self, other):
""""
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
sage: b = K(rows=[[1]])
sage: c = K(rows=[[2]])
sage: c>=b
True
sage: b>=b
True
sage: b>=c
False
"""
return parent(self) is parent(other) and self.value >= other.value

def __cmp__(self, other):
""""
EXAMPLES::
sage: K = crystals.KirillovReshetikhin(['A',2,1],1,1)
sage: b = K(rows=[[1]])
sage: c = K(rows=[[2]])
sage: cmp(b,c)
-1
sage: cmp(b,b)
0
If the parent are different, it uses comparison of the parents::
sage: cmp(b,1) == cmp(b.parent(), ZZ)
True
"""
return cmp(parent(self), parent(other)) or cmp(self.value, other.value)

AffineCrystalFromClassical.Element = AffineCrystalFromClassicalElement

Expand Down
43 changes: 21 additions & 22 deletions src/sage/combinat/crystals/affinization.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# http://www.gnu.org/licenses/
#****************************************************************************

from sage.structure.element import parent
from sage.structure.parent import Parent
from sage.structure.unique_representation import UniqueRepresentation
from sage.structure.element import Element
Expand Down Expand Up @@ -187,11 +188,23 @@ def _latex_(self):
from sage.misc.latex import latex
return latex(self._b) + "({})".format(self._m)

def __eq__(self, other):
def __hash__(self):
r"""
TESTS::
sage: A = crystals.KirillovReshetikhin(['A',2,1], 2, 2).affinization()
sage: mg = A.module_generators[0]
sage: hash(mg)
-6948036233304877976 # 64-bit
-1420700568 # 32-bit
"""
Check equality.
return hash(self._b) ^ hash(self._m)

EXAMPLES::
def __cmp__(self, other):
"""
Comparison.
TESTS::
sage: A = crystals.KirillovReshetikhin(['A',2,1], 2, 2).affinization()
sage: mg = A.module_generators[0]
Expand All @@ -203,32 +216,14 @@ def __eq__(self, other):
sage: A = crystals.AffinizationOf(KT)
sage: A(KT.module_generators[3], 1).f(0) == A.module_generators[0]
True
"""
if not isinstance(other, AffinizationOfCrystal.Element):
return False
return self.parent() == other.parent() \
and self._b == other._b and self._m == other._m

def __ne__(self, other):
"""
Check inequality.
EXAMPLES::
sage: A = crystals.KirillovReshetikhin(['A',2,1], 2, 2).affinization()
sage: mg = A.module_generators[0]
sage: mg != mg.f(2)
True
sage: mg != mg.f(2).e(2)
False
"""
return not self.__eq__(other)
def __lt__(self, other):
"""
Check less than.
EXAMPLES::
sage: A = crystals.KirillovReshetikhin(['A',2,1], 2, 2).affinization()
sage: S = A.subcrystal(max_depth=2)
Expand All @@ -241,8 +236,12 @@ def __lt__(self, other):
[[1, 2], [2, 3]](1),
[[1, 2], [3, 3]](1),
[[2, 2], [3, 3]](2)]
"""
return self._m < other._m or (self._m == other._m and self._b < other._b)
if parent(self) is parent(other):
return cmp(self._m, other._m) or cmp(self._b, other._b)
else:
return cmp(parent(self), parent(other))

def e(self, i):
"""
Expand Down
24 changes: 23 additions & 1 deletion src/sage/combinat/crystals/elementary_crystals.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
from sage.combinat.root_system.cartan_type import CartanType
from sage.combinat.root_system.root_system import RootSystem
from sage.rings.integer import Integer
from sage.rings.integer_ring import ZZ

class AbstractSingleCrystalElement(Element):
r"""
Expand All @@ -102,6 +103,17 @@ def __lt__(self,other):
"""
return False

def __hash__(self):
r"""
TESTS::
sage: C = crystals.elementary.Component("D7")
sage: c = C.highest_weight_vector()
sage: hash(c) # random
879
"""
return hash(self.parent())

def __eq__(self,other):
r"""
EXAMPLES::
Expand Down Expand Up @@ -787,7 +799,7 @@ def _element_constructor_(self, m):
sage: B(721)
721
"""
return self.element_class(self, m)
return self.element_class(self, ZZ(m))

def weight_lattice_realization(self):
"""
Expand Down Expand Up @@ -817,6 +829,16 @@ def __init__(self, parent, m):
self._m = m
Element.__init__(self, parent)

def __hash__(self):
r"""
TESTS::
sage: B = crystals.elementary.Elementary(['B',7],7)
sage: hash(B(17))
17
"""
return hash(self._m)

def _repr_(self):
r"""
EXAMPLES::
Expand Down

0 comments on commit 01bc1e4

Please sign in to comment.