Add validation for noise channels parameters #1357
Conversation
nahumsa
changed the title
nahumsa
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1357 +/- ##
=======================================
Coverage 98.15% 98.15%
=======================================
Files 154 154
Lines 11492 11512 +20
=======================================
+ Hits 11280 11300 +20
Misses 212 212
Continue to review full report at Codecov.
|
pennylane/ops/channel.py
Outdated
| @@ -58,6 +58,10 @@ class AmplitudeDamping(Channel): | |||
| @classmethod | |||
| def _kraus_matrices(cls, *params): | |||
| gamma = params[0] | |||
|
|
|||
| if gamma < 0.0 or gamma > 1.0: | |||
There was a problem hiding this comment.
Alternatively, you could have
| if gamma < 0.0 or gamma > 1.0: | |
| if not 0 <= gamma <= 1.0: |
I don't think this is any more efficient, but it might be more readable?
There was a problem hiding this comment.
Sure, it looks more readable to me.
Co-authored-by: Josh Izaac <josh146@gmail.com>
There was a problem hiding this comment.
Hi @nahumsa, thanks for another great contribution!
Everything looks in order, but I have an important concern. I remember removing a warning like this because it was throwing errors when the channel parameters where trainable parameters. If I'm not mistaken, the issue was that a check like
if not 0 <= gamma <= 1.0:
would fail if gamma was not a float. The ability to train noise parameters is a unique feature of PennyLane, as shown in the noisy circuit tutorial, so we should be mindful not to break this functionality.
Please add some tests that gradients can be computed correctly and circuits can be trained, and I'll be happy to approve. If the tests fail, then you should still be able to adapt the checks so that no issues occur
|
Hi @ixfoduap! Thanks for pointing that out! I have a question regarding this test, it would be an end-to-end test just like the example, or just computing the gradient for a given example would be enough? |
|
I would like to test that the gradient of a simple circuit is computed correctly. Actually maybe such tests are already done for some of the channels. If that's the case, then I guess no more code needs to be written. Basically I just want to make sure that we don't ship breaking code. This doesn't have to appear as an actual integration test, but I would be more comfortable if you could run the examples from the tutorial from a pennylane version from this branch and check that everything goes well. |
|
I think that this test is covered here: @pytest.mark.parametrize("angle", np.linspace(0, 2 * np.pi, 7))
def test_grad_bitflip(self, angle, tol):
"""Test that analytical gradient is computed correctly for different states. Channel
grad recipes are independent of channel parameter"""
dev = qml.device("default.mixed", wires=1)
prob = 0.5
@qml.qnode(dev)
def circuit(p):
qml.RX(angle, wires=0)
qml.BitFlip(p, wires=0)
return qml.expval(qml.PauliZ(0))
gradient = np.squeeze(qml.grad(circuit)(prob))
assert gradient == circuit(1) - circuit(0)
assert np.allclose(gradient, (-2 * np.cos(angle)))Please, let me know if it needs more tests about this. |
Context: Noise channels do not validate parameter values
Description of the Change: Add validation for noise channel parameters, now it raises an
ValueErrorwhennoise parameters are invalid.
Benefits: It no longer raises a
RuntimeWarningwhen using invalid parameters on noise channels.Possible Drawbacks:
Related GitHub Issues: Closes #1337