-
Notifications
You must be signed in to change notification settings - Fork 10
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 support for Zterms. #5
base: main
Are you sure you want to change the base?
Conversation
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 think this looks good. Can you add a test for this in the test folder? The test should cover:
- Create a circuit for a problem with first-order and second-order terms at depth 1.
- Create a circuit for a problem with first-order and second-order terms at depth 2.
I would work with three qubits on a line and a problem the requires full connectivity. The resulting circuit should require 1 SWAP and be fairly easy to test.
""" | ||
Create the circuit for QAOA. | ||
Notes: This circuit construction for QAOA works for quadratic terms in `Z` as well as |
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.
""" | |
Create the circuit for QAOA. | |
Notes: This circuit construction for QAOA works for quadratic terms in `Z` as well as | |
"""Create the circuit for QAOA. | |
Notes: This circuit construction for QAOA works for quadratic terms in `Z` as well as |
Some IDE allow for a short comment followed by a long detailed comment. This is determined by the formating e.g.
def foo():
"""A brief comment.
A long comment that describes the functions
and will typically span multiple lines.
"""
@@ -116,19 +116,27 @@ def apply_qaoa_layers( | |||
|
|||
|
|||
def create_qaoa_swap_circuit( | |||
cost_operator: SparsePauliOp, | |||
cost_operator: list[tuple[str, float]], |
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.
Why this change? Looking at the code below you call cost_operator.paulis
. Therefore it seems that the type hint should be on SparsePauliOp
and not list[tuple[...
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.
Agreed, I changed this now.
# Create H2 as an operator | ||
for pauli, gate_weight in zip(gate_list, weights_list): | ||
if sum(pauli.x) != 0 or sum(pauli.z) > 2: | ||
raise Exception("This method can only handle first order and second order Pauli Z terms.") |
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.
raise Exception("This method can only handle first order and second order Pauli Z terms.") | |
raise NotImplementedError("This method can only handle first order and second order Pauli Z terms.") |
I think it would be good to later on handle higher-order terms and then use the standard Qiskit transpiler to start of with.
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 @sabinadragoi, I have just enabled automated testing and branch protection rules in the repo. To do that, I had to fix the pylint complaints, as well as the failing tests. I recommend that you rebase this branch to have a "clean slate" and make sure that your changes don't break the tests (you can see below that there will be conflicts, but you should be able to fix them easily). If you have any questions about what this change means, how to fix the conflicts, etc, feel free to reach out to me offline and I'll walk you through it :)
No description provided.