From 05de97ed2cc0164b2777a2e61ea11f62dcfa5640 Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 12:41:51 +0200 Subject: [PATCH 1/9] Fix possible overflow in line() constructor If nr_qubits = 0, the subtraction will wrap around and we get a panic because a Vec of usize::MAX can not be allocated. By changing to 1..nr_qubits the range will be empty if nr_qubits = 0 and hence edges will be [] without problems (and the i - 1 in the closure is also fine because i >= 1). --- src/architecture/connectivity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index f2f3c247..b429bad2 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -41,7 +41,7 @@ impl Connectivity { } pub fn line(nr_qubits: usize) -> Self { - let edges: Vec<(usize, usize)> = (0..nr_qubits - 1).map(|i| (i, i + 1)).collect(); + let edges: Vec<(usize, usize)> = (1..nr_qubits).map(|i| (i - 1, i)).collect(); Connectivity::from_edges(&edges) } From 45c16d3db665911527c97b0048ddb9e9eef0dbfe Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 13:01:58 +0200 Subject: [PATCH 2/9] Rename nr_qubits to num_qubits for consistency --- src/architecture/connectivity.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index b429bad2..54ef1645 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -40,8 +40,8 @@ impl Connectivity { } } - pub fn line(nr_qubits: usize) -> Self { - let edges: Vec<(usize, usize)> = (1..nr_qubits).map(|i| (i - 1, i)).collect(); + pub fn line(num_qubits: usize) -> Self { + let edges: Vec<(usize, usize)> = (1..num_qubits).map(|i| (i - 1, i)).collect(); Connectivity::from_edges(&edges) } From a985a2066eda8bdabb115c920ac38fe8843f2601 Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 13:02:31 +0200 Subject: [PATCH 3/9] Initialize edge vecs with capacity in constructors --- src/architecture/connectivity.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index 54ef1645..01460e01 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -46,7 +46,7 @@ impl Connectivity { } pub fn grid(num_rows: usize, num_cols: usize) -> Self { - let mut edges = Vec::new(); + let mut edges = Vec::with_capacity(2 * num_rows * num_cols); for r in 0..num_rows { for c in 0..num_cols { @@ -62,7 +62,7 @@ impl Connectivity { } pub fn complete(num_qubits: usize) -> Self { - let mut edges = Vec::new(); + let mut edges = Vec::with_capacity(num_qubits * num_qubits / 2); for i in 0..num_qubits { for j in (i + 1)..num_qubits { edges.push((i, j)); From d29dd51bd745f8ec4747bda4788028eaa947a52a Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 13:07:06 +0200 Subject: [PATCH 4/9] Fix possible infinite loop in path_from_shortest_path_tree The code while u != v { if let Some(x) = ... { } } will run forever if a None is encountered at a point. Breaking in that case doesn't make sense because we are returning a partial path without warning. Since we already checked that self.prev[u][v] is not None, we expect there exists a path from u to v and, hence, self.prev should also contain all nodes along this path. --- src/architecture/connectivity.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index 01460e01..b08ae935 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -145,17 +145,17 @@ impl Connectivity { } fn path_from_shortest_path_tree(&self, u: GraphIndex, mut v: GraphIndex) -> Vec { - let mut path = vec![v]; - if self.prev[u][v].is_none() { return Vec::new(); } + let mut path = vec![v]; while u != v { - if let Some(new_v) = self.prev[u][v] { - v = new_v; - path.push(v); - } + let Some(new_v) = self.prev[u][v] else { + panic!("broken path from {u} to {v}"); + }; + v = new_v; + path.push(v); } path.reverse(); From 5182e2ef34b8789d8ef3b7554a006c10d19498a5 Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 16:32:40 +0200 Subject: [PATCH 5/9] Reduce code duplication in graph update --- src/architecture/connectivity.rs | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index b08ae935..a1077cf5 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -116,15 +116,9 @@ impl Connectivity { } fn update(&mut self) { - let non_cutting = get_non_cutting_vertices(&self.graph); - - let (distance, prev) = floyd_warshall_path(&self.graph, |e| *e.weight()).unwrap(); - - let distance = distance.iter().map(|(k, v)| (*k, *v)).collect(); - - self.non_cutting = non_cutting; - self.distance = distance; - self.prev = prev; + let graph = std::mem::take(&mut self.graph); + let updated_self = Self::from_graph(graph); + *self = updated_self; } pub fn remove_node(&mut self, i: GraphIndex) { @@ -133,9 +127,7 @@ impl Connectivity { } pub fn add_edge(&mut self, i: GraphIndex, j: GraphIndex) { - self.graph - .add_edge(self.graph.from_index(i), self.graph.from_index(j), 1); - self.update(); + self.add_weighted_edge(i, j, 1); } pub fn add_weighted_edge(&mut self, i: GraphIndex, j: GraphIndex, weight: EdgeWeight) { @@ -258,17 +250,8 @@ impl Architecture for Connectivity { fn disconnect(&self, i: GraphIndex) -> Connectivity { let mut graph = self.graph.clone(); - graph.retain_nodes(|_, index| index.index() != i); - let non_cutting = get_non_cutting_vertices(&graph); - let (distance, prev) = floyd_warshall_path(&graph, |e| *e.weight()).unwrap(); - let distance = distance.into_iter().collect(); - - Connectivity { - graph, - non_cutting, - prev, - distance, - } + graph.remove_node(graph.from_index(i)); + Connectivity::from_graph(graph) } } From 8e889232d8a24032e9f6479f388115eafbe001d4 Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 16:33:37 +0200 Subject: [PATCH 6/9] Use petgraph from_index and .index() consistently --- src/architecture/connectivity.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index a1077cf5..5a1ff650 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -102,17 +102,10 @@ impl Connectivity { } pub fn edges(&self) -> Vec<(GraphIndex, GraphIndex)> { - let graph_edges: Vec<(GraphIndex, GraphIndex)> = self - .graph + self.graph .edge_references() - .map(|node| { - ( - self.graph.to_index(node.source()), - self.graph.to_index(node.target()), - ) - }) - .collect(); - graph_edges + .map(|node| (node.source().index(), node.target().index())) + .collect() } fn update(&mut self) { @@ -177,7 +170,7 @@ impl Architecture for Connectivity { j < self.graph.node_count(), "architecture does not contain node {j}" ); - self.distance[&(self.graph.from_index(i), self.graph.from_index(j))] as usize + self.distance[&(self.graph.from_index(i), self.graph.from_index(j))] } fn neighbors(&self, i: GraphIndex) -> Vec { @@ -186,7 +179,7 @@ impl Architecture for Connectivity { "architecture does not contain node {i}" ); self.graph - .neighbors(NodeIndex::new(i)) + .neighbors(self.graph.from_index(i)) .map(|neighbor| neighbor.index()) .collect() } @@ -241,7 +234,7 @@ impl Architecture for Connectivity { for neighbor in tree.neighbors(node) { if !visited.is_visited(&neighbor) { visited.visit(neighbor); - edge_list.push((self.graph.to_index(node), self.graph.to_index(neighbor))); + edge_list.push((node.index(), neighbor.index())); } } } From b83b848aa4feeb5a39b4b84dd7296f5c1d9be88e Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Tue, 26 Aug 2025 16:54:40 +0200 Subject: [PATCH 7/9] Iterate over graph nodes instead of simple Range Iterating over nodes seems to be the safer option in presence of removed nodes which could leave gaps in the node ids. --- src/architecture/connectivity.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index 5a1ff650..6a0c50df 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -15,8 +15,15 @@ fn get_non_cutting_vertices( graph: &StableUnGraph, ) -> Vec { let art_points = articulation_points(&graph); - (0..graph.node_count()) - .filter(|node| !art_points.contains(&graph.from_index(*node))) + graph + .node_indices() + .filter_map(|node| { + if !art_points.contains(&node) { + Some(node.index()) + } else { + None + } + }) .collect() } From 9f96c2162381ee1fea574ab81aa652b821b54b0b Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Wed, 27 Aug 2025 10:18:58 +0200 Subject: [PATCH 8/9] Use GraphIndex type alias more consistently --- src/architecture.rs | 4 ++-- src/architecture/connectivity.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/architecture.rs b/src/architecture.rs index aabb3d3b..b96a4c96 100644 --- a/src/architecture.rs +++ b/src/architecture.rs @@ -12,13 +12,13 @@ pub enum LadderError { pub trait Architecture { fn best_path(&self, i: GraphIndex, j: GraphIndex) -> Vec; - fn distance(&self, i: GraphIndex, j: GraphIndex) -> GraphIndex; + fn distance(&self, i: GraphIndex, j: GraphIndex) -> usize; fn neighbors(&self, i: GraphIndex) -> Vec; fn non_cutting(&mut self) -> &Vec; fn get_cx_ladder( &self, nodes: &[GraphIndex], root: &GraphIndex, - ) -> Result, LadderError>; + ) -> Result, LadderError>; fn disconnect(&self, i: GraphIndex) -> Self; } diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index 6a0c50df..d0f81e66 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -180,7 +180,7 @@ impl Architecture for Connectivity { self.distance[&(self.graph.from_index(i), self.graph.from_index(j))] } - fn neighbors(&self, i: GraphIndex) -> Vec { + fn neighbors(&self, i: GraphIndex) -> Vec { assert!( i < self.graph.node_count(), "architecture does not contain node {i}" @@ -200,7 +200,7 @@ impl Architecture for Connectivity { &self, nodes: &[GraphIndex], root: &GraphIndex, - ) -> Result, LadderError> { + ) -> Result, LadderError> { let mut nodes = nodes.to_vec(); let terminals: Vec<_> = self .graph From 0f044cd56e4c1c89aad0396015a9e6dfb7eae2ee Mon Sep 17 00:00:00 2001 From: Manuel Geiger <40306539+Ectras@users.noreply.github.com> Date: Wed, 27 Aug 2025 10:20:58 +0200 Subject: [PATCH 9/9] Use swap_remove to filter nodes in get_cx_ladder contains() and retain() were two passes over the array, pos() and swap_remove() only take one. Since we don't use the nodes afterwards (except for error reporting), order need not be preserved. --- src/architecture/connectivity.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/architecture/connectivity.rs b/src/architecture/connectivity.rs index d0f81e66..d6f0c963 100644 --- a/src/architecture/connectivity.rs +++ b/src/architecture/connectivity.rs @@ -1,4 +1,5 @@ use super::{Architecture, EdgeWeight, GraphIndex, LadderError, NodeWeight}; +use itertools::Itertools; use petgraph::algo::floyd_warshall::floyd_warshall_path; use petgraph::algo::steiner_tree::stable_steiner_tree; use petgraph::prelude::{EdgeRef, StableUnGraph}; @@ -202,18 +203,20 @@ impl Architecture for Connectivity { root: &GraphIndex, ) -> Result, LadderError> { let mut nodes = nodes.to_vec(); - let terminals: Vec<_> = self + let terminals = self .graph .node_references() .filter_map(|(node_index, _)| { - if nodes.contains(&node_index.index()) { - nodes.retain(|&x| x != node_index.index()); - Some(node_index) - } else { - None - } + // Try to remove node from `nodes` + nodes + .iter() + .position(|x| *x == node_index.index()) + .map(|pos| { + nodes.swap_remove(pos); + node_index + }) }) - .collect(); + .collect_vec(); if !nodes.is_empty() { return Err(LadderError::NodesNotFound(nodes));