`MCMTGate` & plugins #13150

Cryoris · 2024-09-13T12:36:53Z

Summary

Make the MCMT circuits a gate with pluggable synthesis method, plus port the V-chain synthesis to Rust. This is part of #13046.

Details and comments

Even though the focus of this PR is the restructuring into a block that the compiler can reason about, there's also a nice speedup due to the port to Rust:

New runtime / Old runtime = 0.63 (geo mean)

-- n = 50 ntarg = 1
main: 5.604e-04 +- 1.514e-04
this: 3.490e-04 +- 1.771e-04
fraction: 0.64

-- n = 50 targ = 5
main: 5.612e-04 +- 1.180e-04
this: 3.051e-04 +- 1.267e-04
fraction: 0.54

-- n = 50 targ = 25
main: 7.027e-04 +- 1.188e-04
this: 3.727e-04 +- 1.225e-04
fraction: 0.53

-- n = 200 targ = 1
main: 2.074e-03 +- 2.704e-04
this: 1.631e-03 +- 2.565e-03
fraction: 0.78

-- n = 200 targ = 20
main: 2.693e-03 +- 2.488e-03
this: 1.646e-03 +- 2.433e-03
fraction: 0.62

-- n = 200 targ = 100
main: 2.616e-03 +- 2.670e-04
this: 1.880e-03 +- 2.594e-03
fraction: 0.71

-- n = 1000 targ = 1
main: 1.087e-02 +- 4.227e-03
this: 6.820e-03 +- 5.478e-03
fraction: 0.62

-- n = 1000 targ = 100
main: 1.251e-02 +- 6.234e-03
this: 7.843e-03 +- 6.380e-03
fraction: 0.62

-- n = 1000 targ = 500
main: 1.415e-02 +- 5.152e-03
this: 9.360e-03 +- 6.755e-03
fraction: 0.66

qiskit-bot · 2024-09-13T12:36:58Z

One or more of the following people are relevant to this code:

@Cryoris
@Qiskit/terra-core
@ajavadia
@kevinhartman
@mtreinish

and fix tests

coveralls · 2024-09-13T14:50:52Z

Pull Request Test Coverage Report for Build 11176416050

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

193 of 209 (92.34%) changed or added relevant lines in 9 files are covered.
14 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.02%) to 88.893%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/synthesis/hls_plugins.py	25	27	92.59%
crates/accelerate/src/synthesis/multi_controlled/mcmt.rs	94	101	93.07%
qiskit/circuit/library/generalized_gates/mcmt.py	57	64	89.06%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/parse.rs	6	97.61%
crates/qasm2/src/lex.rs	8	91.48%

Totals
Change from base Build 11161049152:	0.02%
Covered Lines:	74476
Relevant Lines:	83782

💛 - Coveralls

ShellyGarion · 2024-09-15T06:53:24Z

thanks @Cryoris for opening this PR! It also handles issue #12863.
What do you think about issue #10698 ?
I think it should be a small addition, but could also be handled in a follow-up PR.

alexanderivrii

Thanks @Cryoris, this looks very nice. I have left a few minor comments and questions.

alexanderivrii · 2024-09-23T09:33:43Z

qiskit/circuit/library/generalized_gates/mcmt.py

+    This default implementations requires no ancilla qubits, by broadcasting the target gate
+    to the number of target qubits and using Qiskit's generic control routine to control the
+    broadcasted target on the control qubits. If ancilla qubits are available, a more efficient
+    variant using the so-called V-chain decomposition can be used. This is implemented in
+    :class:`~qiskit.circuit.library.MCMTVChain`.


This part of the docstring probably needs to be updated.

Good point 👍🏻 related question: do you know whether the synthesis methods like synth_mcx_... are API-documented?

I think so. The synthesis methods are described in https://docs.quantum.ibm.com/api/qiskit/synthesis. Also, the synthesis plugins are described in https://docs.quantum.ibm.com/api/qiskit/transpiler_synthesis_plugins.

Great, I added the Sphinx doc in 646279a 👍🏻

qiskit/circuit/library/generalized_gates/mcmt.py

alexanderivrii · 2024-09-23T09:42:34Z

qiskit/circuit/library/generalized_gates/mcmt.py

+        if isinstance(gate, ControlledGate):
+            base_gate = gate.base_gate
+        elif isinstance(gate, Gate):
+            base_gate = gate
+        else:
+            raise TypeError(f"Invalid gate type {type(gate)}.")


Would it make sense to consider a control-annotated operation as well?

We could if we additionally ran HLS to apply the other modifiers that are not control. A cleaner way might be to completely disallow 2q gates, from which we only extract the 1q base gate. That would require us to handle less cases and might be a leaner workflow.

qiskit/circuit/library/generalized_gates/mcmt.py

qiskit/synthesis/multi_controlled/mcmt_vchain.py

crates/accelerate/src/synthesis/multi_controlled/mcmt.rs

alexanderivrii · 2024-09-23T10:37:09Z

Lol, the tests have failed due to cargo clippy warnings. And I caught one of them in the review. :)

alexanderivrii · 2024-10-01T06:45:14Z

Thank you very much! As per your question, could you please add a table for documenting MCMT synthesis plugins (in hls_plugins.py, the output should be in https://docs.quantum.ibm.com/api/qiskit/transpiler_synthesis_plugins) and the new synthesis function (the output should be in https://docs.quantum.ibm.com/api/qiskit/synthesis). Also this deserves a release note, listing new functionality and deprecations.

alexanderivrii · 2024-10-01T07:32:29Z

releasenotes/notes/mcmt-gate-a201d516f05c7d56.yaml

+    Added the :class:`.MCMTGate` to represent multi-control multi-target
+    operations in a gate. This gate representation of the existing :class:`.MCMT`
+    circuit allows the compiler to select the best available implementation
+    according to the number of auxiliary qubits present in the circuit.


This is super-minor, please feel free to ignore it:

Suggested change

Added the :class:`.MCMTGate` to represent multi-control multi-target

operations in a gate. This gate representation of the existing :class:`.MCMT`

circuit allows the compiler to select the best available implementation

according to the number of auxiliary qubits present in the circuit.

Added the :class:`.MCMTGate` to represent a multi-control multi-target

operation as a gate. This gate representation of the existing :class:`.MCMT`

circuit allows the compiler to select the best available implementation

according to the number and the state of auxiliary qubits present in the circuit.

Do we want to list pending deprecations?

I would just list the deprecations once they are not pending anymore 🙂 (I think that's what we typically have as well 🙂)

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

alexanderivrii

LGTM!

qiskit/transpiler/passes/synthesis/hls_plugins.py

alexanderivrii

Reapproving after the symengine fix.

mtreinish · 2024-10-03T09:40:42Z

This has a merge conflict now that 642debf has merged. The interface for CircuitData::from_packed_operations has changed to include a result and its usage here will need to be updated.

…/mcmt

Cryoris · 2024-10-03T14:19:43Z

Done in 60a82a3 👍🏻

ShellyGarion · 2024-10-04T07:18:20Z

qiskit/circuit/library/__init__.py

@@ -501,6 +501,7 @@
    Permutation,
    PermutationGate,
    GMS,
+    MCMTGate,


You should also add it to line 188 in this file - it currently does not appear in the circuit library docs!

* MCMT in Rust & Plugin * test plugins * more docs * add support for ``ctrl_state`` and fix tests * Sasha's review comments * fix cyclic imports * add sphinx doc for mcmt_vchain * review comments & reno * Update releasenotes/notes/mcmt-gate-a201d516f05c7d56.yaml Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> * Fix plugin name * Update from_packed_operations usage --------- Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

alexanderivrii

Reapproving once more now that both Matthew's and Shelly's comments have been addressed! Thanks all!

* MCMT in Rust & Plugin * test plugins * more docs * add support for ``ctrl_state`` and fix tests * Sasha's review comments * fix cyclic imports * add sphinx doc for mcmt_vchain * review comments & reno * Update releasenotes/notes/mcmt-gate-a201d516f05c7d56.yaml Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> * Fix plugin name * Update from_packed_operations usage * add missing MCMTGate to docs --------- Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

Cryoris added 4 commits September 11, 2024 17:18

MCMT in Rust & Plugin

21b05ee

test plugins

0e89bf7

more docs

9db4015

Merge branch 'main' into rust/mcmt

aacf9b5

Cryoris requested review from alexanderivrii, ShellyGarion and a team as code owners September 13, 2024 12:36

Cryoris added Rust This PR or issue is related to Rust code in the repository mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Sep 13, 2024

Cryoris changed the title ~~Rust/mcmt~~ MCMTGate & plugins Sep 13, 2024

add support for ctrl_state

4a9172e

and fix tests

ShellyGarion mentioned this pull request Sep 15, 2024

Organize the synthesis library of multi-controlled gates #12863

Open

10 tasks

ShellyGarion mentioned this pull request Sep 16, 2024

Circuit library modernization #13046

Open

alexanderivrii reviewed Sep 23, 2024

View reviewed changes

Cryoris added 4 commits September 26, 2024 16:54

Sasha's review comments

19b7eca

Merge branch 'main' into rust/mcmt

d28bf80

fix cyclic imports

73f8e0a

add sphinx doc for mcmt_vchain

646279a

review comments & reno

708fd7c

alexanderivrii reviewed Oct 1, 2024

View reviewed changes

Update releasenotes/notes/mcmt-gate-a201d516f05c7d56.yaml

830fb5f

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

alexanderivrii previously approved these changes Oct 1, 2024

View reviewed changes

alexanderivrii enabled auto-merge October 1, 2024 07:57

ShellyGarion reviewed Oct 1, 2024

View reviewed changes

qiskit/transpiler/passes/synthesis/hls_plugins.py Outdated Show resolved Hide resolved

Fix plugin name

3a9859e

Merge branch 'main' into rust/mcmt

cf5687a

alexanderivrii previously approved these changes Oct 3, 2024

View reviewed changes

alexanderivrii added this pull request to the merge queue Oct 3, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 3, 2024

Cryoris added this pull request to the merge queue Oct 3, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 3, 2024

Cryoris added 3 commits October 3, 2024 16:13

Merge branch 'main' into rust/mcmt

13d2b64

Update from_packed_operations usage

60a82a3

Merge branch 'rust/mcmt' of github.com:Cryoris/qiskit-terra into rust…

22e8f2b

…/mcmt

Cryoris dismissed alexanderivrii’s stale review via 22e8f2b October 3, 2024 14:19

ShellyGarion reviewed Oct 4, 2024

View reviewed changes

alexanderivrii enabled auto-merge October 4, 2024 08:03

alexanderivrii previously approved these changes Oct 4, 2024

View reviewed changes

alexanderivrii added this pull request to the merge queue Oct 4, 2024

alexanderivrii removed this pull request from the merge queue due to a manual request Oct 4, 2024

add missing MCMTGate to docs

e41a735

Cryoris dismissed alexanderivrii’s stale review via e41a735 October 4, 2024 08:12

alexanderivrii approved these changes Oct 4, 2024

View reviewed changes

alexanderivrii enabled auto-merge October 4, 2024 08:15

alexanderivrii added this pull request to the merge queue Oct 4, 2024

Merged via the queue into Qiskit:main with commit 8b6e47e Oct 4, 2024
15 checks passed

ShellyGarion mentioned this pull request Oct 4, 2024

add ctrl_state to MCMT gate #10698

Closed

Cryoris deleted the rust/mcmt branch October 4, 2024 11:19

ShellyGarion added the Changelog: New Feature Include in the "Added" section of the changelog label Nov 5, 2024

1ucian0 mentioned this pull request Dec 19, 2024

Deprecation of MCMT in favor of MCMTGate #13584

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`MCMTGate` & plugins #13150

`MCMTGate` & plugins #13150

Cryoris commented Sep 13, 2024

qiskit-bot commented Sep 13, 2024

coveralls commented Sep 13, 2024 •

edited

Loading

ShellyGarion commented Sep 15, 2024

alexanderivrii left a comment

alexanderivrii Sep 23, 2024

Cryoris Sep 26, 2024

alexanderivrii Oct 1, 2024

Cryoris Oct 1, 2024

alexanderivrii Sep 23, 2024

Cryoris Sep 26, 2024

alexanderivrii commented Sep 23, 2024

alexanderivrii commented Oct 1, 2024

alexanderivrii Oct 1, 2024

Cryoris Oct 1, 2024

alexanderivrii left a comment

alexanderivrii left a comment

mtreinish commented Oct 3, 2024

Cryoris commented Oct 3, 2024

ShellyGarion Oct 4, 2024

alexanderivrii left a comment

MCMTGate & plugins #13150

MCMTGate & plugins #13150

Conversation

Cryoris commented Sep 13, 2024

Summary

Details and comments

qiskit-bot commented Sep 13, 2024

coveralls commented Sep 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11176416050

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

ShellyGarion commented Sep 15, 2024

alexanderivrii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderivrii commented Sep 23, 2024

alexanderivrii commented Oct 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderivrii left a comment

Choose a reason for hiding this comment

alexanderivrii left a comment

Choose a reason for hiding this comment

mtreinish commented Oct 3, 2024

Cryoris commented Oct 3, 2024

Choose a reason for hiding this comment

alexanderivrii left a comment

Choose a reason for hiding this comment

`MCMTGate` & plugins #13150

`MCMTGate` & plugins #13150

coveralls commented Sep 13, 2024 •

edited

Loading