respond to review comments

- fix typo - update model drawing in comment - more comment
Qiskit · Feb 21, 2022 · 59698ac · 59698ac
1 parent d91569c
commit 59698ac
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 68 deletions.
diff --git a/qiskit/transpiler/passes/scheduling/alap.py b/qiskit/transpiler/passes/scheduling/alap.py
@@ -52,7 +52,10 @@ def run(self, dag):
         for node in reversed(list(dag.topological_op_nodes())):
             op_duration = self._get_node_duration(node, bit_indices, dag)
 
-            # compute t0, t1: instruction interval
+            # compute t0, t1: instruction interval, note that
+            # t0: start time of instruction
+            # t1: end time of instruction
+
             # since this is alap scheduling, node is scheduled in reversed topological ordering
             # and nodes are packed from the very end of the circuit.
             # the physical meaning of t0 and t1 is flipped here.
@@ -61,10 +64,10 @@ def run(self, dag):
                 t0 = max(idle_before[bit] for bit in node.qargs + node.cargs)
                 t1 = t0 + op_duration
                 #
-                #       t1 = t0 + duration
-                # Q ░░░░|▒▒▒▒▒▒▒▒▒▒▒
-                # C ░░░░░░░░|▒▒▒▒▒▒▒
-                #           t0 + (duration - clbit_write_latency)
+                #        |t1 = t0 + duration
+                # Q ░░░░░▒▒▒▒▒▒▒▒▒▒▒
+                # C ░░░░░░░░░▒▒▒▒▒▒▒
+                #            |t0 + (duration - clbit_write_latency)
                 #
                 for clbit in node.cargs:
                     idle_before[clbit] = t0 + (op_duration - self.clbit_write_latency)
@@ -73,31 +76,28 @@ def run(self, dag):
                 if node.op.condition_bits:
                     # conditional is bit tricky due to conditional_latency
                     t0c = max(idle_before[c] for c in node.op.condition_bits)
-                    if t0c > t0q:
-                        # this is situation something like below
-                        #
-                        #              t0q
-                        # Q ░░░░░░░░░░░|▒▒▒▒
-                        # C ░░░░░░|▒▒▒▒▒▒▒▒▒
-                        #         t0c
-                        #
-                        # In this case, there is no actual clbit read before gate.
-                        #
-                        #         t1 = t0c
-                        # Q ░░░░░░|▒▒▒||▒▒▒▒
-                        # C ░░░|▒▒|▒▒▒▒▒▒▒▒▒
-                        #      t1 + conditional_latency
-                        #
-                        # rather than naively doing
-                        #
-                        #     t1 = t0c + duration
-                        # Q ░░|▒▒▒|░░░░|▒▒▒▒
-                        # C |▒▒|░░|▒▒▒▒▒▒▒▒▒
-                        #   t0c + duration + conditional_latency
-                        #
-                        t0 = max(t0q, t0c - op_duration)
-                    else:
-                        t0 = t0q
+                    # Assume following case (t0c > t0q):
+                    #
+                    #                |t0q
+                    # Q ░░░░░░░░░░░░░▒▒▒
+                    # C ░░░░░░░░▒▒▒▒▒▒▒▒
+                    #           |t0c
+                    #
+                    # In this case, there is no actual clbit read before gate.
+                    #
+                    #             |t0q' = t0c - conditional_latency
+                    # Q ░░░░░░░░▒▒▒░░▒▒▒
+                    # C ░░░░░░▒▒▒▒▒▒▒▒▒▒
+                    #         |t1c' = t0c + conditional_latency
+                    #
+                    # rather than naively doing
+                    #
+                    #        |t1q' = t0c + duration
+                    # Q ░░░░░▒▒▒░░░░░▒▒▒
+                    # C ░░▒▒░░░░▒▒▒▒▒▒▒▒
+                    #     |t1c' = t0c + duration + conditional_latency
+                    #
+                    t0 = max(t0q, t0c - op_duration)
                     t1 = t0 + op_duration
                     for clbit in node.op.condition_bits:
                         idle_before[clbit] = t1 + self.conditional_latency

diff --git a/qiskit/transpiler/passes/scheduling/asap.py b/qiskit/transpiler/passes/scheduling/asap.py
@@ -53,36 +53,36 @@ def run(self, dag):
         for node in dag.topological_op_nodes():
             op_duration = self._get_node_duration(node, bit_indices, dag)
 
-            # compute t0, t1: instruction interval
+            # compute t0, t1: instruction interval, note that
+            # t0: start time of instruction
+            # t1: end time of instruction
             if isinstance(node.op, Measure):
                 # measure instruction handling is bit tricky due to clbit_write_latency
                 t0q = max(idle_after[q] for q in node.qargs)
                 t0c = max(idle_after[c] for c in node.cargs)
-                if t0q > t0c:
-                    t0 = t0q
-                else:
-                    # This is situation something like below
-                    #
-                    #       t0q
-                    # Q ▒▒▒▒|░░░░░░░░░░░
-                    # C ▒▒▒▒▒▒▒▒|░░░░░░░
-                    #           t0c
-                    #
-                    # In this case, there is no actual clbit access until clbit_write_latency.
-                    # The node t0 can be push backward by this amount.
-                    #
-                    #         t0 = t0c - clbit_write_latency
-                    # Q ▒▒▒▒|░▒▒▒▒▒▒▒▒▒▒
-                    # C ▒▒▒▒▒▒▒▒|▒▒▒▒▒▒▒
-                    #           t0c
-                    #
-                    # rather than naively doing
-                    #
-                    #           t0 = t0c
-                    # Q ▒▒▒▒|░░░|▒▒▒▒▒▒▒
-                    # C ▒▒▒▒▒▒▒▒|░░▒▒▒▒▒
-                    #
-                    t0 = max(t0q, t0c - self.clbit_write_latency)
+                # Assume following case (t0c > t0q)
+                #
+                #       |t0q
+                # Q ▒▒▒▒░░░░░░░░░░░░
+                # C ▒▒▒▒▒▒▒▒░░░░░░░░
+                #           |t0c
+                #
+                # In this case, there is no actual clbit access until clbit_write_latency.
+                # The node t0 can be push backward by this amount.
+                #
+                #         |t0q' = t0c - clbit_write_latency
+                # Q ▒▒▒▒░░▒▒▒▒▒▒▒▒▒▒
+                # C ▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒▒
+                #           |t0c' = t0c
+                #
+                # rather than naively doing
+                #
+                #           |t0q' = t0c
+                # Q ▒▒▒▒░░░░▒▒▒▒▒▒▒▒
+                # C ▒▒▒▒▒▒▒▒░░░▒▒▒▒▒
+                #              |t0c' = t0c + clbit_write_latency
+                #
+                t0 = max(t0q, t0c - self.clbit_write_latency)
                 t1 = t0 + op_duration
                 for clbit in node.cargs:
                     idle_after[clbit] = t1
@@ -94,17 +94,17 @@ def run(self, dag):
                     if t0q > t0c:
                         # This is situation something like below
                         #
-                        #           t0q
-                        # Q ▒▒▒▒▒▒▒▒|░░
-                        # C ▒▒▒|░░░░░░░
-                        #      t0c
+                        #           |t0q
+                        # Q ▒▒▒▒▒▒▒▒▒░░
+                        # C ▒▒▒░░░░░░░░
+                        #     |t0c
                         #
                         # In this case, you can insert readout access before tq0
                         #
-                        #           t0q
-                        # Q ▒▒▒▒▒▒▒▒|▒▒
-                        # C ▒▒▒|░░|▒|░░
-                        #         t0q - conditional_latency
+                        #           |t0q
+                        # Q ▒▒▒▒▒▒▒▒▒▒▒
+                        # C ▒▒▒░░░▒▒░░░
+                        #         |t0q - conditional_latency
                         #
                         t0c = max(t0q - self.conditional_latency, t0c)
                     t1c = t0c + self.conditional_latency

diff --git a/qiskit/transpiler/passes/scheduling/base_scheduler.py b/qiskit/transpiler/passes/scheduling/base_scheduler.py
@@ -73,8 +73,8 @@ class BaseScheduler(TransformationPass):
             c: 1/════════╩═════════╡ c_0=0x1 ╞
                          0         └─────────┘
 
-        It looks like the topological order of nodes is inverted. This behavior can be
-        understood by considering the control flow model described above,
+        It looks like the ordering of nodes is inverted (actually not).
+        This behavior can be understood by considering the control flow model described above,
 
         .. parsed-literal::
 

diff --git a/releasenotes/notes/upgrade-alap-asap-passes-bcacc0f1053c9828.yaml b/releasenotes/notes/upgrade-alap-asap-passes-bcacc0f1053c9828.yaml
@@ -18,7 +18,7 @@ upgrade:
     To reproduce conventional behavior, create a pass manager with one of
     scheduling passes with ``clbit_write_latency`` identical to the measurement instruction length.
 
-    The followign example may clearly explain the change of scheduler spec.
+    The following example may clearly explain the change of scheduler spec.
 
     .. parsed-literal::
 

diff --git a/test/python/transpiler/test_scheduling_pass.py b/test/python/transpiler/test_scheduling_pass.py
@@ -639,7 +639,9 @@ def test_random_complicated_circuit(self):
         expected_asap.delay(400, 1)
         expected_asap.cx(0, 1).c_if(0, 0)
         expected_asap.delay(700, 0)  # creg is released at t0 of cx(0,1).c_if(0,0)
-        expected_asap.delay(700, 1)  # no creg write until 100dt. thus measure can move left by 300dt.
+        expected_asap.delay(
+            700, 1
+        )  # no creg write until 100dt. thus measure can move left by 300dt.
         expected_asap.delay(300, 2)
         expected_asap.measure(2, 0)
         self.assertEqual(expected_asap, actual_asap)