-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 linear synthesis algorithm for LNN #9098
Conversation
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 the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 3575999269
💛 - Coveralls |
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 @ShellyGarion! Overall, this looks good to (and I have the benefit of knowing that this code works correctly in practice). I have left a few minor comments, nothing too deep.
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 a lot for the improvements! I have left a few more requests for minor changes, otherwise this looks very good.
My main problem when looking at this code (which has nothing to do with the code quality itself) it that the proof for lemma 7.3 in the paper is quite involved and does not fully describe how to implement it efficiently. I now see that Ben came up with the idea based on using the inverse matrix to get an efficient implementation.
Other than that the code follows the paper quite closely, and contains very helpful comments.
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 @ShellyGarion, LGTM!
return cx_instructions_rows_m2nw, cx_instructions_rows_nw2id | ||
|
||
|
||
def synth_cnot_depth_line_kms(mat): |
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.
Is this including in the docs tree anywhere? We should make sure that the new function we're advertising in the release notes has its documentation published.
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 not sure if we want to have this standalone function in the API docs, or allow to access it only via the HLS synthesis plugin
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.
But if we add this algorithm to docs, then we should also add Patel-Markov-Hayes as well
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, @ShellyGarion, looks good to me (including downloading documentation artifacts to see how docs look like).
* update init file * add a test for kms synthesis for lnn * add a synthesis algorithm based on kms for lnn in depth 5n. Co-authored-by: Ben Zindorf <[email protected]> * minor fixes in linear_depth_lnn * fix import * add release notes * updates following review * more updates following review * add synth_cnot_depth_line_kms to docs * updates following review comments * fix init * add Patel-Markov-Hayes to docs * update docstring * add Patel-Markov-Hayes to release notes Co-authored-by: Ben Zindorf <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
Adding a synthesis algorithm for linear reversible circuits (containing only CX gates) for linear-nearest-neighbor connectivity.
See #9036
Co-authored by Ben Zindorf.
Details and comments
According to the paper: https://arxiv.org/abs/quant-ph/0701194
The depth of an n-qubit linear reversible circuit is bounded by 5n.