Repository: PennyLaneAI/pennylane
Pull request #4978 (2024-01-02 20:02:36)

Test all operators with assert_valid, or fail with a reason
**Context:**
Continuation of the work I started in #4922 (this is part 1 of the future work listed in that PR)

**Description of the Change:**
1. Implement `_flatten` for `BlockEncode` and `SpecialUnitary` to pytreeify those types
2. Organize `tests/ops/functions/conftest.py` to better parametrize tests. The parametrization fixtures are easier to make using the following:
  - `_INSTANCES_TO_TEST` is a list of operator instances that couldn't be auto-generated, but do pass validation
  - `_INSTANCES_TO_FAIL` is a list of tuples (`(Operator, Type[Exception])`) where the first element is an operator instance that will fail validation, and the second element is the exception type raised during validation. Ideally, it also comes with a comment explaining what's failing/why it should fail.

All test collection logic is now in `conftest.py`, making the tests much smaller and simpler. The only "extra" logic in the tests is the xfail I added for all qutrit ops before auto-generation. Now there are 3 tests:
1. `test_generated_list_of_ops`, which auto-generates instances of every Operator type and asserts its validity
2. `test_explicit_list_of_ops`, which tests the validity of manual operator instances
3. `test_explicit_list_of_failing_ops`, which ensures that invalid operator instances fail to validate as expected

**Benefits:**
Every single pennylane operator (except for templates) is (in)validated in a single test file, and most of it is done automatically.

