Update synth_mcx_kg24 to use relative phase Toffolis #14394
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qiskit-bot
added
the
Community PR
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
|
Some benchmarks to justify the change in HLS transpiler: CX CountsDepthU CountsCode |
Pull Request Test Coverage Report for Build 15348049412Warning: 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
💛 - Coveralls |
There was a problem hiding this comment.
@patelvyom - this is a great idea to replace all toffoli gates by relative toffolis, and I'm extremely happy to see that the CX counts bounds have improved by 2x!
My main comment is that we already have an rccx method in qiskit and using it could simplify your code, and maybe make it faster (since it's written in rust).
Could you also fix your previous release notes for the plugins?
https://github.com/Qiskit/qiskit/blob/main/releasenotes/notes/add-new-mcx-plugins-2177db9195e2b680.yaml
If you make these changes in the next days, I think this PR could go into qiskit 2.1 release (planned on June 6). Otherwise, it will go into qiskit 2.2 (and require new release notes).
There is no reason to hurry here - it's up to you :)
Co-authored-by: Shelly Garion <[email protected]>
…thout x with rccx
|
@ShellyGarion I've addressed your comments. Let me know if you have more suggestions. |
There was a problem hiding this comment.
@patelvyom - thanks for doing all these changes so quickly!
I have one final small comment.
ShellyGarion
approved these changes
May 30, 2025
There was a problem hiding this comment.
This PR enhances #13922 by replacing ccx gates with rccx gates, thus improving the CX count by 2x!
I think it should definitely go into qiskit 2.1 release.
@alexanderivrii - do you have any final comments?
rahaman-quantum
pushed a commit
to rahaman-quantum/qiskit
that referenced
this pull request
Jun 20, 2025
* add relative phase toffoli in ccx+x decomp * update cx counts tests * update HLS transpiler * Update qiskit/synthesis/multi_controlled/mcx_synthesis.py Co-authored-by: Shelly Garion <[email protected]> * use in-built rccx instead of custom implementation, repalce rccx_x without x with rccx * use rccx in logn_depth_ladder * update dosctring * replace inverse with regular rccx_x * update synth order in release notes * use rccx directly --------- Co-authored-by: Shelly Garion <[email protected]>
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR enhances the recently introduced MCX synthesis methods (see #13922) by replacing standard Toffoli gates with relative-phase Toffolis, resulting in significantly improved CX gate counts.
Details and Comments
By leveraging relative-phase Toffolis, this PR reduces CX counts by up to a factor of 2 across several synthesis strategies. Notably, the CX counts for the following methods now match!
synth_mcx_2_clean_kg24synth_mcx_1_clean_kg24synth_mcx_n_clean_m15Key improvements:
CCX + Xwith_n_parallel_ccx_x, using relative-phase Toffolis internally.Tagging @ShellyGarion, as you're familiar with the recent changes in this area.