Skip to content

Commit 9c8f4fc

Browse files
raynelfssCryoris
raynelfss
and
Cryoris
authored
Fix: BasisTranslator processing of nested ControlFlowOp (#15875)
* Fix: `BasisTranslator` processing of `ControlFlowOp` - Remove the bool output `is_updated` from `apply_translation` helper method in `BasisTranslator`. The removal of this argument ensures that when a translation is found it is always applied. The `is_modified` boolean output, while intended as a way of optimizing the operation of the `BasisTranslator` did not achieve much as a new `DAGCircuit would be created each time. During the `ControlFlowOp` handling step of this method we relied on `is_updated` to determine whether to use the original `DAGCircuit` or not, which in the end would erroneously skip certain transformations in the `Operation`'s blocks. By removing this tracking value, which was already ignored by the main `BasisTranslator` process. We end up with a more correct processing of the instructions at hand. - Update test `test_inner_wire_map_control_op` from the `TestCircuitControlFlowOps` as better outcome is now found for said circuit. * Test: Add test - Inspired on [comment](#13162 (comment)) from @jakelishman. * Docs: Add release note * Address review comments: - Add new test using a backend and the full transpiler pipeline. - Add all issues to release note. - Small language fix. * Lint: formatting * Fix: Make new test use aer * Update releasenotes/notes/fix_basis_control_flow-033ecba233d5f9ed.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> --------- Co-authored-by: Julien Gacon <gaconju@gmail.com>
1 parent 3e80326 commit 9c8f4fc

4 files changed

Lines changed: 112 additions & 27 deletions

File tree

‎crates/transpiler/src/passes/basis_translator/mod.rs‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ pub fn run_basis_translator(
202202
})
203203
.collect::<Result<_, BasisTranslatorError>>()?;
204204

205-
let (out_dag, _) = apply_translation(
205+
let out_dag = apply_translation(
206206
dag,
207207
&new_target_basis,
208208
&instr_map,
@@ -334,8 +334,7 @@ fn apply_translation(
334334
min_qubits: usize,
335335
qargs_with_non_global_operation: &AhashIndexMap<Qargs, AhashIndexSet<&str>>,
336336
qarg_mapping: Option<&HashMap<Qubit, Qubit>>,
337-
) -> Result<(DAGCircuit, bool), BasisTranslatorError> {
338-
let mut is_updated = false;
337+
) -> Result<DAGCircuit, BasisTranslatorError> {
339338
let out_dag = dag
340339
.copy_empty_like(VarsMode::Alike, BlocksMode::Keep)
341340
.map_err(|_| {
@@ -372,7 +371,7 @@ fn apply_translation(
372371
.map(|(k, v)| (v, *k))
373372
.collect()
374373
};
375-
(updated_dag, is_updated) = apply_translation(
374+
updated_dag = apply_translation(
376375
dag_block,
377376
target_basis,
378377
instr_map,
@@ -381,12 +380,7 @@ fn apply_translation(
381380
qargs_with_non_global_operation,
382381
Some(&qarg_mapping),
383382
)?;
384-
let flow_block = if is_updated {
385-
updated_dag
386-
} else {
387-
dag_block.clone()
388-
};
389-
flow_blocks.push(out_dag_builder.add_block(flow_block));
383+
flow_blocks.push(out_dag_builder.add_block(updated_dag));
390384
}
391385
let new_instr = PackedInstruction::from_control_flow(
392386
node_obj.op.control_flow().clone(),
@@ -474,9 +468,8 @@ fn apply_translation(
474468
node_obj.op.name().to_string(),
475469
));
476470
}
477-
is_updated = true;
478471
}
479-
Ok((out_dag_builder.build(), is_updated))
472+
Ok(out_dag_builder.build())
480473
}
481474

482475
fn replace_node(

‎releasenotes/notes/fix_basis_control_flow-033ecba233d5f9ed.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
fixes:
2+
- |
3+
Fixed error in :class:`.BasisTranslator` in which a circuit with nested
4+
control flow operations would have its blocks skip the calculated
5+
transformations.
6+
7+
See `#13162 <https://github.com/Qiskit/qiskit/issues/13162>`__,
8+
`#14025 <https://github.com/Qiskit/qiskit/issues/14025>`__,
9+
and `#15734 <https://github.com/Qiskit/qiskit/issues/15734>`__.

‎test/python/transpiler/test_basis_translator.py‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"""Test the BasisTranslator pass"""
1515

1616
import os
17+
import unittest
1718

1819
from numpy import pi
1920
import scipy
@@ -37,7 +38,10 @@
3738
CXGate,
3839
RXGate,
3940
RZZGate,
41+
TGate,
42+
TdgGate,
4043
)
44+
from qiskit.circuit.controlflow import IfElseOp
4145
from qiskit.converters import circuit_to_dag, dag_to_circuit, circuit_to_instruction
4246
from qiskit.exceptions import QiskitError
4347
from qiskit.providers.fake_provider import GenericBackendV2
@@ -49,6 +53,7 @@
4953
from qiskit.circuit.library.standard_gates.equivalence_library import (
5054
StandardEquivalenceLibrary as std_eqlib,
5155
)
56+
from qiskit.utils.optionals import HAS_AER
5257
from test import QiskitTestCase
5358

5459

