PennyLaneAI / pennylane

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.
https://pennylane.ai
Apache License 2.0
2.25k stars 584 forks source link

Improve QNode keyword input validation #2853

Open antalszava opened 2 years ago

antalszava commented 2 years ago

Feature details

Any keyword argument can be passed to a QNode or its qnode decorator without errors arising.

This can result in unexpected behaviour if users make a typo when attempting to use a valid option.

E.g., the following example uses the Autograd interface although inteface="jax" is being passed (because inteface is not an accepted kwarg):

import pennylane as qml

dev = qml.device('default.qubit', wires=3)

@qml.qnode(dev, inteface="jax")
def circuit():
    return qml.expval(qml.PauliZ(0))
>>> circuit()
tensor(1., requires_grad=True)

Implementation

No response

How important would you say this feature is?

2: Somewhat important. Needed this quarter.

Additional information

No response

nahumsa commented 2 years ago

Hi! I would like to solve this issue. Is checking the kwarg from the decorator in a list with all arguments that are accepted to the function and if it is not in this list it will raise a TypeError a valid solution? For instance:

TypeError: __init__() got an unexpected keyword argument 'inteface'
antalszava commented 2 years ago

Hi @nahumsa, great! :)

I think that sounds good. The main subtlety here is that kwargs passed to qml.qnode would be passed as gradient_kwargs to qml.execute. So it first has to be considered if the possible options for gradient_kwargs can be collected in advance or if not, if we can somehow create patterns and/or query options from the ML framework determined by a specific interface.

nahumsa commented 2 years ago

I see that the gradient_kwargs are arguments for the diff_method, so knowing the diff_method we could know which gradient_kwargs are accepted. Do you think that this is correct?

antalszava commented 2 years ago

I think so, yes, although it requires more consideration for each possible gradient function.

Josh (the author of this logic) mentioned that the reason why such a validation check has been avoided in the past was because of maintenance overhead: adding the validation check creates a connection between the QNode and the gradient function signatures where there isn't currently one, and changing one component may break the other.

josh146 commented 2 years ago

Another approach could be to raise an error in the gradient transform if it gets a kwarg it doesn't understand --- that way an unknown keyword argument in the QNode would raise an error for the user, without the QNode needing to know the 'allowed' gradient keywords. I'm not sure if there is any nuance I am missing here though

nahumsa commented 2 years ago

Another approach could be to raise an error in the gradient transform if it gets a kwarg it doesn't understand --- that way an unknown keyword argument in the QNode would raise an error for the user, without the QNode needing to know the 'allowed' gradient keywords. I'm not sure if there is any nuance I am missing here though

I think that raising an error in the gradient transform is the best way to solve this issue. I will try to implement that 😄

nahumsa commented 2 years ago

I have noticed that on pennylane.interfaces.execution.cache_execute:

https://github.com/PennyLaneAI/pennylane/blob/1e3a73107d1a897ba04910492c1c698d41e8787e/pennylane/interfaces/execution.py#L126-L128

The gradient_kwargs variable when running the default interface (which I think is autograd) is modified to an empty dict because of the pennylane.interfaces.autograd._execute

https://github.com/PennyLaneAI/pennylane/blob/1e3a73107d1a897ba04910492c1c698d41e8787e/pennylane/interfaces/autograd.py#L109-L110

Any thoughts about this?

P.S.: I'm trying to understand more about the codebase, sorry if the commentary wasn't that useful :)