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

clarify that qnodes with qml.sample are not differentiable #5237

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

timmysilv
Copy link
Contributor

@timmysilv timmysilv commented Feb 20, 2024

Context:
Autograd allows the differentiation of integers, although this doesn't entirely make sense (see #2556). On top of this, our docstring for qml.sample currently suggests that it is possible, when it should really just point the user elsewhere.

Description of the Change:

  • Update the docstring of qml.sample to suggest the use of expval instead
  • Add tests to show that autograd allows computing the jacobian of a circuit which returns samples, whereas jax (for example) does not

Benefits:
More useful docs

Possible Drawbacks:
We are testing something that shouldn't exist just because Autograd said it should exist

Related GitHub Issues:
Fixes #4840

@timmysilv timmysilv marked this pull request as ready for review February 20, 2024 22:38
@timmysilv timmysilv requested a review from a team February 20, 2024 22:38
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (3407a9e) to head (90c0a43).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5237      +/-   ##
==========================================
- Coverage   99.68%   99.67%   -0.01%     
==========================================
  Files         399      399              
  Lines       36853    36567     -286     
==========================================
- Hits        36736    36449     -287     
- Misses        117      118       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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 @timmysilv!

@astralcai
Copy link
Contributor

astralcai commented Feb 22, 2024

Although it is possible with autograd, should we at least raise a warning saying that what the user is trying to do doesn't necessarily make sense?

@timmysilv
Copy link
Contributor Author

timmysilv commented Feb 22, 2024

the trouble with all this is that auto-diff can be rather opaque. we don't generally have the QuantumScripts available to us ahead of time, so we can't inspect the measurements in order to know when to raise the warning. an alternative way to think about this is, QNodes won't know that they're being executed within a qml.grad (or other auto-diff) context. We have to be cognizant of this at all times when modifying the execution pipeline, but sometimes things just don't work, like this bug.

@timmysilv timmysilv enabled auto-merge (squash) February 22, 2024 20:44
@timmysilv timmysilv merged commit af718c6 into master Feb 22, 2024
39 checks passed
@timmysilv timmysilv deleted the sc-50257-sample-grad-fixups branch February 22, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] QNodes that return qml.sample(obs) and use parameter-shift are not differentiable
4 participants