Adding `coupling-map` and `target` to `HighLevelSynthesis` #10477

alexanderivrii · 2023-07-23T09:06:27Z

Summary

This PR adds the arguments coupling_map, target and use_qubit_indices to HighLevelSynthesis transpiler pass, and adds the arguments coupling_map, targets and qubits to HighLevelSynthesisPlugin. The change is backward-compatible.

Details and comments

This reimplements yet another component of #9250, and consists of defining a cleaner API for HighLevelSynthesis and HighLevelSynthesisPlugin that takes coupling_map and target into account. See discussion here and here.

The argument use_qubit_indices provides the context to HighLevelSynthesis whether it's running before or after the layout has been set, that is, whether the qubit indices of higher-level-objects correspond to qubit indices on the target backend. One thing which I did not realize initially is that even when passes like unroll3q or high-level-synthesis are running on the abstract circuit, they have access to target and hence to the coupling map of the device (this was not this way before target was added).

…ith target and coupling_map

qiskit-bot · 2023-07-23T09:06:31Z

One or more of the the following people are requested to review this:

@Qiskit/terra-core

coveralls · 2023-07-23T09:38:34Z

Pull Request Test Coverage Report for Build 5834603069

25 of 25 (100.0%) changed or added relevant lines in 3 files are covered.
6 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.001%) to 87.243%

Files with Coverage Reduction	New Missed Lines	%
qiskit/extensions/quantum_initializer/squ.py	2	80.0%
crates/qasm2/src/lex.rs	4	90.89%

Totals
Change from base Build 5825072876:	0.001%
Covered Lines:	74283
Relevant Lines:	85145

💛 - Coveralls

mtreinish

Overall this LGTM, the interface makes sense as it's explicitly setting the required arguments. Just a few small questions/comments inline. One other thing is do you think we should add some details about this to the plugin interface docs: https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/transpiler/passes/synthesis/plugin.py#L159 because we're adding on to the interface.

qiskit/transpiler/passes/synthesis/high_level_synthesis.py

mtreinish · 2023-07-24T20:26:07Z

releasenotes/notes/reno-691a6036754487df.yaml

+  - |
+    Added the arguments ``coupling_map``, ``target`` and ``qubits`` to :class:`.HighLevelSynthesisPlugin`.
+    This change is backward-compatible.


I think having some examples here would go a long way for plugin maintainers.

I agree. At the moment I don't have any good examples, but I would after the last part of #9250 gets cleaned up, i.e. actually having some plugins that take coupling_map and qubits into account. And then we would be able to mention/reference these specific plugins in the plugin interface docs. By the way, thanks for reminding to update these docs, that's done in 54bd00b.

mtreinish · 2023-07-24T20:27:23Z

test/python/transpiler/test_high_level_synthesis.py

+        if coupling_map is None:
+            raise TranspilerError("Coupling map should be specified!")


I guess it doesn't matter in this case since we're just testing the raise, but you can do target.build_coupling_map() to get a coupling map from a target.

Yes, good to know, thanks!

I have also added a test that when only the target is set, the synthesis plugin does see a valid coupling map (because indeed the HighLevelSynthesis pass builds it using the function above).

mtreinish · 2023-07-24T20:30:18Z

qiskit/transpiler/preset_passmanagers/common.py

@@ -217,7 +217,7 @@ def generate_unroll_3q(
            target=target,
        )
    )
-    unroll_3q.append(HighLevelSynthesis(hls_config=hls_config))
+    unroll_3q.append(HighLevelSynthesis(hls_config=hls_config, coupling_map=None, target=target))


I'm wondering if there is a potential issue here because unroll3q is typically run before layout so the qubit indices that get set in the pass will not be indices in the coupling map (ie qubit 0 in the circuit is not yet decided to be qubit 0 on the target backend). Maybe we need a flag to skip the qubits or otherwise flag that we've yet to set a layout.

That's a good point. At the moment I think having qubits=None is exactly an indication that the layout has not yet been chosen. I have added a sentence on that to the plugin reference docs.

But won't this still set qubits to a value because it's unconditionally populated in the plugin_method.run() call?

Update: I spoke with Matthew in private, and he has convinced me that HighLevelSynthesis needs to know whether it's running before or after the layout is set, and suggested to add a flag use_qubit_indices to HighLevelSynthesis transpiler pass. This is now done in e3211fb.

