Skip to content

Commit

Permalink
trac sagemath#33255: merged with 9.8.beta6
Browse files Browse the repository at this point in the history
  • Loading branch information
dcoudert committed Jan 8, 2023
2 parents 2114066 + c379a13 commit 569ec3c
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 11 deletions.
33 changes: 32 additions & 1 deletion src/sage/graphs/bipartite_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ class BipartiteGraph(Graph):
- ``weighted`` -- boolean (default: ``None``); whether graph thinks of
itself as weighted or not. See ``self.weighted()``
- ``hash_labels`` - boolean (default: ``False``); whether to include labels
/ weights during hashing. Will raise a warning when __hash__ is invoked
and default to true.
.. NOTE::
All remaining arguments are passed to the ``Graph`` constructor
Expand Down Expand Up @@ -345,7 +349,7 @@ class BipartiteGraph(Graph):
"""

def __init__(self, data=None, partition=None, check=True, *args, **kwds):
def __init__(self, data=None, partition=None, check=True, hash_labels=None, *args, **kwds):
"""
Create a bipartite graph.
Expand Down Expand Up @@ -407,6 +411,8 @@ def __init__(self, data=None, partition=None, check=True, *args, **kwds):
self.add_edges = MethodType(Graph.add_edges, self)
alist_file = True

self.hash_labels=hash_labels

from sage.structure.element import is_Matrix
if isinstance(data, BipartiteGraph):
Graph.__init__(self, data, *args, **kwds)
Expand Down Expand Up @@ -544,6 +550,31 @@ def __init__(self, data=None, partition=None, check=True, *args, **kwds):

return

def __hash__(self):

"""
Compute a hash for ``self``, if ``self`` is immutable.
"""
if self.is_immutable():

# determine whether to hash labels
# warn user if not manually specified
use_labels=self._use_hash_labels()



edge_iter = self.edge_iterator(labels=use_labels)

if self.allows_multiple_edges():
from collections import Counter
edge_items = Counter(edge_iter).items()
else:
edge_items = edge_iter

return hash((frozenset(self.left), frozenset(self.right), frozenset(edge_items)))
else:
raise TypeError("This graph is mutable, and thus not hashable. Create an immutable copy by `g.copy(immutable=True)`")

def _upgrade_from_graph(self):
"""
Set the left and right sets of vertices from the input graph.
Expand Down
4 changes: 3 additions & 1 deletion src/sage/graphs/digraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
weighted=None, data_structure="sparse",
vertex_labels=True, name=None,
multiedges=None, convert_empty_dict_labels_to_None=None,
sparse=True, immutable=False):
sparse=True, immutable=False, hash_labels=None):
"""
TESTS::
Expand Down Expand Up @@ -855,6 +855,8 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
self._backend = ib
self._immutable = True

self.hash_labels=hash_labels

# Formats
def dig6_string(self):
r"""
Expand Down
106 changes: 98 additions & 8 deletions src/sage/graphs/generic_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,13 @@ def __eq__(self, other):

return self._backend.is_subgraph(other._backend, self, ignore_labels=not self.weighted())

# check if specified by the user, if not then fallback
def _use_labels_for_hash(self):
if not hasattr(self, "hash_labels") or self.hash_labels is None:
fallback=self.weighted()
self.hash_labels=fallback
return self.hash_labels

@cached_method
def __hash__(self):
"""
Expand Down Expand Up @@ -685,9 +692,13 @@ def __hash__(self):
sage: G1.__hash__() == G2.__hash__()
True

Make sure hash_labels parameter behaves as expected:


"""
if self.is_immutable():
edge_items = self.edge_iterator(labels=self._weighted)
use_labels=self._use_labels_for_hash()
edge_items = self.edge_iterator(labels=use_labels)
if self.allows_multiple_edges():
from collections import Counter
edge_items = Counter(edge_items).items()
Expand Down Expand Up @@ -955,7 +966,7 @@ def is_immutable(self):

# Formats

def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None):
def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None, hash_labels=None):
"""
Change the graph implementation

