From af81f9b1011e04367b3bf1368064e9cf43076075 Mon Sep 17 00:00:00 2001
From: Agustin Borgna <agustin.borgna@quantinuum.com>
Date: Thu, 19 Oct 2023 09:24:02 +0100
Subject: [PATCH 1/3] Add sparse convex check benchmarks
---
benches/benchmarks/convex.rs | 29 +++++++++++++++++++++++++----
src/algorithms/convex.rs | 10 +++++-----
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/benches/benchmarks/convex.rs b/benches/benchmarks/convex.rs
index 7daf59f..2dfa1aa 100644
--- a/benches/benchmarks/convex.rs
+++ b/benches/benchmarks/convex.rs
@@ -1,4 +1,5 @@
use criterion::{black_box, criterion_group, AxisScale, BenchmarkId, Criterion, PlotConfiguration};
+use itertools::Itertools;
use portgraph::{algorithms::ConvexChecker, PortView};
use super::generators::make_two_track_dag;
@@ -22,15 +23,15 @@ fn bench_convex_construction(c: &mut Criterion) {
/// We benchmark the worst case scenario, where the "subgraph" is the
/// entire graph itself.
-fn bench_convex(c: &mut Criterion) {
- let mut g = c.benchmark_group("Runtime convexity check");
+fn bench_convex_full(c: &mut Criterion) {
+ let mut g = c.benchmark_group("Runtime convexity check. Full graph.");
g.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
for size in [100, 1_000, 10_000] {
let graph = make_two_track_dag(size);
let mut checker = ConvexChecker::new(&graph);
g.bench_with_input(
- BenchmarkId::new("check_convexity", size),
+ BenchmarkId::new("check_convexity_full", size),
&size,
|b, _size| b.iter(|| black_box(checker.is_node_convex(graph.nodes_iter()))),
);
@@ -38,10 +39,30 @@ fn bench_convex(c: &mut Criterion) {
g.finish();
}
+/// We benchmark the an scenario where the size of the "subgraph" is sub-linear on the size of the graph.
+fn bench_convex_sparse(c: &mut Criterion) {
+ let mut g = c.benchmark_group("Runtime convexity check. Sparse subgraph on an n^2 size graph.");
+ g.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
+
+ for size in [100usize, 1_000, 5_000] {
+ let graph_size = size.pow(2);
+ let graph = make_two_track_dag(graph_size);
+ let mut checker = ConvexChecker::new(&graph);
+ let nodes = graph.nodes_iter().step_by(graph_size / size).collect_vec();
+ g.bench_with_input(
+ BenchmarkId::new("check_convexity_sparse", size),
+ &size,
+ |b, _size| b.iter(|| black_box(checker.is_node_convex(nodes.iter().copied()))),
+ );
+ }
+ g.finish();
+}
+
criterion_group! {
name = benches;
config = Criterion::default();
targets =
- bench_convex,
+ bench_convex_full,
+ bench_convex_sparse,
bench_convex_construction
}
diff --git a/src/algorithms/convex.rs b/src/algorithms/convex.rs
index 551de0a..c966b9a 100644
--- a/src/algorithms/convex.rs
+++ b/src/algorithms/convex.rs
@@ -14,14 +14,14 @@ use crate::{Direction, LinkView, NodeIndex, PortIndex, SecondaryMap, UnmanagedDe
use super::TopoSort;
-/// A pre-computed datastructure for fast convexity checking.
+/// A pre-computed data structure for fast convexity checking.
pub struct ConvexChecker<G> {
graph: G,
// The nodes in topological order
topsort_nodes: Vec<NodeIndex>,
// The index of a node in the topological order (the inverse of topsort_nodes)
topsort_ind: UnmanagedDenseMap<NodeIndex, usize>,
- // A temporary datastructure used during `is_convex`
+ // A temporary data structure used during `is_convex`
causal: CausalVec,
}
@@ -60,7 +60,7 @@ where
/// past and in the future of another node of the subgraph.
///
/// This function requires mutable access to `self` because it uses a
- /// temporary datastructure within the object.
+ /// temporary data structure within the object.
///
/// ## Arguments
///
@@ -111,7 +111,7 @@ where
/// Whether a subgraph is convex.
///
/// A subgraph is convex if there is no path between two nodes of the
- /// sugraph that has an edge outside of the subgraph.
+ /// subgraph that has an edge outside of the subgraph.
///
/// Equivalently, we check the following two conditions:
/// - There is no node that is both in the past and in the future of
@@ -119,7 +119,7 @@ where
/// - There is no edge from an output port to an input port.
///
/// This function requires mutable access to `self` because it uses a
- /// temporary datastructure within the object.
+ /// temporary data structure within the object.
///
/// ## Arguments
///
From 5ff42a4bfbddaa3bc6dead7ea9d7c36a03ecc37d Mon Sep 17 00:00:00 2001
From: Agustin Borgna <agustin.borgna@quantinuum.com>
Date: Thu, 19 Oct 2023 10:36:28 +0100
Subject: [PATCH 2/3] Reduce the number of nodes explored in is_node_convex
By using the toposorted order and keeping track
of nodes in the causal future.
---
src/algorithms/convex.rs | 108 +++++++++++++++------------------------
1 file changed, 40 insertions(+), 68 deletions(-)
diff --git a/src/algorithms/convex.rs b/src/algorithms/convex.rs
index c966b9a..3ad92a6 100644
--- a/src/algorithms/convex.rs
+++ b/src/algorithms/convex.rs
@@ -6,9 +6,6 @@
use std::collections::BTreeSet;
-use bitvec::bitvec;
-use bitvec::vec::BitVec;
-
use crate::algorithms::toposort;
use crate::{Direction, LinkView, NodeIndex, PortIndex, SecondaryMap, UnmanagedDenseMap};
@@ -21,8 +18,6 @@ pub struct ConvexChecker<G> {
topsort_nodes: Vec<NodeIndex>,
// The index of a node in the topological order (the inverse of topsort_nodes)
topsort_ind: UnmanagedDenseMap<NodeIndex, usize>,
- // A temporary data structure used during `is_convex`
- causal: CausalVec,
}
impl<G> ConvexChecker<G>
@@ -40,12 +35,10 @@ where
for (i, &n) in topsort_nodes.iter().enumerate() {
topsort_ind.set(n, i);
}
- let causal = CausalVec::new(topsort_nodes.len());
Self {
graph,
topsort_nodes,
topsort_ind,
- causal,
}
}
@@ -78,31 +71,53 @@ where
/// in some topological order. In the worst case this will traverse every
/// node in the graph and can be improved on in the future.
pub fn is_node_convex(&mut self, nodes: impl IntoIterator<Item = NodeIndex>) -> bool {
+ // The nodes in the subgraph, in topological order.
let nodes: BTreeSet<_> = nodes.into_iter().map(|n| self.topsort_ind[n]).collect();
+ if nodes.is_empty() {
+ return true;
+ }
+
+ // The range of considered nodes, as positions in the toposorted vector.
+ // Since the nodes are ordered, any node outside of this range will
+ // necessarily be outside the convex hull.
let min_ind = *nodes.first().unwrap();
let max_ind = *nodes.last().unwrap();
- for ind in min_ind..=max_ind {
- let n = self.topsort_nodes[ind];
- let mut in_inds = {
- let in_neighs = self.graph.input_neighbours(n);
- in_neighs
+ let node_range = min_ind..=max_ind;
+
+ let mut node_iter = nodes.iter().copied().peekable();
+
+ // Nodes in the causal future of `nodes` (inside `node_range`).
+ let mut other_nodes = BTreeSet::new();
+
+ loop {
+ if node_iter.peek().is_none() {
+ break;
+ }
+ if other_nodes.is_empty() || node_iter.peek() < other_nodes.first() {
+ let current = node_iter.next().unwrap();
+ let current_node = self.topsort_nodes[current];
+ for neighbour in self.graph.output_neighbours(current_node)
.map(|n| self.topsort_ind[n])
- .filter(|&ind| ind >= min_ind)
- };
- if nodes.contains(&ind) {
- if in_inds.any(|ind| self.causal.get(ind) == Causal::Future) {
- // There is a node in the past that is also in the future!
- return false;
+ .filter(|ind| node_range.contains(ind))
+ {
+ if !nodes.contains(&neighbour) {
+ other_nodes.insert(neighbour);
+ }
}
- self.causal.set(ind, Causal::Past);
} else {
- let ind_causal = match in_inds
- .any(|ind| nodes.contains(&ind) || self.causal.get(ind) == Causal::Future)
+ let current = other_nodes.pop_first().unwrap();
+ let current_node = self.topsort_nodes[current];
+ for neighbour in self.graph.output_neighbours(current_node)
+ .map(|n| self.topsort_ind[n])
+ .filter(|ind| node_range.contains(ind))
{
- true => Causal::Future,
- false => Causal::Past,
- };
- self.causal.set(ind, ind_causal);
+ if nodes.contains(&neighbour) {
+ // A non-subgraph node in the causal future of the subgraph has an output neighbour in the subgraph.
+ return false;
+ } else {
+ other_nodes.insert(neighbour);
+ }
+ }
}
}
true
@@ -148,49 +163,6 @@ where
}
}
-/// Whether a node is in the past or in the future of a subgraph.
-#[derive(Default, Clone, Debug, PartialEq, Eq)]
-enum Causal {
- #[default]
- Past,
- Future,
-}
-
-/// A memory-efficient substitute for `Vec<Causal>`.
-struct CausalVec(BitVec);
-
-impl From<bool> for Causal {
- fn from(b: bool) -> Self {
- match b {
- true => Self::Future,
- false => Self::Past,
- }
- }
-}
-
-impl From<Causal> for bool {
- fn from(c: Causal) -> Self {
- match c {
- Causal::Past => false,
- Causal::Future => true,
- }
- }
-}
-
-impl CausalVec {
- fn new(len: usize) -> Self {
- Self(bitvec![0; len])
- }
-
- fn set(&mut self, index: usize, causal: Causal) {
- self.0.set(index, causal.into());
- }
-
- fn get(&self, index: usize) -> Causal {
- self.0[index].into()
- }
-}
-
#[cfg(test)]
mod tests {
use crate::{LinkMut, NodeIndex, PortGraph, PortMut, PortView};
From 2c2dfa2defe51c0abbe064475a98950d224df243 Mon Sep 17 00:00:00 2001
From: Agustin Borgna <agustin.borgna@quantinuum.com>
Date: Thu, 19 Oct 2023 20:09:59 +0100
Subject: [PATCH 3/3] convexity checks no longer require &mut
---
benches/benchmarks/convex.rs | 4 ++--
src/algorithms/convex.rs | 20 ++++++++++++--------
src/view/subgraph.rs | 6 +++---
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/benches/benchmarks/convex.rs b/benches/benchmarks/convex.rs
index 2dfa1aa..237a658 100644
--- a/benches/benchmarks/convex.rs
+++ b/benches/benchmarks/convex.rs
@@ -29,7 +29,7 @@ fn bench_convex_full(c: &mut Criterion) {
for size in [100, 1_000, 10_000] {
let graph = make_two_track_dag(size);
- let mut checker = ConvexChecker::new(&graph);
+ let checker = ConvexChecker::new(&graph);
g.bench_with_input(
BenchmarkId::new("check_convexity_full", size),
&size,
@@ -47,7 +47,7 @@ fn bench_convex_sparse(c: &mut Criterion) {
for size in [100usize, 1_000, 5_000] {
let graph_size = size.pow(2);
let graph = make_two_track_dag(graph_size);
- let mut checker = ConvexChecker::new(&graph);
+ let checker = ConvexChecker::new(&graph);
let nodes = graph.nodes_iter().step_by(graph_size / size).collect_vec();
g.bench_with_input(
BenchmarkId::new("check_convexity_sparse", size),
diff --git a/src/algorithms/convex.rs b/src/algorithms/convex.rs
index 3ad92a6..b034139 100644
--- a/src/algorithms/convex.rs
+++ b/src/algorithms/convex.rs
@@ -70,7 +70,7 @@ where
/// that are in the interval between the first and last node of the subgraph
/// in some topological order. In the worst case this will traverse every
/// node in the graph and can be improved on in the future.
- pub fn is_node_convex(&mut self, nodes: impl IntoIterator<Item = NodeIndex>) -> bool {
+ pub fn is_node_convex(&self, nodes: impl IntoIterator<Item = NodeIndex>) -> bool {
// The nodes in the subgraph, in topological order.
let nodes: BTreeSet<_> = nodes.into_iter().map(|n| self.topsort_ind[n]).collect();
if nodes.is_empty() {
@@ -96,7 +96,9 @@ where
if other_nodes.is_empty() || node_iter.peek() < other_nodes.first() {
let current = node_iter.next().unwrap();
let current_node = self.topsort_nodes[current];
- for neighbour in self.graph.output_neighbours(current_node)
+ for neighbour in self
+ .graph
+ .output_neighbours(current_node)
.map(|n| self.topsort_ind[n])
.filter(|ind| node_range.contains(ind))
{
@@ -107,7 +109,9 @@ where
} else {
let current = other_nodes.pop_first().unwrap();
let current_node = self.topsort_nodes[current];
- for neighbour in self.graph.output_neighbours(current_node)
+ for neighbour in self
+ .graph
+ .output_neighbours(current_node)
.map(|n| self.topsort_ind[n])
.filter(|ind| node_range.contains(ind))
{
@@ -147,7 +151,7 @@ where
/// Any edge between two nodes of the subgraph that does not have an explicit
/// input or output port is considered within the subgraph.
pub fn is_convex(
- &mut self,
+ &self,
nodes: impl IntoIterator<Item = NodeIndex>,
inputs: impl IntoIterator<Item = PortIndex>,
outputs: impl IntoIterator<Item = PortIndex>,
@@ -197,7 +201,7 @@ mod tests {
#[test]
fn induced_convexity_test() {
let (g, [i1, i2, i3, n1, n2, o1, o2]) = graph();
- let mut checker = ConvexChecker::new(&g);
+ let checker = ConvexChecker::new(&g);
assert!(checker.is_node_convex([i1, i2, i3]));
assert!(checker.is_node_convex([i1, n2]));
@@ -212,7 +216,7 @@ mod tests {
#[test]
fn edge_convexity_test() {
let (g, [i1, i2, _, n1, n2, _, o2]) = graph();
- let mut checker = ConvexChecker::new(&g);
+ let checker = ConvexChecker::new(&g);
assert!(checker.is_convex(
[i1, n2],
@@ -245,7 +249,7 @@ mod tests {
fn dangling_input() {
let mut g = PortGraph::new();
let n = g.add_node(1, 1);
- let mut checker = ConvexChecker::new(&g);
+ let checker = ConvexChecker::new(&g);
assert!(checker.is_node_convex([n]));
}
@@ -254,7 +258,7 @@ mod tests {
let mut g = PortGraph::new();
let n = g.add_node(1, 1);
g.add_node(1, 1);
- let mut checker = ConvexChecker::new(&g);
+ let checker = ConvexChecker::new(&g);
assert!(checker.is_node_convex([n]));
}
}
diff --git a/src/view/subgraph.rs b/src/view/subgraph.rs
index c7ef184..cc82b43 100644
--- a/src/view/subgraph.rs
+++ b/src/view/subgraph.rs
@@ -96,12 +96,12 @@ where
/// Whether the subgraph is convex.
pub fn is_convex(&self) -> bool {
- let mut checker = ConvexChecker::new(self.graph());
- self.is_convex_with_checker(&mut checker)
+ let checker = ConvexChecker::new(self.graph());
+ self.is_convex_with_checker(&checker)
}
/// Whether the subgraph is convex, using a pre-existing checker.
- pub fn is_convex_with_checker(&self, checker: &mut ConvexChecker<G>) -> bool {
+ pub fn is_convex_with_checker(&self, checker: &ConvexChecker<G>) -> bool {
checker.is_convex(
self.nodes_iter(),
self.context().inputs.iter().copied(),