-
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
Add circuit generation from set of stabilizers #11483
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 7541194064Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
@Randl - thank you very much for your contribution to Qiskit! Here are a few initial comments:
See similar pseudo-random tests in: https://github.com/Qiskit/qiskit/blob/main/test/python/synthesis/test_stabilizer_synthesis.py
|
* Move to synthesis library and rename accordingly * Add pseudo-random tests * Fix docstrings
@ShellyGarion |
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.
Hi @Randl - I very much like your PR and think it will be an important contribution to Qiskit.
I have a few additional minor comments.
I would also like @alexanderivrii to review this PR.
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.
Overall, this looks good.
I have left inline a few minor nitpicks about formatting and possibility for code reuse, but nothing blocking.
A quick question about the implementation: is there an advantage to working with the string representation of stabilizers (such as "+XIZI") vs. converting them to Pauli
s and working with the underlying nd.array
s?
Also a question about the scalability of the approach: how much time does this take to synthesize stabilizer states for 100 or so qubits?
Return: | ||
StabilizerState: a state stabilized by stabilizers. |
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.
We usually use Returns:
instead of Return:
(though I do see Return:
in multiple places in the code as well). Let me address this to a higher autority, @jakelishman.
releasenotes/notes/add-synth-circuit-from-stabilizer-list-4cf9cfa01bbc7ddf.yaml
Outdated
Show resolved
Hide resolved
@alexanderivrii you're correct about using |
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.
Thank you for quickly addressing the previous comments (and indeed the code is now much easier to read). LGTM!
* Add stabilizer_to_circuit function * Fix verification test * Mention `stabilizer_to_circuit` in `StabilizerState` documentation * * Add `from_stabilizer_list` function to Stabilizer state * Move to synthesis library and rename accordingly * Add pseudo-random tests * Fix docstrings * doc fixes * Allow any Collection and not just list, fix naming, tests, and docs accordingly. * typo * missed rename * Documentation fixes * fix indent * fix indent 2 * fix indent 3 * Rewrite using `Pauli` instead of string.
Summary
Add
stabilizer_to_circuit
function that given a set of stabilizers returns a (Clifford) circuit whose output (on zero input) is stabilized by this set.Closes #11478
Details and comments