Expand Down Expand Up @@ -1141,7 +1152,86 @@ def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None):
sage: G._immutable = True
sage: G.copy()._backend
<sage.graphs.base.sparse_graph.SparseGraphBackend object at ...>
"""

Copying and changing hash_labels parameter::

sage: G1 = Graph({0: {1: 'edge label A'}}, immutable=True, hash_labels=False)
sage: G1c = G1.copy(hash_labels=True, immutable=True)
sage: hash(G1)==hash(G1c)
False

sage: G1 = Graph({0: {1: 'edge label A'}}, immutable=True, hash_labels=False)
sage: G2 = Graph({0: {1: 'edge label B'}}, immutable=True, hash_labels=False)
sage: hash(G1)==hash(G2)
True
sage: G1c = G1.copy(hash_labels=True)
sage: G2c = G2.copy(hash_labels=True)
sage: hash(G1c)==hash(G2c)
False

Making sure the .copy behaviour works correctly with hash_labels and immutable in all 54 cases::
sage: for old_immutable in [True, False]:
....: for new_immutable in [True, False, None]:
....: for old_hash_labels in [True, False, None]:
....: for new_hash_labels in [True, False, None]:
....:
....: # make a graph with old_immutable, old_hash_labels
....: G = Graph({0: {1: 'edge label A'}}, immutable=old_immutable, hash_labels=old_hash_labels)
....: old_immutable=G.is_immutable()
....: old_hash_labels=G.hash_labels
....:
....: # copy the graph, passing the overrides
....: G2 = G.copy(immutable=new_immutable, hash_labels=new_hash_labels)
....:
....: if new_immutable is None:
....: # make sure immutability is preserved if we don't update it
....: assert G2.is_immutable() == old_immutable, [old_immutable, new_immutable, old_hash_labels, new_hash_labels]
....: else:
....: # make sure that updating immutability works
....: assert G2.is_immutable() == new_immutable, [old_immutable, new_immutable, old_hash_labels, new_hash_labels]
....:
....: if new_hash_labels is None:
....: # make sure hash_labels is preserved if we don't update it
....: assert G2.hash_labels == old_hash_labels, [old_immutable, new_immutable, old_hash_labels, new_hash_labels]
....: else:
....: # make sure updating hash labels works
....: assert G2.hash_labels == new_hash_labels, [old_immutable, new_immutable, old_hash_labels, new_hash_labels]

"""

# This is an ugly hack but it works.
# This function contains some fairly complex logic
# There is a comment further down that says

### Immutable copy of an immutable graph ? return self !

# The issue being that if we want to change the hash_labels behaviour, then
# returning self is no longer a good option. I'd argue that a copy function
# returning self is always bad behaviour, but that's out of the scope for this ticket.
# Trying to weaken the if statement to include something like

### and (hash_labels is None or (hash_labels==self._use_labels_for_hash()))

# doesn't work, since there is no fallback logic for making
# an immutable copy of an immutable graph, and my attempts at
# implementing one caused other tests to break in different
# bits of the code

# the hack I've used creates a mutable copy of the graph
# and then makes an immutable copy of that one. I think this is a fairly
# inobtrusive implementation, since the function still runs as normally,
# assuming that they pass nothing into hash_labels, and seems to behave
# correctly otherwise.

# However, this is obviously not optimal, and could definitely be improved upon
# by someone who understands the logic better.
if hash_labels is not None:
desired_immutable = self.is_immutable() if immutable is None else immutable
forced_mutable_copy = self.copy(weighted=weighted, data_structure=data_structure, sparse=sparse, immutable=False)
fresh_copy = forced_mutable_copy.copy(weighted=weighted, data_structure=data_structure, sparse=sparse, immutable=desired_immutable)
fresh_copy.hash_labels=hash_labels
return fresh_copy

# Which data structure should be used ?
if data_structure is not None:
# data_structure is already defined so there is nothing left to do
Expand Down Expand Up @@ -1174,19 +1264,19 @@ def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None):
if (isinstance(self._backend, StaticSparseBackend) 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
if isinstance(self._backend, DenseGraphBackend):
data_structure = "dense"
else:
data_structure = "sparse"

G = self.__class__(self, name=self.name(), pos=copy(self._pos),
weighted=weighted,
data_structure=data_structure)
weighted=weighted,
data_structure=data_structure)

attributes_to_copy = ('_assoc', '_embedding')
attributes_to_copy = ('_assoc', '_embedding', 'hash_labels')
for attr in attributes_to_copy:
if hasattr(self, attr):
copy_attr = {}
Expand Down
4 changes: 3 additions & 1 deletion src/sage/graphs/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
weighted=None, data_structure="sparse",
vertex_labels=True, name=None,
multiedges=None, convert_empty_dict_labels_to_None=None,
sparse=True, immutable=False):
sparse=True, immutable=False, hash_labels=None):
"""
TESTS::
Expand Down Expand Up @@ -1266,6 +1266,8 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
self._backend = ib
self._immutable = True

self.hash_labels = hash_labels

# Formats

@doc_index("Basic methods")
Expand Down

0 comments on commit 569ec3c

Please sign in to comment.