Repository: PennyLaneAI/pennylane
Pull request #6039 (2024-07-24 22:29:16)

CNOT decomposition method returns DecompositionUndefinedError
**Context:**
Before CNOT inherited from `Controlled`, its decomposition method returned a `DecompositionUndefinedError`. Now it uses the smart decomposition functions from `Controlled`, which in this case make it decompose to itself.

```
>>> op = qml.CNOT([0, 1])
>>> op.decomposition()
[CNOT(wires=[0, 1])]
```

The change in the decomposition method of CNOT was unintentional, and operators should not decompose to themselves.

**Description of the Change:**
We put it back how it was:

```
>>> op = qml.CNOT([0, 1])
>>> op.decomposition()
DecompositionUndefinedError
```

**Related GitHub Issues:**
#5711 

github-actions[bot]: Hello. You may have forgotten to update the changelog!
Please edit [`doc/releases/changelog-dev.md`](/PennyLaneAI/pennylane/blob/master/doc/releases/changelog-dev.md) with:
* A one-to-two sentence description of the change. You may include a small working example for new features.
* A link back to this PR.
* Your name (or GitHub username) in the contributors section.
lillian542: [sc-63619]
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/6039?dropdown=coverage&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:
> Project coverage is 99.65%. Comparing base [(`a456fce`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/a456fce7f837ac3ea08be3af35c8454925c761eb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) to head [(`1e21841`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/1e2184153119bffa133f9dce538d8919f55bb012?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).
> Report is 2 commits behind head on master.

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


```diff
@@            Coverage Diff             @@
##           master    #6039      +/-   ##
==========================================
- Coverage   99.65%   99.65%   -0.01%     
==========================================
  Files         428      428              
  Lines       41160    40894     -266     
==========================================
- Hits        41020    40753     -267     
- Misses        140      141       +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/6039?dropdown=coverage&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 3b9c85ace..9c030886b 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -121,6 +121,9 @@
   Hamiltonians.
   [(#5950)](https://github.com/PennyLaneAI/pennylane/pull/5950)
 
+* The `CNOT` operator no longer decomposes to itself. Instead, it raises a `qml.DecompositionUndefinedError`.
+  [(#6039)](https://github.com/PennyLaneAI/pennylane/pull/6039)
+
 <h4>Community contributions 🥳</h4>
 
 * `DefaultQutritMixed` readout error has been added using parameters `readout_relaxation_probs` and 
diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py
index bc92514a4..e3795ff66 100644
--- a/pennylane/ops/functions/assert_valid.py
+++ b/pennylane/ops/functions/assert_valid.py
@@ -56,6 +56,9 @@ def _check_decomposition(op, skip_wire_mapping):
 
         assert isinstance(decomp, list), "decomposition must be a list"
         assert isinstance(compute_decomp, list), "decomposition must be a list"
+        assert op.__class__ not in [
+            decomp_op.__class__ for decomp_op in decomp
+        ], "an operator should not be included in its own decomposition"
 
         for o1, o2, o3 in zip(decomp, compute_decomp, processed_queue):
             assert o1 == o2, "decomposition must match compute_decomposition"
@@ -67,12 +70,17 @@ def _check_decomposition(op, skip_wire_mapping):
         # Check that mapping wires transitions to the decomposition
         wire_map = {w: ascii_lowercase[i] for i, w in enumerate(op.wires)}
         mapped_op = op.map_wires(wire_map)
-        mapped_decomp = mapped_op.decomposition()
-        orig_decomp = op.decomposition()
-        for mapped_op, orig_op in zip(mapped_decomp, orig_decomp):
-            assert (
-                mapped_op.wires == qml.map_wires(orig_op, wire_map).wires
-            ), "Operators in decomposition of wire-mapped operator must have mapped wires."
+        # calling `map_wires` on a Controlled operator generates a new `op` from the controls and
+        # base, so may return a different class of operator. We only compare decomps of `op` and
+        # `mapped_op` if `mapped_op` **has** a decomposition.
+        # see MultiControlledX([0, 1]) and CNOT([0, 1]) as an example
+        if mapped_op.has_decomposition:
+            mapped_decomp = mapped_op.decomposition()
+            orig_decomp = op.decomposition()
+            for mapped_op, orig_op in zip(mapped_decomp, orig_decomp):
+                assert (
+                    mapped_op.wires == qml.map_wires(orig_op, wire_map).wires
+                ), "Operators in decomposition of wire-mapped operator must have mapped wires."
     else:
         failure_comment = "If has_decomposition is False, then decomposition must raise a ``DecompositionUndefinedError``."
         _assert_error_raised(
diff --git a/pennylane/ops/op_math/controlled_ops.py b/pennylane/ops/op_math/controlled_ops.py
index d2c5d3afb..4d2a8475f 100644
--- a/pennylane/ops/op_math/controlled_ops.py
+++ b/pennylane/ops/op_math/controlled_ops.py
@@ -749,10 +749,10 @@ class CNOT(ControlledOp):
     The controlled-NOT operator
 
     .. math:: CNOT = \begin{bmatrix}
-            1 & 0 & 0 & 0 \\
-            0 & 1 & 0 & 0\\
-            0 & 0 & 0 & 1\\
-            0 & 0 & 1 & 0
+        1 & 0 & 0 & 0 \\
+        0 & 1 & 0 & 0\\
+        0 & 0 & 0 & 1\\
+        0 & 0 & 1 & 0
         \end{bmatrix}.
 
     .. note:: The first wire provided corresponds to the **control qubit**.
@@ -791,6 +791,32 @@ class CNOT(ControlledOp):
     def __init__(self, wires, id=None):
         super().__init__(qml.PauliX(wires=wires[1:]), wires[:1], id=id)
 
+    @property
+    def has_decomposition(self):
+        return False
+
+    @staticmethod
+    def compute_decomposition(*params, wires=None, **hyperparameters):  # -> List["Operator"]:
+        r"""Representation of the operator as a product of other operators (static method).
+
+        .. math:: O = O_1 O_2 \dots O_n.
+
+        .. note::
+            Operations making up the decomposition should be queued within the
+            ``compute_decomposition`` method.
+
+        .. seealso:: :meth:`~.Operator.decomposition`.
+
+        Args:
+            *params (list): trainable parameters of the operator, as stored in the ``parameters`` attribute
+            wires (Iterable[Any], Wires): wires that the operator acts on
+            **hyperparams (dict): non-trainable hyperparameters of the operator, as stored in the ``hyperparameters`` attribute
+
+        Raises:
+            qml.DecompositionUndefinedError
+        """
+        raise qml.operation.DecompositionUndefinedError
+
     def __repr__(self):
         return f"CNOT(wires={self.wires.tolist()})"
 
diff --git a/pennylane/transforms/tape_expand.py b/pennylane/transforms/tape_expand.py
index f3e9b0d26..60580eb3f 100644
--- a/pennylane/transforms/tape_expand.py
+++ b/pennylane/transforms/tape_expand.py
@@ -297,14 +297,17 @@ def _custom_decomp_context(custom_decomps):
             obj = getattr(qml, obj)
 
         original_decomp_method = obj.compute_decomposition
+        original_has_decomp_property = obj.has_decomposition
 
         try:
             # Explicitly set the new compute_decomposition method
             obj.compute_decomposition = staticmethod(fn)
+            obj.has_decomposition = lambda obj: True
             yield
 
         finally:
             obj.compute_decomposition = staticmethod(original_decomp_method)
+            obj.has_decomposition = original_has_decomp_property
 
     # Loop through the decomposition dictionary and create all the contexts
     try:


Test patch:
diff --git a/tests/ops/functions/test_assert_valid.py b/tests/ops/functions/test_assert_valid.py
index b9a6cf453..9fff77583 100644
--- a/tests/ops/functions/test_assert_valid.py
+++ b/tests/ops/functions/test_assert_valid.py
@@ -80,6 +80,17 @@ class TestDecompositionErrors:
         with pytest.raises(AssertionError, match="If has_decomposition is False"):
             assert_valid(BadDecomp(wires=0), skip_pickle=True)
 
+    def test_decomposition_must_not_contain_op(self):
+        """Test that the decomposition of an operator doesn't include the operator itself"""
+
+        class BadDecomp(Operator):
+            @staticmethod
+            def compute_decomposition(wires):
+                return [BadDecomp(wires)]
+
+        with pytest.raises(AssertionError, match="should not be included in its own decomposition"):
+            assert_valid(BadDecomp(wires=0), skip_pickle=True)
+
 
 class TestBadMatrix:
     """Tests involving matrix validation."""
diff --git a/tests/ops/op_math/test_controlled_ops.py b/tests/ops/op_math/test_controlled_ops.py
index d82308daf..7a002cccd 100644
--- a/tests/ops/op_math/test_controlled_ops.py
+++ b/tests/ops/op_math/test_controlled_ops.py
@@ -763,3 +763,15 @@ def test_tuple_control_wires_parametric_ops(op_type):
     """Test that tuples can be provided as control wire labels."""
 
     assert op_type(0.123, [(0, 1), 2]).wires == qml.wires.Wires([(0, 1), 2])
+
+
+def test_CNOT_decomposition():
+    """Test that CNOT raises a DecompositionUndefinedError instead of using the
+    controlled_op decomposition functions"""
+    assert not qml.CNOT((0, 1)).has_decomposition
+
+    with pytest.raises(qml.operation.DecompositionUndefinedError):
+        qml.CNOT.compute_decomposition()
+
+    with pytest.raises(qml.operation.DecompositionUndefinedError):
+        qml.CNOT([0, 1]).decomposition()