Add dynamic circuits scheduling pass #365

taalexander · 2022-07-06T01:01:45Z

Summary

This PR Is based on #366 and should only be merged after.

This PR adds two components

A module for IBM systems related infrastructure for Qiskit that are not otherwise acceptable in Qiskit as it must remain IBM agnostic.
An initial implementation of a scheduling pass that is designed to function with the dynamic circuits capability of IBM hardware.

This PR will be followed with additional PRs rounding out dynamical decoupling and dynamic circuit utility functionality. We should also encourage IBM specific infrastructure to be productized and added to these modules going forward.

Details and comments

…ical bits.

…e scheduling.

azure-pipelines · 2022-07-06T04:00:24Z

There was an error handling pipeline event d1c778eb-96d1-4148-a0a6-acf71695e171.

coveralls · 2022-07-06T04:39:26Z

Pull Request Test Coverage Report for Build 2620326171

-208 of 208 (0.0%) changed or added relevant lines in 14 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-4.3%) to 28.939%

Changes Missing Coverage	Changed/Added Lines	%
qiskit_ibm_provider/jupyter/init.py	1	0.0%
qiskit_ibm_provider/jupyter/backend_info.py	1	0.0%
qiskit_ibm_provider/jupyter/config_widget.py	1	0.0%
qiskit_ibm_provider/jupyter/gates_widget.py	1	0.0%
qiskit_ibm_provider/jupyter/jobs_widget.py	1	0.0%
qiskit_ibm_provider/jupyter/live_data_widget.py	1	0.0%
qiskit_ibm_provider/jupyter/qubits_widget.py	1	0.0%
qiskit_ibm_provider/jupyter/utils.py	1	0.0%
qiskit_ibm_provider/transpiler/init.py	1	0.0%
qiskit_ibm_provider/transpiler/passes/init.py	3	0.0%

Totals
Change from base Build 2422190848:	-4.3%
Covered Lines:	1664
Relevant Lines:	5750

💛 - Coveralls

coveralls · 2022-07-06T04:39:26Z

Pull Request Test Coverage Report for Build 2836986822

206 of 212 (97.17%) changed or added relevant lines in 6 files are covered.
10 unchanged lines in 2 files lost coverage.
Overall coverage increased (+2.4%) to 35.642%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit_ibm_provider/transpiler/passes/scheduling/block_base_padder.py	91	94	96.81%
qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py	92	95	96.84%

Files with Coverage Reduction	New Missed Lines	%
qiskit_ibm_provider/utils/backend_converter.py	3	15.22%
qiskit_ibm_provider/api/rest/backend.py	7	34.04%

Totals
Change from base Build 2622080332:	2.4%
Covered Lines:	2053
Relevant Lines:	5760

💛 - Coveralls

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py

reza-j · 2022-07-07T16:32:58Z

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py

+    def _visit_reset(self, node: DAGNode) -> None:
+        """Visit a reset node.
+
+        Reset currently triggers the end of a pulse block in IBM dynamic circuits hardware
+        as conditional reset is performed internally using a c_if.
+        This means that it is possible to schedule *up to* a reset (and during its measurement pulses)
+        but the reset will be followed by a period of conditional indeterminism.
+        All resets on disjoint qubits will be collected on the same qubits to be run simultaneously.
+        This means that from the perspective of scheduling resets have the same behaviour and duration
+        as a measurement.
+        """
+        self._visit_measure(node)


What if the measurement result is |1>? Shouldn't reset operation always replace the data stored in qubit to |0>?

Yes, the reset maps |0> -> |0> and |1> -> |0>. From the perspective of scheduling, measurement ends the scheduling - therefore what happens after (such as c_if) has no impact on the scheduling

reza-j · 2022-07-07T16:50:37Z

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py

+                # In this case, you can insert readout access before tq0
+                #
+                #           |t0q
+                # Q ▒▒▒▒▒▒▒▒▒▒▒
+                # C ▒▒▒░░░▒▒░░░
+                #         |t0q
+                #


In this case, you can insert readout access before tq0

Is this true even if t0q-t0c is very small? It seems like we will not have enough time to do the readout access then, right?

Also is the label below (i.e., |t0q ) correct?

# C ▒▒▒░░░▒▒░░░ # |t0q

This code is just trying not to overly occupy classical bits on the timeline. it won't have an impact on QASM3 as the model of classical bits used for scheduling here does not apply.

ajavadia · 2022-07-20T19:22:17Z

I skimmed this briefly. So what is the main thing that makes this IBM specific and not general purpose? Is it that measurements terminate a block? I'm just trying to figure out what exactly it is that prevents us from having this in terra (because right now i think terra is unable to produce any schedule for dynamic circuits)