mtreinish · 2023-07-24T20:31:37Z

releasenotes/notes/reno-691a6036754487df.yaml

+    transpiler pass.
+  - |
+    Added the arguments ``coupling_map``, ``target`` and ``qubits`` to :class:`.HighLevelSynthesisPlugin`.
+    This change is backward-compatible.


Also it might be worthwhile to call out that if they're not explicitly added to a plugin's run() method kwargs the coupling map, target, and qubits will end up in the options input.

I made a pass over release notes in 5a1365a.

mtreinish

This LGTM, thanks for the quick update. I had a couple of small suggestions inline but nothing worth really blocking over. If you don't think we should make these changes I'm happy to approve and move forward with this.

qiskit/transpiler/passes/synthesis/plugin.py

qiskit/transpiler/preset_passmanagers/common.py

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

mtreinish

LGTM, thanks for the quick update.

qiskit/transpiler/preset_passmanagers/common.py

alexanderivrii · 2023-08-11T17:33:33Z

@mtreinish, many thanks for the improvements! Though the commit af27ba8 was problematic: the function generate_unroll_3q does not have the argument coupling_map. However, it does accept target that gets passed to HighLevelSynthesis, allowing it to build the coupling map from the target. And this is indeed very nice -- exactly as you say this allows us to choose a better method for synthesizing a high-level-object before layout/routing,

mtreinish

Thanks, you're correct, I neglected that the coupling_map isn't available there. Sorry for applying an incorrect suggestion. This is ready to go from my perspective then.

jakelishman · 2023-08-11T18:51:11Z

Sorry, I just removed this from the queue because I think it may be worth holding off merging anything until the cross-repo merge I'm just writing up now is complete.

…kit#10477) * extending HighLevelSynthesis and HighLevelSynthesisPlugin interface with target and coupling_map * improving docstrings and adjusting arguments to implemented plugins * adding test that the coupling map gets passed correctly * release notes * docs * using DAGCircuit's find_bit * removing debug print * updating plugin interface docs * improvements to release notes * removing unused variable * adding use_qubit_indices; updating release notes and tests * Update qiskit/transpiler/passes/synthesis/plugin.py Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Update qiskit/transpiler/passes/synthesis/plugin.py Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Update qiskit/transpiler/preset_passmanagers/common.py Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Update qiskit/transpiler/preset_passmanagers/common.py Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Update qiskit/transpiler/preset_passmanagers/common.py Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Update qiskit/transpiler/preset_passmanagers/common.py * setting coupling_map to None --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

### Summary Make the Clifford synthesis algorithm for RB circuits pluggable (implementing it as a `HighLevelSynthesisPlugin`). Fixes #1279 and #1023. Change to accept Clifford elements consisting only of instructions supported by the backend for `interleaved_element` option in `InterleavedRB`. Speed up 2Q RB/IRB for backends with unidirectional 2q gates, e.g. IBM's 127Q Eagle processors. ### Details and comments Previously, for 3Q+ RB circuits, entire circuit is transpiled at once and hence for each of the resulting Cliffords, the initial and the final layout may differ, that means sampled Cliffords are changed during the transpilation. Also in the worst case, the resulting circuit may use physical qubits not in the supplied `physical_qubits`. To avoid that, this commit changes to transpile an RB sequence Clifford by Clifford. The Clifford synthesis algorithm (`rb_default`) is implemented as a `HighLevelSynthesisPlugin` (see `RBDefaultCliffordSynthesis` in `clifford_synthesis.py`), which forces the initial layout (i.e. guarantees the initial layout = the final layout) and physical qubits to use. As a byproduct, the performance of 2Q RB/IRB for backends with directed 2q gates (e.g. IBM's 127Q Eagle processors) is drastically improved. For those cases, previously we had to rely on `transpile` function to make generated circuits comply with the coupling map, however, after this commit, we can synthesize Cliffords with considering the 2q-gate direction and go through the fast path introduced in #982. Depends on Qiskit/qiskit#10477 (qiskit 0.45) --------- Co-authored-by: Helena Zhang <Helena.Zhang@ibm.com>

alexanderivrii added 5 commits July 12, 2023 15:09

extending HighLevelSynthesis and HighLevelSynthesisPlugin interface w…

04ef67c

…ith target and coupling_map

improving docstrings and adjusting arguments to implemented plugins

