Fix #7006 by tracking availability of clbits in scheduling (#7085)

* Add condition_bits to Instruction

* Fix a bug in schduling of circuits using clbits

* Fix small bugs, style and lint

* Update docstrings to tell the limitation of schedulers

* Add release note

* Update docstrings

* Replace with more robust assertions

* Update test/python/transpiler/test_instruction_alignments.py

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com>
Co-authored-by: Kevin Krsulich <kevin@krsulich.net>

qiskit/circuit/instruction.py

@@ -33,6 +33,7 @@
 import warnings
 import copy
 from itertools import zip_longest
+from typing import List
 
 import numpy
 
@@ -528,3 +529,13 @@ def repeat(self, n):
             qc.data = [(self, qargs[:], cargs[:])] * n
         instruction.definition = qc
         return instruction
+
+    @property
+    def condition_bits(self) -> List[Clbit]:
+        """Get Clbits in condition."""
+        if self.condition is None:
+            return []
+        if isinstance(self.condition[0], Clbit):
+            return [self.condition[0]]
+        else:  # ClassicalRegister
+            return list(self.condition[0])

qiskit/circuit/quantumcircuit.py

@@ -1978,12 +1978,8 @@ def num_connected_components(self, unitary_only: bool = False) -> int:
                 num_touched = 0
                 # Controls necessarily join all the cbits in the
                 # register that they use.
-                if instr.condition and not unitary_only:
-                    if isinstance(instr.condition[0], Clbit):
-                        condition_bits = [instr.condition[0]]
-                    else:
-                        condition_bits = instr.condition[0]
-                    for bit in condition_bits:
+                if not unitary_only:
+                    for bit in instr.condition_bits:
                         idx = bit_indices[bit]
                         for k in range(num_sub_graphs):
                             if idx in sub_graphs[k]:

qiskit/transpiler/passes/scheduling/alap.py

@@ -11,18 +11,29 @@
 # that they have been altered from the originals.
 
 """ALAP Scheduling."""
+import itertools
 from collections import defaultdict
 from typing import List
-from qiskit.transpiler.passes.scheduling.time_unit_conversion import TimeUnitConversion
-from qiskit.circuit.delay import Delay
+
+from qiskit.circuit import Delay, Measure
 from qiskit.circuit.parameterexpression import ParameterExpression
 from qiskit.dagcircuit import DAGCircuit
 from qiskit.transpiler.basepasses import TransformationPass
 from qiskit.transpiler.exceptions import TranspilerError
+from qiskit.transpiler.passes.scheduling.time_unit_conversion import TimeUnitConversion
 
 
 class ALAPSchedule(TransformationPass):
