Skip to content

Commit

Permalink
sagemathgh-37942: Graph constructors: Remove gratuitous nondeterminism
Browse files Browse the repository at this point in the history
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Since Python 3.x, `dict`s preserve insertion order.

Here we modify the processing of "dict-of-dicts" and "dist-of-lists"
input formats to avoid going through `set`, which introduces gratuitous
nondeterminism regarding the order of vertices.

This change gives for example this improvement:
```sage
sage: G = graphs.CubeGraph(3)
sage: G.vertices(sort=False)
['000', '001', '010', '011', '100', '101', '110', '111']
```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37942
Reported by: Matthias Köppe
Reviewer(s): David Coudert, Martin Rubey, Matthias Köppe
  • Loading branch information
Release Manager committed May 8, 2024
2 parents 6970fe4 + d1f3f4a commit cc8ea2e
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/sage/graphs/generic_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -15719,7 +15719,7 @@ def cluster_triangles(self, nbunch=None, implementation=None):

sage: F = graphs.FruchtGraph()
sage: list(F.cluster_triangles().values())
[1, 1, 0, 1, 1, 1, 1, 1, 0, 1, 1, 0]
[1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 0, 0]
sage: F.cluster_triangles()
{0: 1, 1: 1, 2: 0, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 0, 9: 1, 10: 1, 11: 0}
sage: F.cluster_triangles(nbunch=[0, 1, 2])
Expand Down
20 changes: 10 additions & 10 deletions src/sage/graphs/graph_decompositions/modular_decomposition.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,17 +371,17 @@ def print_md_tree(root):
sage: from sage.graphs.graph_decompositions.modular_decomposition import *
sage: print_md_tree(modular_decomposition(graphs.IcosahedralGraph()))
PRIME
3
4
7
9
11
1
5
7
8
11
0
2
6
3
9
4
10
"""

Expand Down Expand Up @@ -494,17 +494,17 @@ def habib_maurer_algorithm(graph, g_classes=None):
sage: from sage.graphs.graph_decompositions.modular_decomposition import *
sage: print_md_tree(habib_maurer_algorithm(graphs.IcosahedralGraph()))
PRIME
3
4
7
9
11
1
5
7
8
11
0
2
6
3
9
4
10
The Octahedral graph is not Prime::
Expand Down
33 changes: 27 additions & 6 deletions src/sage/graphs/graph_input.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,14 @@ def from_dict_of_dicts(G, M, loops=False, multiedges=False, weighted=False, conv
sage: g.is_isomorphic(graphs.PetersenGraph())
True
The resulting order of vertices is unspecified but deterministic::
sage: from sage.graphs.graph_input import from_dict_of_dicts
sage: g = Graph()
sage: from_dict_of_dicts(g, {i: {} for i in range(99, 90, -1)})
sage: g.vertices(sort=False)
[99, 98, 97, 96, 95, 94, 93, 92, 91]
TESTS:
:issue:`32831` is fixed::
Expand Down Expand Up @@ -493,8 +501,11 @@ def from_dict_of_dicts(G, M, loops=False, multiedges=False, weighted=False, conv

G.allow_loops(loops, check=False)
G.allow_multiple_edges(multiedges, check=False)
verts = set().union(M.keys(), *M.values())
G.add_vertices(verts)
# Use keys of a dictionary instead of a set, to preserve insertion order
verts = dict(M)
for d in M.values():
verts.update(d)
G.add_vertices(verts.keys())
if convert_empty_dict_labels_to_None:
def relabel(x):
return x if x != {} else None
Expand All @@ -504,7 +515,7 @@ def relabel(x):

is_directed = G.is_directed()
if not is_directed and multiedges:
v_to_id = {v: i for i, v in enumerate(verts)}
v_to_id = {v: i for i, v in enumerate(verts.keys())}
for u in M:
for v in M[u]:
if v_to_id[u] <= v_to_id[v] or v not in M or u not in M[v] or u == v:
Expand Down Expand Up @@ -543,8 +554,18 @@ def from_dict_of_lists(G, D, loops=False, multiedges=False, weighted=False):
sage: from_dict_of_lists(g, graphs.PetersenGraph().to_dictionary())
sage: g.is_isomorphic(graphs.PetersenGraph())
True
The resulting order of vertices is unspecified but deterministic::
sage: from sage.graphs.graph_input import from_dict_of_lists
sage: g = Graph()
sage: from_dict_of_lists(g, {i: [] for i in range(99, 90, -1)})
sage: g.vertices(sort=False)
[99, 98, 97, 96, 95, 94, 93, 92, 91]
"""
verts = set().union(D.keys(), *D.values())
# Use keys of a dictionary instead of a set, to preserve insertion order
verts = dict(D)
verts.update({v: None for l in D.values() for v in l})
if not loops:
if any(u in neighb for u, neighb in D.items()):
if loops is False:
Expand All @@ -567,11 +588,11 @@ def from_dict_of_lists(G, D, loops=False, multiedges=False, weighted=False):
multiedges = False
G.allow_loops(loops, check=False)
G.allow_multiple_edges(multiedges, check=False)
G.add_vertices(verts)
G.add_vertices(verts.keys())

is_directed = G.is_directed()
if not is_directed and multiedges:
v_to_id = {v: i for i, v in enumerate(verts)}
v_to_id = {v: i for i, v in enumerate(verts.keys())}
for u in D:
for v in D[u]:
if (v_to_id[u] <= v_to_id[v] or
Expand Down

0 comments on commit cc8ea2e

Please sign in to comment.