Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

properly catch exceptions in Dijkstra_Boost #29781

Closed
dimpase opened this issue Jun 2, 2020 · 5 comments
Closed

properly catch exceptions in Dijkstra_Boost #29781

dimpase opened this issue Jun 2, 2020 · 5 comments

Comments

@dimpase
Copy link
Member

dimpase commented Jun 2, 2020

with a recent Boost (1.73 on Gentoo) I see doctest errors like

File "src/sage/graphs/base/boost_graph.pyx", line 915, in sage.graphs.base.boost_graph.shortest_paths
Failed example:
    shortest_paths(g, 1, algorithm='Dijkstra')
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: Dijkstra algorithm does not work with negative weights, use Bellman-Ford instead
Got:
    terminate called after throwing an instance of 'boost::wrapexcept<boost::negative_edge>'
      what():  The graph may not contain an edge with negative weight.
    Traceback (most recent call last):
      File "/mnt/opt/Sage/sage-dev/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 680, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/opt/Sage/sage-dev/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1104, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.graphs.base.boost_graph.shortest_paths[13]>", line 1, in <module>
        shortest_paths(g, Integer(1), algorithm='Dijkstra')
      File "sage/graphs/base/boost_graph.pyx", line 827, in sage.graphs.base.boost_graph.shortest_paths (build/cythonized/sage/graphs
/base/boost_graph.cpp:12063)
        cpdef shortest_paths(g, start, weight_function=None, algorithm=None):
      File "sage/graphs/base/boost_graph.pyx", line 983, in sage.graphs.base.boost_graph.shortest_paths (build/cythonized/sage/graphs
/base/boost_graph.cpp:11320)
        sig_on()
    RuntimeError: Aborted

we fix these by using proper try/catch block.

CC: @dcoudert @antonio-rojas @kiwifb

Component: graph theory

Author: Dima Pasechnik

Branch/Commit: 746982b

Reviewer: David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/29781

@dimpase dimpase added this to the sage-9.2 milestone Jun 2, 2020
@dimpase
Copy link
Member Author

dimpase commented Jun 2, 2020

comment:1

The diff, ignoring whitespace change, is just

--- a/src/sage/graphs/base/boost_graph.pyx
+++ b/src/sage/graphs/base/boost_graph.pyx
@@ -977,6 +977,7 @@ cpdef shortest_paths(g, start, weight_function=None, algorithm=None):
             raise ValueError("the graph contains a negative cycle")
 
     elif algorithm in ['Dijkstra', 'Dijkstra_Boost']:
+        try:
             if g.is_directed():
                 boost_weighted_graph_from_sage_graph(&g_boost_dir, g, v_to_int, weight_function)
                 vi = v_to_int[start]
@@ -991,6 +992,8 @@ cpdef shortest_paths(g, start, weight_function=None, algorithm=None):
                 sig_off()
             if not result.distances.size():
                 raise RuntimeError("Dijkstra algorithm does not work with negative weights, use Bellman-Ford instead")
+        except RuntimeError:
+            raise RuntimeError("Dijkstra algorithm does not work with negative weights, use Bellman-Ford instead")
 
     else:
         raise ValueError(f"unknown algorithm {algorithm!r}")

@dcoudert
Copy link
Contributor

dcoudert commented Jun 2, 2020

comment:2

LGTM.

I added a note in #29657 since the usage of weights must be strengthen in boost_graph.pyx and unified with other weighted methods.

@dcoudert
Copy link
Contributor

dcoudert commented Jun 2, 2020

Reviewer: David Coudert

@dcoudert
Copy link
Contributor

dcoudert commented Jun 2, 2020

comment:3

LGTM

@vbraun
Copy link
Member

vbraun commented Jun 5, 2020

Changed branch from u/dimpase/graphs/dijkstraboost to 746982b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants