diff --git a/crates/transpiler/src/passes/constrained_reschedule.rs b/crates/transpiler/src/passes/constrained_reschedule.rs index 04f356f71a18..749a8015109b 100644 --- a/crates/transpiler/src/passes/constrained_reschedule.rs +++ b/crates/transpiler/src/passes/constrained_reschedule.rs @@ -43,6 +43,16 @@ fn get_next_gate(dag: &DAGCircuit, node_index: NodeIndex) -> impl Iterator) -> bool { + matches!( + op_view, + OperationRef::StandardInstruction(StandardInstruction::Measure) + | OperationRef::StandardInstruction(StandardInstruction::Reset) + ) || matches!(op_view.name(), "measure" | "reset") +} + /// Update the start time of the current node to satisfy alignment constraints. /// Immediate successors are pushed back to avoid overlap and will be processed later. /// @@ -74,19 +84,21 @@ fn push_node_back( }; let op_view = op.op.view(); - let alignment = match op_view { - OperationRef::Gate(_) | OperationRef::StandardGate(_) => Some(pulse_align), - OperationRef::StandardInstruction(StandardInstruction::Reset) - | OperationRef::StandardInstruction(StandardInstruction::Measure) => Some(acquire_align), - OperationRef::StandardInstruction(StandardInstruction::Delay(_)) => None, - _ => { - if !op_view.directive() { - None - } else { - return Err(TranspilerError::new_err(format!( - "Unknown operation type for '{}'.", - op_view.name() - ))); + let alignment = if is_measure_or_reset(&op_view) { + Some(acquire_align) + } else { + match op_view { + OperationRef::Gate(_) | OperationRef::StandardGate(_) => Some(pulse_align), + OperationRef::StandardInstruction(StandardInstruction::Delay(_)) => None, + _ => { + if op_view.directive() { + return Err(TranspilerError::new_err(format!( + "Unknown operation type for '{}'.", + op_view.name() + ))); + } else { + None + } } } }; @@ -109,35 +121,33 @@ fn push_node_back( .or_insert(this_t0); } - let new_t1q = if let Some(target) = target { - let qargs: Vec = dag - .qargs_interner() - .get(op.qubits) - .iter() - .map(|q| PhysicalQubit(q.index() as u32)) - .collect(); - let duration = target.get_duration(op.op.name(), &qargs).unwrap_or(0.0); - this_t0 + duration as u64 - } else if matches!( + let new_t1q = if matches!( op_view, OperationRef::StandardInstruction(StandardInstruction::Delay(_)) ) { + // Delay is not in the target gate table, so always read the duration + // from the instruction parameter regardless of whether target is present. let params = op.params_view(); let param = params .first() .ok_or_else(|| PyValueError::new_err("Delay instruction missing duration parameter"))?; let duration = match param { - Param::Obj(val) => { - // Try to extract as different numeric types - Python::attach(|py| val.bind(py).extract::()) - } + Param::Obj(val) => Python::attach(|py| val.bind(py).extract::()), Param::Float(f) => Ok(*f as u64), _ => Err(TranspilerError::new_err( "The provided Delay duration is not in terms of dt.", )), }?; - this_t0 + duration + } else if let Some(target) = target { + let qargs: Vec = dag + .qargs_interner() + .get(op.qubits) + .iter() + .map(|q| PhysicalQubit(q.index() as u32)) + .collect(); + let duration = target.get_duration(op.op.name(), &qargs).unwrap_or(0.0); + this_t0 + duration as u64 } else { this_t0 }; @@ -150,11 +160,7 @@ fn push_node_back( .collect(); // Handle classical bits based on operation type - let (new_t1c, this_clbits) = if matches!( - op_view, - OperationRef::StandardInstruction(StandardInstruction::Measure) - | OperationRef::StandardInstruction(StandardInstruction::Reset) - ) { + let (new_t1c, this_clbits) = if is_measure_or_reset(&op_view) { // creg access ends at the end of instruction let new_t1c = Some(new_t1q); let this_clbits: HashSet<_> = dag @@ -188,11 +194,7 @@ fn push_node_back( .collect(); let next_op_view = next_node.op.view(); - let (next_t0c, next_clbits) = if matches!( - next_op_view, - OperationRef::StandardInstruction(StandardInstruction::Measure) - | OperationRef::StandardInstruction(StandardInstruction::Reset) - ) { + let (next_t0c, next_clbits) = if is_measure_or_reset(&next_op_view) { // creg access starts after write latency let next_t0c = Some(next_t0q + clbit_write_latency as u64); let next_clbits: HashSet<_> = dag @@ -208,7 +210,7 @@ fn push_node_back( // Compute overlap if there is qubits overlap let qreg_overlap = if !this_qubits.is_disjoint(&next_qubits) { - new_t1q - next_t0q + new_t1q.saturating_sub(next_t0q) } else { 0 }; @@ -219,7 +221,7 @@ fn push_node_back( && !this_clbits.is_disjoint(&next_clbits) { if let (Some(t1c), Some(t0c)) = (new_t1c, next_t0c) { - t1c - t0c + t1c.saturating_sub(t0c) } else { 0 } @@ -292,8 +294,8 @@ pub fn run_constrained_reschedule( node_index, node_start_time, clbit_write_latency, - acquire_align, pulse_align, + acquire_align, target, )?; } diff --git a/releasenotes/notes/constrained-reschedule-fix-958c531d4114ca99.yaml b/releasenotes/notes/constrained-reschedule-fix-958c531d4114ca99.yaml new file mode 100644 index 000000000000..694336cc114e --- /dev/null +++ b/releasenotes/notes/constrained-reschedule-fix-958c531d4114ca99.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + Fixed three bugs in the Rust port of :class:`.ConstrainedReschedule` + that caused incorrect ``node_start_time`` values when a :class:`.Target` was + provided. First, the :class:`.Delay` duration was read from the target + gate table (where it is absent) instead of the instruction parameter, + producing a zero duration and corrupting successor start times via unsigned + integer underflow. Second, the ``pulse_align`` and ``acquire_align`` + arguments were passed in the wrong order to the inner scheduling function, + so :class:`.Measure` and :class:`.Reset` operations were never shifted to + the correct acquire-alignment boundary. Third, the qubit- and clbit-overlap + calculations used plain subtraction on unsigned integers, which wrapped to a + large value when there was no real overlap; changed to saturating subtraction. diff --git a/test/python/transpiler/test_scheduling_padding_pass.py b/test/python/transpiler/test_scheduling_padding_pass.py index 2b6c90571ff5..9b2cb5e9689e 100644 --- a/test/python/transpiler/test_scheduling_padding_pass.py +++ b/test/python/transpiler/test_scheduling_padding_pass.py @@ -23,7 +23,9 @@ from qiskit.transpiler.passes import ( ASAPScheduleAnalysis, ALAPScheduleAnalysis, + ConstrainedReschedule, PadDelay, + TimeUnitConversion, ) from qiskit.transpiler.passmanager import PassManager from qiskit.transpiler.exceptions import TranspilerError @@ -368,5 +370,82 @@ def test_respect_target_instruction_constraints(self, schedule_pass): self.assertEqual(qc, scheduled) +class TestConstrainedReschedule(QiskitTestCase): + """Tests for ConstrainedReschedule.""" + + def _make_target(self, x_duration, measure_duration, acquire_alignment, pulse_alignment=1): + """Build a minimal Target with given durations and alignment constraints.""" + from qiskit.circuit.library import XGate + from qiskit.circuit import Measure + + target = Target( + dt=1, + acquire_alignment=acquire_alignment, + pulse_alignment=pulse_alignment, + ) + target.add_instruction(XGate(), {(0,): InstructionProperties(duration=x_duration)}) + target.add_instruction(Measure(), {(0,): InstructionProperties(duration=measure_duration)}) + return target + + def test_delay_duration_respected_with_target(self): + """Regression test for #16186: ConstrainedReschedule must read Delay duration from the + instruction parameter, not the target, because Delay is not in the target gate table. + + Circuit: X(0) -> Delay(100 dt, 0) -> Measure(0) + With X duration=160 dt, acquire_alignment=16: + ALAP yields x=0, delay=160, measure=260. + ConstrainedReschedule should push measure to 272 (next multiple of 16 above 260). + Before the fix, the Rust port returned measure=160 due to Delay duration being read as 0. + """ + target = self._make_target( + x_duration=160, measure_duration=800, acquire_alignment=16 + ) + + qc = QuantumCircuit(1, 1) + qc.x(0) + qc.delay(100, 0, unit="dt") + qc.measure(0, 0) + + pm = PassManager( + [ + TimeUnitConversion(target.durations()), + ALAPScheduleAnalysis(target.durations()), + ConstrainedReschedule(target=target), + ] + ) + pm.run(qc) + + times = {n.op.name: t for n, t in pm.property_set["node_start_time"].items()} + self.assertEqual(times["x"], 0) + self.assertEqual(times["delay"], 160) + # 260 is not a multiple of 16; next multiple is 272. + self.assertEqual(times["measure"], 272) + + def test_already_aligned_measure_unchanged(self): + """When the delay already ends on an alignment boundary, measure must not be shifted.""" + target = self._make_target( + x_duration=160, measure_duration=800, acquire_alignment=16 + ) + + qc = QuantumCircuit(1, 1) + qc.x(0) + qc.delay(96, 0, unit="dt") # 160 + 96 = 256, which is 16*16 — already aligned + qc.measure(0, 0) + + pm = PassManager( + [ + TimeUnitConversion(target.durations()), + ALAPScheduleAnalysis(target.durations()), + ConstrainedReschedule(target=target), + ] + ) + pm.run(qc) + + times = {n.op.name: t for n, t in pm.property_set["node_start_time"].items()} + self.assertEqual(times["x"], 0) + self.assertEqual(times["delay"], 160) + self.assertEqual(times["measure"], 256) + + if __name__ == "__main__": unittest.main()