-    """ALAP Scheduling."""
+    """ALAP Scheduling pass, which schedules the **stop** time of instructions as late as possible.
+
+    For circuits with instructions writing or reading clbits (e.g. measurements, conditional gates),
+    the scheduler assumes clbits I/O operations take no time, ``measure`` locks clbits to be written
+    at its end and ``c_if`` locks clbits to be read at its beginning.
+
+    Notes:
+        The ALAP scheduler may not schedule a circuit exactly the same as any real backend does
+        when the circuit contains control flows (e.g. conditional instructions).
+    """
 
     def __init__(self, durations):
         """ALAPSchedule initializer.
@@ -58,6 +69,8 @@ def run(self, dag):
             new_dag.add_creg(creg)
 
         qubit_time_available = defaultdict(int)
+        clbit_readable = defaultdict(int)
+        clbit_writeable = defaultdict(int)
 
         def pad_with_delays(qubits: List[int], until, unit) -> None:
             """Pad idle time-slots in ``qubits`` with delays in ``unit`` until ``until``."""
@@ -68,11 +81,6 @@ def pad_with_delays(qubits: List[int], until, unit) -> None:
 
         bit_indices = {bit: index for index, bit in enumerate(dag.qubits)}
         for node in reversed(list(dag.topological_op_nodes())):
-            start_time = max(qubit_time_available[q] for q in node.qargs)
-            pad_with_delays(node.qargs, until=start_time, unit=time_unit)
-
-            new_dag.apply_operation_front(node.op, node.qargs, node.cargs)
-
             # validate node.op.duration
             if node.op.duration is None:
                 indices = [bit_indices[qarg] for qarg in node.qargs]
@@ -85,11 +93,32 @@ def pad_with_delays(qubits: List[int], until, unit) -> None:
                     f"Parameterized duration ({node.op.duration}) "
                     f"of {node.op.name} on qubits {indices} is not bounded."
                 )
+            # choose appropriate clbit available time depending on op
+            clbit_time_available = (
+                clbit_writeable if isinstance(node.op, Measure) else clbit_readable
+            )
+            # correction to change clbit start time to qubit start time
+            delta = 0 if isinstance(node.op, Measure) else node.op.duration
+            # must wait for op.condition_bits as well as node.cargs
+            start_time = max(
+                itertools.chain(
+                    (qubit_time_available[q] for q in node.qargs),
+                    (clbit_time_available[c] - delta for c in node.cargs + node.op.condition_bits),
+                )
+            )
+
+            pad_with_delays(node.qargs, until=start_time, unit=time_unit)
+
+            new_dag.apply_operation_front(node.op, node.qargs, node.cargs)
 
             stop_time = start_time + node.op.duration
             # update time table
             for q in node.qargs:
                 qubit_time_available[q] = stop_time
+            for c in node.cargs:  # measure
+                clbit_writeable[c] = clbit_readable[c] = start_time
+            for c in node.op.condition_bits:  # conditional op
+                clbit_writeable[c] = max(stop_time, clbit_writeable[c])
 
         working_qubits = qubit_time_available.keys()
         circuit_duration = max(qubit_time_available[q] for q in working_qubits)

qiskit/transpiler/passes/scheduling/asap.py

@@ -11,18 +11,29 @@
 # that they have been altered from the originals.
 
 """ASAP Scheduling."""
+import itertools
 from collections import defaultdict
 from typing import List
-from qiskit.transpiler.passes.scheduling.time_unit_conversion import TimeUnitConversion
-from qiskit.circuit.delay import Delay
+
+from qiskit.circuit import Delay, Measure
 from qiskit.circuit.parameterexpression import ParameterExpression
 from qiskit.dagcircuit import DAGCircuit
 from qiskit.transpiler.basepasses import TransformationPass
 from qiskit.transpiler.exceptions import TranspilerError
+from qiskit.transpiler.passes.scheduling.time_unit_conversion import TimeUnitConversion
 
 
 class ASAPSchedule(TransformationPass):
-    """ASAP Scheduling."""
+    """ASAP Scheduling pass, which schedules the start time of instructions as early as possible..
+
+    For circuits with instructions writing or reading clbits (e.g. measurements, conditional gates),
+    the scheduler assumes clbits I/O operations take no time, ``measure`` locks clbits to be written
+    at its end and ``c_if`` locks clbits to be read at its beginning.
+
+    Notes:
+        The ASAP scheduler may not schedule a circuit exactly the same as any real backend does
+        when the circuit contains control flows (e.g. conditional instructions).
+    """
 
     def __init__(self, durations):
         """ASAPSchedule initializer.
@@ -59,6 +70,8 @@ def run(self, dag):
             new_dag.add_creg(creg)
 
         qubit_time_available = defaultdict(int)
+        clbit_readable = defaultdict(int)
+        clbit_writeable = defaultdict(int)
 
         def pad_with_delays(qubits: List[int], until, unit) -> None:
             """Pad idle time-slots in ``qubits`` with delays in ``unit`` until ``until``."""
@@ -67,13 +80,8 @@ def pad_with_delays(qubits: List[int], until, unit) -> None:
                     idle_duration = until - qubit_time_available[q]
                     new_dag.apply_operation_back(Delay(idle_duration, unit), [q])
 
-        bit_indices = {bit: index for index, bit in enumerate(dag.qubits)}
+        bit_indices = {q: index for index, q in enumerate(dag.qubits)}
         for node in dag.topological_op_nodes():
-            start_time = max(qubit_time_available[q] for q in node.qargs)
-            pad_with_delays(node.qargs, until=start_time, unit=time_unit)
-
-            new_dag.apply_operation_back(node.op, node.qargs, node.cargs)
-
             # validate node.op.duration
             if node.op.duration is None:
                 indices = [bit_indices[qarg] for qarg in node.qargs]
@@ -86,11 +94,32 @@ def pad_with_delays(qubits: List[int], until, unit) -> None:
                     f"Parameterized duration ({node.op.duration}) "
                     f"of {node.op.name} on qubits {indices} is not bounded."
                 )
+            # choose appropriate clbit available time depending on op
+            clbit_time_available = (
+                clbit_writeable if isinstance(node.op, Measure) else clbit_readable
+            )
+            # correction to change clbit start time to qubit start time
+            delta = node.op.duration if isinstance(node.op, Measure) else 0
+            # must wait for op.condition_bits as well as node.cargs
+            start_time = max(
+                itertools.chain(
+                    (qubit_time_available[q] for q in node.qargs),
+                    (clbit_time_available[c] - delta for c in node.cargs + node.op.condition_bits),
+                )
+            )
+
+            pad_with_delays(node.qargs, until=start_time, unit=time_unit)
+
+            new_dag.apply_operation_back(node.op, node.qargs, node.cargs)
 
             stop_time = start_time + node.op.duration
             # update time table
             for q in node.qargs:
                 qubit_time_available[q] = stop_time