nkanazawa1989 · 2022-07-21T01:12:03Z

I think this is reasonable approach. It is of course possible to upgrade Terra's scheduling pass, however, this means we must guarantee the backward compatibility, and this prevents IBM hardwares from upgrading their behavior. Provider specific passes can be much flexible because we assume the output program will be executed on a particular hardware, and backward compatibility doesn't matter since there is no hardware that can execute old program.

(edit) I think it's also good idea to support dynamic circuit scheduling in terra. We need more investigation for controller of other vendors to design some generic model of control flow, since terra is backend agnostic.

ajavadia · 2022-07-28T16:12:30Z

I'm running the teleportation example in the documentation here:
Here's the circuit before/after scheduling:

global phase: π
                                                ┌─────────┐┌────┐┌─────────┐┌─┐
q0_0 -> 0 ───────────────────────────────────■──┤ Rz(π/2) ├┤ √X ├┤ Rz(π/2) ├┤M├───────────────────
          ┌─────────┐┌────┐┌─────────┐     ┌─┴─┐└───┬─┬───┘└────┘└─────────┘└╥┘
q0_1 -> 1 ┤ Rz(π/2) ├┤ √X ├┤ Rz(π/2) ├──■──┤ X ├────┤M├──────────────────────╫────────────────────
          └─────────┘└────┘└─────────┘┌─┴─┐└───┘    └╥┘                      ║ ┌───────┐ ┌───┐ ┌─┐
q0_2 -> 2 ────────────────────────────┤ X ├──────────╫───────────────────────╫─┤ Rz(π) ├─┤ X ├─┤M├
                                      └───┘          ║                       ║ └───╥───┘ └─╥─┘ └╥┘
                                                     ║                       ║  ┌──╨──┐    ║    ║
   crz: 1/═══════════════════════════════════════════╬═══════════════════════╩══╡ 0x1 ╞════╬════╬═
                                                     ║                       0  └─────┘ ┌──╨──┐ ║
   crx: 1/═══════════════════════════════════════════╩══════════════════════════════════╡ 0x1 ╞═╬═
                                                     0                                  └─────┘ ║
result: 1/══════════════════════════════════════════════════════════════════════════════════════╩═
                                                                                                0
global phase: π
          ┌─────────────────┐                                              ┌─────────┐     ┌────┐┌─────────┐┌─┐ ░           ░ ┌──────────────────┐    ░         ░ ┌──────────────────┐    ░
     q_0: ┤ Delay(1440[dt]) ├───────────────────────────────■──────────────┤ Rz(π/2) ├─────┤ √X ├┤ Rz(π/2) ├┤M├─░───────────░─┤ Delay(24080[dt]) ├────░─────────░─┤ Delay(24080[dt]) ├────░─
          └───┬─────────┬───┘┌────┐┌─────────┐            ┌─┴─┐        ┌───┴─────────┴────┐└────┘└─────────┘└╥┘ ░           ░ └──────────────────┘┌─┐ ░         ░ ├──────────────────┤    ░
     q_1: ────┤ Rz(π/2) ├────┤ √X ├┤ Rz(π/2) ├──■─────────┤ X ├────────┤ Delay(24240[dt]) ├──────────────────╫──░───────────░─────────────────────┤M├─░─────────░─┤ Delay(24080[dt]) ├────░─
           ┌──┴─────────┴───┐└────┘└─────────┘┌─┴─┐┌──────┴───┴───────┐└──────────────────┘                  ║  ░ ┌───────┐ ░ ┌──────────────────┐└╥┘ ░  ┌───┐  ░ └──────────────────┘┌─┐ ░
     q_2: ─┤ Delay(160[dt]) ├─────────────────┤ X ├┤ Delay(25296[dt]) ├──────────────────────────────────────╫──░─┤ Rz(π) ├─░─┤ Delay(24080[dt]) ├─╫──░──┤ X ├──░─────────────────────┤M├─░─
           └────────────────┘                 └───┘└──────────────────┘                                      ║  ░ └───╥───┘ ░ └──────────────────┘ ║  ░  └─╥─┘  ░                     └╥┘ ░
                                                                                                             ║     ┌──╨──┐                         ║       ║                           ║
   crz: 1/═══════════════════════════════════════════════════════════════════════════════════════════════════╩═════╡ 0x1 ╞═════════════════════════╬═══════╬═══════════════════════════╬════
                                                                                                             0     └─────┘                         ║    ┌──╨──┐                        ║
   crx: 1/═════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════╩════╡ 0x1 ╞════════════════════════╬════
                                                                                                                                                   0    └─────┘                        ║
result: 1/═════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════╩════
                                                                                                                                                                                       0