@@ -1231,3 +1236,81 @@ def test_fractional_gate_in_basis_from_custom_target(self):
12311236
pm = generate_preset_pass_manager(optimization_level=1, target=target, seed_transpiler=134)
12321237
cqc = pm.run(qc)
12331238
self.assertEqual(Operator(qc), Operator.from_circuit(cqc))
1239+
1240+
def test_basis_nested_control_flow_op(self):
1241+
"""Test nested handling of nested control flow operations under the basis translator"""
1242+
phi = Parameter("φ")
1243+
lam = Parameter("λ")
1244+
1245+
target = Target(num_qubits=3)
1246+
target.add_instruction(U2Gate(phi, lam))
1247+
target.add_instruction(IfElseOp, name="if_else")
1248+
target.add_instruction(TGate())
1249+
target.add_instruction(TdgGate())
1250+
target.add_instruction(CXGate())
1251+
1252+
qc = QuantumCircuit(3, 1)
1253+
1254+
with qc.if_test((0, False)) as else_:
1255+
pass
1256+
with else_:
1257+
with qc.if_test((0, False)) as else2:
1258+
qc.rccx(0, 1, 2)
1259+
with else2:
1260+
pass
1261+
1262+
transpiled = BasisTranslator(std_eqlib, target_basis=None, target=target)(qc)
1263+
1264+
expected_qc = QuantumCircuit(3, 1)
1265+
with expected_qc.if_test((0, False)) as else_:
1266+
pass
1267+
with else_:
1268+
with expected_qc.if_test((0, False)) as else2:
1269+
expected_qc.append(U2Gate(0, pi), [2])
1270+
expected_qc.t(2)
1271+
expected_qc.cx(1, 2)
1272+
expected_qc.tdg(2)
1273+
expected_qc.cx(0, 2)
1274+
expected_qc.t(2)
1275+
expected_qc.cx(1, 2)
1276+
expected_qc.tdg(2)
1277+
expected_qc.append(U2Gate(0, pi), [2])
1278+
with else2:
1279+
pass
1280+
1281+
self.assertEqual(transpiled, expected_qc)
1282+
1283+
@unittest.skipUnless(HAS_AER, "Aer backend required for simulation")
1284+
def test_basis_nested_control_flow_op_aer(self):
1285+
"""Test nested handling of nested control flow operations under the basis translator
1286+
using an Aer Simulator backend"""
1287+
from qiskit_aer import AerSimulator
1288+
1289+
backend = AerSimulator()
1290+
1291+
qc = QuantumCircuit(3, 1)
1292+
1293+
with qc.if_test((0, False)) as else_:
1294+
pass
1295+
with else_:
1296+
with qc.if_test((0, False)) as else2:
1297+
qc.dcx(0, 2)
1298+
with else2:
1299+
pass
1300+
1301+
pm = generate_preset_pass_manager(
1302+
optimization_level=1, backend=backend, seed_transpiler=134
1303+
)
1304+
transpiled = pm.run(qc)
1305+
1306+
expected = QuantumCircuit(3, 1)
1307+
with expected.if_test((0, False)) as else_:
1308+
pass
1309+
with else_:
1310+
with expected.if_test((0, False)) as else2:
1311+
expected.cx(0, 2)
1312+
expected.cx(2, 0)
1313+
with else2:
1314+
pass
1315+
1316+
self.assertEqual(transpiled, expected)

‎test/python/visualization/test_circuit_text_drawer.py‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4431,21 +4431,21 @@ def test_inner_wire_map_control_op(self):
44314431
"""Test that the gates inside ControlFlowOps land on correct qubits when transpiled"""
44324432
expected = "\n".join(
44334433
[
4434-
" ",
4435-
" qr_1 -> 0 ───────────────────────────────────────────────────",
4436-
" ",
4437-
"ancilla_0 -> 1 ───────────────────────────────────────────────────",
4438-
" ┌────── ┌────────┐┌────── ┌───┐ ───────┐ ───────┐ ",
4439-
" qr_0 -> 2 ┤ If-0 ┤ Rz(-π) ├┤ If-1 ┤ X ├ End-1 ├─ End-0 ├─",
4440-
" └──╥─── └────────┘└──╥─── └───┘ ───────┘ ───────┘ ",
4441-
"ancilla_1 -> 3 ───╫─────────────────╫─────────────────────────────",
4442-
" ║ ║ ",
4443-
"ancilla_2 -> 4 ───╫─────────────────╫─────────────────────────────",
4444-
" ║ ║ ",
4445-
" cr_0: ═══o═════════════════╬═════════════════════════════",
4446-
" ║ ║ ",
4447-
" cr_1: ═══■═════════════════■═════════════════════════════",
4448-
" 0x2 ",
4434+
" ",
4435+
" qr_1 -> 0 ──────────────────────────────────────────────────",
4436+
" ",
4437+
"ancilla_0 -> 1 ──────────────────────────────────────────────────",
4438+
" ┌────── ┌───────┐┌────── ┌───┐ ───────┐ ───────┐ ",
4439+
" qr_0 -> 2 ┤ If-0 ┤ Rz(π) ├┤ If-1 ┤ X ├ End-1 ├─ End-0 ├─",
4440+
" └──╥─── └───────┘└──╥─── └───┘ ───────┘ ───────┘ ",
4441+
"ancilla_1 -> 3 ───╫────────────────╫─────────────────────────────",
4442+
" ║ ║ ",
4443+
"ancilla_2 -> 4 ───╫────────────────╫─────────────────────────────",
4444+
" ║ ║ ",
4445+
" cr_0: ═══o════════════════╬═════════════════════════════",
4446+
" ║ ║ ",
4447+
" cr_1: ═══■════════════════■═════════════════════════════",
4448+
" 0x2 ",
44494449
]
44504450
)
44514451

0 commit comments

Comments
 (0)
⚡