From 2fdb5e5b0a7677ba4d7efda337fa9a952e739030 Mon Sep 17 00:00:00 2001 From: dcoudert Date: Sun, 10 Sep 2023 10:42:47 +0200 Subject: [PATCH 1/4] make min_spanning_tree robust to incomparable vertices --- src/sage/graphs/base/boost_graph.pyx | 15 +++- src/sage/graphs/generic_graph.py | 116 ++++++++++++++++----------- src/sage/graphs/spanning_tree.pyx | 66 +++++++++++++++ 3 files changed, 149 insertions(+), 48 deletions(-) diff --git a/src/sage/graphs/base/boost_graph.pyx b/src/sage/graphs/base/boost_graph.pyx index ff222837117..573ff440e48 100644 --- a/src/sage/graphs/base/boost_graph.pyx +++ b/src/sage/graphs/base/boost_graph.pyx @@ -686,6 +686,16 @@ cpdef min_spanning_tree(g, Traceback (most recent call last): ... TypeError: float() argument must be a string or a... number... + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)], weighted=True) + sage: E = min_spanning_tree(G, algorithm='Kruskal') + sage: sum(w for _, _, w in E) + 3 + sage: F = min_spanning_tree(G, algorithm='Prim') + sage: sum(w for _, _, w in F) + 3 """ from sage.graphs.graph import Graph @@ -719,9 +729,8 @@ cpdef min_spanning_tree(g, if result.size() != 2 * (n - 1): return [] - else: - edges = [(int_to_vertex[ result[2*i]], int_to_vertex[ result[2*i + 1]]) for i in range(n - 1)] - return [(min(e[0], e[1]), max(e[0], e[1]), g.edge_label(e[0], e[1])) for e in edges] + edges = [(int_to_vertex[ result[2*i]], int_to_vertex[ result[2*i + 1]]) for i in range(n - 1)] + return [(u, v, g.edge_label(u, v)) for u, v in edges] cpdef blocks_and_cut_vertices(g): diff --git a/src/sage/graphs/generic_graph.py b/src/sage/graphs/generic_graph.py index 2deb533f7f1..347e7dcb767 100644 --- a/src/sage/graphs/generic_graph.py +++ b/src/sage/graphs/generic_graph.py @@ -4704,17 +4704,23 @@ def min_spanning_tree(self, sage: len(g.min_spanning_tree()) 4 sage: weight = lambda e: 1 / ((e[0] + 1) * (e[1] + 1)) - sage: sorted(g.min_spanning_tree(weight_function=weight)) - [(0, 4, None), (1, 4, None), (2, 4, None), (3, 4, None)] - sage: sorted(g.min_spanning_tree(weight_function=weight, - ....: algorithm='Kruskal_Boost')) - [(0, 4, None), (1, 4, None), (2, 4, None), (3, 4, None)] + sage: E = g.min_spanning_tree(weight_function=weight) + sage: T = Graph(E) + sage: set(g) == set(T) and T.order() == T.size() + 1 and T.is_tree() + True + sage: sum(map(weight, E)) + 5/12 + sage: E = g.min_spanning_tree(weight_function=weight, + ....: algorithm='Kruskal_Boost') + sage: Graph(E).is_tree(); sum(map(weight, E)) + True + 5/12 sage: g = graphs.PetersenGraph() sage: g.allow_multiple_edges(True) sage: g.add_edges(g.edge_iterator()) - sage: sorted(g.min_spanning_tree()) - [(0, 1, None), (0, 4, None), (0, 5, None), (1, 2, None), (1, 6, None), - (3, 8, None), (5, 7, None), (5, 8, None), (6, 9, None)] + sage: T = Graph(g.min_spanning_tree()) + sage: set(g) == set(T) and T.order() == T.size() + 1 and T.is_tree() + True Boruvka's algorithm:: @@ -4725,15 +4731,13 @@ def min_spanning_tree(self, Prim's algorithm:: sage: g = graphs.CompleteGraph(5) - sage: sorted(g.min_spanning_tree(algorithm='Prim_edge', - ....: starting_vertex=2, weight_function=weight)) - [(0, 4, None), (1, 4, None), (2, 4, None), (3, 4, None)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_fringe', - ....: starting_vertex=2, weight_function=weight)) - [(0, 4, None), (1, 4, None), (2, 4, None), (3, 4, None)] - sage: sorted(g.min_spanning_tree(weight_function=weight, - ....: algorithm='Prim_Boost')) - [(0, 4, None), (1, 4, None), (2, 4, None), (3, 4, None)] + sage: for algo in ['Prim_edge', 'Prim_fringe', 'Prim_Boost']: + ....: E = g.min_spanning_tree(algorithm=algo, weight_function=weight) + ....: T = Graph(E) + ....: print(set(g) == set(T) and T.order() == T.size() + 1 and T.is_tree()) + True + True + True NetworkX algorithm:: @@ -4745,30 +4749,43 @@ def min_spanning_tree(self, sage: G = Graph([(0, 1, {'name': 'a', 'weight': 1}), ....: (0, 2, {'name': 'b', 'weight': 3}), ....: (1, 2, {'name': 'b', 'weight': 1})]) - sage: sorted(G.min_spanning_tree(weight_function=lambda e: e[2]['weight'])) + sage: sorted(G.min_spanning_tree(algorithm='Boruvka', + ....: weight_function=lambda e: e[2]['weight'])) [(0, 1, {'name': 'a', 'weight': 1}), (1, 2, {'name': 'b', 'weight': 1})] If the graph is not weighted, edge labels are not considered, even if they are numbers:: sage: g = Graph([(1, 2, 1), (1, 3, 2), (2, 3, 1)]) - sage: sorted(g.min_spanning_tree()) + sage: sorted(g.min_spanning_tree(algorithm='Boruvka')) [(1, 2, 1), (1, 3, 2)] In order to use weights, we need either to set variable ``weighted`` to ``True``, or to specify a weight function or set by_weight to ``True``:: sage: g.weighted(True) - sage: sorted(g.min_spanning_tree()) + sage: Graph(g.min_spanning_tree()).edges(sort=True) [(1, 2, 1), (2, 3, 1)] sage: g.weighted(False) - sage: sorted(g.min_spanning_tree()) + sage: Graph(g.min_spanning_tree()).edges(sort=True) [(1, 2, 1), (1, 3, 2)] - sage: sorted(g.min_spanning_tree(by_weight=True)) + sage: Graph(g.min_spanning_tree(by_weight=True)).edges(sort=True) + [(1, 2, 1), (2, 3, 1)] + sage: Graph(g.min_spanning_tree(weight_function=lambda e: e[2])).edges(sort=True) [(1, 2, 1), (2, 3, 1)] - sage: sorted(g.min_spanning_tree(weight_function=lambda e: e[2])) + + Note that the order of the vertices on each edge is not guaranteed and + may differ from an algorithm to the other:: + + sage: g.weighted(True) + sage: sorted(g.min_spanning_tree()) + [(2, 1, 1), (3, 2, 1)] + sage: sorted(g.min_spanning_tree(algorithm='Boruvka')) + [(1, 2, 1), (2, 3, 1)] + sage: Graph(g.min_spanning_tree()).edges(sort=True) [(1, 2, 1), (2, 3, 1)] + TESTS: Check that, if ``weight_function`` is not provided, then edge weights @@ -4776,21 +4793,21 @@ def min_spanning_tree(self, sage: g = Graph(weighted=True) sage: g.add_edges([[0, 1, 1], [1, 2, 1], [2, 0, 10]]) - sage: sorted(g.min_spanning_tree()) + sage: Graph(g.min_spanning_tree()).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Filter_Kruskal')) + sage: Graph(g.min_spanning_tree(algorithm='Filter_Kruskal')).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Kruskal_Boost')) + sage: Graph(g.min_spanning_tree(algorithm='Kruskal_Boost')).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_fringe')) + sage: Graph(g.min_spanning_tree(algorithm='Prim_fringe')).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_edge')) + sage: Graph(g.min_spanning_tree(algorithm='Prim_edge')).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_Boost')) + sage: Graph(g.min_spanning_tree(algorithm='Prim_Boost')).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='NetworkX')) # needs networkx + sage: Graph(g.min_spanning_tree(algorithm='Boruvka')).edges(sort=True) [(0, 1, 1), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Boruvka')) + sage: Graph(g.min_spanning_tree(algorithm='NetworkX')).edges(sort=True) # needs networkx [(0, 1, 1), (1, 2, 1)] Check that, if ``weight_function`` is provided, it overrides edge @@ -4798,29 +4815,27 @@ def min_spanning_tree(self, sage: g = Graph([[0, 1, 1], [1, 2, 1], [2, 0, 10]], weighted=True) sage: weight = lambda e: 3 - e[0] - e[1] - sage: sorted(g.min_spanning_tree(weight_function=weight)) + sage: Graph(g.min_spanning_tree(weight_function=weight)).edges(sort=True) [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Filter_Kruskal', weight_function=weight)) + sage: Graph(g.min_spanning_tree(algorithm='Filter_Kruskal', weight_function=weight)).edges(sort=True) [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Kruskal_Boost', weight_function=weight)) + sage: Graph(g.min_spanning_tree(algorithm='Kruskal_Boost', weight_function=weight)).edges(sort=True) [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_fringe', weight_function=weight)) + sage: Graph(g.min_spanning_tree(algorithm='Prim_fringe', weight_function=weight)).edges(sort=True) [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_edge', weight_function=weight)) + sage: Graph(g.min_spanning_tree(algorithm='Prim_edge', weight_function=weight)).edges(sort=True) [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Prim_Boost', weight_function=weight)) + sage: Graph(g.min_spanning_tree(algorithm='Prim_Boost', weight_function=weight)).edges(sort=True) [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='NetworkX', weight_function=weight)) # needs networkx - [(0, 2, 10), (1, 2, 1)] - sage: sorted(g.min_spanning_tree(algorithm='Boruvka', weight_function=weight)) + sage: Graph(g.min_spanning_tree(algorithm='NetworkX', weight_function=weight)).edges(sort=True) # needs networkx [(0, 2, 10), (1, 2, 1)] If the graph is directed, it is transformed into an undirected graph:: sage: g = digraphs.Circuit(3) - sage: sorted(g.min_spanning_tree(weight_function=weight)) + sage: Graph(g.min_spanning_tree(weight_function=weight)).edges(sort=True) [(0, 2, None), (1, 2, None)] - sage: sorted(g.to_undirected().min_spanning_tree(weight_function=weight)) + sage: Graph(g.to_undirected().min_spanning_tree(weight_function=weight)).edges(sort=True) [(0, 2, None), (1, 2, None)] If at least an edge weight is not convertible to a float, an error is @@ -4841,6 +4856,17 @@ def min_spanning_tree(self, sage: graphs.EmptyGraph().min_spanning_tree() [] + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: E = G.min_spanning_tree(algorithm='Prim_Boost', by_weight=True) + sage: E = G.min_spanning_tree(algorithm='Prim_fringe', by_weight=True) + sage: E = G.min_spanning_tree(algorithm='Prim_edge', by_weight=True) + sage: E = G.min_spanning_tree(algorithm='Kruskal_Boost', by_weight=True) + sage: E = G.min_spanning_tree(algorithm='Filter_Kruskal', by_weight=True) + sage: E = G.min_spanning_tree(algorithm='Boruvka', by_weight=True) + sage: E = G.min_spanning_tree(algorithm='NetworkX', by_weight=True) # needs networkx """ if not self.order(): return [] @@ -5136,9 +5162,9 @@ def cycle_basis(self, output='vertex'): sage: [sorted(c) for c in G.cycle_basis()] # needs networkx [['Hey', 'Really ?', 'Wuuhuu'], [0, 2], [0, 1, 2]] sage: [sorted(c) for c in G.cycle_basis(output='edge')] # needs networkx - [[('Hey', 'Wuuhuu', None), - ('Really ?', 'Hey', None), - ('Wuuhuu', 'Really ?', None)], + [[('Hey', 'Really ?', None), + ('Really ?', 'Wuuhuu', None), + ('Wuuhuu', 'Hey', None)], [(0, 2, 'a'), (2, 0, 'b')], [(0, 2, 'b'), (1, 0, 'c'), (2, 1, 'd')]] diff --git a/src/sage/graphs/spanning_tree.pyx b/src/sage/graphs/spanning_tree.pyx index 08f151a65ad..7576a341701 100644 --- a/src/sage/graphs/spanning_tree.pyx +++ b/src/sage/graphs/spanning_tree.pyx @@ -241,6 +241,13 @@ def kruskal(G, by_weight=True, weight_function=None, check_weight=False, check=F Traceback (most recent call last): ... ValueError: the input graph must be undirected + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: E = kruskal(G, by_weight=True) + sage: sum(w for _, _, w in E) + 3 """ return list(kruskal_iterator(G, by_weight=by_weight, weight_function=weight_function, check_weight=check_weight, check=check)) @@ -313,6 +320,13 @@ def kruskal_iterator(G, by_weight=True, weight_function=None, check_weight=False Traceback (most recent call last): ... ValueError: the input graph must be undirected + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: E = list(kruskal_iterator(G, by_weight=True)) + sage: sum(w for _, _, w in E) + 3 """ from sage.graphs.graph import Graph if not isinstance(G, Graph): @@ -382,6 +396,14 @@ def kruskal_iterator_from_edges(edges, union_find, by_weight=True, sage: union_set = DisjointSet(G) sage: next(kruskal_iterator_from_edges(G.edges(sort=False), union_set, by_weight=G.weighted())) (1, 6, 10) + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: union_set = DisjointSet(G) + sage: E = list(kruskal_iterator_from_edges(G.edges(sort=False), union_set, by_weight=True)) + sage: sum(w for _, _, w in E) + 3 """ # We sort edges, as specified. if weight_function is not None: @@ -472,6 +494,15 @@ def filter_kruskal(G, threshold=10000, by_weight=True, weight_function=None, sage: filter_kruskal(Graph(2), check=True) [] + + TESTS: + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: E = filter_kruskal(G, by_weight=True) + sage: sum(w for _, _, w in E) + 3 """ return list(filter_kruskal_iterator(G, threshold=threshold, by_weight=by_weight, weight_function=weight_function, @@ -563,6 +594,13 @@ def filter_kruskal_iterator(G, threshold=10000, by_weight=True, weight_function= sage: len(list(filter_kruskal_iterator(graphs.HouseGraph(), threshold=1))) 4 + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: E = list(filter_kruskal_iterator(G, by_weight=True)) + sage: sum(w for _, _, w in E) + 3 """ from sage.graphs.graph import Graph if not isinstance(G, Graph): @@ -776,6 +814,13 @@ def boruvka(G, by_weight=True, weight_function=None, check_weight=True, check=Fa Traceback (most recent call last): ... ValueError: the input graph must be undirected + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: E = boruvka(G, by_weight=True) + sage: sum(w for _, _, w in E) + 3 """ from sage.graphs.graph import Graph if not isinstance(G, Graph): @@ -985,6 +1030,13 @@ def random_spanning_tree(G, output_as_graph=False, by_weight=False, weight_funct Traceback (most recent call last): ... ValueError: works only for non-empty connected graphs + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: T = G.random_spanning_tree(by_weight=True, output_as_graph=True) + sage: T.is_tree() + True """ from sage.misc.prandom import randint from sage.misc.prandom import random @@ -1113,6 +1165,12 @@ def spanning_trees(g, labels=False): Traceback (most recent call last): ... ValueError: this method is for undirected graphs only + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph([(1, 2, 10), (1, 'a', 1), ('a', 'b', 1), ('b', 2, 1)]) + sage: len(list(G.spanning_trees(labels=False))) + 4 """ from sage.graphs.graph import Graph if not isinstance(g, Graph): @@ -1257,6 +1315,14 @@ def edge_disjoint_spanning_trees(G, k, by_weight=False, weight_function=None, ch Traceback (most recent call last): ... ValueError: this method is for undirected graphs only + + Check that the method is robust to incomparable vertices:: + + sage: G = Graph() + sage: G.add_clique([0, 1, 2, 'a', 'b']) + sage: F = G.edge_disjoint_spanning_trees(k=2) + sage: len(F) + 2 """ if G.is_directed(): raise ValueError("this method is for undirected graphs only") From 36ac7e4751f9b0f89c86d50f9ca2d9cf738590be Mon Sep 17 00:00:00 2001 From: dcoudert Date: Sun, 10 Sep 2023 10:44:03 +0200 Subject: [PATCH 2/4] fix method lift_cross_ratios in sage/matroids/utilities.py --- src/sage/matroids/utilities.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/sage/matroids/utilities.py b/src/sage/matroids/utilities.py index a8612dab29f..211c435c5ee 100644 --- a/src/sage/matroids/utilities.py +++ b/src/sage/matroids/utilities.py @@ -572,26 +572,28 @@ def lift_cross_ratios(A, lift_map=None): G = Graph([((r, 0), (c, 1), (r, c)) for r, c in A.nonzero_positions()]) # write the entries of (a scaled version of) A as products of cross ratios of A - T = set() + T = Graph() for C in G.connected_components_subgraphs(): - T.update(C.min_spanning_tree()) + T.add_edges(C.min_spanning_tree()) # - fix a tree of the support graph G to units (= empty dict, product of 0 terms) - F = {entry[2]: dict() for entry in T} - W = set(G.edge_iterator()) - set(T) - H = G.subgraph(edges=T) + F = {entry: dict() for entry in T.edge_labels()} + W = set(G.edge_iterator()) - set(T.edge_iterator()) + H = G.subgraph(edges=T.edge_iterator()) while W: # - find an edge in W to process, closing a circuit in H which is induced in G edge = W.pop() path = H.shortest_path(edge[0], edge[1]) + path_s = set(path) retry = True while retry: retry = False for edge2 in W: - if edge2[0] in path and edge2[1] in path: + if edge2[0] in path_s and edge2[1] in path_s: W.add(edge) edge = edge2 W.remove(edge) path = H.shortest_path(edge[0], edge[1]) + path_s = set(path) retry = True break entry = edge[2] From f647f671d0bc92b3f2c093cfd8333b8036babc75 Mon Sep 17 00:00:00 2001 From: dcoudert Date: Sun, 10 Sep 2023 11:21:53 +0200 Subject: [PATCH 3/4] fix method fundamental_group in sage/topology/simplicial_complex.py --- src/sage/topology/simplicial_complex.py | 27 +++++++------------------ 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/sage/topology/simplicial_complex.py b/src/sage/topology/simplicial_complex.py index d3959b55b92..a0037a43a39 100644 --- a/src/sage/topology/simplicial_complex.py +++ b/src/sage/topology/simplicial_complex.py @@ -4173,29 +4173,16 @@ def fundamental_group(self, base_point=None, simplify=True): from sage.groups.free_group import FreeGroup from sage.libs.gap.libgap import libgap as gap G = self.graph() - # If the vertices and edges of G are not sortable, e.g., a mix - # of str and int, Sage+Python 3 may raise a TypeError when - # trying to find the spanning tree. So create a graph - # isomorphic to G but with sortable vertices. Use a copy of G, - # because self.graph() is cached, and relabeling its vertices - # would relabel the cached version. - int_to_v = dict(enumerate(G.vertex_iterator())) - v_to_int = {v: i for i, v in int_to_v.items()} - G2 = G.copy(immutable=False) - G2.relabel(v_to_int) - spanning_tree = G2.min_spanning_tree() - gens = [(int_to_v[e[0]], int_to_v[e[1]]) - for e in G2.edges(sort=True) - if e not in spanning_tree] - if len(gens) == 0: - return gap.TrivialGroup() - # Edges in the graph may be sorted differently than in the # simplicial complex, so convert the edges to frozensets so we # don't have to worry about it. Convert spanning_tree to a set # to make lookup faster. - spanning_tree = set(frozenset((int_to_v[e[0]], int_to_v[e[1]])) - for e in spanning_tree) + spanning_tree = set(frozenset((u, v)) for u, v, _ in G.min_spanning_tree()) + gens = [e for e in G.edge_iterator(labels=False) + if frozenset(e) not in spanning_tree] + if not gens: + return gap.TrivialGroup() + gens_dict = {frozenset(g): i for i, g in enumerate(gens)} FG = FreeGroup(len(gens), 'e') rels = [] @@ -4204,7 +4191,7 @@ def fundamental_group(self, base_point=None, simplify=True): z = dict() for i in range(3): x = frozenset(bdry[i]) - if (x in spanning_tree): + if x in spanning_tree: z[i] = FG.one() else: z[i] = FG.gen(gens_dict[x]) From 5efb88e8b65dac07ac5fa7bbbb8a265b405f889c Mon Sep 17 00:00:00 2001 From: dcoudert Date: Mon, 11 Sep 2023 10:58:08 +0200 Subject: [PATCH 4/4] PR #36232: alignment --- src/sage/topology/simplicial_complex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/topology/simplicial_complex.py b/src/sage/topology/simplicial_complex.py index 89fc0fdac7d..6816c128115 100644 --- a/src/sage/topology/simplicial_complex.py +++ b/src/sage/topology/simplicial_complex.py @@ -4183,7 +4183,7 @@ def fundamental_group(self, base_point=None, simplify=True): # to make lookup faster. spanning_tree = set(frozenset((u, v)) for u, v, _ in G.min_spanning_tree()) gens = [e for e in G.edge_iterator(labels=False) - if frozenset(e) not in spanning_tree] + if frozenset(e) not in spanning_tree] if not gens: return gap.TrivialGroup()