I think q0 and q1 should be measured simultaneously, since there's no dependency. Putting barrier between them will cause a long decay.

taalexander · 2022-08-10T13:27:39Z

I think q0 and q1 should be measured simultaneously, since there's no dependency. Putting barrier between them will cause a long decay.

This is a problem with Qiskit's topological sort being used as the basis for the pass. I will look into fixes to reorder this, however, I think in general this requires a "measurement grouping" pass. Does such a pass already exist in Qiskit?

taalexander · 2022-08-10T13:28:58Z

@ajavadia

I skimmed this briefly. So what is the main thing that makes this IBM specific and not general purpose? Is it that measurements terminate a block? I'm just trying to figure out what exactly it is that prevents us from having this in terra (because right now i think terra is unable to produce any schedule for dynamic circuits)

All limitations are explained in the code. Basically indeterminism in control-flow, and measurement/reset grouping behavior currently. With future hardware updates, this may get even more complicated as we add additional scheduling constraints.

nkanazawa1989

Thanks @taalexander, the code looks good to me. Some minor comments before approving.

nkanazawa1989 · 2022-08-22T02:04:43Z

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py

+            t0 = max(t0q, t1c)  # pylint: disable=invalid-name
+
+            t1 = t0 + op_duration  # pylint: disable=invalid-name
+            self._update_idles(node, t0, t1)


Just to confirm; in the current backend classical registers are also blocked until qubit operation completes? I think this code should be fine anyways because there is no instruction that can be conditional and takes both quantum and classical registers.

nkanazawa1989 · 2022-08-22T02:15:34Z

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py

+
+        # now update all measure qarg times.
+
+        self._current_block_measures.add(node)


Why this line appears twice?

nkanazawa1989 · 2022-08-22T02:22:43Z

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py

