Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the code for decomposing MCX and MCPhase gates, so that the number of CX gates won't grow exponentially #11993

Merged
merged 8 commits into from
Mar 31, 2024

Conversation

ShellyGarion
Copy link
Member

@ShellyGarion ShellyGarion commented Mar 12, 2024

Summary

close #11823
see the discussion in #5872
Update the code for decomposing MCX and MCPhase gates, so that the number of CX gates won't grow exponentially

Details and comments

The current code for synthesizing MCX and MCPhase gates (without ancillas) is based on Gray decomposition and hence the number of CX gates grows exponentially in the number of qubits n.
We already have a code for MCRZ synthesis in linear number of CXs (~16*n) based on https://arxiv.org/abs/2302.06377.
In this PR, we add code for MCPhase synthesis in quadratic number of CXs (~8*n^2), and then to extend it to MCX.

Here is a comparison of the CX count in the current and new Qiskit code:

num_qubits= 4 cx count in current code= 20 cx count in new code= 20
num_qubits= 5 cx count in current code= 44 cx count in new code= 44
num_qubits= 6 cx count in current code= 92 cx count in new code= 84
num_qubits= 7 cx count in current code= 188 cx count in new code= 140
num_qubits= 8 cx count in current code= 380 cx count in new code= 220
num_qubits= 9 cx count in current code= 764 cx count in new code= 324
num_qubits= 10 cx count in current code= 1532 cx count in new code= 452
num_qubits= 11 cx count in current code= 3068 cx count in new code= 604
num_qubits= 12 cx count in current code= 6140 cx count in new code= 780
num_qubits= 13 cx count in current code= 12284 cx count in new code= 980
num_qubits= 14 cx count in current code= 24572 cx count in new code= 1204
num_qubits= 15 cx count in current code= 49148 cx count in new code= 1452
num_qubits= 16 cx count in current code= 98300 cx count in new code= 1724
num_qubits= 17 cx count in current code= 196604 cx count in new code= 2020
num_qubits= 18 cx count in current code= 393212 cx count in new code= 2340
num_qubits= 19 cx count in current code= 786428 cx count in new code= 2684
num_qubits= 20 cx count in current code= 1572860 cx count in new code= 3052

@ShellyGarion ShellyGarion added this to the 1.1.0 milestone Mar 12, 2024
@ShellyGarion ShellyGarion requested a review from a team as a code owner March 12, 2024 09:07
@qiskit-bot
Copy link
Collaborator

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

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia

@ShellyGarion ShellyGarion changed the title [WIP] Update the code for decomposing MCX and MCPhase gate, so that the number of CX gates won't grow exponentially [WIP] Update the code for decomposing MCX and MCPhase gates, so that the number of CX gates won't grow exponentially Mar 12, 2024
@coveralls
Copy link

coveralls commented Mar 12, 2024

Pull Request Test Coverage Report for Build 8497005635

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.

Details

  • 28 of 28 (100.0%) changed or added relevant lines in 2 files are covered.
  • 211 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.1%) to 89.317%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.03%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py 4 90.04%
crates/accelerate/src/convert_2q_block_matrix.rs 4 90.7%
crates/qasm2/src/lex.rs 8 91.6%
crates/accelerate/src/euler_one_qubit_decomposer.rs 10 89.78%
qiskit/synthesis/two_qubit/two_qubit_decompose.py 15 93.61%
crates/qasm2/src/parse.rs 18 96.69%
crates/accelerate/src/two_qubit_decompose.rs 151 89.33%
Totals Coverage Status
Change from base Build 8456962099: -0.1%
Covered Lines: 60239
Relevant Lines: 67444

💛 - Coveralls

@ShellyGarion ShellyGarion added performance Changelog: Bugfix Include in the "Fixed" section of the changelog labels Mar 12, 2024
@ShellyGarion ShellyGarion changed the title [WIP] Update the code for decomposing MCX and MCPhase gates, so that the number of CX gates won't grow exponentially Update the code for decomposing MCX and MCPhase gates, so that the number of CX gates won't grow exponentially Mar 12, 2024
@ShellyGarion
Copy link
Member Author

This code handles the two cases of lam differently:

  • If lam is a float (or an int) then we use the mcrz synthesis to reduce the number of CX gates.

  • If lam is a ParameterExpression we use the existing code since mcrz gate does not handle this type in general.

@ShellyGarion
Copy link
Member Author

ShellyGarion commented Mar 20, 2024

After completing this PR, we will be able to write a code to efficiently synthesize any multi-controlled 1-qubit unitary using Euler's decomposition in quadratic number of CX gates, based on the synthesis of mcp, mcry, and mcrz gates (this will be done in a separate PR).

Copy link
Contributor

@alexanderivrii alexanderivrii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ShellyGarion, this leads to a significant reduction in the number of CX-gates when decomposing MCX and MCP gates. This looks good to me, with a minor nitpick about the wording in release notes. I am also wondering if this should be labeled as "new feature" rather than "bug fix".

Another question: it seems that the old method actually produced fewer CX-gates for small values of num_qubits (e.g., 5 gates instead of 6 with 2 controls). I think it would make sense to use the new method only when it's better.

Comment on lines 4 to 5
Fix the decomposition of the gates :class:`.MCXGate` and :class:`.MCPhaseGate`,
so that the number of :class:`.CXGate` will grow quadratically in the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's slightly better to say "replace the decomposition" instead of "fix the decomposition" since the previous decomposition was technically correct. Maybe you also want to explicitly mention that this is the decomposition without ancilla qubits.

@ShellyGarion
Copy link
Member Author

Another question: it seems that the old method actually produced fewer CX-gates for small values of num_qubits (e.g., 5 gates instead of 6 with 2 controls). I think it would make sense to use the new method only when it's better.

Thanks @alexanderivrii ! that's a good catch, it's due to this:

# the C3XGate and C4XGate will be implemented in the MCXGrayCode class.

Since we don't rely on the MCXGrayCode class anymore, I'll add the cases of C3XGate and C4XGate directly to the MCXGate definition.

Copy link
Contributor

@alexanderivrii alexanderivrii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Shelly for doing the updates. Having qiskit transpiler produce efficient circuits for MCX gates will make many people happy.

@alexanderivrii alexanderivrii added this pull request to the merge queue Mar 31, 2024
Merged via the queue into Qiskit:main with commit 4f7b54a Mar 31, 2024
12 checks passed
@ShellyGarion ShellyGarion mentioned this pull request May 15, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog performance synthesis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recursively controlling a circuit takes exponential time and memory
4 participants