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.
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
Adding MCX synthesis plugins #12961
Adding MCX synthesis plugins #12961
Changes from 1 commit
d3e882c
a64597a
e548257
dccd30f
d5458ce
014b8a2
cab6cb5
a79e2f9
364cf75
bc4238c
fa94424
306361f
e54706b
c3c1927
b21f3ea
f0f3de9
01d76b2
d739065
d856c88
08051f3
d73d941
73d4502
4135344
efb55c2
b6702c5
247d414
458c10d
055e365
6bb46d5
e5cd0b8
d6226b9
9278d26
3879cdd
1c05061
40228f0
0649bc9
5a8ba64
5422990
e0765ec
5f14493
82586cb
c0746ec
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any better suggestions for this function name? the mcp synth is based on iteratively using mcrz synth.
I would suggest
synth_mcx_basic
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't
mcrz
use the_mcsu2
work? Maybe we can use that paper as reference?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking for a better name for this too, so ideas are welcome. I am not particularly fond of
synth_mcx_basic
though, as the word "basic" is too generic. By the way, do we have a reference for this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is a reference for the mcrz (mcsu2) paper, but this code is a variant that does not appear in the paper.
[1] R. Vale et al. Decomposition of Multi-controlled Special Unitary Single-Qubit Gates
arXiv:2302.06377 (2023) https://arxiv.org/abs/2302.06377
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reference; adding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think that we should find a better name than " synth_mcx_mcphase"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After an offline discussion with @ShellyGarion and @Cryoris, we have settled on
synth_v24
. I am not really fond of this or other names, but this is the best we could think of and consistent with the naming of other methods.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we should definitely introduce a human-readable alias for these methods, which users can call in the HLS 🙂