From 33efaa92b86b77f89e752572d2002af84c6502cf Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 08:35:06 +0200 Subject: [PATCH 1/7] Generalise toposort to LinkView --- src/algorithms/toposort.rs | 72 ++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/src/algorithms/toposort.rs b/src/algorithms/toposort.rs index ff1a266..30098af 100644 --- a/src/algorithms/toposort.rs +++ b/src/algorithms/toposort.rs @@ -1,4 +1,4 @@ -use crate::{Direction, LinkView, NodeIndex, PortGraph, PortIndex, PortView, SecondaryMap}; +use crate::{Direction, LinkView, NodeIndex, PortIndex, SecondaryMap}; use bitvec::prelude::BitVec; use std::{collections::VecDeque, fmt::Debug, iter::FusedIterator}; @@ -23,21 +23,21 @@ use std::{collections::VecDeque, fmt::Debug, iter::FusedIterator}; /// graph.link_nodes(node_a, 0, node_b, 0).unwrap(); /// /// // Run a topological sort on the graph starting at node A. -/// let topo: TopoSort = toposort(&graph, [node_a], Direction::Outgoing); +/// let topo: TopoSort<_> = toposort(&graph, [node_a], Direction::Outgoing); /// assert_eq!(topo.collect::>(), [node_a, node_b]); /// ``` -pub fn toposort( - graph: &PortGraph, +pub fn toposort( + graph: G, source: impl IntoIterator, direction: Direction, -) -> TopoSort<'_, Map> +) -> TopoSort where Map: SecondaryMap, { TopoSort::new(graph, source, direction, None, None) } -/// Returns an iterator over a [`PortGraph`] in topological order, applying a +/// Returns an iterator over a [`LinkView`] in topological order, applying a /// filter to the nodes and ports. No filtered nodes are returned, and neither /// are any nodes only accessible via filtered nodes or filtered ports. /// @@ -66,7 +66,7 @@ where /// graph.link_nodes(node_a, 1, node_c, 0).unwrap(); /// /// // Run a topological sort on the graph starting at node A. -/// let topo: TopoSort = toposort_filtered( +/// let topo: TopoSort<_> = toposort_filtered( /// &graph, /// [node_a, node_d], /// Direction::Outgoing, @@ -75,15 +75,16 @@ where /// ); /// assert_eq!(topo.collect::>(), [node_a, node_b]); /// ``` -pub fn toposort_filtered<'graph, Map>( - graph: &'graph PortGraph, +pub fn toposort_filtered( + graph: G, source: impl IntoIterator, direction: Direction, - node_filter: impl FnMut(NodeIndex) -> bool + 'graph, - port_filter: impl FnMut(NodeIndex, PortIndex) -> bool + 'graph, -) -> TopoSort<'_, Map> + node_filter: impl FnMut(NodeIndex) -> bool, + port_filter: impl FnMut(NodeIndex, PortIndex) -> bool, +) -> TopoSort where Map: SecondaryMap, + G: LinkView, { TopoSort::new( graph, @@ -94,13 +95,13 @@ where ) } -/// Iterator over a [`PortGraph`] in topological order. +/// Iterator over a [`LinkView`] in topological order. /// /// See [`toposort`] for more information. /// /// Implements [Kahn's algorithm](https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm). -pub struct TopoSort<'graph, Map = BitVec> { - graph: &'graph PortGraph, +pub struct TopoSort { + graph: G, visited_ports: Map, /// A VecDeque is used for the node list to produce a canonical ordering, /// as successors of nodes already have a canonical ordering due to ports. @@ -111,26 +112,27 @@ pub struct TopoSort<'graph, Map = BitVec> { nodes_seen: usize, /// A filter closure for the nodes to visit. If the closure returns false, /// the node is skipped. - node_filter: Option bool + 'graph>>, + node_filter: Option bool>>, /// A filter closure for the ports to visit. If the closure returns false, /// the port is skipped. - port_filter: Option bool + 'graph>>, + port_filter: Option bool>>, } -impl<'graph, Map> TopoSort<'graph, Map> +impl TopoSort where Map: SecondaryMap, + G: LinkView, { /// Initialises a new topological sort of a portgraph in a specified direction /// starting at a collection of `source` nodes. /// /// If the default value of `Map` is not `false`, this requires O(#ports) time. fn new( - graph: &'graph PortGraph, + graph: G, source: impl IntoIterator, direction: Direction, - mut node_filter: Option bool + 'graph>>, - port_filter: Option bool + 'graph>>, + mut node_filter: Option bool>>, + port_filter: Option bool>>, ) -> Self { let mut visited_ports: Map = SecondaryMap::new(); @@ -167,7 +169,8 @@ where /// Returns whether there are ports that have not been visited yet. /// If the iterator has seen all nodes this implies that there is a cycle. - pub fn ports_remaining(&self) -> impl DoubleEndedIterator + '_ { + // pub fn ports_remaining(&self) -> impl DoubleEndedIterator + '_ { + pub fn ports_remaining(&self) -> impl Iterator + '_ { self.graph .ports_iter() .filter(move |&p| !self.visited_ports.get(p)) @@ -175,12 +178,12 @@ where /// Checks if a node becomes ready once it is visited from `from_port`, i.e. /// it has been reached from all its linked ports. - fn becomes_ready(&mut self, node: NodeIndex, from_port: PortIndex) -> bool { + fn becomes_ready(&mut self, node: NodeIndex, from_port: impl Into) -> bool { if self.ignore_node(node) { return false; } self.graph.ports(node, self.direction.reverse()).all(|p| { - if p == from_port { + if p == from_port.into() { // This port must have not been visited yet. Otherwise, the node // would have been already been reported as ready and added to // the candidate list. @@ -216,9 +219,10 @@ where } } -impl<'graph, Map> Iterator for TopoSort<'graph, Map> +impl Iterator for TopoSort where Map: SecondaryMap, + G: LinkView, { type Item = NodeIndex; @@ -238,7 +242,7 @@ where if self.becomes_ready(target, link) { self.candidate_nodes.push_back(target); } - self.visited_ports.set(link, true); + self.visited_ports.set(link.into(), true); } } @@ -255,11 +259,17 @@ where } } -impl<'graph, Map> FusedIterator for TopoSort<'graph, Map> where Map: SecondaryMap {} +impl FusedIterator for TopoSort +where + Map: SecondaryMap, + G: LinkView, +{ +} -impl<'graph, Map> Debug for TopoSort<'graph, Map> +impl Debug for TopoSort where Map: Debug, + G: Debug, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("TopoSort") @@ -275,7 +285,7 @@ where mod test { use super::*; - use crate::{Direction, LinkMut, PortMut, PortView}; + use crate::{Direction, PortGraph, PortMut, LinkMut, PortView}; #[test] fn small_toposort() { @@ -294,13 +304,13 @@ mod test { graph.link_nodes(node_c, 0, node_d, 0).unwrap(); // Run a topological sort on the graph starting at node A. - let topo: TopoSort = toposort(&graph, [node_a, node_d], Direction::Outgoing); + let topo: TopoSort<_> = toposort(&graph, [node_a, node_d], Direction::Outgoing); assert_eq!( topo.collect::>(), [node_a, node_d, node_b, node_e, node_c] ); - let topo_filtered: TopoSort = toposort_filtered( + let topo_filtered: TopoSort<_> = toposort_filtered( &graph, [node_a, node_d], Direction::Outgoing, From 0be165fb80faf7b720b67f4a4839d7fa1ddfb471 Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 08:40:47 +0200 Subject: [PATCH 2/7] Format --- src/algorithms/toposort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/algorithms/toposort.rs b/src/algorithms/toposort.rs index 30098af..4973bcd 100644 --- a/src/algorithms/toposort.rs +++ b/src/algorithms/toposort.rs @@ -285,7 +285,7 @@ where mod test { use super::*; - use crate::{Direction, PortGraph, PortMut, LinkMut, PortView}; + use crate::{Direction, LinkMut, PortGraph, PortMut, PortView}; #[test] fn small_toposort() { From ff4f5048a69278f4a671fb90ae61c53906ad8e54 Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 08:57:48 +0200 Subject: [PATCH 3/7] Add closure lifetime to Toposort --- benches/benchmarks/toposort.rs | 2 +- src/algorithms/toposort.rs | 43 ++++++++++++++++++---------------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/benches/benchmarks/toposort.rs b/benches/benchmarks/toposort.rs index be854d0..810429c 100644 --- a/benches/benchmarks/toposort.rs +++ b/benches/benchmarks/toposort.rs @@ -9,7 +9,7 @@ use portgraph::{ use super::generators::*; fn run_toposort(graph: &PortGraph, roots: impl IntoIterator) { - let topo: TopoSort = toposort(graph, roots, Direction::Outgoing); + let topo: TopoSort<_> = toposort(graph, roots, Direction::Outgoing); for n in topo { black_box(n); } diff --git a/src/algorithms/toposort.rs b/src/algorithms/toposort.rs index 4973bcd..3820dd5 100644 --- a/src/algorithms/toposort.rs +++ b/src/algorithms/toposort.rs @@ -26,11 +26,11 @@ use std::{collections::VecDeque, fmt::Debug, iter::FusedIterator}; /// let topo: TopoSort<_> = toposort(&graph, [node_a], Direction::Outgoing); /// assert_eq!(topo.collect::>(), [node_a, node_b]); /// ``` -pub fn toposort( +pub fn toposort<'f, Map, G: LinkView>( graph: G, source: impl IntoIterator, direction: Direction, -) -> TopoSort +) -> TopoSort<'f, G, Map> where Map: SecondaryMap, { @@ -75,16 +75,16 @@ where /// ); /// assert_eq!(topo.collect::>(), [node_a, node_b]); /// ``` -pub fn toposort_filtered( +pub fn toposort_filtered<'f, Map, G>( graph: G, - source: impl IntoIterator, + source: impl IntoIterator + 'f, direction: Direction, - node_filter: impl FnMut(NodeIndex) -> bool, - port_filter: impl FnMut(NodeIndex, PortIndex) -> bool, -) -> TopoSort + node_filter: impl FnMut(NodeIndex) -> bool + 'f, + port_filter: impl FnMut(NodeIndex, PortIndex) -> bool + 'f, +) -> TopoSort<'f, G, Map> where Map: SecondaryMap, - G: LinkView, + G: LinkView + 'f, { TopoSort::new( graph, @@ -100,7 +100,7 @@ where /// See [`toposort`] for more information. /// /// Implements [Kahn's algorithm](https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm). -pub struct TopoSort { +pub struct TopoSort<'f, G, Map = BitVec> { graph: G, visited_ports: Map, /// A VecDeque is used for the node list to produce a canonical ordering, @@ -112,13 +112,13 @@ pub struct TopoSort { nodes_seen: usize, /// A filter closure for the nodes to visit. If the closure returns false, /// the node is skipped. - node_filter: Option bool>>, + node_filter: Option bool + 'f>>, /// A filter closure for the ports to visit. If the closure returns false, /// the port is skipped. - port_filter: Option bool>>, + port_filter: Option bool + 'f>>, } -impl TopoSort +impl<'f, Map, G> TopoSort<'f, G, Map> where Map: SecondaryMap, G: LinkView, @@ -131,8 +131,8 @@ where graph: G, source: impl IntoIterator, direction: Direction, - mut node_filter: Option bool>>, - port_filter: Option bool>>, + mut node_filter: Option bool + 'f>>, + port_filter: Option bool + 'f>>, ) -> Self { let mut visited_ports: Map = SecondaryMap::new(); @@ -179,11 +179,13 @@ where /// Checks if a node becomes ready once it is visited from `from_port`, i.e. /// it has been reached from all its linked ports. fn becomes_ready(&mut self, node: NodeIndex, from_port: impl Into) -> bool { + let from_port = from_port.into(); if self.ignore_node(node) { return false; } - self.graph.ports(node, self.direction.reverse()).all(|p| { - if p == from_port.into() { + let ports: Vec<_> = self.graph.ports(node, self.direction.reverse()).collect(); + ports.into_iter().all(|p| { + if p == from_port { // This port must have not been visited yet. Otherwise, the node // would have been already been reported as ready and added to // the candidate list. @@ -219,7 +221,7 @@ where } } -impl Iterator for TopoSort +impl<'f, Map, G> Iterator for TopoSort<'f, G, Map> where Map: SecondaryMap, G: LinkView, @@ -228,8 +230,9 @@ where fn next(&mut self) -> Option { let node = self.candidate_nodes.pop_front()?; + let ports = self.graph.ports(node, self.direction).collect::>(); - for port in self.graph.ports(node, self.direction) { + for port in ports { self.visited_ports.set(port, true); if self.ignore_port(node, port) { @@ -259,14 +262,14 @@ where } } -impl FusedIterator for TopoSort +impl<'f, Map, G> FusedIterator for TopoSort<'f, G, Map> where Map: SecondaryMap, G: LinkView, { } -impl Debug for TopoSort +impl<'f, Map, G> Debug for TopoSort<'f, G, Map> where Map: Debug, G: Debug, From 14f9b8b33ce0916eb0bf52dc12c37338d3973213 Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 08:59:49 +0200 Subject: [PATCH 4/7] Fix docs --- src/algorithms/toposort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/algorithms/toposort.rs b/src/algorithms/toposort.rs index 3820dd5..cabdf9b 100644 --- a/src/algorithms/toposort.rs +++ b/src/algorithms/toposort.rs @@ -2,7 +2,7 @@ use crate::{Direction, LinkView, NodeIndex, PortIndex, SecondaryMap}; use bitvec::prelude::BitVec; use std::{collections::VecDeque, fmt::Debug, iter::FusedIterator}; -/// Returns an iterator over a [`PortGraph`] in topological order. +/// Returns an iterator over a [`LinkView`] in topological order. /// /// The `Map` type parameter specifies the type of the secondary map that is /// used to store the dominator tree data. The default is [`BitVec`], which is From a496c90af78f427ace01400e02b2a0e401ce7af1 Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 09:02:30 +0200 Subject: [PATCH 5/7] Remove superfluous lifetime bounds --- src/algorithms/toposort.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/algorithms/toposort.rs b/src/algorithms/toposort.rs index cabdf9b..0d197d6 100644 --- a/src/algorithms/toposort.rs +++ b/src/algorithms/toposort.rs @@ -77,14 +77,14 @@ where /// ``` pub fn toposort_filtered<'f, Map, G>( graph: G, - source: impl IntoIterator + 'f, + source: impl IntoIterator, direction: Direction, node_filter: impl FnMut(NodeIndex) -> bool + 'f, port_filter: impl FnMut(NodeIndex, PortIndex) -> bool + 'f, ) -> TopoSort<'f, G, Map> where Map: SecondaryMap, - G: LinkView + 'f, + G: LinkView, { TopoSort::new( graph, From f8a53db4e3c621bec060b5bed3d662fafdb450b9 Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 09:03:36 +0200 Subject: [PATCH 6/7] Fix docs (2) --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index afd41fc..03610bd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,7 @@ //! graph.link_ports(port_a, port_b).unwrap(); //! //! // Run a topological sort on the graph starting at node A. -//! let topo: TopoSort = toposort(&graph, [node_a], Direction::Outgoing); +//! let topo: TopoSort<_> = toposort(&graph, [node_a], Direction::Outgoing); //! assert_eq!(topo.collect::>(), [node_a, node_b]); //! ``` //! From 38d02f5aa20a58498d24ef04a6edc62987791bf7 Mon Sep 17 00:00:00 2001 From: Luca Mondada Date: Fri, 28 Jul 2023 09:29:04 +0200 Subject: [PATCH 7/7] Improve type params docs --- src/algorithms/toposort.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/algorithms/toposort.rs b/src/algorithms/toposort.rs index 0d197d6..18d2db3 100644 --- a/src/algorithms/toposort.rs +++ b/src/algorithms/toposort.rs @@ -4,6 +4,10 @@ use std::{collections::VecDeque, fmt::Debug, iter::FusedIterator}; /// Returns an iterator over a [`LinkView`] in topological order. /// +/// ## Type parameters +/// - `G`: The graph type implementing [`LinkView`], +/// - `Map`: Internal workspace for graph traversal (see below). +/// /// The `Map` type parameter specifies the type of the secondary map that is /// used to store the dominator tree data. The default is [`BitVec`], which is /// efficient for full graph traversal, i.e. when all nodes are reachable from @@ -26,13 +30,14 @@ use std::{collections::VecDeque, fmt::Debug, iter::FusedIterator}; /// let topo: TopoSort<_> = toposort(&graph, [node_a], Direction::Outgoing); /// assert_eq!(topo.collect::>(), [node_a, node_b]); /// ``` -pub fn toposort<'f, Map, G: LinkView>( +pub fn toposort( graph: G, source: impl IntoIterator, direction: Direction, -) -> TopoSort<'f, G, Map> +) -> TopoSort<'static, G, Map> where Map: SecondaryMap, + G: LinkView, { TopoSort::new(graph, source, direction, None, None) } @@ -43,6 +48,11 @@ where /// /// If the filter closures return false for a node or port, it is skipped. /// +/// ## Type parameters +/// - `'f`: The lifetime of the filter closures, +/// - `G`: The graph type implementing [`LinkView`], +/// - `Map`: Internal workspace for graph traversal (see below). +/// /// The `Map` type parameter specifies the type of the secondary map that is /// used to store the dominator tree data. The default is [`BitVec`], which is /// efficient for full graph traversal, i.e. when all nodes are reachable from @@ -75,7 +85,7 @@ where /// ); /// assert_eq!(topo.collect::>(), [node_a, node_b]); /// ``` -pub fn toposort_filtered<'f, Map, G>( +pub fn toposort_filtered<'f, G, Map>( graph: G, source: impl IntoIterator, direction: Direction,