Fix an edge case in Sabre's release valve (#13114)

* Fix edge case in Sabre release valve

If a Sabre trial does not find a set of Swaps to route nodes, the "release valve" adds Swaps to route the two-qubit gate in between the the closest two qubits. In rare cases, this leads to _more_ than one gate being routable, which was not handled correctly previously.

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

* add reno and test

* Use sensible syntax

Co-authored-by: Kevin Hartman <kevin@hart.mn>

* Use sensible grammar

Co-authored-by: Kevin Hartman <kevin@hart.mn>

* clippy

* Check if the new test breaks CI

* Revert "Check if the new test breaks CI"

This reverts commit 01bcdc7.

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Kevin Hartman <kevin@hart.mn>

Author: 3 people

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;
     }

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() {

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 <https://github.com/Qiskit/qiskit/issues/13081>`__.

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."""