+            t0 = t0q  # pylint: disable=invalid-name
+            bit_indices = {bit: index for index, bit in enumerate(self._dag.qubits)}
+            measure_duration = self.durations.get(
+                Measure(), [bit_indices[qarg] for qarg in node.qargs], unit="dt"


Ah likely I misunderstood the behavior of the code. Since this iterates over all measurements in the block individually, it doesn't change duration depending on the combination of qubits (and node.qargs here is a single qubit in our device configuration). This is another question, why you cannot use _get_duration method? This also takes pulse gate into account, and we can in principle override measurement with the pulse gate.

kdk · 2022-09-13T13:41:48Z

I think q0 and q1 should be measured simultaneously, since there's no dependency. Putting barrier between them will cause a long decay.

This is a problem with Qiskit's topological sort being used as the basis for the pass. I will look into fixes to reorder this, however, I think in general this requires a "measurement grouping" pass. Does such a pass already exist in Qiskit?

Agree this is tricky to do if using topological sort. There is a pass which performs a similar but different grouping for barriers MergeAdjacentBarriers that might give an idea of one way this could be done:

https://github.com/Qiskit/qiskit-terra/blob/780099972c0dc8bb073665ebd1a137e56126b479/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py

taalexander added 23 commits July 6, 2022 00:26

Fix lint for Qiskit main branch.

450922b

Fix sphinx language configuration.

5c61448

Add initial scaffolding for dynamic_circuits module.

0658c7a

Add a dummy test.

4dbe92a

Add reno.

44a7afb

Add initial qiskit DynamicalDecoupling scheduling scaffolding and tests.

8aa3365

Refactor scheduler with visitor pattern.

06f9d86

Refactor nodes to no longer return. Remove special handling for class…

a518281

…ical bits.

Add block based scheduling and updated tests for dynamic circuits typ…

bfcfe2e

…e scheduling.

Refactor padder class.

9a0955c

Base padding is working.

1744ae4

Remove debug statements.

4340be0

Update non conditional tests to pass.

9e2d5ae

c_if scheduling tests are working.

3cd3859

All tests passing.

b48951d

All dynamic circuits tests passing.

3efa7da

Black formatting.

96e02fb

Move the dynamic_circuits module to a generic transpiler module.

9dddf1e

Blackify

2442d1e

Linting.

7e144e9

Blackify.

42c8bc8

Reach black and pylint fixed point.

615f03c

Mypyify the code.

70ae09d

taalexander force-pushed the add-scheduling-pass branch from c814d58 to 70ae09d Compare July 6, 2022 03:59

rathishcholarajan and others added 3 commits July 6, 2022 06:18

Merge branch 'main' into add-scheduling-pass

36c611e

Add initial scaffolding for dynamic_circuits module.

b4f8f9b

Add a dummy test.

af86472

taalexander added 4 commits July 6, 2022 16:03

Move scheduler unit tests.

2d995df

Make scheduler idempotent.

931fe00

Operations on qubits already measured trigger block termination.

783d608

Black.

b777249

taalexander mentioned this pull request Jul 7, 2022

Add dynamical decoupling pass #367

Merged

reza-j reviewed Jul 7, 2022

View reviewed changes

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py Outdated Show resolved Hide resolved

reza-j reviewed Jul 7, 2022

View reviewed changes

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py Show resolved Hide resolved

reza-j reviewed Jul 7, 2022

View reviewed changes

qiskit_ibm_provider/transpiler/passes/scheduling/scheduler.py Outdated Show resolved Hide resolved

reza-j reviewed Jul 7, 2022

View reviewed changes

mtreinish mentioned this pull request Aug 9, 2022

Add deprecation warning Qiskit/qiskit-ibmq-provider#1140

Merged

taalexander added 4 commits August 10, 2022 23:19

Update comment on _visit_measure qargs.

81596b1

Update comment typo.

d50b28b

Update comment on neighbor ordering.

97e5bcd

Add comments.

b5d9e31

taalexander requested review from nkanazawa1989 and reza-j and removed request for nkanazawa1989 August 11, 2022 02:32

nkanazawa1989 reviewed Aug 22, 2022

View reviewed changes

renier approved these changes Aug 24, 2022

View reviewed changes

Merge branch 'main' into add-scheduling-pass

ff25a88

kt474 merged commit 3c5408d into Qiskit:main Aug 24, 2022

taalexander mentioned this pull request Sep 18, 2022

Update scheduling passes to group measurements #391

Merged

samanthavbarron mentioned this pull request Mar 19, 2024

Performance of block_order_op_nodes Qiskit/qiskit-ibm-runtime#1531

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dynamic circuits scheduling pass #365

Add dynamic circuits scheduling pass #365

taalexander commented Jul 6, 2022 •

edited

Loading

azure-pipelines bot commented Jul 6, 2022

coveralls commented Jul 6, 2022

coveralls commented Jul 6, 2022 •

edited

Loading

reza-j Jul 7, 2022

taalexander Aug 11, 2022 •

edited

Loading

reza-j Jul 7, 2022 •

edited

Loading

reza-j Jul 7, 2022

taalexander Aug 11, 2022 •

edited

Loading

ajavadia commented Jul 20, 2022

nkanazawa1989 commented Jul 21, 2022 •

edited

Loading

ajavadia commented Jul 28, 2022

taalexander commented Aug 10, 2022 •

edited

Loading

taalexander commented Aug 10, 2022

nkanazawa1989 left a comment

nkanazawa1989 Aug 22, 2022

nkanazawa1989 Aug 22, 2022

nkanazawa1989 Aug 22, 2022

kdk commented Sep 13, 2022


		# now update all measure qarg times.

		self._current_block_measures.add(node)

Add dynamic circuits scheduling pass #365

Add dynamic circuits scheduling pass #365

Conversation

taalexander commented Jul 6, 2022 • edited Loading

Summary

Details and comments

azure-pipelines bot commented Jul 6, 2022

coveralls commented Jul 6, 2022

Pull Request Test Coverage Report for Build 2620326171

💛 - Coveralls

coveralls commented Jul 6, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2836986822

💛 - Coveralls

reza-j Jul 7, 2022

Choose a reason for hiding this comment

taalexander Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

reza-j Jul 7, 2022 • edited Loading

Choose a reason for hiding this comment

reza-j Jul 7, 2022

Choose a reason for hiding this comment

taalexander Aug 11, 2022 • edited Loading

Choose a reason for hiding this comment

ajavadia commented Jul 20, 2022

nkanazawa1989 commented Jul 21, 2022 • edited Loading

ajavadia commented Jul 28, 2022

taalexander commented Aug 10, 2022 • edited Loading

taalexander commented Aug 10, 2022

nkanazawa1989 left a comment

Choose a reason for hiding this comment

nkanazawa1989 Aug 22, 2022

Choose a reason for hiding this comment

nkanazawa1989 Aug 22, 2022

Choose a reason for hiding this comment

nkanazawa1989 Aug 22, 2022

Choose a reason for hiding this comment

kdk commented Sep 13, 2022

taalexander commented Jul 6, 2022 •

edited

Loading

coveralls commented Jul 6, 2022 •

edited

Loading

taalexander Aug 11, 2022 •

edited

Loading

reza-j Jul 7, 2022 •

edited

Loading

taalexander Aug 11, 2022 •

edited

Loading

nkanazawa1989 commented Jul 21, 2022 •

edited

Loading

taalexander commented Aug 10, 2022 •

edited

Loading