Fix ConstrainedReschedule Rust port giving wrong node_start_time (#16186)

crates/transpiler/src/passes/constrained_reschedule.rs

@@ -43,6 +43,16 @@ fn get_next_gate(dag: &DAGCircuit, node_index: NodeIndex) -> impl Iterator<Item
         .filter(|&idx| matches!(dag[idx], NodeType::Operation(_)))
 }
 
+/// Returns true if the operation is a measure or reset, whether stored as a Rust
+/// StandardInstruction or as a Python-level Instruction.
+fn is_measure_or_reset(op_view: &OperationRef<'_>) -> 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<PhysicalQubit> = 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::<u64>())
-            }
+            Param::Obj(val) => Python::attach(|py| val.bind(py).extract::<u64>()),
             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<PhysicalQubit> = 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,
         )?;
     }

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.

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()