Better cycle detection

pytools/graph.py

@@ -299,9 +299,26 @@ def compute_topological_order(graph: Mapping[T, Collection[T]],
                 heappush(heap, HeapEntry(child, keyfunc(child)))
 
     if len(order) != total_num_nodes:
-        # any node which has a predecessor left is a part of a cycle
-        raise CycleError(next(iter(n for n, num_preds in
-            nodes_to_num_predecessors.items() if num_preds != 0)))
+        # There is a cycle in the graph
+        try:
+            validate_graph(graph)
+        except ValueError:
+            # Graph is invalid, we can't compute SCCs or return a meaningful node
+            # that is part of a cycle
+            raise CycleError(None)
+
+        sccs = compute_sccs(graph)
+        cycles = [scc for scc in sccs if len(scc) > 1]
+
+        if cycles:
+            # Cycles that are not self-loops
+            node = cycles[0][0]
+        else:
+            # Self-loop SCCs also have a length of 1
+            node = next(iter(n for n, num_preds in
+                             nodes_to_num_predecessors.items() if num_preds != 0))
+
+        raise CycleError(node)
 
     return order
 

test/test_graph_tools.py

@@ -395,6 +395,25 @@ def test_is_connected():
     assert is_connected({})
 
 
+def test_cycle_detection():
+    from pytools.graph import compute_topological_order, CycleError
+
+    # Non-Self Loop
+    graph = {1: {}, 5: {1, 8}, 8: {5}}
+    with pytest.raises(CycleError, match="5|8"):
+        compute_topological_order(graph)
+
+    # Self-Loop
+    graph = {1: {1}, 5: {8}, 8: {}}
+    with pytest.raises(CycleError, match="1"):
+        compute_topological_order(graph)
+
+    # Invalid graph with loop
+    graph = {1: {42}, 5: {8}, 8: {5}}
+    with pytest.raises(CycleError, match="None"):
+        compute_topological_order(graph)
+
+
 if __name__ == "__main__":
     if len(sys.argv) > 1:
         exec(sys.argv[1])