From 5a273243a934697f39f432af434f10939ea1a094 Mon Sep 17 00:00:00 2001 From: dcoudert Date: Sun, 29 Oct 2023 19:13:30 +0100 Subject: [PATCH 1/4] address siggestions from issue 36530 --- src/sage/graphs/generic_graph.py | 71 +++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/src/sage/graphs/generic_graph.py b/src/sage/graphs/generic_graph.py index eedbc36bef3..33f50fd712d 100644 --- a/src/sage/graphs/generic_graph.py +++ b/src/sage/graphs/generic_graph.py @@ -7865,25 +7865,35 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", argument is set to ``None`` by default, which means that no constraint is set upon the first vertex in the path. + This parameter can only be used when ``algorithm`` is ``"MILP"``. + - ``t`` -- a vertex (default: ``None``); forces the destination of the path (the method then returns the longest path ending at ``t``). The argument is set to ``None`` by default, which means that no constraint is set upon the last vertex in the path. + This parameter can only be used when ``algorithm`` is ``"MILP"``. + - ``use_edge_labels`` -- boolean (default: ``False``); whether to compute a path with maximum weight where the weight of an edge is defined by its label (a label set to ``None`` or ``{}`` being considered as a weight of `1`), or to compute a path with the longest possible number of edges (i.e., edge weights are set to 1) + This parameter can only be used when ``algorithm`` is ``"MILP"``. + - ``algorithm`` -- string (default: ``"MILP"``); the algorithm to use - among ``"MILP"`` and ``"backtrack"``. Two remarks on this respect: + among ``"MILP"``, ``"backtrack"`` and ``"heuristic"``: - * While the MILP formulation returns an exact answer, the backtrack - algorithm is a randomized heuristic. + * ``"MILP"`` returns an exact answer. - * As the backtrack algorithm does not support edge weighting, setting - ``use_edge_labels=True`` will force the use of the MILP algorithm. + * ``"backtrack"`` will be renamed ``"heuristic"`` in the future. A + warning is raised when used. + + * ``"heuristic"`` is a randomized heuristic for finding a long path in + an unweighted (di)graph. This heuristic does not take into account + parameters ``s``, ``t`` and ``use_edge_labels``. An error is raised + if these parameters are set. - ``solver`` -- string (default: ``None``); specify a Mixed Integer Linear Programming (MILP) solver to be used. If set to ``None``, the @@ -7928,7 +7938,7 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", The heuristic totally agrees:: sage: g = graphs.PetersenGraph() - sage: p = g.longest_path(algorithm="backtrack").edges(sort=True, labels=False) + sage: p = g.longest_path(algorithm="heuristic").edges(sort=True, labels=False) sage: len(p) 9 @@ -7950,13 +7960,13 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", TESTS: - The argument ``algorithm`` must be either ``'backtrack'`` or - ``'MILP'``:: + The argument ``algorithm`` must be either ``'backtrack'``, + ``'heuristic'`` or ``'MILP'``:: sage: graphs.PetersenGraph().longest_path(algorithm="abc") Traceback (most recent call last): ... - ValueError: algorithm must be either 'backtrack' or 'MILP' + ValueError: algorithm must be either 'backtrack', 'heuristic' or 'MILP' Disconnected graphs not weighted:: @@ -8029,13 +8039,44 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", sage: H = {(0, 3), (2, 0), (3, 4)} sage: H == {x for x in G.longest_path().edge_iterator(labels=False)} # needs sage.numerical.mip True + + :gh:`12345`:: + + sage: G = graphs.PathGraph(3) + sage: P = G.longest_path(algorithm='backtrack') + doctest:...: FutureWarning: algorithm 'backtrack' will be renamed 'heuristic' in the future. + See https://github.com/sagemath/sage/issues/12345 for details. + sage: G.longest_path(algorithm='heuristic', s=0) + Traceback (most recent call last): + ... + ValueError: parameters s, t, and use_edge_labels can not be used in + combinaiton with algorithms 'backtrack' and 'heuristic' + sage: G.longest_path(algorithm='heuristic', t=2) + Traceback (most recent call last): + ... + ValueError: parameters s, t, and use_edge_labels can not be used in + combinaiton with algorithms 'backtrack' and 'heuristic' + sage: G.longest_path(algorithm='heuristic', use_edge_labels=True) + Traceback (most recent call last): + ... + ValueError: parameters s, t, and use_edge_labels can not be used in + combinaiton with algorithms 'backtrack' and 'heuristic' """ self._scream_if_not_simple() - if use_edge_labels: - algorithm = "MILP" - if algorithm not in ("backtrack", "MILP"): - raise ValueError("algorithm must be either 'backtrack' or 'MILP'") + if algorithm not in ("backtrack", "heuristic", "MILP"): + raise ValueError("algorithm must be either 'backtrack', 'heuristic' or 'MILP'") + if algorithm == "backtrack": + from sage.misc.superseded import warning + warning(12345, + "algorithm 'backtrack' will be renamed 'heuristic' in the future.", + FutureWarning) + algorithm = 'heuristic' + if algorithm == 'heuristic': + if s is not None or t is not None or use_edge_labels: + raise ValueError("parameters s, t, and use_edge_labels can not " + "be used in combinaiton with algorithms " + "'backtrack' and 'heuristic'") # Quick improvement if not self.is_connected(): @@ -8080,8 +8121,8 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", from sage.graphs.graph import Graph return [0, Graph()] if use_edge_labels else Graph() - # Calling the backtrack heuristic if asked - if algorithm == "backtrack": + # Calling the heuristic if asked + if algorithm == "heuristic": from sage.graphs.generic_graph_pyx import find_hamiltonian as fh x = fh(self, find_path=True)[1] return self.subgraph(vertices=x, edges=list(zip(x[:-1], x[1:]))) From 2aee48ba969c3f2e92b00574abba36ab7d4adee5 Mon Sep 17 00:00:00 2001 From: dcoudert Date: Sun, 29 Oct 2023 19:24:27 +0100 Subject: [PATCH 2/4] add the number of this PR --- src/sage/graphs/generic_graph.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sage/graphs/generic_graph.py b/src/sage/graphs/generic_graph.py index 33f50fd712d..0c2f0f88725 100644 --- a/src/sage/graphs/generic_graph.py +++ b/src/sage/graphs/generic_graph.py @@ -8040,12 +8040,12 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", sage: H == {x for x in G.longest_path().edge_iterator(labels=False)} # needs sage.numerical.mip True - :gh:`12345`:: + :issue:`36574`:: sage: G = graphs.PathGraph(3) sage: P = G.longest_path(algorithm='backtrack') doctest:...: FutureWarning: algorithm 'backtrack' will be renamed 'heuristic' in the future. - See https://github.com/sagemath/sage/issues/12345 for details. + See https://github.com/sagemath/sage/issues/36574 for details. sage: G.longest_path(algorithm='heuristic', s=0) Traceback (most recent call last): ... @@ -8068,7 +8068,7 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", raise ValueError("algorithm must be either 'backtrack', 'heuristic' or 'MILP'") if algorithm == "backtrack": from sage.misc.superseded import warning - warning(12345, + warning(36574, "algorithm 'backtrack' will be renamed 'heuristic' in the future.", FutureWarning) algorithm = 'heuristic' From 5f96b24e3cfc7c77ad436c00bb92ecdbaedd1006 Mon Sep 17 00:00:00 2001 From: dcoudert Date: Thu, 9 Nov 2023 08:27:06 +0100 Subject: [PATCH 3/4] typos --- src/sage/graphs/generic_graph.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sage/graphs/generic_graph.py b/src/sage/graphs/generic_graph.py index 9e1923cc9fa..af107b74f31 100644 --- a/src/sage/graphs/generic_graph.py +++ b/src/sage/graphs/generic_graph.py @@ -8070,17 +8070,17 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", Traceback (most recent call last): ... ValueError: parameters s, t, and use_edge_labels can not be used in - combinaiton with algorithms 'backtrack' and 'heuristic' + combination with algorithms 'backtrack' and 'heuristic' sage: G.longest_path(algorithm='heuristic', t=2) Traceback (most recent call last): ... ValueError: parameters s, t, and use_edge_labels can not be used in - combinaiton with algorithms 'backtrack' and 'heuristic' + combination with algorithms 'backtrack' and 'heuristic' sage: G.longest_path(algorithm='heuristic', use_edge_labels=True) Traceback (most recent call last): ... ValueError: parameters s, t, and use_edge_labels can not be used in - combinaiton with algorithms 'backtrack' and 'heuristic' + combination with algorithms 'backtrack' and 'heuristic' """ self._scream_if_not_simple() @@ -8095,7 +8095,7 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", if algorithm == 'heuristic': if s is not None or t is not None or use_edge_labels: raise ValueError("parameters s, t, and use_edge_labels can not " - "be used in combinaiton with algorithms " + "be used in combination with algorithms " "'backtrack' and 'heuristic'") # Quick improvement From 2fbaf8afc0216f318898d7066f6e318e8b7f3e96 Mon Sep 17 00:00:00 2001 From: dcoudert Date: Thu, 9 Nov 2023 08:55:01 +0100 Subject: [PATCH 4/4] deprecate algorithm backtrack --- src/sage/graphs/generic_graph.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/sage/graphs/generic_graph.py b/src/sage/graphs/generic_graph.py index af107b74f31..b6d2fc13917 100644 --- a/src/sage/graphs/generic_graph.py +++ b/src/sage/graphs/generic_graph.py @@ -7907,8 +7907,7 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", * ``"MILP"`` returns an exact answer. - * ``"backtrack"`` will be renamed ``"heuristic"`` in the future. A - warning is raised when used. + * ``"backtrack"`` is renamed ``"heuristic"`` (:issue:`36574`). * ``"heuristic"`` is a randomized heuristic for finding a long path in an unweighted (di)graph. This heuristic does not take into account @@ -8064,39 +8063,38 @@ def longest_path(self, s=None, t=None, use_edge_labels=False, algorithm="MILP", sage: G = graphs.PathGraph(3) sage: P = G.longest_path(algorithm='backtrack') - doctest:...: FutureWarning: algorithm 'backtrack' will be renamed 'heuristic' in the future. + doctest:...: DeprecationWarning: algorithm 'backtrack' is deprecated. + Use algorithm 'heuristic' instead. See https://github.com/sagemath/sage/issues/36574 for details. sage: G.longest_path(algorithm='heuristic', s=0) Traceback (most recent call last): ... ValueError: parameters s, t, and use_edge_labels can not be used in - combination with algorithms 'backtrack' and 'heuristic' + combination with algorithm 'heuristic' sage: G.longest_path(algorithm='heuristic', t=2) Traceback (most recent call last): ... ValueError: parameters s, t, and use_edge_labels can not be used in - combination with algorithms 'backtrack' and 'heuristic' + combination with algorithm 'heuristic' sage: G.longest_path(algorithm='heuristic', use_edge_labels=True) Traceback (most recent call last): ... ValueError: parameters s, t, and use_edge_labels can not be used in - combination with algorithms 'backtrack' and 'heuristic' + combination with algorithm 'heuristic' """ self._scream_if_not_simple() if algorithm not in ("backtrack", "heuristic", "MILP"): raise ValueError("algorithm must be either 'backtrack', 'heuristic' or 'MILP'") if algorithm == "backtrack": - from sage.misc.superseded import warning - warning(36574, - "algorithm 'backtrack' will be renamed 'heuristic' in the future.", - FutureWarning) + from sage.misc.superseded import deprecation + deprecation(36574, "algorithm 'backtrack' is deprecated. " + "Use algorithm 'heuristic' instead.") algorithm = 'heuristic' if algorithm == 'heuristic': if s is not None or t is not None or use_edge_labels: raise ValueError("parameters s, t, and use_edge_labels can not " - "be used in combination with algorithms " - "'backtrack' and 'heuristic'") + "be used in combination with algorithm 'heuristic'") # Quick improvement if not self.is_connected():