Repository: PennyLaneAI/pennylane
Pull request #4922 (2023-12-06 19:58:31)

Automatically test operator validity
**Context:**
Now that we have the `assert_valid` function (#4764), we can easily validate operators meeting our expectations. The trouble is, we'd have to test every single operator manually. It would be nice to automatically generate an instance of each operator (with valid parameters) and use that to validate the class. 

**Description of the Change:**
Add `tests/ops/functions/conftest.py`, which generates a parametrization of all subclasses of `Operator` (specifics described below). This had to be put in `conftest.py` because pytest-xdist panics otherwise. Also update `test_assert_valid.py` to auto-generate an instance of each of these subclasses and assert that instance's validity. There are some details to mention about what I've done: I use `op_cls.ndim_params` and `op_cls.num_params` to generate valid parameters to construct the instance of the class. This doesn't always work well for various reasons:
- If they are defined as `property` variables, you can't view it at the class-level, so I just guess
- I use `len(ndim_params)` to guess the number of parameters, resorting to `num_params` if that fails
- I use `1.`, `[1.]*(2**n)`, or `np.eye(n)` if the `ndim_params` is 0, 1 or 2 respectively (for each parameter). I assume no other values exist, and that these will be the appropriate magnitudes. Not always true, eg. `BasisState` has `ndim_params=(1,)` to get an array, but the length is `n`, not `2**n`

`_SKIP_OP_TYPES` is a mish-mash of things we don't test. I wrote in-line comments in the dict to clarify what's what here. Anyway, they are:
- arithmetic ops
- templates
- ops that fail for unknown reasons
- ops that fail BUT have explicit test cases added (currently just a handful so we can have a test available with a parametrization for the future)

`_ABSTRACT_OR_META_TYPES` is a set of class types that we shouldn't try to create instances of. In `get_all_classes`, if `c` is an abstract (eg. `Operator`, although it's not explicitly stated to be abstract bc python) or meta (eg. `ControlledOp`, for internal construction only) class, we don't add it to the final list of classes to make instances of. We do still add their child classes.

There are also some types that I explicitly xfail in the test. These _succeed_ in being auto-generated, but they _fail_ validation. I've made this into a dict, so you have to give a reason explaining why it fails validation if you're putting it here.

**Benefits:**
- We are more confident in our existing operators!
- Any newly-added Operator will automatically get picked up by this test. Good ones get added easily, bad ones fail this test in CI and the dev knows how to fix their operator.

**Possible Drawbacks:**
Only thing I'm worried about now is ensuring that adding tests to make this totally complete is as easy as possible. might be tough because the configuration is in `conftest.py`, and not the test file itself. I've added comments to the test itself so hopefully people look at them.

I'll list possible future work. In order of what I'd like to see:
1. Start adding explicit test cases for everything in `_SKIP_OP_TYPES` (except qutrit ops, see next task)
2. make `assert_valid` work for qutrit ops then remove the xfail

