Skip to content

Commit 8402923

Browse files
author
Release Manager
committed
gh-35982: some care in sage/graphs/domination.py We do some minor changes in `sage/graphs/domination.py`. The main change is the default value of parameter `work_on_copy` from `False` to `True`. Firstly, it is safer to ensure that the input graph is not modified by default, unless the user asks for such behavior. Secondly, the method was actually working on a copy when the parameter was `False` (wrong order of tests). We also add some tests to show that the methods are robust to vertices with incomparable labels (see #35902). ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #35982 Reported by: David Coudert Reviewer(s): David Coudert, Matthias Köppe
2 parents e86a48f + 945e476 commit 8402923

File tree

1 file changed

+59
-21
lines changed

1 file changed

+59
-21
lines changed

src/sage/graphs/domination.py

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def is_redundant(G, dom, focus=None):
137137
False
138138
"""
139139
dom = list(dom)
140-
focus = list(G) if focus is None else list(focus)
140+
focus = G if focus is None else set(focus)
141141

142142
# dominator[v] (for v in focus) will be equal to:
143143
# - (0, None) if v has no neighbor in dom
@@ -345,13 +345,20 @@ def dominating_sets(g, k=1, independent=False, total=False,
345345
Traceback (most recent call last):
346346
...
347347
ValueError: the domination distance must be a non-negative integer
348+
349+
The method is robust to vertices with incomparable labels::
350+
351+
sage: G = Graph([(1, 'A'), ('A', 2), (2, 3), (3, 1)])
352+
sage: L = list(G.dominating_sets())
353+
sage: len(L)
354+
6
348355
"""
349356
g._scream_if_not_simple(allow_multiple_edges=True, allow_loops=not total)
350357

351358
if not k:
352359
yield list(g)
353360
return
354-
elif k < 0:
361+
if k < 0:
355362
raise ValueError("the domination distance must be a non-negative integer")
356363

357364
from sage.numerical.mip import MixedIntegerLinearProgram
@@ -721,8 +728,8 @@ def _aux_with_rep(H, to_dom, u_next):
721728

722729
# Here we use aux_with_rep twice to enumerate the minimal
723730
# dominating sets while avoiding repeated outputs
724-
for (X, i) in _aux_with_rep(G, to_dom, u_next):
725-
for (Y, j) in _aux_with_rep(G, to_dom, u_next):
731+
for X, i in _aux_with_rep(G, to_dom, u_next):
732+
for Y, j in _aux_with_rep(G, to_dom, u_next):
726733
if j >= i:
727734
# This is the first time we meet X: we output it
728735
yield X
@@ -732,7 +739,7 @@ def _aux_with_rep(H, to_dom, u_next):
732739
break
733740

734741

735-
def minimal_dominating_sets(G, to_dominate=None, work_on_copy=False, k=1):
742+
def minimal_dominating_sets(G, to_dominate=None, work_on_copy=True, k=1):
736743
r"""
737744
Return an iterator over the minimal dominating sets of a graph.
738745
@@ -743,7 +750,7 @@ def minimal_dominating_sets(G, to_dominate=None, work_on_copy=False, k=1):
743750
- ``to_dominate`` -- vertex iterable or ``None`` (default: ``None``);
744751
the set of vertices to be dominated.
745752
746-
- ``work_on_copy`` -- boolean (default: ``False``); whether or not to work on
753+
- ``work_on_copy`` -- boolean (default: ``True``); whether or not to work on
747754
a copy of the input graph; if set to ``False``, the input graph will be
748755
modified (relabeled).
749756
@@ -836,6 +843,19 @@ def minimal_dominating_sets(G, to_dominate=None, work_on_copy=False, k=1):
836843
sage: list(G.minimal_dominating_sets(k=3))
837844
[{(0, 0)}, {(0, 1)}, {(0, 2)}, {(1, 0)}, {(1, 1)}, {(1, 2)}]
838845
846+
When parameter ``work_on_copy`` is ``False``, the input graph is modified
847+
(relabeled)::
848+
849+
sage: G = Graph([('A', 'B')])
850+
sage: _ = list(G.minimal_dominating_sets(work_on_copy=True))
851+
sage: set(G) == {'A', 'B'}
852+
True
853+
sage: _ = list(G.minimal_dominating_sets(work_on_copy=False))
854+
sage: set(G) == {'A', 'B'}
855+
False
856+
sage: set(G) == {0, 1}
857+
True
858+
839859
TESTS:
840860
841861
The empty graph is handled correctly::
@@ -891,6 +911,15 @@ def minimal_dominating_sets(G, to_dominate=None, work_on_copy=False, k=1):
891911
Traceback (most recent call last):
892912
...
893913
ValueError: vertex (foo) is not a vertex of the graph
914+
915+
The method is robust to vertices with incomparable labels::
916+
917+
sage: G = Graph([(1, 'A'), ('A', 2), (2, 3), (3, 1)])
918+
sage: L = list(G.minimal_dominating_sets())
919+
sage: len(L)
920+
6
921+
sage: {3, 'A'} in L
922+
True
894923
"""
895924
def tree_search(H, plng, dom, i):
896925
r"""
@@ -944,7 +973,8 @@ def tree_search(H, plng, dom, i):
944973
# We complete dom with can_ext -> canD
945974
canD = set().union(can_ext, dom)
946975

947-
if (not H.is_redundant(canD, V_next)) and set(dom) == set(_parent(H, canD, plng[i][1])):
976+
if (not H.is_redundant(canD, V_next)
977+
and set(dom) == set(_parent(H, canD, plng[i][1]))):
948978
# By construction, can_ext is a dominating set of
949979
# `V_next - N[dom]`, so canD dominates V_next.
950980
# If canD is a legitimate child of dom and is not redundant, we
@@ -954,6 +984,12 @@ def tree_search(H, plng, dom, i):
954984
##
955985
# end of tree-search routine
956986

987+
if k < 0:
988+
raise ValueError("the domination distance must be a non-negative integer")
989+
if not k:
990+
yield set(G) if to_dominate is None else set(to_dominate)
991+
return
992+
957993
int_to_vertex = list(G)
958994
vertex_to_int = {u: i for i, u in enumerate(int_to_vertex)}
959995

@@ -965,28 +1001,24 @@ def tree_search(H, plng, dom, i):
9651001
raise ValueError(f"vertex ({u}) is not a vertex of the graph")
9661002
vertices_to_dominate = {vertex_to_int[u] for u in to_dominate}
9671003

968-
if k < 0:
969-
raise ValueError("the domination distance must be a non-negative integer")
970-
elif not k:
971-
yield set(int_to_vertex) if to_dominate is None else set(to_dominate)
1004+
if not vertices_to_dominate:
1005+
# base case: vertices_to_dominate is empty
1006+
# the empty set/list is the only minimal DS of the empty set
1007+
yield set()
9721008
return
973-
elif k > 1:
1009+
if k > 1:
9741010
# We build a graph H with an edge between u and v if these vertices are
9751011
# at distance at most k in G
9761012
H = G.__class__(G.order())
9771013
for u, ui in vertex_to_int.items():
978-
H.add_edges((ui, vertex_to_int[v]) for v in G.breadth_first_search(u, distance=k) if u != v)
1014+
H.add_edges((ui, vertex_to_int[v])
1015+
for v in G.breadth_first_search(u, distance=k) if u != v)
9791016
G = H
9801017
elif work_on_copy:
981-
G.relabel(perm=vertex_to_int)
982-
else:
9831018
G = G.relabel(perm=vertex_to_int, inplace=False)
984-
985-
if not vertices_to_dominate:
986-
# base case: vertices_to_dominate is empty
987-
# the empty set/list is the only minimal DS of the empty set
988-
yield set()
989-
return
1019+
else:
1020+
# The input graph is modified
1021+
G.relabel(perm=vertex_to_int, inplace=True)
9901022

9911023
peeling = _peel(G, vertices_to_dominate)
9921024

@@ -1119,6 +1151,12 @@ def greedy_dominating_set(G, k=1, vertices=None, ordering=None, return_sets=Fals
11191151
sage: G = graphs.PathGraph(5)
11201152
sage: dom = greedy_dominating_set(G, vertices=[0, 1, 3, 4])
11211153
1154+
The method is robust to vertices with incomparable labels::
1155+
1156+
sage: G = Graph([(1, 'A')])
1157+
sage: len(greedy_dominating_set(G))
1158+
1
1159+
11221160
Check parameters::
11231161
11241162
sage: greedy_dominating_set(G, ordering="foo")

0 commit comments

Comments
 (0)