-
Notifications
You must be signed in to change notification settings - Fork 575
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
[TAPE] Adds the CV parameter-shift tape #811
Conversation
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com> Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
"There is a bug in the second order CV parameter-shift rule; " | ||
"phase arguments return the incorrect derivative." |
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.
@nquesada, I was wondering if you have any ideas to fix this? I think you might be the person who knows the logic best
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
if A.ndim == 2: | ||
A = A + A.T |
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.
Just for my own understanding: how come this happens here?
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.
If A is a 2D matrix, than we have to symmetrize it here (if it isn't symmetrized, than it won't correspond to a valid Hermitian observable).
Essentially, what we are doing here is A @ Z + Z @ A.T
(since Z is always symmetric).
I believe this is the same as doing Z.T @ A @ Z / 2
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.
Oh right, thanks! When would A.ndim == 2
not be satisfied?
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.
If the observable is first order in x and p, then A
will be a 1D vector rather than a matrix. That should be the only other case --- alternatives could only occur if the developer doesn't follow the guidelines for heisenberg_rep
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! :) maybe worth leaving a comment? Such that it's clarified that having a first-order observable would be a possibility when e.g. the second-order param shift is enforced on a first-order observable. (as _transform_observable
seems to be solely used by parameter_shift_second_order
, which could be slightly misleading at first).
Co-authored-by: antalszava <antalszava@gmail.com>
"""If an op has grad_method=F, this should be respected | ||
by the CVParamShiftTape""" |
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.
Based on this docstring, would have thought it's an example for a fallback to finite differences, although it doesn't seem to be the case
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.
It should be! Here, I am monkeypatching the rotation option to only specify grad_method="F"
. The CV tape should respect this, and return a grad method of "F" for that particular gate argument.
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.
Note that one F parameter does not cause the entire circuit to require finite-difference---only that parameter gradient is computed using finite diff, the rest continue to use the parameter shift rule.
Co-authored-by: antalszava <antalszava@gmail.com>
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.
Looks good to me, awesome @josh146! 💯 Thanks for answering my questions already 😊
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.
Looks awesome @josh146! Approved.
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Context: Adds a new quantum tape, which implements the CV parameter-shift method.
Description of the Change:
The new tape is pretty much a direct port of the existing
qml.qnodes.CVQNode
to the new quantum tape paradigm.Minor changes include:
The
CVParamShiftTape
inherits fromQubitParamShiftTape
, since this avoids duplicating some logic (such as determining when to switch the parameter-shift rule to one that understands variances).New checks have been added to ensure the device supports
PolyXP
for second-order observables. If not, we raise a warning and fallback to finite differences. This fixes a bug where we cannot train on<N>
using our remote hardware.Benefits: n/a
Possible Drawbacks:
CVQNode
: phase arguments return the incorrect derivative when forcing the second-order CV parameter-shift rule.Related GitHub Issues: