diff --git a/hydroflow_lang/src/graph/flat_to_partitioned.rs b/hydroflow_lang/src/graph/flat_to_partitioned.rs index 4c732fe5b0aa..da192516c2d9 100644 --- a/hydroflow_lang/src/graph/flat_to_partitioned.rs +++ b/hydroflow_lang/src/graph/flat_to_partitioned.rs @@ -165,7 +165,8 @@ fn make_subgraph_collect( !matches!(pred, GraphNode::Handoff { .. }) }) }, - ); + ) + .expect("Subgraphs are in-out trees."); let mut grouped_nodes: SecondaryMap> = Default::default(); for node_id in topo_sort { diff --git a/hydroflow_lang/src/graph/graph_algorithms.rs b/hydroflow_lang/src/graph/graph_algorithms.rs index 96ec1e357df2..dfbaed243f39 100644 --- a/hydroflow_lang/src/graph/graph_algorithms.rs +++ b/hydroflow_lang/src/graph/graph_algorithms.rs @@ -23,16 +23,21 @@ where let topo_sort_order = { // Condensed each SCC into a single node for toposort. let mut condensed_preds: BTreeMap> = Default::default(); - for u in (nodes_fn)() { - condensed_preds - .entry(scc[&u]) - .or_default() - .extend((preds_fn)(u).into_iter().map(|v| scc[&v])); + for v in (nodes_fn)() { + let v = scc[&v]; + condensed_preds.entry(v).or_default().extend( + (preds_fn)(v) + .into_iter() + .map(|u| scc[&u]) + .filter(|&u| v != u), + ); } topo_sort((nodes_fn)(), |v| { condensed_preds.get(&v).into_iter().flatten().cloned() }) + .map_err(drop) + .expect("No cycles after SCC condensing.") }; topo_sort_order } @@ -40,13 +45,16 @@ where /// Topologically sorts a set of nodes. Returns a list where the order of `Id`s will agree with /// the order of any path through the graph. /// -/// This naturally requires a directed acyclic graph (DAG). +/// This succeeds if the input is a directed acyclic graph (DAG). +/// +/// If the input has a cycle, an `Err` will be returned containing the cycle. Each node in the +/// cycle will be listed exactly once. /// /// pub fn topo_sort( node_ids: NodeIds, mut preds_fn: PredsFn, -) -> Vec +) -> Result, Vec> where Id: Copy + Eq + Ord, NodeIds: IntoIterator, @@ -58,28 +66,49 @@ where fn pred_dfs_postorder( node_id: Id, preds_fn: &mut PredsFn, - marked: &mut BTreeSet, + marked: &mut BTreeMap, // `false` => temporary, `true` => permanent. order: &mut Vec, - ) where + ) -> Result<(), ()> + where Id: Copy + Eq + Ord, PredsFn: FnMut(Id) -> PredsIter, PredsIter: IntoIterator, { - if marked.insert(node_id) { - for next_pred in (preds_fn)(node_id) { - pred_dfs_postorder(next_pred, preds_fn, marked, order); + match marked.get(&node_id) { + Some(_permanent @ true) => Ok(()), + Some(_temporary @ false) => { + // Cycle found! + order.clear(); + order.push(node_id); + Err(()) + } + None => { + marked.insert(node_id, false); + for next_pred in (preds_fn)(node_id) { + pred_dfs_postorder(next_pred, preds_fn, marked, order).map_err(|()| { + if order.len() == 1 || order.first().unwrap() != order.last().unwrap() { + order.push(node_id); + } + })?; + } + order.push(node_id); + marked.insert(node_id, true); + Ok(()) } - order.push(node_id); - } else { - // TODO(mingwei): cycle found! } } for node_id in node_ids { - pred_dfs_postorder(node_id, &mut preds_fn, &mut marked, &mut order); + if pred_dfs_postorder(node_id, &mut preds_fn, &mut marked, &mut order).is_err() { + // Cycle found. + let end = order.last().unwrap(); + let beg = order.iter().position(|n| n == end).unwrap(); + order.drain(0..=beg); + return Err(order); + } } - order + Ok(order) } /// Finds the strongly connected components in the graph. A strongly connected component is a @@ -156,6 +185,8 @@ where #[cfg(test)] mod test { + use itertools::Itertools; + use super::*; #[test] @@ -179,6 +210,13 @@ mod test { .filter(move |&&(_, dst)| v == dst) .map(|&(src, _)| src) }); + assert!( + sort.is_ok(), + "Did not expect cycle: {:?}", + sort.unwrap_err() + ); + + let sort = sort.unwrap(); println!("{:?}", sort); let position: BTreeMap<_, _> = sort.iter().enumerate().map(|(i, &x)| (x, i)).collect(); @@ -187,6 +225,54 @@ mod test { } } + #[test] + pub fn test_toposort_cycle() { + // https://commons.wikimedia.org/wiki/File:Directed_graph,_cyclic.svg + // ┌────►C──────┐ + // │ │ + // │ ▼ + // A───────►B E ─────►F + // ▲ │ + // │ │ + // └─────D◄─────┘ + let edges = [ + ('A', 'B'), + ('B', 'C'), + ('C', 'E'), + ('D', 'B'), + ('E', 'F'), + ('E', 'D'), + ]; + let ids = edges + .iter() + .flat_map(|&(a, b)| [a, b]) + .collect::>(); + let cycle_rotations = BTreeSet::from_iter([ + ['B', 'C', 'E', 'D'], + ['C', 'E', 'D', 'B'], + ['E', 'D', 'B', 'C'], + ['D', 'B', 'C', 'E'], + ]); + + let permutations = ids.iter().copied().permutations(ids.len()); + for permutation in permutations { + let result = topo_sort(permutation.iter().copied(), |v| { + edges + .iter() + .filter(move |&&(_, dst)| v == dst) + .map(|&(src, _)| src) + }); + assert!(result.is_err()); + let cycle = result.unwrap_err(); + assert!( + cycle_rotations.contains(&*cycle), + "cycle: {:?}, vertex order: {:?}", + cycle, + permutation + ); + } + } + #[test] pub fn test_scc_kosaraju() { // https://commons.wikimedia.org/wiki/File:Scc-1.svg