+            for c in node.cargs:  # measure
+                clbit_writeable[c] = clbit_readable[c] = stop_time
+            for c in node.op.condition_bits:  # conditional op
+                clbit_writeable[c] = max(start_time, clbit_writeable[c])
 
         working_qubits = qubit_time_available.keys()
         circuit_duration = max(qubit_time_available[q] for q in working_qubits)

qiskit/transpiler/passes/scheduling/instruction_alignment.py

@@ -11,7 +11,7 @@
 # that they have been altered from the originals.
 
 """Align measurement instructions."""
-
+import itertools
 import warnings
 from collections import defaultdict
 from typing import List, Union
@@ -20,8 +20,8 @@
 from qiskit.circuit.instruction import Instruction
 from qiskit.circuit.measure import Measure
 from qiskit.circuit.parameterexpression import ParameterExpression
-from qiskit.pulse import Play
 from qiskit.dagcircuit import DAGCircuit
+from qiskit.pulse import Play
 from qiskit.transpiler.basepasses import TransformationPass, AnalysisPass
 from qiskit.transpiler.exceptions import TranspilerError
 
@@ -133,8 +133,10 @@ def run(self, dag: DAGCircuit):
         # * pad_with_delay is called only with non-delay node to avoid consecutive delay
         new_dag = dag._copy_circuit_metadata()
 
-        qubit_time_available = defaultdict(int)
-        qubit_stop_times = defaultdict(int)
+        qubit_time_available = defaultdict(int)  # to track op start time
+        qubit_stop_times = defaultdict(int)  # to track delay start time for padding
+        clbit_readable = defaultdict(int)
+        clbit_writeable = defaultdict(int)
 
         def pad_with_delays(qubits: List[int], until, unit) -> None:
             """Pad idle time-slots in ``qubits`` with delays in ``unit`` until ``until``."""
@@ -144,25 +146,37 @@ def pad_with_delays(qubits: List[int], until, unit) -> None:
                     new_dag.apply_operation_back(Delay(idle_duration, unit), [q])
 
         for node in dag.topological_op_nodes():
-            start_time = max(qubit_time_available[q] for q in node.qargs)
+            # choose appropriate clbit available time depending on op
+            clbit_time_available = (
+                clbit_writeable if isinstance(node.op, Measure) else clbit_readable
+            )
+            # correction to change clbit start time to qubit start time
+            delta = node.op.duration if isinstance(node.op, Measure) else 0
+            start_time = max(
+                itertools.chain(
+                    (qubit_time_available[q] for q in node.qargs),
+                    (clbit_time_available[c] - delta for c in node.cargs + node.op.condition_bits),
+                )
+            )
 
             if isinstance(node.op, Measure):
                 if start_time % self.alignment != 0:
                     start_time = ((start_time // self.alignment) + 1) * self.alignment
 
-            if not isinstance(node.op, Delay):
+            if not isinstance(node.op, Delay):  # exclude delays for combining consecutive delays
                 pad_with_delays(node.qargs, until=start_time, unit=time_unit)
                 new_dag.apply_operation_back(node.op, node.qargs, node.cargs)
 
-                stop_time = start_time + node.op.duration
-                # update time table
-                for q in node.qargs:
-                    qubit_time_available[q] = stop_time
+            stop_time = start_time + node.op.duration
+            # update time table
+            for q in node.qargs:
+                qubit_time_available[q] = stop_time
+                if not isinstance(node.op, Delay):
                     qubit_stop_times[q] = stop_time
-            else:
-                stop_time = start_time + node.op.duration
-                for q in node.qargs:
-                    qubit_time_available[q] = stop_time
+            for c in node.cargs:  # measure
+                clbit_writeable[c] = clbit_readable[c] = stop_time
+            for c in node.op.condition_bits:  # conditional op
+                clbit_writeable[c] = max(start_time, clbit_writeable[c])
 
         working_qubits = qubit_time_available.keys()
         circuit_duration = max(qubit_time_available[q] for q in working_qubits)

releasenotes/notes/fix-scheduling-circuits-with-clbits-operations-e5d8bfa90e9a3ae1.yaml

@@ -0,0 +1,12 @@
+---
+fixes:
+  - |
+    Fixed an issue in scheduling of circuits with clbits operations, e.g. measurements,
+    conditional gates, updating
+    :class:`~qiskit.transpiler.passes.ASAPSchedule`,
+    :class:`~qiskit.transpiler.passes.ALAPSchedule`, and
+    :class:`~qiskit.transpiler.passes.AlignMeasures`.
+    The updated schedulers assume all clbits I/O operations take no time,
+    ``measure`` writes the measured value to a clbit at the end, and
+    ``c_if`` reads the conditional value in clbit(s) at the beginning.
+    Fixed `#7006 <https://github.com/Qiskit/qiskit-terra/issues/7006>`__

test/python/transpiler/test_instruction_alignments.py

@@ -40,8 +40,7 @@ def setUp(self):
                 ("sx", (1,), 160),
                 ("cx", (0, 1), 800),
                 ("cx", (1, 0), 800),
-                ("measure", (0,), 1600),
-                ("measure", (1,), 1600),
+                ("measure", None, 1600),
             ]
         )
         self.time_conversion_pass = TimeUnitConversion(inst_durations=instruction_durations)
