diff --git a/crates/accelerate/src/sabre/layer.rs b/crates/accelerate/src/sabre/layer.rs index 899321a96681..dec8253ad282 100644 --- a/crates/accelerate/src/sabre/layer.rs +++ b/crates/accelerate/src/sabre/layer.rs @@ -70,7 +70,10 @@ impl FrontLayer { pub fn remove(&mut self, index: &NodeIndex) { // The actual order in the indexmap doesn't matter as long as it's reproducible. // Swap-remove is more efficient than a full shift-remove. - let [a, b] = self.nodes.swap_remove(index).unwrap(); + let [a, b] = self + .nodes + .swap_remove(index) + .expect("Tried removing index that does not exist."); self.qubits[a.index()] = None; self.qubits[b.index()] = None; } diff --git a/crates/accelerate/src/sabre/route.rs b/crates/accelerate/src/sabre/route.rs index bef6d501b4aa..0fd594993943 100644 --- a/crates/accelerate/src/sabre/route.rs +++ b/crates/accelerate/src/sabre/route.rs @@ -27,6 +27,7 @@ use rustworkx_core::petgraph::prelude::*; use rustworkx_core::petgraph::visit::EdgeRef; use rustworkx_core::shortest_path::dijkstra; use rustworkx_core::token_swapper::token_swapper; +use smallvec::{smallvec, SmallVec}; use crate::getenv_use_multiple_threads; use crate::nlayout::{NLayout, PhysicalQubit}; @@ -286,7 +287,7 @@ impl<'a, 'b> RoutingState<'a, 'b> { fn force_enable_closest_node( &mut self, current_swaps: &mut Vec<[PhysicalQubit; 2]>, - ) -> NodeIndex { + ) -> SmallVec<[NodeIndex; 2]> { let (&closest_node, &qubits) = { let dist = &self.target.distance; self.front_layer @@ -328,7 +329,32 @@ impl<'a, 'b> RoutingState<'a, 'b> { current_swaps.push([shortest_path[end], shortest_path[end - 1]]); } current_swaps.iter().for_each(|&swap| self.apply_swap(swap)); - closest_node + + // If we apply a single swap it could be that we route 2 nodes; that is a setup like + // A - B - A - B + // and we swap the middle two qubits. This cannot happen if we apply 2 or more swaps. + if current_swaps.len() > 1 { + smallvec![closest_node] + } else { + // check if the closest node has neighbors that are now routable -- for that we get + // the other physical qubit that was swapped and check whether the node on it + // is now routable + let mut possible_other_qubit = current_swaps[0] + .iter() + // check if other nodes are in the front layer that are connected by this swap + .filter_map(|&swap_qubit| self.front_layer.qubits()[swap_qubit.index()]) + // remove the closest_node, which we know we already routed + .filter(|(node_index, _other_qubit)| *node_index != closest_node) + .map(|(_node_index, other_qubit)| other_qubit); + + // if there is indeed another candidate, check if that gate is routable + if let Some(other_qubit) = possible_other_qubit.next() { + if let Some(also_routed) = self.routable_node_on_qubit(other_qubit) { + return smallvec![closest_node, also_routed]; + } + } + smallvec![closest_node] + } } /// Return the swap of two virtual qubits that produces the best score of all possible swaps. @@ -573,14 +599,14 @@ pub fn swap_map_trial( } if routable_nodes.is_empty() { // If we exceeded the max number of heuristic-chosen swaps without making progress, - // unwind to the last progress point and greedily swap to bring a ndoe together. + // unwind to the last progress point and greedily swap to bring a node together. // Efficiency doesn't matter much; this path never gets taken unless we're unlucky. current_swaps .drain(..) .rev() .for_each(|swap| state.apply_swap(swap)); let force_routed = state.force_enable_closest_node(&mut current_swaps); - routable_nodes.push(force_routed); + routable_nodes.extend(force_routed); } state.update_route(&routable_nodes, current_swaps); if state.heuristic.decay.is_some() { diff --git a/releasenotes/notes/fix-sabre-releasevalve-7f9af9bfc0482e04.yaml b/releasenotes/notes/fix-sabre-releasevalve-7f9af9bfc0482e04.yaml new file mode 100644 index 000000000000..17432259be29 --- /dev/null +++ b/releasenotes/notes/fix-sabre-releasevalve-7f9af9bfc0482e04.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed an edge case in :class:`.SabreLayout`, where in rare cases on large + devices and challenging circuits, the routing would fail. This was due to the + release valve making more than one two-qubit gate routable, where only one was expected. + Fixed `#13081 `__. \ No newline at end of file diff --git a/test/python/transpiler/test_sabre_layout.py b/test/python/transpiler/test_sabre_layout.py index a5ebef43540f..7565ee17655f 100644 --- a/test/python/transpiler/test_sabre_layout.py +++ b/test/python/transpiler/test_sabre_layout.py @@ -18,16 +18,16 @@ from qiskit import QuantumRegister, QuantumCircuit from qiskit.circuit.classical import expr, types -from qiskit.circuit.library import EfficientSU2 +from qiskit.circuit.library import EfficientSU2, QuantumVolume from qiskit.transpiler import CouplingMap, AnalysisPass, PassManager -from qiskit.transpiler.passes import SabreLayout, DenseLayout, StochasticSwap +from qiskit.transpiler.passes import SabreLayout, DenseLayout, StochasticSwap, Unroll3qOrMore from qiskit.transpiler.exceptions import TranspilerError from qiskit.converters import circuit_to_dag from qiskit.compiler.transpiler import transpile from qiskit.providers.fake_provider import GenericBackendV2 from qiskit.transpiler.passes.layout.sabre_pre_layout import SabrePreLayout from qiskit.transpiler.preset_passmanagers import generate_preset_pass_manager -from test import QiskitTestCase # pylint: disable=wrong-import-order +from test import QiskitTestCase, slow_test # pylint: disable=wrong-import-order from ..legacy_cmaps import ALMADEN_CMAP, MUMBAI_CMAP @@ -315,6 +315,25 @@ def test_support_var_with_explicit_routing_pass(self): layout = pass_.property_set["layout"] self.assertEqual([layout[q] for q in qc.qubits], [2, 3, 4, 1, 5]) + @slow_test + def test_release_valve_routes_multiple(self): + """Test Sabre works if the release valve routes more than 1 operation. + + Regression test of #13081. + """ + qv = QuantumVolume(500, seed=42) + qv.measure_all() + qc = Unroll3qOrMore()(qv) + + cmap = CouplingMap.from_heavy_hex(21) + pm = PassManager( + [ + SabreLayout(cmap, swap_trials=20, layout_trials=20, max_iterations=4, seed=100), + ] + ) + _ = pm.run(qc) + self.assertIsNotNone(pm.property_set.get("layout")) + class DensePartialSabreTrial(AnalysisPass): """Pass to run dense layout as a sabre trial."""