Skip to content

Commit 1ad3f20

Browse files
author
Release Manager
committed
gh-40717: Fix enumerating the same cycle twice in undirected graph <!-- ^ 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 #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 #12345". --> The output for `algorithm="A"` and `algorithm="B"` in all_cycles_iterator function is inconsistent as below. This happens when a graph is undirected. `algorithm="A"` outputs the same cycle in reverse order, while `algorithm="B"` does not. ```python sage: g = Graph({0: [1, 2], 1: [0, 2], 2: [0, 1]}) sage: list(g.all_cycles_iterator(algorithm='A', simple=True)) [[0, 1, 2, 0], [0, 2, 1, 0]] sage: list(g.all_cycles_iterator(algorithm='B', simple=True)) [[0, 1, 2, 0]] ``` This PR fixes this issue by changing the current bahavior of `algorithm="A"` and output each cycle once. ### 📝 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. - [x] I have linked a relevant issue or discussion. - [x] 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, --> <!-- - #12345: short description why this is a dependency --> <!-- - #34567: ... --> URL: #40717 Reported by: Yuta Inoue Reviewer(s): David Coudert, Yuta Inoue
2 parents eb7e296 + 7b8e9a8 commit 1ad3f20

File tree

1 file changed

+29
-14
lines changed

1 file changed

+29
-14
lines changed

src/sage/graphs/cycle_enumeration.py

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,22 +156,22 @@ def _all_cycles_iterator_vertex(self, vertex, starting_vertices=None, simple=Fal
156156
...
157157
ValueError: negative weight is not allowed
158158
159-
The function works for an undirected graph::
159+
The function works for an undirected graph. Specifically, each cycle is
160+
enumerated exactly once, meaning a cycle and its reverse are not listed separately::
160161
161162
sage: g = Graph({0: [1, 2], 1: [0, 2], 2: [0, 1]})
162163
sage: it = g._all_cycles_iterator_vertex(0, simple=False)
163164
sage: for i in range(7): print(next(it))
164165
[0, 1, 0]
165166
[0, 2, 0]
166167
[0, 1, 2, 0]
167-
[0, 2, 1, 0]
168168
[0, 1, 0, 1, 0]
169169
[0, 1, 0, 2, 0]
170170
[0, 1, 2, 1, 0]
171+
[0, 2, 0, 2, 0]
171172
sage: for cycle in g._all_cycles_iterator_vertex(0, simple=True):
172173
....: print(cycle)
173174
[0, 1, 2, 0]
174-
[0, 2, 1, 0]
175175
"""
176176
if starting_vertices is None:
177177
starting_vertices = [vertex]
@@ -195,6 +195,8 @@ def _all_cycles_iterator_vertex(self, vertex, starting_vertices=None, simple=Fal
195195
h.delete_edges((u, v) for u, v in h.edge_iterator(labels=False) if d[u] != d[v])
196196
else:
197197
h = self
198+
int_to_vertex = list(h)
199+
vertex_to_int = {v: i for i, v in enumerate(int_to_vertex)}
198200

199201
by_weight, weight_function = self._get_weight_function(by_weight=by_weight,
200202
weight_function=weight_function,
@@ -212,12 +214,24 @@ def _all_cycles_iterator_vertex(self, vertex, starting_vertices=None, simple=Fal
212214
while heap_queue:
213215
length, path = heappop(heap_queue)
214216
# Checks if a cycle has been found
215-
if len(path) > 1 and path[0] == path[-1] and \
216-
(not simple or self.is_directed() or len(path) > 3):
217-
if report_weight:
218-
yield (length, path)
219-
else:
220-
yield path
217+
if len(path) > 1 and path[0] == path[-1]:
218+
report = True
219+
if not self.is_directed():
220+
if simple:
221+
report = len(path) > 3 and vertex_to_int[path[1]] < vertex_to_int[path[-2]]
222+
else:
223+
L = len(path)
224+
for i in range(1, L // 2):
225+
if vertex_to_int[path[i]] > vertex_to_int[path[L - i - 1]]:
226+
report = False
227+
break
228+
if vertex_to_int[path[i]] < vertex_to_int[path[L - i - 1]]:
229+
break
230+
if report:
231+
if report_weight:
232+
yield (length, path)
233+
else:
234+
yield path
221235
# If simple is set to True, only simple cycles are
222236
# allowed, Then it discards the current path
223237
if (not simple or path.count(path[-1]) == 1):
@@ -559,7 +573,7 @@ def all_cycles_iterator(self, starting_vertices=None, simple=False,
559573
[0, 1, 2, 0]
560574
[2, 3, 4, 5, 2]
561575
562-
The algorithm ``'B'`` is available only when `simple=True`::
576+
The algorithm ``'B'`` is available only when ``simple=True``::
563577
564578
sage: g = DiGraph()
565579
sage: g.add_edges([('a', 'b', 1), ('b', 'a', 1)])
@@ -569,13 +583,13 @@ def all_cycles_iterator(self, starting_vertices=None, simple=False,
569583
...
570584
ValueError: The algorithm 'B' is available only when simple=True.
571585
572-
The algorithm ``'A'`` works for undirected graphs as well::
586+
The algorithm ``'A'`` works for undirected graphs as well. Specifically, each cycle is
587+
enumerated exactly once, meaning a cycle and its reverse are not listed separately::
573588
574589
sage: g = Graph({0: [1, 2], 1: [0, 2], 2: [0, 1]})
575590
sage: for cycle in g.all_cycles_iterator(algorithm='A', simple=True):
576591
....: print(cycle)
577592
[0, 1, 2, 0]
578-
[0, 2, 1, 0]
579593
"""
580594
if starting_vertices is None:
581595
starting_vertices = self
@@ -872,11 +886,12 @@ def all_simple_cycles(self, starting_vertices=None, rooted=False,
872886
sage: cycles.sort() == cycles_B.sort()
873887
True
874888
875-
The algorithm ``'A'`` is available for undirected graphs::
889+
The algorithm ``'A'`` is available for undirected graphs. Specifically, each cycle is
890+
enumerated exactly once, meaning a cycle and its reverse are not listed separately::
876891
877892
sage: g = Graph({0: [1, 2], 1: [0, 2], 2: [0, 1]})
878893
sage: g.all_simple_cycles(algorithm='A')
879-
[[0, 1, 2, 0], [0, 2, 1, 0]]
894+
[[0, 1, 2, 0]]
880895
"""
881896
return list(self.all_cycles_iterator(starting_vertices=starting_vertices,
882897
simple=True, rooted=rooted,

0 commit comments

Comments
 (0)