@@ -263,6 +262,63 @@ def test_alignment_is_not_processed(self):
 
         self.assertEqual(transpiled, circuit)
 
+    def test_circuit_using_clbit(self):
+        """Test a circuit with instructions using a common clbit.
+
+        (input)
+             ┌───┐┌────────────────┐┌─┐
+        q_0: ┤ X ├┤ Delay(100[dt]) ├┤M├──────────────
+             └───┘└────────────────┘└╥┘   ┌───┐
+        q_1: ────────────────────────╫────┤ X ├──────
+                                     ║    └─╥─┘   ┌─┐
+        q_2: ────────────────────────╫──────╫─────┤M├
+                                     ║ ┌────╨────┐└╥┘
+        c: 1/════════════════════════╩═╡ c_0 = T ╞═╩═
+                                     0 └─────────┘ 0
+
+        (aligned)
+                    ┌───┐       ┌────────────────┐┌─┐┌────────────────┐
+        q_0: ───────┤ X ├───────┤ Delay(112[dt]) ├┤M├┤ Delay(160[dt]) ├───
+             ┌──────┴───┴──────┐└────────────────┘└╥┘└─────┬───┬──────┘
+        q_1: ┤ Delay(1872[dt]) ├───────────────────╫───────┤ X ├──────────
+             └┬────────────────┤                   ║       └─╥─┘       ┌─┐
+        q_2: ─┤ Delay(432[dt]) ├───────────────────╫─────────╫─────────┤M├
+              └────────────────┘                   ║    ┌────╨────┐    └╥┘
+        c: 1/══════════════════════════════════════╩════╡ c_0 = T ╞═════╩═
+                                                   0    └─────────┘     0
+
+        Looking at the q_0, the total schedule length T becomes
+        160 (x) + 112 (aligned delay) + 1600 (measure) + 160 (delay) = 2032.
+        The last delay comes from ALAP scheduling called before the AlignMeasure pass,
+        which aligns stop times as late as possible, so the start time of x(1).c_if(0)
+        and the stop time of measure(0, 0) become T - 160.
+        """
+        circuit = QuantumCircuit(3, 1)
+        circuit.x(0)
+        circuit.delay(100, 0, unit="dt")
+        circuit.measure(0, 0)
+        circuit.x(1).c_if(0, 1)
+        circuit.measure(2, 0)
+
+        timed_circuit = self.time_conversion_pass(circuit)
+        scheduled_circuit = self.scheduling_pass(timed_circuit, property_set={"time_unit": "dt"})
+        aligned_circuit = self.align_measure_pass(
+            scheduled_circuit, property_set={"time_unit": "dt"}
+        )
+        self.assertEqual(aligned_circuit.duration, 2032)
+
+        ref_circuit = QuantumCircuit(3, 1)
+        ref_circuit.x(0)
+        ref_circuit.delay(112, 0, unit="dt")
+        ref_circuit.delay(1872, 1, unit="dt")  # 2032 - 160
+        ref_circuit.delay(432, 2, unit="dt")  # 2032 - 1600
+        ref_circuit.measure(0, 0)
+        ref_circuit.x(1).c_if(0, 1)
+        ref_circuit.delay(160, 0, unit="dt")
+        ref_circuit.measure(2, 0)
+
+        self.assertEqual(aligned_circuit, ref_circuit)
+
 
 class TestPulseGateValidation(QiskitTestCase):
     """A test for pulse gate validation pass."""