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.

Sign up for GitHub

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

Generic trait for SecondaryMaps #51

Merged
merged 7 commits into from
May 30, 2023
Merged

Generic trait for SecondaryMaps #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3e7ffee
Generic trait for SecondaryMaps
aborgna-q May 25, 2023
a2fbad8
Fix docs
aborgna-q May 25, 2023
e30a4cf
Address comments
aborgna-q May 30, 2023
fe7b3d8
s/UnmanagedMap/UnmanagedDenseMap/
aborgna-q May 30, 2023
27cabfd
Drop `shrink_to` from `SecondaryMap`
aborgna-q May 30, 2023
c242e37
Check default value of the map in `toposort`
aborgna-q May 30, 2023
e0a9b98
Fix docs
aborgna-q May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 43 additions & 24 deletions src/algorithms/dominators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use super::postorder_filtered;
use crate::secondary::SecondaryMap;
use crate::{Direction, NodeIndex, PortGraph, PortIndex};
use crate::unmanaged::UnmanagedDenseMap;
use crate::{Direction, NodeIndex, PortGraph, PortIndex, SecondaryMap};
use std::cmp::Ordering;

/// Returns a dominator tree for a [`PortGraph`], where each node is dominated
/// by its parent.
///
/// The `Map` type parameter specifies the type of the secondary map that is used
/// to store the dominator tree data. The default is [`UnmanagedDenseMap`].
///
/// # Example
///
/// The following example runs the dominator algorithm on the following branching graph:
Expand All @@ -14,7 +17,7 @@ use std::cmp::Ordering;
/// ┗> d ┛
///
/// ```
/// # use portgraph::{algorithms::dominators, Direction, PortGraph};
/// # use portgraph::{algorithms::{dominators, DominatorTree}, Direction, PortGraph};
/// let mut graph = PortGraph::with_capacity(5, 10);
/// let a = graph.add_node(0,2);
/// let b = graph.add_node(1,1);
Expand All @@ -28,15 +31,22 @@ use std::cmp::Ordering;
/// graph.link_nodes(d, 0, c, 1).unwrap();
/// graph.link_nodes(c, 0, e, 0).unwrap();
///
/// let tree = dominators(&graph, a, Direction::Outgoing);
/// let tree: DominatorTree = dominators(&graph, a, Direction::Outgoing);
/// assert_eq!(tree.root(), a);
/// assert_eq!(tree.immediate_dominator(a), None);
/// assert_eq!(tree.immediate_dominator(b), Some(a));
/// assert_eq!(tree.immediate_dominator(c), Some(a));
/// assert_eq!(tree.immediate_dominator(d), Some(a));
/// assert_eq!(tree.immediate_dominator(e), Some(c));
/// ```
pub fn dominators(graph: &PortGraph, entry: NodeIndex, direction: Direction) -> DominatorTree {
pub fn dominators<Map>(
graph: &PortGraph,
entry: NodeIndex,
direction: Direction,
) -> DominatorTree<Map>
where
Map: SecondaryMap<NodeIndex, Option<NodeIndex>>,
{
DominatorTree::new(graph, entry, direction, |_| true, |_, _| true)
}

Expand All @@ -46,15 +56,18 @@ pub fn dominators(graph: &PortGraph, entry: NodeIndex, direction: Direction) ->
/// If the filter predicate returns `false` for a node or port, it is ignored
/// when computing the dominator tree.
///
/// The `Map` type parameter specifies the type of the secondary map that is
/// used to store the dominator tree data. The default is [`UnmanagedDenseMap`]. For
/// dominator trees over sparse node indices, `HashMap` or `BTreeMap` may be
/// more efficient.
///
/// # Example
///
/// This example runs the dominator algorithm on the following branching graph:
/// a ─┬> b ┐
/// │ ├─> c ─> e
/// f ─┴> d ┴────────^
/// a ─┬> b ┐ │ ├─> c ─> e f ─┴> d ┴────────^
///
/// ```
/// # use portgraph::{algorithms::dominators_filtered, Direction, PortGraph};
/// # use portgraph::{algorithms::{dominators_filtered, DominatorTree}, Direction, PortGraph};
/// let mut graph = PortGraph::with_capacity(5, 10);
/// let a = graph.add_node(0,2);
/// let b = graph.add_node(1,1);
Expand All @@ -71,7 +84,7 @@ pub fn dominators(graph: &PortGraph, entry: NodeIndex, direction: Direction) ->
/// graph.link_nodes(d, 1, e, 1).unwrap();
/// graph.link_nodes(f, 0, d, 1).unwrap();
///
/// let tree = dominators_filtered(
/// let tree: DominatorTree = dominators_filtered(
/// &graph,
/// a,
/// Direction::Outgoing,
Expand All @@ -86,26 +99,32 @@ pub fn dominators(graph: &PortGraph, entry: NodeIndex, direction: Direction) ->
/// assert_eq!(tree.immediate_dominator(e), Some(c));
/// assert_eq!(tree.immediate_dominator(f), None);
/// ```
pub fn dominators_filtered(
pub fn dominators_filtered<Map>(
graph: &PortGraph,
entry: NodeIndex,
direction: Direction,
node_filter: impl FnMut(NodeIndex) -> bool,
port_filter: impl FnMut(NodeIndex, PortIndex) -> bool,
) -> DominatorTree {
) -> DominatorTree<Map>
where
Map: SecondaryMap<NodeIndex, Option<NodeIndex>>,
{
DominatorTree::new(graph, entry, direction, node_filter, port_filter)
}

/// A dominator tree for a [`PortGraph`].
///
/// See [`dominators`] for more information.
pub struct DominatorTree {
pub struct DominatorTree<Map = UnmanagedDenseMap<NodeIndex, Option<NodeIndex>>> {
root: NodeIndex,
/// The immediate dominator of each node.
idom: SecondaryMap<NodeIndex, Option<NodeIndex>>,
idom: Map,
}

impl DominatorTree {
impl<Map> DominatorTree<Map>
where
Map: SecondaryMap<NodeIndex, Option<NodeIndex>>,
{
fn new(
graph: &PortGraph,
entry: NodeIndex,
Expand All @@ -115,7 +134,7 @@ impl DominatorTree {
) -> Self {
// We traverse the graph in post order starting at the `entry` node.
// We associate each node that we encounter with its index within the traversal.
let mut node_to_index = SecondaryMap::with_capacity(graph.node_capacity());
let mut node_to_index = UnmanagedDenseMap::with_capacity(graph.node_capacity());
let mut index_to_node = Vec::with_capacity(graph.node_capacity());

for (index, node) in postorder_filtered(
Expand Down Expand Up @@ -178,11 +197,11 @@ impl DominatorTree {
}

// Translate into a secondary map with `NodeIndex`s.
let mut idom = SecondaryMap::with_capacity(graph.node_capacity());
let mut idom = Map::with_capacity(graph.node_capacity());

for (index, dominator) in dominators.into_iter().take(num_nodes - 1).enumerate() {
debug_assert_ne!(dominator, usize::MAX);
idom[index_to_node[index]] = Some(index_to_node[dominator]);
idom.set(index_to_node[index], Some(index_to_node[dominator]));
}

Self { root: entry, idom }
Expand All @@ -197,7 +216,7 @@ impl DominatorTree {
#[inline]
/// Returns the immediate dominator of a node.
pub fn immediate_dominator(&self, node: NodeIndex) -> Option<NodeIndex> {
self.idom[node]
*self.idom.get(node)
}
}

Expand Down Expand Up @@ -235,7 +254,7 @@ mod tests {
graph.link_nodes(c, 0, e, 0).unwrap();

// From `a`
let tree = dominators(&graph, a, Direction::Outgoing);
let tree: DominatorTree = dominators(&graph, a, Direction::Outgoing);
assert_eq!(tree.root(), a);
assert_eq!(tree.immediate_dominator(a), None);
assert_eq!(tree.immediate_dominator(b), Some(a));
Expand All @@ -244,7 +263,7 @@ mod tests {
assert_eq!(tree.immediate_dominator(e), Some(c));

// Backwards from `c`
let tree = dominators(&graph, c, Direction::Incoming);
let tree: DominatorTree = dominators(&graph, c, Direction::Incoming);
assert_eq!(tree.root(), c);
assert_eq!(tree.immediate_dominator(a), Some(c));
assert_eq!(tree.immediate_dominator(b), Some(c));
Expand Down Expand Up @@ -275,7 +294,7 @@ mod tests {
graph.link_nodes(f, 0, d, 1).unwrap();

// From `a`
let tree = dominators_filtered(
let tree: DominatorTree = dominators_filtered(
&graph,
a,
Direction::Outgoing,
Expand All @@ -291,7 +310,7 @@ mod tests {
assert_eq!(tree.immediate_dominator(f), None);

// Backwards from `c`
let tree = dominators(&graph, c, Direction::Incoming);
let tree: DominatorTree = dominators(&graph, c, Direction::Incoming);
assert_eq!(tree.root(), c);
assert_eq!(tree.immediate_dominator(a), Some(c));
assert_eq!(tree.immediate_dominator(b), Some(c));
Expand Down Expand Up @@ -323,7 +342,7 @@ mod tests {
graph.link_nodes(d, 1, e, 1).unwrap();
graph.link_nodes(e, 0, d, 2).unwrap();

let dominators = dominators(&graph, entry, Direction::Outgoing);
let dominators: DominatorTree = dominators(&graph, entry, Direction::Outgoing);

assert_eq!(dominators.root(), entry);
assert_eq!(dominators.immediate_dominator(entry), None);
Expand Down
72 changes: 48 additions & 24 deletions src/algorithms/toposort.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use crate::{Direction, NodeIndex, PortGraph, PortIndex};
use crate::{Direction, NodeIndex, PortGraph, PortIndex, SecondaryMap};
use bitvec::prelude::BitVec;
use std::{collections::VecDeque, iter::FusedIterator};

/// Returns an iterator over a [`PortGraph`] in topological order.
///
/// Optimized for full graph traversal, i.e. when all nodes are reachable from the source nodes.
/// It uses O(n) memory, where n is the number of ports in the graph.
/// 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
/// the source nodes. For sparse traversals, `HashMap` or `BTreeMap` may be more
/// efficient.
///
/// Implements [Kahn's algorithm](https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm).
///
Expand All @@ -22,11 +25,14 @@ use std::{collections::VecDeque, iter::FusedIterator};
/// let topo = toposort(&graph, [node_a], Direction::Outgoing);
/// assert_eq!(topo.collect::<Vec<_>>(), [node_a, node_b]);
/// ```
pub fn toposort(
pub fn toposort<Map>(
graph: &PortGraph,
source: impl IntoIterator<Item = NodeIndex>,
direction: Direction,
) -> TopoSort {
) -> TopoSort<'_, Map>
where
Map: SecondaryMap<PortIndex, bool>,
{
TopoSort::new(graph, source, direction, None, None)
}

Expand All @@ -36,9 +42,11 @@ pub fn toposort(
///
/// If the filter closures return false for a node or port, it is skipped.
///
/// Optimized for full graph traversal, i.e. when all nodes are reachable from
/// the source nodes. It uses O(n) memory, where n is the number of ports in the
/// graph.
/// 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
/// the source nodes. For sparse traversals, `HashMap` or `BTreeMap` may be more
/// efficient.
///
/// Implements [Kahn's
/// algorithm](https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm).
Expand Down Expand Up @@ -66,13 +74,16 @@ pub fn toposort(
/// );
/// assert_eq!(topo.collect::<Vec<_>>(), [node_a, node_b]);
/// ```
pub fn toposort_filtered<'graph>(
pub fn toposort_filtered<'graph, Map>(
graph: &'graph PortGraph,
source: impl IntoIterator<Item = NodeIndex>,
direction: Direction,
node_filter: impl FnMut(NodeIndex) -> bool + 'graph,
port_filter: impl FnMut(NodeIndex, PortIndex) -> bool + 'graph,
) -> TopoSort {
) -> TopoSort<'_, Map>
where
Map: SecondaryMap<PortIndex, bool>,
{
TopoSort::new(
graph,
source,
Expand All @@ -87,9 +98,9 @@ pub fn toposort_filtered<'graph>(
/// See [`toposort`] for more information.
///
/// Implements [Kahn's algorithm](https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm).
pub struct TopoSort<'graph> {
pub struct TopoSort<'graph, Map = BitVec> {
graph: &'graph PortGraph,
remaining_ports: BitVec,
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.
candidate_nodes: VecDeque<NodeIndex>,
Expand All @@ -105,35 +116,46 @@ pub struct TopoSort<'graph> {
port_filter: Option<Box<dyn FnMut(NodeIndex, PortIndex) -> bool + 'graph>>,
}

impl<'graph> TopoSort<'graph> {
impl<'graph, Map> TopoSort<'graph, Map>
where
Map: SecondaryMap<PortIndex, bool>,
{
/// 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,
source: impl IntoIterator<Item = NodeIndex>,
direction: Direction,
mut node_filter: Option<Box<dyn FnMut(NodeIndex) -> bool + 'graph>>,
port_filter: Option<Box<dyn FnMut(NodeIndex, PortIndex) -> bool + 'graph>>,
) -> Self {
let mut remaining_ports = BitVec::with_capacity(graph.port_capacity());
remaining_ports.resize(graph.port_capacity(), true);
let mut visited_ports: Map = SecondaryMap::new();

let candidate_nodes: VecDeque<_> = if let Some(node_filter) = node_filter.as_mut() {
source.into_iter().filter(|&n| node_filter(n)).collect()
} else {
source.into_iter().collect()
};

// If the default value of `Map` is not `false`, we must mark all ports as not visited.
if visited_ports.default_value() {
for port in graph.ports_iter() {
visited_ports.set(port, false);
}
}

// Mark all the candidate ports as visited, so we don't visit them again.
for node in candidate_nodes.iter() {
for port in graph.ports(*node, direction.reverse()) {
remaining_ports.set(port.index(), false);
visited_ports.set(port, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an assumption here that the default value of Map is always false or is that guranteed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is part of the definition of SecondaryMap:

/// A map from keys to values that does not manage it's indices.
///
/// Querying a key that has not been set returns a default value.

(and default_value returns false for BitVec)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right but that doesn't stop me using a struct for Map that returns true for default_value right? Might need a debug assert at least?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the check to toposort. If the default value is true it initializes everything (and states so in the docs).

}
}

Self {
graph,
remaining_ports,
visited_ports,
candidate_nodes,
direction,
nodes_seen: 0,
Expand All @@ -144,8 +166,10 @@ impl<'graph> TopoSort<'graph> {

/// 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 ExactSizeIterator<Item = PortIndex> + '_ {
self.remaining_ports.iter_ones().map(PortIndex::new)
pub fn ports_remaining(&self) -> impl DoubleEndedIterator<Item = PortIndex> + '_ {
self.graph
.ports_iter()
.filter(move |&p| !self.visited_ports.get(p))
}

/// Checks if a node becomes ready once it is visited from `from_port`, i.e.
Expand All @@ -159,12 +183,12 @@ impl<'graph> TopoSort<'graph> {
// 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.
self.remaining_ports[p.index()]
} else if !self.remaining_ports[p.index()] {
!self.visited_ports.get(p)
} else if *self.visited_ports.get(p) {
true
} else if self.graph.port_link(p).is_none() || self.ignore_port(node, p) {
// If the port is not linked or should be ignored, mark it as visited.
self.remaining_ports.set(p.index(), false);
self.visited_ports.set(p, true);
true
} else {
false
Expand Down Expand Up @@ -198,7 +222,7 @@ impl<'graph> Iterator for TopoSort<'graph> {
let node = self.candidate_nodes.pop_front()?;

for port in self.graph.ports(node, self.direction) {
self.remaining_ports.set(port.index(), false);
self.visited_ports.set(port.index(), true);

if self.ignore_port(node, port) {
continue;
Expand All @@ -210,7 +234,7 @@ impl<'graph> Iterator for TopoSort<'graph> {
if self.becomes_ready(target, link) {
self.candidate_nodes.push_back(target);
}
self.remaining_ports.set(link.index(), false);
self.visited_ports.set(link.index(), true);
}
}

Expand Down
Loading