Trac #15278: Hash and equality for graphs

At #14806, an immutable graph backend has been introduced. However, the
resulting graphs are not known to be immutable.
{{{
sage: g = graphs.CompleteGraph(400)
sage: gi = Graph(g,data_structure="static_sparse")
sage: hash(gi)
Traceback (most recent call last):
...
TypeError: graphs are mutable, and thus not hashable
}}}
So, when the immutable graph backend is used, then the `._immutable`
attribute should be set to True.

But hang on: Does using the immutable graph backend really result in
immutable graphs? I.e., would it really be impossible to change the
`==`- and `cmp`-classes of a graph by "official" methods such as
"`add_vertex()`"? And is the current equality testing really what we
want to test?

Currently, `__eq__()` starts like this:
{{{
        # inputs must be (di)graphs:
        if not isinstance(other, GenericGraph):
            raise TypeError("cannot compare graph to non-graph
(%s)"%str(other))
        from sage.graphs.all import Graph
        g1_is_graph = isinstance(self, Graph) # otherwise, DiGraph
        g2_is_graph = isinstance(other, Graph) # otherwise, DiGraph

        if (g1_is_graph != g2_is_graph):
            return False
        if self.allows_multiple_edges() !=
other.allows_multiple_edges():
            return False
        if self.allows_loops() != other.allows_loops():
            return False
        if self.order() != other.order():
            return False
        if self.size() != other.size():
            return False
        if any(x not in other for x in self):
            return False
        if self._weighted != other._weighted:
            return False
}}}

1. Is it really a good idea to raise an error when testing equality of a
graph with a non-graph? Most likely not!
2. For sure, a graph that ''has'' multiple edges can not be equal to a
graph that does not have multiple edges. But should it really matter
whether a graph ''allows'' multiple edges when it actually doesn't
''have'' multiple edges?
3. Similarly for `.allows_loops()`.
4. Should `._weighted` be totally removed? Note the following comment in
the documentation of the `.weighted()` method: "edge weightings can
still exist for (di)graphs `G` where `G.weighted()` is `False`". Ouch.

Note the following annoying example from the docs of `.weighted()`:
{{{
            sage: G = Graph({0:{1:'a'}}, sparse=True)
            sage: H = Graph({0:{1:'b'}}, sparse=True)
            sage: G == H
            True

        Now one is weighted and the other is not, and thus the graphs
are
        not equal::

            sage: G.weighted(True)
            sage: H.weighted()
            False
            sage: G == H
            False
}}}
So, calling the method `weighted` with an argument changes the
`==`-class, even though I guess most mathematicians would agree that `G`
has not changed at all. And this would actually be the case even if `G`
was using the immutable graph backend!!!

The aim of this ticket is to sort these things out. To the very least,
graphs using the immutable graph backend should not be allowed to change
their `==`-class by calling `G.weighted(True)`.

URL: http://trac.sagemath.org/15278
Reported by: SimonKing
Ticket author(s): Simon King
Reviewer(s): Nathann Cohen

src/sage/graphs/digraph.py

@@ -253,7 +253,8 @@ class DiGraph(GenericGraph):
 
        * ``"static_sparse"`` -- selects the
          :mod:`~sage.graphs.base.static_sparse_backend` (this backend is faster
-         than the sparse backend and smaller in memory, but it is immutable).
+         than the sparse backend and smaller in memory, and it is immutable, so
+         that the resulting graphs can be used as dictionary keys).
 
        *Only available when* ``implementation == 'c_graph'``
 
@@ -411,6 +412,26 @@ class DiGraph(GenericGraph):
         sage: D._boundary
         []
 
+    Demonstrate that digraphs using the static backend are equal to mutable
+    graphs but can be used as dictionary keys::
+
+        sage: import networkx
+        sage: g = networkx.DiGraph({0:[1,2,3], 2:[4]})
+        sage: G = DiGraph(g, implementation='networkx')
+        sage: G_imm = DiGraph(G, data_structure="static_sparse")
+        sage: H_imm = DiGraph(G, data_structure="static_sparse")
+        sage: H_imm is G_imm
+        False
+        sage: H_imm == G_imm == G
+        True
+        sage: {G_imm:1}[H_imm]
+        1
+        sage: {G_imm:1}[G]
+        Traceback (most recent call last):
+        ...
+        TypeError: This graph is mutable, and thus not hashable. Create an
+        immutable copy by `g.copy(data_structure='static_sparse')`
+
     """
     _directed = True
 
@@ -917,6 +938,7 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
             from sage.graphs.base.static_sparse_backend import StaticSparseBackend
             ib = StaticSparseBackend(self, loops = loops, multiedges = multiedges)
             self._backend = ib
+            self._immutable = True
 
     ### Formats
     def dig6_string(self):

src/sage/graphs/generic_graph.py

@@ -302,6 +302,7 @@
 """
 
 from sage.misc.decorators import options