**Possible Drawbacks:**
Yet another test file that needs maintenance.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/4978?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 [(`b6b1f16`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/b6b1f164b33bb9919ead465007a05ea628d8d551?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.67% compared to head [(`6433acf`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/4978?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.67%.


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


```diff
@@            Coverage Diff             @@
##           master    #4978      +/-   ##
==========================================
- Coverage   99.67%   99.67%   -0.01%     
==========================================
  Files         392      392              
  Lines       35556    35292     -264     
==========================================
- Hits        35442    35177     -265     
- Misses        114      115       +1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/4978?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 e884eb72b..4d252aed7 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -6,6 +6,9 @@
 
 <h3>Improvements 🛠</h3>
 
+* Update `tests/ops/functions/conftest.py` to ensure all operator types are tested for validity.
+  [(#4978)](https://github.com/PennyLaneAI/pennylane/pull/4978)
+
 <h3>Breaking changes 💔</h3>
 
 <h3>Deprecations 👋</h3>
@@ -17,3 +20,5 @@
 <h3>Contributors ✍️</h3>
 
 This release contains contributions from (in alphabetical order):
+
+Matthew Silverman
diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py
index 514667169..0f0252a84 100644
--- a/pennylane/ops/functions/assert_valid.py
+++ b/pennylane/ops/functions/assert_valid.py
@@ -277,7 +277,8 @@ def assert_valid(op: qml.operation.Operator, skip_pickle=False) -> None:
         assert isinstance(d, qml.typing.TensorLike), "each data element must be tensorlike"
         assert qml.math.allclose(d, p), "data and parameters must match."
 
-    _check_wires(op)
+    if len(op.wires) <= 26:
+        _check_wires(op)
     _check_copy(op)
     _check_pytree(op)
     if not skip_pickle:
diff --git a/pennylane/ops/qubit/matrix_ops.py b/pennylane/ops/qubit/matrix_ops.py
index 82d04261d..691488448 100644
--- a/pennylane/ops/qubit/matrix_ops.py
+++ b/pennylane/ops/qubit/matrix_ops.py
@@ -542,6 +542,9 @@ class BlockEncode(Operation):
         self.hyperparameters["norm"] = normalization
         self.hyperparameters["subspace"] = subspace
 
+    def _flatten(self):
+        return self.data, (self.wires, ())
+
     @staticmethod
     def compute_matrix(*params, **hyperparams):
         r"""Representation of the operator as a canonical matrix in the computational basis (static method).
diff --git a/pennylane/ops/qubit/special_unitary.py b/pennylane/ops/qubit/special_unitary.py
index 997e73551..63ec7cb2b 100644
--- a/pennylane/ops/qubit/special_unitary.py
+++ b/pennylane/ops/qubit/special_unitary.py
@@ -422,6 +422,9 @@ class SpecialUnitary(Operation):
 
         super().__init__(theta, wires=wires, id=id)
 
+    def _flatten(self):
+        return self.data, (self.wires, ())
+
     @staticmethod
     def compute_matrix(theta, num_wires):
         r"""Representation of the operator as a canonical matrix in the computational basis
diff --git a/pennylane/resource/first_quantization.py b/pennylane/resource/first_quantization.py
index 06809fc5e..f93e32f6e 100644
--- a/pennylane/resource/first_quantization.py
+++ b/pennylane/resource/first_quantization.py
@@ -135,6 +135,19 @@ class FirstQuantization(Operation):
 
         super().__init__(wires=range(self.qubits))
 
+    def _flatten(self):
+        return (self.n, self.eta), (
+            ("omega", self.omega),
+            ("error", self.error),
+            ("charge", self.charge),
+            ("br", self.br),
+            ("vectors", self.vectors),
+        )
+
+    @classmethod
+    def _unflatten(cls, data, metadata):
+        return cls(*data, **dict(metadata))
+
     @staticmethod
     def success_prob(n, br):
         r"""Return the probability of success for state preparation.
diff --git a/pennylane/resource/second_quantization.py b/pennylane/resource/second_quantization.py
index 602abeef7..21777cdd3 100644
--- a/pennylane/resource/second_quantization.py
+++ b/pennylane/resource/second_quantization.py
@@ -158,6 +158,24 @@ class DoubleFactorization(Operation):
 
         super().__init__(wires=range(self.qubits))
 
+    def _flatten(self):
+        return (self.one_electron, self.two_electron), (
+            ("error", self.error),
+            ("rank_r", self.rank_r),
+            ("rank_m", self.rank_m),
+            ("rank_max", self.rank_max),
+            ("tol_factor", self.tol_factor),
+            ("tol_eigval", self.tol_eigval),
+            ("br", self.br),
+            ("alpha", self.alpha),
+            ("beta", self.beta),
+            ("chemist_notation", True),
+        )
+
+    @classmethod
+    def _unflatten(cls, data, metadata):
+        return cls(*data, **dict(metadata))
+
     @staticmethod
     def estimation_cost(lamb, error):
         r"""Return the number of calls to the unitary needed to achieve the desired error in quantum


Test patch:
diff --git a/tests/ops/functions/conftest.py b/tests/ops/functions/conftest.py
index 276013fc1..c4617301e 100644
--- a/tests/ops/functions/conftest.py
+++ b/tests/ops/functions/conftest.py
@@ -14,78 +14,122 @@
 """
 Pytest configuration file for ops.functions submodule.
 
-Generates a parametrization of valid operators to test.
+Generates parametrizations of operators to test in test_assert_valid.py.
 """
 from inspect import getmembers, isclass
 import pytest
+import numpy as np
 
 import pennylane as qml
 from pennylane.operation import Operator, Operation, Observable, Tensor, Channel
+from pennylane.operation import DiagGatesUndefinedError, MatrixUndefinedError
 from pennylane.ops.op_math.adjoint import Adjoint, AdjointObs, AdjointOperation, AdjointOpObs
 from pennylane.ops.op_math.pow import PowObs, PowOperation, PowOpObs
 
-# 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,
-}
+_INSTANCES_TO_TEST = [
+    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.8]]), np.array([[0, 0.6], [0, 0]])], wires=0),
+    qml.MultiControlledX(wires=[0, 1]),
+    qml.Projector([1], 0),  # the state-vector version is already tested
+    qml.SpecialUnitary([1, 1, 1], 0),
+    qml.IntegerComparator(1, wires=[0, 1]),
+    qml.PauliRot(1.1, "X", wires=[0]),
+    qml.StatePrep([0, 1], 0),
+    qml.PCPhase(0.27, dim=2, wires=[0, 1]),
+    qml.BlockEncode([[0.1, 0.2], [0.3, 0.4]], wires=[0, 1]),
+    qml.adjoint(qml.PauliX(0)),
+    qml.adjoint(qml.RX(1.1, 0)),
+    Tensor(qml.PauliX(0), qml.PauliX(1)),
+    qml.Hamiltonian([1.1, 2.2], [qml.PauliX(0), qml.PauliZ(0)]),
+    qml.s_prod(1.1, qml.RX(1.1, 0)),
+    qml.prod(qml.PauliX(0), qml.PauliY(1), qml.PauliZ(0)),
+    qml.ctrl(qml.RX(1.1, 0), 1),
+    qml.exp(qml.PauliX(0), 1.1),
+    qml.pow(qml.IsingXX(1.1, [0, 1]), 2.5),
+    qml.ops.Evolution(qml.PauliX(0), 5.2),
+    qml.QutritBasisState([1, 2, 0], wires=[0, 1, 2]),
+    qml.resource.FirstQuantization(1, 2, 1),
+]
+"""Valid operator instances that could not be auto-generated."""
+
+
+_INSTANCES_TO_FAIL = [
+    (
+        qml.SparseHamiltonian(qml.Hamiltonian([1.1], [qml.PauliX(0)]).sparse_matrix(), [0]),
+        AssertionError,  # each data element must be tensorlike
+    ),
+    (
+        qml.PauliError("X", 0.5, wires=0),
+        AssertionError,  # each data element must be tensorlike
+    ),
+    (
+        qml.THermitian(np.eye(3), wires=0),
+        AssertionError,  # qutrit ops fail validation
+    ),
+    (
+        qml.ops.qubit.special_unitary.TmpPauliRot(1.1, "X", [0]),
+        AssertionError,  # private type with has_matrix=False despite having one
+    ),
+    (
+        qml.ops.Conditional(qml.measure(1), qml.S(0)),
+        AssertionError,  # needs flattening helpers to be updated, also cannot be pickled
+    ),
+    (
+        qml.prod(qml.RX(1.1, 0), qml.RY(2.2, 0), qml.RZ(3.3, 1)),
+        DiagGatesUndefinedError,  # has_diagonalizing_gates should be False
+    ),
+    (
+        qml.Identity(0),
+        MatrixUndefinedError,  # empty decomposition, matrix differs from decomp's matrix
+    ),
+    (
+        qml.GlobalPhase(1.1),
+        MatrixUndefinedError,  # empty decomposition, matrix differs from decomp's matrix
+    ),
+    (
+        qml.pulse.ParametrizedEvolution(qml.PauliX(0) + sum * qml.PauliZ(0)),
+        ValueError,  # binding parameters fail, and more
+    ),
+    (
+        qml.resource.DoubleFactorization(np.eye(2), np.arange(16).reshape((2,) * 4)),
+        TypeError,  # op.eigvals is a list (overwritten in the init)
+    ),
+]
+"""
+List[Tuple[Operator, Type[Exception]]]: List of tuples containing Operator instances that could
+not be auto-generated, along with the exception type raised when trying to assert its validity.
+
+These operators need to break PL conventions, and each one's reason is specified in a comment.
+"""
 
 
-# types that should not have actual instances created
 _ABSTRACT_OR_META_TYPES = {
+    Adjoint,
+    AdjointOpObs,
+    AdjointOperation,
+    AdjointObs,
     Operator,
     Operation,
     Observable,
     Channel,
     qml.ops.SymbolicOp,
     qml.ops.ScalarSymbolicOp,
+    qml.ops.Pow,
     qml.ops.CompositeOp,
+    qml.ops.Controlled,
     qml.ops.ControlledOp,
     qml.ops.qubit.BasisStateProjector,
     qml.ops.qubit.StateVectorProjector,
     qml.ops.qubit.StatePrepBase,
-    AdjointOpObs,
-    AdjointOperation,
-    AdjointObs,
+    qml.resource.ResourcesOperation,
     PowOpObs,
     PowOperation,
     PowObs,
 }
+"""Types that should not have actual instances created."""
 
 
 def get_all_classes(c):
@@ -100,9 +144,25 @@ def get_all_classes(c):
     return classes
 
 
-_CLASSES_TO_TEST = set(get_all_classes(Operator)) - _SKIP_OP_TYPES
+_CLASSES_TO_TEST = (
+    set(get_all_classes(Operator))
+    - {i[1] for i in getmembers(qml.templates) if isclass(i[1]) and issubclass(i[1], Operator)}
+    - {type(op) for op in _INSTANCES_TO_TEST}
+    - {type(op) for (op, _) in _INSTANCES_TO_FAIL}
+)
+"""All operators, except those tested manually, abstract/meta classes, and templates."""
 
 
 @pytest.fixture(params=sorted(_CLASSES_TO_TEST, key=lambda op: op.__name__))
 def class_to_validate(request):
     yield request.param
+
+
+@pytest.fixture(params=_INSTANCES_TO_TEST)
+def valid_instance(request):
+    yield request.param
+
+
+@pytest.fixture(params=_INSTANCES_TO_FAIL)
+def invalid_instance_and_error(request):
+    yield request.param
diff --git a/tests/ops/functions/test_assert_valid.py b/tests/ops/functions/test_assert_valid.py
index 308bcf085..7a7291097 100644
--- a/tests/ops/functions/test_assert_valid.py
+++ b/tests/ops/functions/test_assert_valid.py
@@ -351,45 +351,33 @@ def create_op_instance(c):
 
 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",
-    }.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
+    #   3. Add an instance of your class to `_INSTANCES_TO_TEST` in ./conftest.py
+    #       Note: if it then fails validation, move it to `_INSTANCES_TO_FAIL` as described 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
+    #   2. Add an instance of your class to `_INSTANCES_TO_FAIL` in ./conftest.py, along with the
+    #       exception type raised by the assertion and a comment explaining the assumption your
+    #       operator makes.
     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):
+def test_explicit_list_of_ops(valid_instance):
     """Test the validity of operators that could not be auto-generated."""
-    assert_valid(op)
+    assert_valid(valid_instance)
+
+
+def test_explicit_list_of_failing_ops(invalid_instance_and_error):
+    """Test instances of ops that fail validation."""
+    op, exc_type = invalid_instance_and_error
+    with pytest.raises(exc_type):
+        assert_valid(op)