e4fad1a

adding test that the coupling map gets passed correctly

ac901a9

Merge branch 'main' into hls-with-target

11a501f

release notes

8d224b2

alexanderivrii requested review from a team and ShellyGarion as code owners July 23, 2023 09:06

docs

29611eb

mtreinish self-assigned this Jul 23, 2023

mtreinish added Changelog: New Feature Include in the "Added" section of the changelog synthesis mod: transpiler Issues and PRs related to Transpiler labels Jul 23, 2023

mtreinish added this to the 0.45.0 milestone Jul 23, 2023

mtreinish reviewed Jul 24, 2023

View reviewed changes

alexanderivrii added 8 commits July 25, 2023 14:12

Merge branch 'main' into hls-with-target

b1f191f

using DAGCircuit's find_bit

11f9236

removing debug print

3ded40c

updating plugin interface docs

54bd00b

improvements to release notes

5a1365a

removing unused variable

5d85f9c

adding use_qubit_indices; updating release notes and tests

e3211fb

Merge branch 'main' into hls-with-target

6d3f1c4

alexanderivrii requested a review from mtreinish July 27, 2023 08:43

mtreinish reviewed Jul 27, 2023

View reviewed changes

alexanderivrii and others added 4 commits July 29, 2023 00:04

Update qiskit/transpiler/passes/synthesis/plugin.py

5044ec0

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

Update qiskit/transpiler/passes/synthesis/plugin.py

b665af7

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

Update qiskit/transpiler/preset_passmanagers/common.py

5111afa

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

Update qiskit/transpiler/preset_passmanagers/common.py

3b77b6b

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

Update qiskit/transpiler/preset_passmanagers/common.py

5a159e3

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

alexanderivrii requested a review from mtreinish July 29, 2023 07:26

mtreinish previously approved these changes Jul 31, 2023

View reviewed changes

qiskit/transpiler/preset_passmanagers/common.py Show resolved Hide resolved

Update qiskit/transpiler/preset_passmanagers/common.py

af27ba8

mtreinish dismissed their stale review via af27ba8 August 4, 2023 19:31

mtreinish enabled auto-merge August 4, 2023 19:31

mtreinish previously approved these changes Aug 4, 2023

View reviewed changes

alexanderivrii added 2 commits August 11, 2023 17:38

Merge branch 'main' into hls-with-target

9a70e46

setting coupling_map to None

a84cd62

alexanderivrii dismissed mtreinish’s stale review via a84cd62 August 11, 2023 16:00

alexanderivrii requested a review from mtreinish August 11, 2023 17:34

mtreinish added this pull request to the merge queue Aug 11, 2023

mtreinish approved these changes Aug 11, 2023

View reviewed changes

jakelishman removed this pull request from the merge queue due to a manual request Aug 11, 2023

jakelishman added this pull request to the merge queue Aug 11, 2023

Merged via the queue into Qiskit:main with commit 242b3c0 Aug 11, 2023

alexanderivrii deleted the hls-with-target branch August 12, 2023 05:35

alexanderivrii mentioned this pull request Aug 17, 2023

Token swapper permutation synthesis plugin #10657

Merged

1ucian0 mentioned this pull request Aug 17, 2023

HLS with coupling map #9250

Closed

itoko mentioned this pull request Oct 17, 2023

Pluggable Clifford synthesis for RB circuits qiskit-community/qiskit-experiments#1288

Merged

		if coupling_map is None:
		raise TranspilerError("Coupling map should be specified!")

Adding coupling-map and target to HighLevelSynthesis #10477

Adding coupling-map and target to HighLevelSynthesis #10477

Uh oh!

Conversation

alexanderivrii commented Jul 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Uh oh!

qiskit-bot commented Jul 23, 2023

Uh oh!

coveralls commented Jul 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 5834603069

💛 - Coveralls

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexanderivrii commented Aug 11, 2023

Uh oh!

mtreinish left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakelishman commented Aug 11, 2023

Uh oh!

Uh oh!

Adding `coupling-map` and `target` to `HighLevelSynthesis` #10477

Adding `coupling-map` and `target` to `HighLevelSynthesis` #10477

alexanderivrii commented Jul 23, 2023 •

edited

Loading

coveralls commented Jul 23, 2023 •

edited

Loading

mtreinish left a comment •

edited

Loading