+from sage.misc.cachefunc import cached_method
 from sage.misc.prandom import random
 from sage.rings.integer_ring import ZZ
 from sage.rings.integer import Integer
@@ -463,21 +464,36 @@ def __eq__(self, other):
                         return False
             return True
 
+    @cached_method
     def __hash__(self):
         """
-        Since graphs are mutable, they should not be hashable, so we return
-        a type error.
+        Only immutable graphs are hashable.
 
-        EXAMPLES::
+        The hash value of an immutable graph relies on the tuple
+        of vertices and the tuple of edges. The resulting value
+        is cached.
+
+        EXAMPLE::
 
-            sage: hash(Graph())
+            sage: G = graphs.PetersenGraph()
+            sage: {G:1}[G]
             Traceback (most recent call last):
             ...
-            TypeError: graphs are mutable, and thus not hashable
+            TypeError: This graph is mutable, and thus not hashable. Create
+            an immutable copy by `g.copy(data_structure='static_sparse')`
+            sage: G_imm = Graph(G, data_structure="static_sparse")
+            sage: G_imm == G
+            True
+            sage: {G_imm:1}[G_imm]  # indirect doctest
+            1
+            sage: G_imm.__hash__() is G_imm.__hash__()
+            True
+
         """
         if getattr(self, "_immutable", False):
             return hash((tuple(self.vertices()), tuple(self.edges())))
-        raise TypeError("graphs are mutable, and thus not hashable")
+        raise TypeError("This graph is mutable, and thus not hashable. "
+                        "Create an immutable copy by `g.copy(data_structure='static_sparse')`")
 
     def __mul__(self, n):
         """
@@ -700,6 +716,12 @@ def __copy__(self, implementation='c_graph', data_structure=None,
         """
         Creates a copy of the graph.
 
+        NOTE:
+
+        If the graph uses :class:`~sage.graphs.base.static_sparse_backend.StaticSparseBackend`
+        and uses the _immutable flag, then ``self`` is returned, rather
+        than a copy, unless one of the optional arguments is used.
+
         INPUT:
 
          - ``implementation`` - string (default: 'networkx') the
