Skip to content

Commit

Permalink
chore!: Update to portgraph 0.13 / petgraph 0.7 (#1878)
Browse files Browse the repository at this point in the history
The new portgraph release comes with some perf improvements.
We require this update for #1869.

Closes #1872.

I added a benchmark variation for subgraphs with few nodes as a
drive-by.
This got improved by the new portgraph (skipping a full graph
traversal), but the runtime is still mostly the `O(n)` full-graph
traversal required for the convexity checking when building the
subgraph.

BREAKING CHANGE: Bumped public dependency `portgraph` to breaking
version `0.13`.
  • Loading branch information
aborgna-q authored Jan 20, 2025
1 parent 2d802e6 commit ad285f5
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 22 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ missing_docs = "warn"
debug_assert_with_mut_call = "warn"

[workspace.dependencies]
portgraph = { version = "0.12.2" }
portgraph = { version = "0.13.0" }
insta = { version = "1.34.0" }
bitvec = "1.0.1"
cgmath = "0.18.0"
Expand All @@ -47,7 +47,7 @@ jsonschema = "0.27.0"
lazy_static = "1.4.0"
num-rational = "0.4.1"
paste = "1.0"
petgraph = { version = "0.6.3", default-features = false }
petgraph = { version = "0.7.1", default-features = false }
proptest = "1.4.0"
proptest-derive = "0.5.0"
regex = "1.9.5"
Expand Down
4 changes: 2 additions & 2 deletions hugr-core/src/hugr/views/descendants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ where
let hugr = hugr.base_hugr();
Ok(Self {
root,
graph: RegionGraph::new_region(&hugr.graph, &hugr.hierarchy, root.pg_index()),
graph: RegionGraph::new(&hugr.graph, &hugr.hierarchy, root.pg_index()),
hugr,
_phantom: std::marker::PhantomData,
})
Expand Down Expand Up @@ -247,7 +247,7 @@ pub(super) mod test {
Some(Signature::new(vec![usize_t()], vec![usize_t()]))
);
assert_eq!(inner_region.node_count(), 3);
assert_eq!(inner_region.edge_count(), 2);
assert_eq!(inner_region.edge_count(), 1);
assert_eq!(inner_region.children(inner).count(), 2);
assert_eq!(inner_region.children(hugr.root()).count(), 0);
assert_eq!(
Expand Down
4 changes: 2 additions & 2 deletions hugr-core/src/hugr/views/sibling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'a, Root: NodeHandle> SiblingGraph<'a, Root> {
let hugr = hugr.base_hugr();
Self {
root,
graph: FlatRegionGraph::new_flat_region(&hugr.graph, &hugr.hierarchy, root.pg_index()),
graph: FlatRegionGraph::new(&hugr.graph, &hugr.hierarchy, root.pg_index()),
hugr,
_phantom: std::marker::PhantomData,
}
Expand Down Expand Up @@ -249,7 +249,7 @@ impl<'g, Root: NodeHandle> HugrInternals for SiblingMut<'g, Root> {
Root: 'p;

fn portgraph(&self) -> Self::Portgraph<'_> {
FlatRegionGraph::new_flat_region(
FlatRegionGraph::new(
&self.base_hugr().graph,
&self.base_hugr().hierarchy,
self.root.pg_index(),
Expand Down
52 changes: 36 additions & 16 deletions hugr-core/src/hugr/views/sibling_subgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use std::mem;

use itertools::Itertools;
use portgraph::algorithms::ConvexChecker;
use portgraph::boundary::Boundary;
use portgraph::{view::Subgraph, Direction, PortView};
use thiserror::Error;

Expand Down Expand Up @@ -179,14 +180,8 @@ impl SiblingSubgraph {
) -> Result<Self, InvalidSubgraph> {
let pg = hugr.portgraph();

let to_pg = |(n, p): (Node, Port)| {
pg.port_index(n.pg_index(), p.pg_offset())
.expect("invalid port")
};

// Ordering of the edges here is preserved and becomes ordering of the signature.
let subpg =
Subgraph::new_subgraph(pg.clone(), combine_in_out(&inputs, &outputs).map(to_pg));
let subpg = Subgraph::new_subgraph(pg.clone(), make_boundary(hugr, &inputs, &outputs));
let nodes = subpg.nodes_iter().map_into().collect_vec();
validate_subgraph(hugr, &nodes, &inputs, &outputs)?;

Expand Down Expand Up @@ -414,7 +409,7 @@ impl SiblingSubgraph {
.is_some_and(|s| s.port_type(p).is_some())
});

if combine_in_out(&vec![out_order_ports], &in_order_ports)
if iter_io(&vec![out_order_ports], &in_order_ports)
.any(|(n, p)| is_order_edge(&replacement, n, p))
{
unimplemented!("Found state order edges in replacement graph");
Expand Down Expand Up @@ -485,15 +480,40 @@ impl SiblingSubgraph {
}
}

fn combine_in_out<'a>(
/// Returns an iterator over the input ports.
fn iter_incoming(inputs: &IncomingPorts) -> impl Iterator<Item = (Node, IncomingPort)> + '_ {
inputs.iter().flat_map(|part| part.iter().copied())
}

/// Returns an iterator over the output ports.
fn iter_outgoing(outputs: &OutgoingPorts) -> impl Iterator<Item = (Node, OutgoingPort)> + '_ {
outputs.iter().copied()
}

/// Returns an iterator over both incoming and outgoing ports.
fn iter_io<'a>(
inputs: &'a IncomingPorts,
outputs: &'a OutgoingPorts,
) -> impl Iterator<Item = (Node, Port)> + 'a {
inputs
.iter()
.flatten()
.map(|(n, p)| (*n, (*p).into()))
.chain(outputs.iter().map(|(n, p)| (*n, (*p).into())))
iter_incoming(inputs)
.map(|(n, p)| (n, Port::from(p)))
.chain(iter_outgoing(outputs).map(|(n, p)| (n, Port::from(p))))
}

fn make_boundary<'a>(
hugr: &impl HugrView,
inputs: &'a IncomingPorts,
outputs: &'a OutgoingPorts,
) -> Boundary {
let to_pg_index = |n: Node, p: Port| {
hugr.portgraph()
.port_index(n.pg_index(), p.pg_offset())
.unwrap()
};
Boundary::new(
iter_incoming(inputs).map(|(n, p)| to_pg_index(n, p.into())),
iter_outgoing(outputs).map(|(n, p)| to_pg_index(n, p.into())),
)
}

/// Precompute convexity information for a HUGR.
Expand Down Expand Up @@ -591,11 +611,11 @@ fn validate_subgraph<H: HugrView>(
}

// Check there are no linked "other" ports
if combine_in_out(inputs, outputs).any(|(n, p)| is_order_edge(hugr, n, p)) {
if iter_io(inputs, outputs).any(|(n, p)| is_order_edge(hugr, n, p)) {
unimplemented!("Connected order edges not supported at the boundary")
}

let boundary_ports = combine_in_out(inputs, outputs).collect_vec();
let boundary_ports = iter_io(inputs, outputs).collect_vec();
// Check that the boundary ports are all in the subgraph.
if let Some(&(n, p)) = boundary_ports.iter().find(|(n, _)| !node_set.contains(n)) {
Err(InvalidSubgraphBoundary::PortNodeNotInSet(n, p))?;
Expand Down
30 changes: 30 additions & 0 deletions hugr/benches/benchmarks/subgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,35 @@ fn bench_singleton_subgraph(c: &mut Criterion) {
group.finish();
}

fn bench_fewnode_subgraph(c: &mut Criterion) {
let mut group = c.benchmark_group("fewnode_subgraph");
group.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));

let num_layers = [10, 100, 1000];

for layers in num_layers.into_iter() {
let (hugr, layer_ids) = circuit(layers);

// Get a subgraph with a fixed number of nodes in the middle of the circuit.
group.bench_with_input(
BenchmarkId::from_parameter(layers),
&layers,
|b, &_layers| {
// Pick all the nodes in four layers in the middle of the circuit.
let nodes: Vec<_> = layer_ids
.iter()
.skip(layers / 2)
.take(4)
.flat_map(|ids| [ids.h, ids.cx1, ids.cx2])
.collect();
b.iter(|| black_box(SiblingSubgraph::try_from_nodes(nodes.clone(), &hugr)))
},
);
}

group.finish();
}

fn bench_multinode_subgraph(c: &mut Criterion) {
let mut group = c.benchmark_group("multinode_subgraph");
group.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
Expand Down Expand Up @@ -59,5 +88,6 @@ criterion_group! {
config = Criterion::default();
targets =
bench_singleton_subgraph,
bench_fewnode_subgraph,
bench_multinode_subgraph,
}

0 comments on commit ad285f5

Please sign in to comment.