Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/accelerate/src/sabre/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
34 changes: 30 additions & 4 deletions crates/accelerate/src/sabre/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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>`__.
25 changes: 22 additions & 3 deletions test/python/transpiler/test_sabre_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -312,6 +312,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."""
Expand Down
Loading