-
Notifications
You must be signed in to change notification settings - Fork 0
Draft: Initial setup for a user guide #54
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
base: main
Are you sure you want to change the base?
Conversation
aymannel
left a comment
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.
LGTM 🚀
| "tmp = QuantumCircuit(2)\n", | ||
| "tmp.h(0)\n", | ||
| "tmp.cx(0, 1)\n", | ||
| "cliff = Clifford(tmp)\n", |
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 would prefer an example that is not trivial (i.e., that shows that the plugin does something at least).
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.
Lemme park this here to add later from qiskit docs
from qiskit import QuantumCircuit
from qiskit.quantum_info import Clifford, random_clifford
qc = QuantumCircuit(3)
cliff = random_clifford(2)
qc.append(cliff, [0, 1])
qc.ccx(0, 1, 2)
qc.draw('mpl')
| "Our next plans include, but are not limited to:\n", | ||
| "- Implement the PSGS algorithm for Pauli Polynomial synthesis (open PR)\n", | ||
| "- Improve Qiskit transpiler integration, e.g. add PauliExponentialSynthesis\n", | ||
| "- Circuit-to-PauliExponential parser for generic circuits\n", | ||
| "- Improve code structure, package structure, and code style." |
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.
Might be better to put this in the README, where it's easier to update.
| "metadata": {}, | ||
| "source": [ | ||
| "### Lexicon\n", | ||
| "Since there are some conflicting terminology in literature, we will define what we mean by some of these terms here in alphabetical order.\n", |
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 would sort in a "top-down" order, explaining higher-level concepts first and then dependent concepts.
E.g.
- Pauli exponential
- Pauli polynomial
- Pauli string
- Pauli letter
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "use syn::data_structures::PropagateClifford; // Would be nice if this was not needed when importing CliffordTableau" |
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.
Traits always have to be imported. Only alternative would be to re-export common types and traits as a "prelude" module such that users can do use syn::prelude::* and get everything. We can discuss this at some point.
| "- Implement the PSGS algorithm for Pauli Polynomial synthesis\n", | ||
| "- Improve Qiskit transpiler integration\n", | ||
| "- Circuit-to-PauliExponential parser for generic circuits\n", | ||
| "- Improve code structure, package structure, and code style" |
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.
Again, better to have this in the README, especially as this is duplicated in both notebooks, making it easy to go out of sync.
| "source": [ | ||
| "type Angle = f64;\n", | ||
| "#[derive(Debug, Default)]\n", | ||
| "pub struct MockCircuit {\n", |
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.
This is quite a bit of code. If we expect users want a Circuit struct like this, it might make sense to provide it as part of the library, even though it was originally omitted by design.
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.
The branch should be rebased, these changes are already on main.
Incomplete because the library is still incomplete.