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
[QAOA] Adds Cost and Mixer Layers #720
[QAOA] Adds Cost and Mixer Layers #720
Changes from 27 commits
fef9b5e
984f4e4
acccb5c
03e1543
069d379
932c561
dedba09
8886d60
8df2bca
620eb6d
9446b64
9f0f90c
c2dc97b
8b164c5
f792997
92ce647
c4643bb
11c21da
2992383
7fb6d18
729d9a8
8127663
b5d590a
a2ceaec
443530f
5cc316b
033f036
ef1b32d
470c3c3
caa80cd
6387e7e
68678fc
48a1f1d
fe8da5c
b34f76a
2f8b013
f90226b
97cc6b9
09a1efb
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.
I find it a bit strange at first sight that the layer does not take
gamma
as an input. I understand that we call ApproxTimeEvolution as a lambda function, but I'm wondering if this is the best choice.Alternatively we could use
def cost_layer(hamiltonian, gamma):
and then useI'm not sure which approach is best. Curious what others think?
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 am ambivalent 🤔
I see pros and cons for both approaches:
In the
cost_layer(cost_h, gamma)
approach,cost_layer
is essentially a template, and must always be called within a QNode.In the
cost = cost_layer(cosh_h)
approach,cost_layer
is more like a 'template generating function'. The returned cost function can only be used within a QNode, like a template.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.
Probably lean more towards
cost_layer(cost_h, gamma)
, unless you see a use-case for having the 'template generating' behaviour.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.
@ixfoduap @josh146 Sounds good, I'll make the change!
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.
As far as I understand, this is testing that the layers are implementing the same sequence of gates as ApproxTimeEvolution, which I'm not sure is the best way to test since ApproxTimeEvolution may be giving the wrong answers, i.e., these tests don't add much more than the tests for ApproxTimeEvolution. Not sure if this is ok...
Maybe it's good to test an output circuit explicitly, without relying on ApproxTimeEvolution
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.
Made these changes 🙂