Skip to content

Commit 7b6af51

Browse files
bnayahuclaude
andcommitted
Fix ConstrainedReschedule Rust port giving wrong node_start_time (#16186)
Three bugs in the Rust port of `ConstrainedReschedule` (PR #14883): 1. **Delay duration ignored when `target` is provided**: `push_node_back` computed `new_t1q` using `target.get_duration("delay", …)`, which returns `None` → `0` because Delay is not in the target gate table. The Delay end-time was therefore `this_t0 + 0` instead of `this_t0 + delay_duration`, causing a u64 underflow in the successor overlap calculation and silently setting the measure start time to a wrapped garbage value (160 dt instead of 272 dt). Fixed by always reading Delay duration from the instruction parameter first, falling back to the target only for non-Delay ops. 2. **`pulse_align`/`acquire_align` swapped in the `push_node_back` call**: `run_constrained_reschedule` passed the two alignment arguments in the wrong order. As a result `acquire_align` inside `push_node_back` received the pulse alignment value (1) and Measure/Reset were never shifted to the correct acquire-alignment boundary. Fixed by correcting the argument order. 3. **u64 underflow in overlap arithmetic**: `new_t1q - next_t0q` and `t1c - t0c` used plain subtraction; when the current node ends before the successor starts (no real overlap) this wraps to a large value and incorrectly shifts successors. Changed to `saturating_sub`. Also adds `is_measure_or_reset()` helper to consolidate the three Measure/Reset checks and to handle Python-level Measure/Reset operations (stored as `OperationRef::Instruction`) that do not match the `StandardInstruction::Measure/Reset` pattern. Adds regression tests in `test_scheduling_padding_pass.py`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1680baf commit 7b6af51

2 files changed

Lines changed: 122 additions & 41 deletions

File tree

crates/transpiler/src/passes/constrained_reschedule.rs

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ fn get_next_gate(dag: &DAGCircuit, node_index: NodeIndex) -> impl Iterator<Item
4343
.filter(|&idx| matches!(dag[idx], NodeType::Operation(_)))
4444
}
4545

46+
/// Returns true if the operation is a measure or reset, whether stored as a Rust
47+
/// StandardInstruction or as a Python-level Instruction.
48+
fn is_measure_or_reset(op_view: &OperationRef<'_>) -> bool {
49+
matches!(
50+
op_view,
51+
OperationRef::StandardInstruction(StandardInstruction::Measure)
52+
| OperationRef::StandardInstruction(StandardInstruction::Reset)
53+
) || matches!(op_view.name(), "measure" | "reset")
54+
}
55+
4656
/// Update the start time of the current node to satisfy alignment constraints.
4757
/// Immediate successors are pushed back to avoid overlap and will be processed later.
4858
///
@@ -74,19 +84,21 @@ fn push_node_back(
7484
};
7585

7686
let op_view = op.op.view();
77-
let alignment = match op_view {
78-
OperationRef::Gate(_) | OperationRef::StandardGate(_) => Some(pulse_align),
79-
OperationRef::StandardInstruction(StandardInstruction::Reset)
80-
| OperationRef::StandardInstruction(StandardInstruction::Measure) => Some(acquire_align),
81-
OperationRef::StandardInstruction(StandardInstruction::Delay(_)) => None,
82-
_ => {
83-
if !op_view.directive() {
84-
None
85-
} else {
86-
return Err(TranspilerError::new_err(format!(
87-
"Unknown operation type for '{}'.",
88-
op_view.name()
89-
)));
87+
let alignment = if is_measure_or_reset(&op_view) {
88+
Some(acquire_align)
89+
} else {
90+
match op_view {
91+
OperationRef::Gate(_) | OperationRef::StandardGate(_) => Some(pulse_align),
92+
OperationRef::StandardInstruction(StandardInstruction::Delay(_)) => None,
93+
_ => {
94+
if op_view.directive() {
95+
return Err(TranspilerError::new_err(format!(
96+
"Unknown operation type for '{}'.",
97+
op_view.name()
98+
)));
99+
} else {
100+
None
101+
}
90102
}
91103
}
92104
};
@@ -109,35 +121,33 @@ fn push_node_back(
109121
.or_insert(this_t0);
110122
}
111123

112-
let new_t1q = if let Some(target) = target {
113-
let qargs: Vec<PhysicalQubit> = dag
114-
.qargs_interner()
115-
.get(op.qubits)
116-
.iter()
117-
.map(|q| PhysicalQubit(q.index() as u32))
118-
.collect();
119-
let duration = target.get_duration(op.op.name(), &qargs).unwrap_or(0.0);
120-
this_t0 + duration as u64
121-
} else if matches!(
124+
let new_t1q = if matches!(
122125
op_view,
123126
OperationRef::StandardInstruction(StandardInstruction::Delay(_))
124127
) {
128+
// Delay is not in the target gate table, so always read the duration
129+
// from the instruction parameter regardless of whether target is present.
125130
let params = op.params_view();
126131
let param = params
127132
.first()
128133
.ok_or_else(|| PyValueError::new_err("Delay instruction missing duration parameter"))?;
129134
let duration = match param {
130-
Param::Obj(val) => {
131-
// Try to extract as different numeric types
132-
Python::attach(|py| val.bind(py).extract::<u64>())
133-
}
135+
Param::Obj(val) => Python::attach(|py| val.bind(py).extract::<u64>()),
134136
Param::Float(f) => Ok(*f as u64),
135137
_ => Err(TranspilerError::new_err(
136138
"The provided Delay duration is not in terms of dt.",
137139
)),
138140
}?;
139-
140141
this_t0 + duration
142+
} else if let Some(target) = target {
143+
let qargs: Vec<PhysicalQubit> = dag
144+
.qargs_interner()
145+
.get(op.qubits)
146+
.iter()
147+
.map(|q| PhysicalQubit(q.index() as u32))
148+
.collect();
149+
let duration = target.get_duration(op.op.name(), &qargs).unwrap_or(0.0);
150+
this_t0 + duration as u64
141151
} else {
142152
this_t0
143153
};
@@ -150,11 +160,7 @@ fn push_node_back(
150160
.collect();
151161

152162
// Handle classical bits based on operation type
153-
let (new_t1c, this_clbits) = if matches!(
154-
op_view,
155-
OperationRef::StandardInstruction(StandardInstruction::Measure)
156-
| OperationRef::StandardInstruction(StandardInstruction::Reset)
157-
) {
163+
let (new_t1c, this_clbits) = if is_measure_or_reset(&op_view) {
158164
// creg access ends at the end of instruction
159165
let new_t1c = Some(new_t1q);
160166
let this_clbits: HashSet<_> = dag
@@ -188,11 +194,7 @@ fn push_node_back(
188194
.collect();
189195

190196
let next_op_view = next_node.op.view();
191-
let (next_t0c, next_clbits) = if matches!(
192-
next_op_view,
193-
OperationRef::StandardInstruction(StandardInstruction::Measure)
194-
| OperationRef::StandardInstruction(StandardInstruction::Reset)
195-
) {
197+
let (next_t0c, next_clbits) = if is_measure_or_reset(&next_op_view) {
196198
// creg access starts after write latency
197199
let next_t0c = Some(next_t0q + clbit_write_latency as u64);
198200
let next_clbits: HashSet<_> = dag
@@ -208,7 +210,7 @@ fn push_node_back(
208210

209211
// Compute overlap if there is qubits overlap
210212
let qreg_overlap = if !this_qubits.is_disjoint(&next_qubits) {
211-
new_t1q - next_t0q
213+
new_t1q.saturating_sub(next_t0q)
212214
} else {
213215
0
214216
};
@@ -219,7 +221,7 @@ fn push_node_back(
219221
&& !this_clbits.is_disjoint(&next_clbits)
220222
{
221223
if let (Some(t1c), Some(t0c)) = (new_t1c, next_t0c) {
222-
t1c - t0c
224+
t1c.saturating_sub(t0c)
223225
} else {
224226
0
225227
}
@@ -292,8 +294,8 @@ pub fn run_constrained_reschedule(
292294
node_index,
293295
node_start_time,
294296
clbit_write_latency,
295-
acquire_align,
296297
pulse_align,
298+
acquire_align,
297299
target,
298300
)?;
299301
}

test/python/transpiler/test_scheduling_padding_pass.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
from qiskit.transpiler.passes import (
2424
ASAPScheduleAnalysis,
2525
ALAPScheduleAnalysis,
26+
ConstrainedReschedule,
2627
PadDelay,
28+
TimeUnitConversion,
2729
)
2830
from qiskit.transpiler.passmanager import PassManager
2931
from qiskit.transpiler.exceptions import TranspilerError
@@ -368,5 +370,82 @@ def test_respect_target_instruction_constraints(self, schedule_pass):
368370
self.assertEqual(qc, scheduled)
369371

370372

373+
class TestConstrainedReschedule(QiskitTestCase):
374+
"""Tests for ConstrainedReschedule."""
375+
376+
def _make_target(self, x_duration, measure_duration, acquire_alignment, pulse_alignment=1):
377+
"""Build a minimal Target with given durations and alignment constraints."""
378+
from qiskit.circuit.library import XGate
379+
from qiskit.circuit import Measure
380+
381+
target = Target(
382+
dt=1,
383+
acquire_alignment=acquire_alignment,
384+
pulse_alignment=pulse_alignment,
385+
)
386+
target.add_instruction(XGate(), {(0,): InstructionProperties(duration=x_duration)})
387+
target.add_instruction(Measure(), {(0,): InstructionProperties(duration=measure_duration)})
388+
return target
389+
390+
def test_delay_duration_respected_with_target(self):
391+
"""Regression test for #16186: ConstrainedReschedule must read Delay duration from the
392+
instruction parameter, not the target, because Delay is not in the target gate table.
393+
394+
Circuit: X(0) -> Delay(100 dt, 0) -> Measure(0)
395+
With X duration=160 dt, acquire_alignment=16:
396+
ALAP yields x=0, delay=160, measure=260.
397+
ConstrainedReschedule should push measure to 272 (next multiple of 16 above 260).
398+
Before the fix, the Rust port returned measure=160 due to Delay duration being read as 0.
399+
"""
400+
target = self._make_target(
401+
x_duration=160, measure_duration=800, acquire_alignment=16
402+
)
403+
404+
qc = QuantumCircuit(1, 1)
405+
qc.x(0)
406+
qc.delay(100, 0, unit="dt")
407+
qc.measure(0, 0)
408+
409+
pm = PassManager(
410+
[
411+
TimeUnitConversion(target.durations()),
412+
ALAPScheduleAnalysis(target.durations()),
413+
ConstrainedReschedule(target=target),
414+
]
415+
)
416+
pm.run(qc)
417+
418+
times = {n.op.name: t for n, t in pm.property_set["node_start_time"].items()}
419+
self.assertEqual(times["x"], 0)
420+
self.assertEqual(times["delay"], 160)
421+
# 260 is not a multiple of 16; next multiple is 272.
422+
self.assertEqual(times["measure"], 272)
423+
424+
def test_already_aligned_measure_unchanged(self):
425+
"""When the delay already ends on an alignment boundary, measure must not be shifted."""
426+
target = self._make_target(
427+
x_duration=160, measure_duration=800, acquire_alignment=16
428+
)
429+
430+
qc = QuantumCircuit(1, 1)
431+
qc.x(0)
432+
qc.delay(96, 0, unit="dt") # 160 + 96 = 256, which is 16*16 — already aligned
433+
qc.measure(0, 0)
434+
435+
pm = PassManager(
436+
[
437+
TimeUnitConversion(target.durations()),
438+
ALAPScheduleAnalysis(target.durations()),
439+
ConstrainedReschedule(target=target),
440+
]
441+
)
442+
pm.run(qc)
443+
444+
times = {n.op.name: t for n, t in pm.property_set["node_start_time"].items()}
445+
self.assertEqual(times["x"], 0)
446+
self.assertEqual(times["delay"], 160)
447+
self.assertEqual(times["measure"], 256)
448+
449+
371450
if __name__ == "__main__":
372451
unittest.main()

0 commit comments

Comments
 (0)