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

[BUG] No warnings or errors are being raised when shots argument is passed arbitrarily to @qnode with no applicable gradient method #5745

Closed
1 task done
Shiro-Raven opened this issue May 27, 2024 · 5 comments
Labels
bug 🐛 Something isn't working

Comments

@Shiro-Raven
Copy link
Contributor

Expected behavior

When shots is passed to the @qnode function, it is typically considered a keyword argument for an underlying gradient transform. However, should this not be the case, a warning or an error should be raised, notifying the user to the fact that this value is not the actual being used for the execution. The user should be directed to supplying this value to the device instead.

Actual behavior

Nothing happens. The code executes successfully with the shots configuration of the QNode's device

Additional information

No response

Source code

@qml.qnode(qml.device('default.qubit'), shots=50)
def f():
    return qml.expval(qml.Z(0))

f()

Tracebacks

No response

System information

Name: PennyLane
Version: 0.37.0.dev0
Summary: PennyLane is a cross-platform Python library for quantum computing, quantum machine learning, and quantum chemistry. Train a quantum computer the same way as a neural network.
Home-page: https://github.com/PennyLaneAI/pennylane
Author: 
Author-email: 
License: Apache License 2.0
Requires: appdirs, autograd, autoray, cachetools, networkx, numpy, pennylane-lightning, requests, rustworkx, scipy, semantic-version, toml, typing-extensions
Required-by: PennyLane_Lightning

Platform info:           macOS-14.5-arm64-arm-64bit
Python version:          3.9.19
Numpy version:           1.26.4
Scipy version:           1.13.0
Installed devices:
- default.clifford (PennyLane-0.37.0.dev0)
- default.gaussian (PennyLane-0.37.0.dev0)
- default.mixed (PennyLane-0.37.0.dev0)
- default.qubit (PennyLane-0.37.0.dev0)
- default.qubit.autograd (PennyLane-0.37.0.dev0)
- default.qubit.jax (PennyLane-0.37.0.dev0)
- default.qubit.legacy (PennyLane-0.37.0.dev0)
- default.qubit.tf (PennyLane-0.37.0.dev0)
- default.qubit.torch (PennyLane-0.37.0.dev0)
- default.qutrit (PennyLane-0.37.0.dev0)
- default.qutrit.mixed (PennyLane-0.37.0.dev0)
- null.qubit (PennyLane-0.37.0.dev0)
- lightning.qubit (PennyLane-Lightning-0.36.0)

Existing GitHub issues

  • I have searched existing GitHub issues to make sure the issue does not already exist.
@Shiro-Raven Shiro-Raven added the bug 🐛 Something isn't working label May 27, 2024
@dwierichs
Copy link
Contributor

dwierichs commented May 28, 2024

I can take on this issue. My suggestion would be to

  1. test that the SUPPORTED_GRADIENT_KWARGS list is up to date (and does not include "shots") using signature inspection, and
  2. [optionally] raise a dedicated warning or even error when "shots" is passed to QNode/qnode.

Does that make sense? @trbromley @albi3ro @josh146

Another consideration would be to make the whole framework more strict by raising an error if an unknown kwarg is passed, as vanilla Python would do. Do we need to support unspecified, uncaught kwargs in QNodes?

@albi3ro
Copy link
Contributor

albi3ro commented May 28, 2024

I wonder if we could keep SUPPORTED_GRADIENT_KWARGS update to date by using inspect.signature on all of our in-build gradient keyword arguments.

We could also add a check that ensures the provided gradient keyword arguments the used gradient_fn. For example, if backprop is chosen as "best", then we would warn if any gradient keyword arguments were specified.

@dwierichs
Copy link
Contributor

I wonder if we could keep SUPPORTED_GRADIENT_KWARGS update to date by using inspect.signature on all of our in-build gradient keyword arguments.

Somehow I preferred having this signature-based list in the tests and checking a static object against it.
However,...

We could also add a check that ensures the provided gradient keyword arguments the used gradient_fn. For example, if backprop is chosen as "best", then we would warn if any gradient keyword arguments were specified.

... I do like this. I suppose the repeated evaluation of the signatures has negligible run time? And would we run into risk of nested logic that differentiates between gradient transforms, vjp/jvp, backprop etc? 🤔 I may be confused about the layer of logic at which this test is performed 😬

dwierichs added a commit that referenced this issue Jun 4, 2024
**Context:**
Additional kwargs passed to a QNode at initialization are interpreted as
gradient kwargs.
They are checked against the hardcoded collection
`qml.gradients.SUPPORTED_GRADIENT_KWARGS` to make sure that this
boilerplate interpretation of additional kwargs makes sense.

**Description of the Change:**
This PR adds a test that makes sure that
`qml.gradients.SUPPORTED_GRADIENT_KWARGS` actually matches all gradient
transform kwargs supported by PL.
In addition, this PR introduces an error being raised when the keyword
argument `shots` is passed at QNode initialization, because this is not
a valid gradient kwarg and even if it were, it would lead to the
confusing behaviour of `QNode(..., shots=100)` not executing with
`shots=100`.

**Benefits:**
Code quality, better user input validation.

**Possible Drawbacks:**

**Related GitHub Issues:**
#5745 

[sc-64175]

---------

Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Co-authored-by: Christina Lee <christina@xanadu.ai>
@Shiro-Raven
Copy link
Contributor Author

Shiro-Raven commented Jun 5, 2024

Is this issue already handled? Should I close it? @dwierichs

@dwierichs
Copy link
Contributor

Oh ,yes good catch! This was handled in #5748

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

No branches or pull requests

3 participants