Skip to content

Commit 6ce7788

Browse files
ElePTmtreinish
andauthored
Fix ignored errors/durations in generate_preset_pass_manager if dt is set (#14065)
* Fix dt oversight in generate_preset_pass_manager * Update warning to mention explicitly the invalidation of custom durations and error rates. * Do not overwrite original backend values inside transpile, iterate over target to reset internal instruction_durations value so that new dt can be taken into account * One iteration is enough to reset the value of self._instruction_durations * Estimate duration instead of hardcoding it * Apply suggestion from Matt's code review Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Fix example * Move cache invalidation to target dt setter. * Add reno --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
1 parent fbdf950 commit 6ce7788

5 files changed

Lines changed: 74 additions & 9 deletions

File tree

crates/accelerate/src/target_transpiler/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ pub(crate) struct Target {
150150
pub description: Option<String>,
151151
#[pyo3(get)]
152152
pub num_qubits: Option<usize>,
153-
#[pyo3(get, set)]
154153
pub dt: Option<f64>,
155154
#[pyo3(get, set)]
156155
pub granularity: u32,
@@ -741,6 +740,17 @@ impl Target {
741740

742741
// Instance attributes
743742

743+
/// The dt attribute.
744+
#[getter(_dt)]
745+
fn get_dt(&self) -> Option<f64> {
746+
self.dt
747+
}
748+
749+
#[setter(_dt)]
750+
fn set_dt(&mut self, dt: Option<f64>) {
751+
self.dt = dt
752+
}
753+
744754
/// The set of qargs in the target.
745755
#[getter]
746756
#[pyo3(name = "qargs")]

qiskit/transpiler/preset_passmanagers/generate_preset_pass_manager.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"""
1414
Preset pass manager generation function
1515
"""
16-
16+
import copy
1717
import warnings
1818

1919
from qiskit.circuit.controlflow import CONTROL_FLOW_OP_NAMES, get_control_flow_name_mapping
@@ -200,12 +200,14 @@ def generate_preset_pass_manager(
200200
# If there are no loose constraints => use backend target if available
201201
_no_loose_constraints = basis_gates is None and coupling_map is None and dt is None
202202

203+
# If the only loose constraint is dt => use backend target and modify dt
204+
_adjust_dt = backend is not None and dt is not None
205+
203206
# Warn about inconsistencies in backend + loose constraints path (dt shouldn't be a problem)
204207
if backend is not None and (coupling_map is not None or basis_gates is not None):
205208
warnings.warn(
206209
"Providing `coupling_map` and/or `basis_gates` along with `backend` is not "
207-
"recommended. This may introduce inconsistencies in the transpilation target, "
208-
"leading to potential errors.",
210+
"recommended, as this will invalidate the backend's gate durations and error rates.",
209211
category=UserWarning,
210212
stacklevel=2,
211213
)
@@ -226,13 +228,17 @@ def generate_preset_pass_manager(
226228
raise ValueError(
227229
f"Gates with 3 or more qubits ({gate}) in `basis_gates` or `backend` are "
228230
"incompatible with a custom `coupling_map`. To include 3-qubit or larger "
229-
" gates in the transpilation basis, use a custom `target` instance instead."
231+
" gates in the transpilation basis, provide a custom `target` instead."
230232
)
231233

232234
if target is None:
233235
if backend is not None and _no_loose_constraints:
234236
# If a backend is specified without loose constraints, use its target directly.
235237
target = backend.target
238+
elif _adjust_dt:
239+
# If a backend is specified with loose dt, use its target and adjust the dt value.
240+
target = copy.deepcopy(backend.target)
241+
target.dt = dt
236242
else:
237243
if basis_gates is not None:
238244
# Build target from constraints.
@@ -255,7 +261,7 @@ def generate_preset_pass_manager(
255261
dt=dt,
256262
)
257263

258-
# update loose constraints to populate pm options
264+
# Update loose constraints to populate pm options
259265
if coupling_map is None:
260266
coupling_map = target.build_coupling_map()
261267
if basis_gates is None and len(target.operation_names) > 0:

qiskit/transpiler/target.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,17 @@ def __init__(
270270
self._instruction_durations = None
271271
self._instruction_schedule_map = None
272272

273+
@property
274+
def dt(self):
275+
"""Return dt."""
276+
return self._dt
277+
278+
@dt.setter
279+
def dt(self, dt):
280+
"""Set dt and invalidate instruction duration cache"""
281+
self._dt = dt
282+
self._instruction_durations = None
283+
273284
def add_instruction(self, instruction, properties=None, name=None):
274285
"""Add a new instruction to the :class:`~qiskit.transpiler.Target`
275286
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
fixes:
3+
- |
4+
Fixed an oversight in the :class:`.Target` class where setting a new value for the ``dt`` attribute
5+
and subsequently calling ``target.durations()`` would not show the updated ``dt`` value in the returned
6+
:class:`.InstructionDurations` object. This is now fixed through an invalidation of the internal target
7+
instruction durations cache in the ``dt`` setter.

test/python/compiler/test_transpiler.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,12 +1677,13 @@ def test_scheduling_dt_constraints(self):
16771677
"""Test that scheduling-related loose transpile constraints
16781678
work with BackendV2."""
16791679

1680-
backend_v2 = GenericBackendV2(num_qubits=2, seed=1)
1680+
original_dt = 2.2222222222222221e-10
1681+
backend_v2 = GenericBackendV2(num_qubits=2, dt=original_dt, seed=3)
16811682
qc = QuantumCircuit(1, 1)
16821683
qc.x(0)
16831684
qc.measure(0, 0)
1684-
original_dt = 2.2222222222222221e-10
1685-
original_duration = 5059
1685+
scheduled = transpile(qc, backend=backend_v2, scheduling_method="asap")
1686+
original_duration = scheduled.duration
16861687

16871688
# halve dt in sec = double duration in dt
16881689
scheduled = transpile(qc, backend=backend_v2, scheduling_method="asap", dt=original_dt / 2)
@@ -2272,6 +2273,36 @@ def test_no_contraction_of_wires_in_routed_box(self, level):
22722273
active_indices = {qubit_map[bit] for instruction in body for bit in instruction.qubits}
22732274
self.assertNotIn(8, active_indices)
22742275

2276+
def test_custom_dt_preserves_properties(self):
2277+
"""Test that setting the `dt` parameter with a `backend` doesn't affect the target properties
2278+
and vf2 runs as expected.
2279+
"""
2280+
2281+
coupling_map = [[0, 1], [1, 0], [1, 2], [1, 3], [2, 1], [3, 1], [3, 4], [4, 3]]
2282+
backend = GenericBackendV2(
2283+
num_qubits=5,
2284+
basis_gates=["id", "sx", "x", "cx", "rz"],
2285+
coupling_map=coupling_map,
2286+
seed=0,
2287+
)
2288+
qubits = 3
2289+
qc = QuantumCircuit(qubits)
2290+
for i in range(5):
2291+
qc.cx(i % qubits, int(i + qubits / 2) % qubits)
2292+
2293+
# transpile with no gate errors
2294+
tqc_no_error = transpile(qc, coupling_map=coupling_map, seed_transpiler=4242)
2295+
# transpile with gate errors
2296+
tqc_no_dt = transpile(qc, backend=backend, seed_transpiler=4242)
2297+
# confirm that the output layouts are different
2298+
self.assertNotEqual(
2299+
tqc_no_dt.layout.final_index_layout(), tqc_no_error.layout.final_index_layout()
2300+
)
2301+
# now modify dt with gate errors
2302+
tqc_dt = transpile(qc, backend=backend, seed_transpiler=4242, dt=backend.dt * 2)
2303+
# confirm that dt doesn't affect layout
2304+
self.assertEqual(tqc_no_dt.layout.final_index_layout(), tqc_dt.layout.final_index_layout())
2305+
22752306

22762307
@ddt
22772308
class TestPostTranspileIntegration(QiskitTestCase):

0 commit comments

Comments
 (0)