@@ -763,8 +785,9 @@ def __copy__(self, implementation='c_graph', data_structure=None,
             sage: G2 is G
             False
 
-        TESTS: We make copies of the _pos and _boundary attributes.
+        TESTS:
 
+        We make copies of the _pos and _boundary attributes.
         ::
 
             sage: g = graphs.PathGraph(3)
@@ -773,13 +796,51 @@ def __copy__(self, implementation='c_graph', data_structure=None,
             False
             sage: h._boundary is g._boundary
             False
+
+        We make sure that one can make immutable copies by providing the
+        ``data_structure`` optional argument, and that copying an immutable graph
+        returns the graph::
+
+            sage: G = graphs.PetersenGraph()
+            sage: hash(G)
+            Traceback (most recent call last):
+            ...
+            TypeError: This graph is mutable, and thus not hashable. Create an
+            immutable copy by `g.copy(data_structure='static_sparse')`
+            sage: g = G.copy(data_structure='static_sparse')
+            sage: hash(g)    # random
+            1833517720
+            sage: g==G
+            True
+            sage: g is copy(g) is g.copy()
+            True
+            sage: g is g.copy(data_structure='static_sparse')
+            True
+
+        If a graph pretends to be immutable, but does not use the static sparse
+        backend, then the copy is not identic with the graph, even though it is
+        considered to be hashable::
+
+            sage: P = Poset(([1,2,3,4], [[1,3],[1,4],[2,3]]), linear_extension=True, facade = False)
+            sage: H = P.hasse_diagram()
+            sage: H._immutable = True
+            sage: hash(H)   # random
+            -1843552882
+            sage: copy(H) is H
+            False
+
         """
         if sparse != None:
             if data_structure != None:
                 raise ValueError("The 'sparse' argument is an alias for "
                                  "'data_structure'. Please do not define both.")
             data_structure = "sparse" if sparse else "dense"
 
+        if getattr(self, '_immutable', False):
+            from sage.graphs.base.static_sparse_backend import StaticSparseBackend
+            if isinstance(self._backend, StaticSparseBackend) and implementation=='c_graph' and (data_structure=='static_sparse' or data_structure is None):
+                return self
+
         if data_structure is None:
             from sage.graphs.base.dense_graph import DenseGraphBackend
             from sage.graphs.base.sparse_graph import SparseGraphBackend
@@ -965,7 +1026,7 @@ def to_dictionary(self, edge_labels=False, multiple_edges=False):
           neighbors.
 
         * If ``edge_labels == False`` and ``multiple_edges == True``, the output
-          is a dictionary the same as previously with one difference : the
+          is a dictionary the same as previously with one difference: the
           neighbors are listed with multiplicity.
 
         * If ``edge_labels == True`` and ``multiple_edges == False``, the output
@@ -2269,8 +2330,18 @@ def weighted(self, new=None):
         """
         Whether the (di)graph is to be considered as a weighted (di)graph.
 
-        Note that edge weightings can still exist for (di)graphs ``G`` where
-        ``G.weighted()`` is ``False``.
+        INPUT:
+
+        - ``new`` (optional bool): If it is provided, then the weightedness
+          flag is set accordingly. This is not allowed for immutable graphs.
+
+        .. NOTE::
+
+            Changing the weightedness flag changes the ``==``-class of
+            a graph and is thus not allowed for immutable graphs.
+
+            Edge weightings can still exist for (di)graphs ``G`` where
+            ``G.weighted()`` is ``False``.
 
         EXAMPLES:
 
@@ -2299,7 +2370,7 @@ def weighted(self, new=None):
 
         TESTS:
 
-        Ensure that ticket #10490 is fixed: allows a weighted graph to be
+        Ensure that :trac:`10490` is fixed: allows a weighted graph to be
         set as unweighted. ::
 
             sage: G = Graph({1:{2:3}})
@@ -2321,8 +2392,35 @@ def weighted(self, new=None):
             sage: G.weighted(True)
             sage: G.weighted()
             True
+
+        Ensure that graphs using the static sparse backend can not be mutated
+        using this method, as fixed in :trac:`15278`::
+
+            sage: G = graphs.PetersenGraph()
+            sage: G.weighted()
+            False
+            sage: H = copy(G)
+            sage: H == G
+            True
+            sage: H.weighted(True)
+            sage: H == G
+            False
+            sage: G_imm = Graph(G, data_structure="static_sparse")
+            sage: G_imm == G
+            True
+            sage: G_imm.weighted()
+            False
+            sage: G_imm.weighted(True)
+            Traceback (most recent call last):
+            ...
+            TypeError: This graph is immutable and can thus not be changed.
+            Create a mutable copy, e.g., by `g.copy(sparse=False)`
+
         """
         if new is not None:
+            if getattr(self, '_immutable', False):
+                raise TypeError("This graph is immutable and can thus not be changed. "
+                                "Create a mutable copy, e.g., by `g.copy(sparse=False)`")
             if new in [True, False]:
                 self._weighted = new
         else:
@@ -6291,7 +6389,7 @@ def hamiltonian_cycle(self, algorithm='tsp' ):
         NOTE:
 
         This function, as ``is_hamiltonian``, computes a Hamiltonian
-        cycle if it exists : the user should *NOT* test for
+        cycle if it exists: the user should *NOT* test for
         Hamiltonicity using ``is_hamiltonian`` before calling this
         function, as it would result in computing it twice.
 
@@ -6527,7 +6625,7 @@ def feedback_vertex_set(self, value_only=False, solver=None, verbose=0, constrai
                     break
 
                 if verbose:
-                    print "Adding a constraint on circuit : ",certificate
+                    print "Adding a constraint on circuit: ",certificate
 
                 # There is a circuit left. Let's add the corresponding
                 # constraint !
@@ -7311,7 +7409,7 @@ def edge_disjoint_paths(self, s, t, method = "FF"):
 
         .. NOTE::
 
-            This function is topological : it does not take the eventual
+            This function is topological: it does not take the eventual
             weights of the edges into account.
 
         EXAMPLE:
@@ -8022,14 +8120,13 @@ def delete_vertex(self, vertex, in_order=False):
         if vertex not in self:
             raise RuntimeError("Vertex (%s) not in the graph."%str(vertex))
 
+        self._backend.del_vertex(vertex)
         attributes_to_update = ('_pos', '_assoc', '_embedding')
         for attr in attributes_to_update:
             if hasattr(self, attr) and getattr(self, attr) is not None:
                 getattr(self, attr).pop(vertex, None)
         self._boundary = [v for v in self._boundary if v != vertex]
 
-        self._backend.del_vertex(vertex)
-
     def delete_vertices(self, vertices):
         """
         Remove vertices from the (di)graph taken from an iterable container
@@ -8052,6 +8149,8 @@ def delete_vertices(self, vertices):
         for vertex in vertices:
             if vertex not in self:
                 raise RuntimeError("Vertex (%s) not in the graph."%str(vertex))
+
+        self._backend.del_vertices(vertices)
         attributes_to_update = ('_pos', '_assoc', '_embedding')
         for attr in attributes_to_update:
             if hasattr(self, attr) and getattr(self, attr) is not None:
@@ -8061,8 +8160,6 @@ def delete_vertices(self, vertices):
 
         self._boundary = [v for v in self._boundary if v not in vertices]
 
-        self._backend.del_vertices(vertices)
-
     def has_vertex(self, vertex):
         """
         Return True if vertex is one of the vertices of this graph.
@@ -17004,7 +17101,7 @@ def is_hamiltonian(self):
 
         This function, as ``hamiltonian_cycle`` and
         ``traveling_salesman_problem``, computes a Hamiltonian
-        cycle if it exists : the user should *NOT* test for
+        cycle if it exists: the user should *NOT* test for
         Hamiltonicity using ``is_hamiltonian`` before calling
         ``hamiltonian_cycle`` or ``traveling_salesman_problem``
         as it would result in computing it twice.

src/sage/graphs/graph.py

@@ -493,6 +493,24 @@
     ...
     \end{tikzpicture}
 
+Mutability
+----------
+
+Graphs are mutable, and thus unusable as dictionary keys, unless
+``data_structure="static_sparse"`` is used::
+
+    sage: G = graphs.PetersenGraph()
+    sage: {G:1}[G]
+    Traceback (most recent call last):
+    ...
+    TypeError: This graph is mutable, and thus not hashable. Create an
+    immutable copy by `g.copy(data_structure='static_sparse')`
+    sage: G_immutable = Graph(G, data_structure="static_sparse")
+    sage: G_immutable == G
+    True
+    sage: {G_immutable:1}[G_immutable]
+    1
+
 Methods
 -------
 """
@@ -677,7 +695,8 @@ class Graph(GenericGraph):
 
        * ``"static_sparse"`` -- selects the
          :mod:`~sage.graphs.base.static_sparse_backend` (this backend is faster
-         than the sparse backend and smaller in memory, but it is immutable).
+         than the sparse backend and smaller in memory, and it is immutable, so
+         that the resulting graphs can be used as dictionary keys).
 
        *Only available when* ``implementation == 'c_graph'``
 
@@ -935,6 +954,29 @@ class Graph(GenericGraph):
           sage: H = Graph(g, implementation='networkx')
           sage: G._backend._nxg is H._backend._nxg
           False
+
+    All these graphs are mutable and can thus not be used as a dictionary
+    key::
+
+          sage: {G:1}[H]
+          Traceback (most recent call last):
+          ...
+          TypeError: This graph is mutable, and thus not hashable. Create
+          an immutable copy by `g.copy(data_structure='static_sparse')`
+
+    If the ``data_structure`` is equal to ``"static_sparse"``, then an
+    immutable graph results. Note that this does not use the NetworkX data
+    structure::
+
+          sage: G_imm = Graph(g, data_structure="static_sparse")
+          sage: H_imm = Graph(g, data_structure="static_sparse")
+          sage: G_imm == H_imm == G == H
+          True
+          sage: hasattr(G_imm._backend, "_nxg")
+          False
+          sage: {G_imm:1}[H_imm]
+          1
+
     """
     _directed = False
 
@@ -1587,6 +1629,7 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
             from sage.graphs.base.static_sparse_backend import StaticSparseBackend
             ib = StaticSparseBackend(self, loops = loops, multiedges = multiedges)
             self._backend = ib
+            self._immutable = True
 
     ### Formats
     def graph6_string(self):