[sc-51653]

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/4922?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`e2dfd64`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/e2dfd64ed008d50de98506678f8c0688deb18fd0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.66% compared to head [(`8fa309b`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/4922?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.66%.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4922      +/-   ##
==========================================
- Coverage   99.66%   99.66%   -0.01%     
==========================================
  Files         387      387              
  Lines       35049    34785     -264     
==========================================
- Hits        34932    34667     -265     
- Misses        117      118       +1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/4922?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).


Patch:
diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md
index 84767d6e3..41bc8429e 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -378,6 +378,10 @@
   done with `qml.ExpvalCost`, but this is the preferred method because `ExpvalCost` is deprecated.
   [(#4896)](https://github.com/PennyLaneAI/pennylane/pull/4896)
 
+* All PennyLane `Operator` subclasses are automatically tested by `ops.functions.assert_valid` to ensure
+  that they follow PennyLane `Operator` standards.
+  [(#4922)](https://github.com/PennyLaneAI/pennylane/pull/4922)
+
 <h3>Breaking changes 💔</h3>
 
 * The function `qml.transforms.classical_jacobian` has been moved to the gradients module
diff --git a/pennylane/ops/channel.py b/pennylane/ops/channel.py
index c3a12a4c4..6464b5c3a 100644
--- a/pennylane/ops/channel.py
+++ b/pennylane/ops/channel.py
@@ -729,6 +729,9 @@ class QubitChannel(Channel):
         if not np.allclose(Kraus_sum, np.eye(K_list[0].shape[0])):
             raise ValueError("Only trace preserving channels can be applied.")
 
+    def _flatten(self):
+        return (self.data,), (self.wires, ())
+
     @staticmethod
     def compute_kraus_matrices(*kraus_matrices):  # pylint:disable=arguments-differ
         """Kraus matrices representing the QubitChannel channel.
diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py
index 7006069bf..5e6e288f5 100644
--- a/pennylane/ops/functions/assert_valid.py
+++ b/pennylane/ops/functions/assert_valid.py
@@ -146,7 +146,7 @@ def _check_eigendecomposition(op):
         assert qml.math.allclose(eg, compute_eg), "eigvals and compute_eigvals must match"
 
     if has_eigvals and op.has_diagonalizing_gates:
-        dg = qml.prod(*dg) if len(dg) > 0 else qml.Identity(op.wires)
+        dg = qml.prod(*dg[::-1]) if len(dg) > 0 else qml.Identity(op.wires)
         eg = qml.QubitUnitary(np.diag(eg), wires=op.wires)
         decomp = qml.prod(qml.adjoint(dg), eg, dg)
         decomp_mat = qml.matrix(decomp)
diff --git a/pennylane/ops/functions/bind_new_parameters.py b/pennylane/ops/functions/bind_new_parameters.py
index d28ac9c6a..e976e43a6 100644
--- a/pennylane/ops/functions/bind_new_parameters.py
+++ b/pennylane/ops/functions/bind_new_parameters.py
@@ -111,6 +111,20 @@ def bind_new_parameters_composite_op(op: CompositeOp, params: Sequence[TensorLik
     return op.__class__(*new_operands)
 
 
+@bind_new_parameters.register
+def bind_new_parameters_cy(
+    op: qml.CY, params: Sequence[TensorLike]
+):  # pylint:disable=unused-argument
+    return copy.copy(op)
+
+
+@bind_new_parameters.register
+def bind_new_parameters_cz(
+    op: qml.CZ, params: Sequence[TensorLike]
+):  # pylint:disable=unused-argument
+    return copy.copy(op)
+
+
 @bind_new_parameters.register
 def bind_new_parameters_symbolic_op(op: SymbolicOp, params: Sequence[TensorLike]):
     new_base = bind_new_parameters(op.base, params)
diff --git a/pennylane/ops/meta.py b/pennylane/ops/meta.py
index e87247c52..c9f6fde37 100644
--- a/pennylane/ops/meta.py
+++ b/pennylane/ops/meta.py
@@ -214,6 +214,13 @@ class Snapshot(Operation):
     def label(self, decimals=None, base_label=None, cache=None):
         return "|Snap|"
 
+    def _flatten(self):
+        return (), (self.tag, self.hyperparameters["measurement"])
+
+    @classmethod
+    def _unflatten(cls, data, metadata):
+        return cls(tag=metadata[0], measurement=metadata[1])
+
     # pylint: disable=W0613
     @staticmethod
     def compute_decomposition(*params, wires=None, **hyperparameters):
diff --git a/pennylane/ops/qubit/observables.py b/pennylane/ops/qubit/observables.py
index 279489487..cfbf88d26 100644
--- a/pennylane/ops/qubit/observables.py
+++ b/pennylane/ops/qubit/observables.py
@@ -58,6 +58,9 @@ class Hermitian(Observable):
     num_params = 1
     """int: Number of trainable parameters that the operator depends on."""
 
+    ndim_params = (2,)
+    """tuple[int]: Number of dimensions per trainable parameter that the operator depends on."""
+
     grad_method = "F"
 
     # Qubit case
@@ -380,6 +383,9 @@ class Projector(Observable):
     num_params = 1
     """int: Number of trainable parameters that the operator depends on."""
 
+    ndim_params = (1,)
+    """tuple[int]: Number of dimensions per trainable parameter that the operator depends on."""
+
     def __new__(cls, state, wires, **_):
         """Changes parents based on the state representation.
 


Test patch:
diff --git a/tests/ops/functions/conftest.py b/tests/ops/functions/conftest.py
new file mode 100644
index 000000000..c0e35cb2e
--- /dev/null
+++ b/tests/ops/functions/conftest.py
@@ -0,0 +1,104 @@
+# Copyright 2018-2023 Xanadu Quantum Technologies Inc.
+
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+Pytest configuration file for ops.functions submodule.
+
+Generates a parametrization of valid operators to test.
+"""
+from inspect import getmembers, isclass
+import pytest
+
+import pennylane as qml
+from pennylane.operation import Operator, Operation, Observable, Tensor, Channel
+from pennylane.ops.op_math.adjoint import Adjoint, AdjointObs, AdjointOperation, AdjointOpObs
+
+# if you would like to validate one of these operators, add an instance to the parametrization
+# of `test_explicit_list_of_ops` in `test_assert_valid.py`, and kindly move it below the
+# "manually validated ops" comment in this dict for easy inspection in the future.
+_SKIP_OP_TYPES = {
+    # ops composed of more than one thing
+    Adjoint,
+    Tensor,
+    qml.Hamiltonian,
+    qml.ops.Pow,
+    qml.ops.SProd,
+    qml.ops.Prod,
+    qml.ops.Controlled,
+    qml.ops.Exp,
+    qml.ops.Evolution,
+    qml.ops.Conditional,
+    # fails for unknown reason - should be registered in the test parametrization below
+    qml.SparseHamiltonian,  # fails because data is not as expected
+    qml.pulse.ParametrizedEvolution,
+    qml.THermitian,
+    qml.resource.FirstQuantization,
+    qml.SpecialUnitary,
+    qml.IntegerComparator,
+    qml.PauliRot,
+    qml.PauliError,
+    qml.ops.qubit.special_unitary.TmpPauliRot,  # private object
+    qml.StatePrep,
+    qml.PCPhase,
+    qml.BlockEncode,
+    qml.resource.DoubleFactorization,
+    qml.resource.ResourcesOperation,
+    # templates
+    *[i[1] for i in getmembers(qml.templates) if isclass(i[1]) and issubclass(i[1], Operator)],
+    # manually validated ops
+    qml.ops.Sum,
+    qml.BasisState,
+    qml.ControlledQubitUnitary,
+    qml.QubitStateVector,
+    qml.QubitChannel,
+    qml.MultiControlledX,
+}
+
+
+# types that should not have actual instances created
+_ABSTRACT_OR_META_TYPES = {
+    Operator,
+    Operation,
+    Observable,
+    Channel,
+    qml.ops.SymbolicOp,
+    qml.ops.ScalarSymbolicOp,
+    qml.ops.CompositeOp,
+    qml.ops.ControlledOp,
+    qml.ops.qubit.BasisStateProjector,
+    qml.ops.qubit.StateVectorProjector,
+    qml.ops.qubit.StatePrepBase,
+    AdjointOpObs,
+    AdjointOperation,
+    AdjointObs,
+}
+
+
+def get_all_classes(c):
+    """Recursive function to generate a flat list of all child classes of ``c``.
+    (first called with ``Operator``)."""
+    if c.__module__[:10] != "pennylane.":
+        return []
+    subs = c.__subclasses__()
+    classes = [] if c in _ABSTRACT_OR_META_TYPES else [c]
+    for sub in subs:
+        classes.extend(get_all_classes(sub))
+    return classes
+
+
+_CLASSES_TO_TEST = set(get_all_classes(Operator)) - _SKIP_OP_TYPES
+
+
+@pytest.fixture(params=sorted(_CLASSES_TO_TEST, key=lambda op: op.__name__))
+def class_to_validate(request):
+    yield request.param
diff --git a/tests/ops/functions/test_assert_valid.py b/tests/ops/functions/test_assert_valid.py
index 3b4f561d3..c3208048e 100644
--- a/tests/ops/functions/test_assert_valid.py
+++ b/tests/ops/functions/test_assert_valid.py
@@ -315,3 +315,82 @@ def test_data_is_tuple():
 
     with pytest.raises(AssertionError, match=r"op.data must be a tuple"):
         assert_valid(BadData(2.0, wires=0))
+
+
+def create_op_instance(c):
+    """Given an Operator class, create an instance of it."""
+    n_wires = c.num_wires
+    if n_wires == qml.operation.AllWires:
+        n_wires = 0
+    elif n_wires == qml.operation.AnyWires:
+        n_wires = 1
+
+    wires = qml.wires.Wires(range(n_wires))
+    if (num_params := c.num_params) == 0:
+        return c(wires) if wires else c()
+    if isinstance(num_params, property):
+        num_params = 1
+
+    # get ndim_params
+    if isinstance((ndim_params := c.ndim_params), property):
+        ndim_params = (0,) * num_params
+
+    # turn ndim_params into valid params
+    [dim] = set(ndim_params)
+    if dim == 0:
+        params = [1] * len(ndim_params)
+    elif dim == 1:
+        params = [[1] * 2**n_wires] * len(ndim_params)
+    elif dim == 2:
+        params = [np.eye(2)] * len(ndim_params)
+    else:
+        raise ValueError("unexpected dim:", dim)
+
+    return c(*params, wires=wires) if wires else c(*params)
+
+
+def test_generated_list_of_ops(class_to_validate):
+    """Test every auto-generated operator instance."""
+    if fail_reason := {
+        qml.Identity: "empty decomposition, matrix differs from decomp's matrix",
+        qml.GlobalPhase: "empty decomposition, matrix differs from decomp's matrix",
+        qml.PhaseShift: "decomposition still needs GlobalPhase, see #4657",
+    }.get(class_to_validate):
+        pytest.xfail(reason=fail_reason)
+    if class_to_validate.__module__[14:20] == "qutrit":
+        # QutritBasisState actually passes validation... but it shouldn't
+        pytest.xfail(reason="qutrit ops fail matrix validation")
+
+    # If you defined a new Operator and this call to `create_op_instance` failed, it might
+    # be the fault of the test and not your Operator. Please do one of the following things:
+    #   1. Update your Operator to meet PL standards so it passes
+    #   2. Improve `create_op_instance` so it can create an instance of your op (it is quite hacky)
+    #   3. Add your class to the "manually validated ops" section of `_SKIP_OP_TYPES`
+    #      in ./conftest.py, and add an instance of it to "test_explicit_list_of_ops" below
+    op = create_op_instance(class_to_validate)
+    # If you defined a new Operator and this call to `assert_valid` failed, the Operator doesn't
+    # follow PL standards. Please do one of the following things:
+    #   1. Preferred action: Update your Operator to meet PL standards so it passes
+    #   2. Add your Operator to the `fail_reason` dict above, with an explanation for why it needs
+    #      to go against PL standards
+    assert_valid(op)
+
+
+@pytest.mark.parametrize(
+    "op",
+    [
+        qml.sum(qml.PauliX(0), qml.PauliZ(0)),
+        qml.BasisState([1], wires=[0]),
+        qml.ControlledQubitUnitary(np.eye(2), control_wires=1, wires=0),
+        qml.QubitStateVector([0, 1], wires=0),
+        qml.QubitChannel(
+            [np.array([[1.0, 0.0], [0.0, 0.9486833]]), np.array([[0.0, 0.31622777], [0.0, 0.0]])],
+            wires=0,
+        ),
+        qml.MultiControlledX(wires=[0, 1]),
+        qml.Projector([1], 0),  # the state-vector version is already tested
+    ],
+)
+def test_explicit_list_of_ops(op):
+    """Test the validity of operators that could not be auto-generated."""
+    assert_valid(op)