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 handling of conditions in SabreDAG creation #8727

Merged
merged 6 commits into from
Sep 16, 2022
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
7 changes: 6 additions & 1 deletion qiskit/transpiler/passes/routing/sabre_swap.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,16 @@ def run(self, dag):

dag_list = []
for node in dag.topological_op_nodes():
cargs = {self._clbit_indices[x] for x in node.cargs}
if node.op.condition is not None:
for clbit in dag._bits_in_condition(node.op.condition):
cargs.add(self._clbit_indices[clbit])

dag_list.append(
(
node._node_id,
[self._qubit_indices[x] for x in node.qargs],
[self._clbit_indices[x] for x in node.cargs],
cargs,
)
)
front_layer = np.asarray([x._node_id for x in dag.front_layer()], dtype=np.uintp)
Expand Down
10 changes: 5 additions & 5 deletions src/sabre_swap/sabre_dag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// copyright notice, and modified files need to carry a notice indicating
// that they have been altered from the originals.

use hashbrown::HashMap;
use hashbrown::{HashMap, HashSet};
use numpy::PyReadonlyArray1;
use pyo3::prelude::*;
use retworkx_core::petgraph::prelude::*;
Expand All @@ -23,7 +23,7 @@ use retworkx_core::petgraph::prelude::*;
#[pyo3(text_signature = "(num_qubits, num_clbits, nodes, front_layer, /)")]
#[derive(Clone, Debug)]
pub struct SabreDAG {
pub dag: DiGraph<(usize, Vec<usize>, Vec<usize>), ()>,
pub dag: DiGraph<(usize, Vec<usize>), ()>,
pub first_layer: Vec<NodeIndex>,
}

Expand All @@ -33,18 +33,18 @@ impl SabreDAG {
pub fn new(
num_qubits: usize,
num_clbits: usize,
nodes: Vec<(usize, Vec<usize>, Vec<usize>)>,
nodes: Vec<(usize, Vec<usize>, HashSet<usize>)>,
front_layer: PyReadonlyArray1<usize>,
) -> PyResult<Self> {
let mut qubit_pos: Vec<usize> = vec![usize::MAX; num_qubits];
let mut clbit_pos: Vec<usize> = vec![usize::MAX; num_clbits];
let mut reverse_index_map: HashMap<usize, NodeIndex> = HashMap::with_capacity(nodes.len());
let mut dag: DiGraph<(usize, Vec<usize>, Vec<usize>), ()> =
let mut dag: DiGraph<(usize, Vec<usize>), ()> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't need to track clbit order within the SabreDAG (which I think you're right - we don't need to), I don't think we need to track qubit order either - it's not important for the Sabre algorithm. That said, it's not really worth bothering about, and if doing it becomes a memory/performance concern in the future we can revisit it.

Graph::with_capacity(nodes.len(), 2 * nodes.len());
for node in &nodes {
let qargs = &node.1;
let cargs = &node.2;
let gate_index = dag.add_node(node.clone());
let gate_index = dag.add_node((node.0, qargs.clone()));
reverse_index_map.insert(node.0, gate_index);
for x in qargs {
if qubit_pos[*x] != usize::MAX {
Expand Down
45 changes: 45 additions & 0 deletions test/python/transpiler/test_sabre_swap.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,51 @@ def test_classical_condition(self):
actual = PassManager([TrivialLayout(cm), SabreSwap(cm)]).run(qc)
self.assertEqual(expected, actual)

def test_classical_condition_cargs(self):
"""Test that classical conditions are preserved even if missing from cargs DAGNode field.

Created from reproduction in https://github.com/Qiskit/qiskit-terra/issues/8675
"""
with self.subTest("missing measurement"):
qc = QuantumCircuit(3, 1)
qc.cx(0, 2).c_if(0, 0)
qc.measure(1, 0)
qc.h(2).c_if(0, 0)
expected = QuantumCircuit(3, 1)
expected.swap(1, 2)
expected.cx(0, 1).c_if(0, 0)
expected.measure(2, 0)
expected.h(1).c_if(0, 0)
result = SabreSwap(CouplingMap.from_line(3), seed=12345)(qc)
self.assertEqual(result, expected)
with self.subTest("reordered measurement"):
qc = QuantumCircuit(3, 1)
qc.cx(0, 1).c_if(0, 0)
qc.measure(1, 0)
qc.h(0).c_if(0, 0)
expected = QuantumCircuit(3, 1)
expected.cx(0, 1).c_if(0, 0)
expected.measure(1, 0)
expected.h(0).c_if(0, 0)
result = SabreSwap(CouplingMap.from_line(3), seed=12345)(qc)
self.assertEqual(result, expected)

def test_conditional_measurement(self):
"""Test that instructions with cargs and conditions are handled correctly."""
qc = QuantumCircuit(3, 2)
qc.cx(0, 2).c_if(0, 0)
qc.measure(2, 0).c_if(1, 0)
qc.h(2).c_if(0, 0)
qc.measure(1, 1)
expected = QuantumCircuit(3, 2)
expected.swap(1, 2)
expected.cx(0, 1).c_if(0, 0)
expected.measure(1, 0).c_if(1, 0)
expected.h(1).c_if(0, 0)
expected.measure(2, 1)
result = SabreSwap(CouplingMap.from_line(3), seed=12345)(qc)
self.assertEqual(result, expected)


if __name__ == "__main__":
unittest.main()