Skip to content
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

Fixes a bug where the CV param-shift would error with more than one 2nd order observable #1197

Merged
merged 5 commits into from
Apr 9, 2021

Conversation

josh146
Copy link
Member

@josh146 josh146 commented Apr 8, 2021

Context:

The CV parameter-shift rule will raise an exception if attempting to differentiate a QNode with more than one second order observable. For example, running the following script:

dev = qml.device("default.gaussian", wires=2)

@qml.qnode(dev)
def circuit(r):
    qml.Squeezing(r, 0, wires=0)
    qml.Squeezing(r, 0, wires=1)
    return [qml.expval(qml.NumberOperator(i)) for i in range(2)]

print(qml.jacobian(circuit)(0.5))

gives the error:

  File "/home/josh/xanadu/pennylane/pennylane/tape/cv_param_shift.py", line 375, in processing_fn
    grad[transformed_obs_idx] = res
ValueError: shape mismatch: value array of shape (2,) could not be broadcast to indexing result of shape (1,)

Description of the Change:

Fixes the bug; the above example now prints

[1.17520119 1.17520119]

Benefits: Bug fixed.

Possible Drawbacks: n/a

Related GitHub Issues: https://discuss.pennylane.ai/t/typeerror-while-using-qml-gradientdescentoptimizer/956/3

…r if attempting to compute the gradient of a QNode with more than one second-order observable.
@josh146 josh146 added the bug 🐛 Something isn't working label Apr 8, 2021
@josh146 josh146 requested a review from co9olguy April 8, 2021 17:25
.github/CHANGELOG.md Outdated Show resolved Hide resolved
@trbromley trbromley self-requested a review April 8, 2021 17:31
grad = np.zeros_like(res)
grad[transformed_obs_idx] = res
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous implementation assumed that res was always a scalar. This is only the case if the QNode has only one observable.

@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #1197 (ea4b90f) into master (816885a) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1197   +/-   ##
=======================================
  Coverage   98.12%   98.12%           
=======================================
  Files         145      145           
  Lines       10980    10980           
=======================================
  Hits        10774    10774           
  Misses        206      206           
Impacted Files Coverage Δ
pennylane/tape/cv_param_shift.py 98.72% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 816885a...ea4b90f. Read the comment docs.

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @josh146!

@josh146 josh146 merged commit 741ba3a into master Apr 9, 2021
@josh146 josh146 deleted the fix-cv2-bug branch April 9, 2021 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants