Repository: PennyLaneAI/pennylane

Issue:
Copying `qml.ControlledQubitUnitary`
The `Operator` base class defines a `__copy__` method for operators.  For almost all `Operator`'s, this method works fine to duplicate a given instance.

But `qml.ControlledQubitUnitary` has a more complicated set of data, and copying it does not work fully.

```python
import pennylane as qml
from pennylane import numpy as np

import copy

op = qml.ControlledQubitUnitary(np.eye(2) * 1j, wires=[0], control_wires=[2])
print(op.matrix)

cop = copy.copy(op) # = op.__copy__()
print(cop.matrix)
```

Error
```
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-6-4a2b6ba0ebcf> in <module>
----> 1 cop.matrix

pennylane/pennylane/operation.py in matrix(self)
    666     @property
    667     def matrix(self):
--> 668         op_matrix = self._matrix(*self.parameters)
    669 
    670         if self.inverse:

pennylane/pennylane/ops/qubit.py in _matrix(self, *params)
   2203 
   2204     def _matrix(self, *params):
-> 2205         if self._CU is None:
   2206             self._CU = block_diag(np.eye(self._padding_left), self.U, np.eye(self._padding_right))
   2207 
```

To remedy this problem, I propose that this class defines its own `__copy__` method.

We should also test `op.__copy__()` for each operation type.  Note that just calling `__copy__()` is not sufficient to tell if there is a problem.  In this situation, you need to call methods like `op_copy.matrix` to see that the copy was insufficient.

Relevant locations in code:
* [``ControlledQubitUnitary``](https://github.com/PennyLaneAI/pennylane/blob/0dcbed53b470eb7f6f93b54077e36ca6287235cf/pennylane/ops/qubit.py#L2110)
* [``Operator.__copy__(self)``](https://github.com/PennyLaneAI/pennylane/blob/0dcbed53b470eb7f6f93b54077e36ca6287235cf/pennylane/operation.py#L255)

Discussion:
github-actions[bot]: Hello. You may have forgotten to update the changelog!
Please edit [.github/CHANGELOG.md](/PennyLaneAI/pennylane/blob/master/.github/CHANGELOG.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.
albi3ro: As this PR doesn't actually change the number of class attributes, we should ignore the pylint error for now.  But maybe that is something that  should be fixed in the future?
codecov[bot]: # [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
> Merging [#1411](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (c19d180) into [master](https://codecov.io/gh/PennyLaneAI/pennylane/commit/ca227436391adf10df1f946cc727a9abb36673eb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (ca22743) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411/graphs/tree.svg?width=650&height=150&src=pr&token=9S3YzlUgGk&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)

```diff
@@           Coverage Diff           @@
##           master    #1411   +/-   ##
=======================================
  Coverage   98.21%   98.22%           
=======================================
  Files         160      160           
  Lines       11901    11905    +4     
=======================================
+ Hits        11689    11694    +5     
+ Misses        212      211    -1     
```


| [Impacted Files](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | Coverage Δ | |
|---|---|---|
| [pennylane/ops/qubit.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL29wcy9xdWJpdC5weQ==) | `98.69% <100.00%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Last update [ca22743...c19d180](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1411?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).

josh146: @albi3ro it has suddenly struck me that there was another fix for this in an old WIP PR, here:

https://github.com/PennyLaneAI/pennylane/pull/1276/files#diff-ed5f9c0de17892f74e5119e2a032fe277eaa330ec4df0c249c7a75af7be6585eR260

This fix modified the parent `Operation.__copy__` to copy _all_ instance attributes (again, except for the operation parameters).

Would this fix work here?
albi3ro: @josh146 Seems like that would also work.  Should we just use that method instead?
josh146: Since it is a more general fix, I think that might be a good idea (to avoid similar bugs appearing in the future)
albi3ro: Instead of creating a operator specific copy, this PR now creates a more general operation copy.

Patch:
diff --git a/.github/CHANGELOG.md b/.github/CHANGELOG.md
index 1ee7f40da..8c8c3da30 100644
--- a/.github/CHANGELOG.md
+++ b/.github/CHANGELOG.md
@@ -577,6 +577,9 @@ random_mat2 = rng.standard_normal(3, requires_grad=False)
 
 <h3>Bug fixes</h3>
 
+* Fixes a bug where a copy of `qml.ControlledQubitUnitary` was non-functional as it did not have all the necessary information.
+[(#1411)](https://github.com/PennyLaneAI/pennylane/pull/1411)
+
 * Warns when adjoint or reversible differentiation specified or called on a device with finite shots.
 [(#1406)](https://github.com/PennyLaneAI/pennylane/pull/1406)
 
diff --git a/pennylane/operation.py b/pennylane/operation.py
index 1142c28cb..fea8a597e 100644
--- a/pennylane/operation.py
+++ b/pennylane/operation.py
@@ -256,11 +256,9 @@ class Operator(abc.ABC):
         cls = self.__class__
         copied_op = cls.__new__(cls)
         copied_op.data = self.data.copy()
-        copied_op._wires = self.wires
-        copied_op._name = self._name
-
-        if hasattr(self, "_inverse"):
-            copied_op._inverse = self._inverse
+        for attr, value in vars(self).items():
+            if attr != "data":
+                setattr(copied_op, attr, value)
 
         return copied_op
 


Test patch:
diff --git a/tests/ops/test_qubit_ops.py b/tests/ops/test_qubit_ops.py
index c19e9c4cf..e6b7a5a80 100644
--- a/tests/ops/test_qubit_ops.py
+++ b/tests/ops/test_qubit_ops.py
@@ -18,6 +18,7 @@ import itertools
 import re
 import pytest
 import functools
+import copy
 import numpy as np
 from numpy.linalg import multi_dot
 from scipy.stats import unitary_group
@@ -407,10 +408,78 @@ NON_PARAMETRIZED_OPERATIONS = [
     (qml.Toffoli, Toffoli),
 ]
 
+PARAMETRIZED_OPERATIONS = [
+    qml.RX(0.123, wires=0),
+    qml.RY(1.434, wires=0),
+    qml.RZ(2.774, wires=0),
+    qml.S(wires=0),
+    qml.SX(wires=0),
+    qml.T(wires=0),
+    qml.CNOT(wires=[0, 1]),
+    qml.CZ(wires=[0, 1]),
+    qml.CY(wires=[0, 1]),
+    qml.SWAP(wires=[0, 1]),
+    qml.ISWAP(wires=[0, 1]),
+    qml.CSWAP(wires=[0, 1, 2]),
+    qml.PauliRot(0.123, "Y", wires=0),
+    qml.IsingXX(0.123, wires=[0, 1]),
+    qml.IsingZZ(0.123, wires=[0, 1]),
+    qml.Rot(0.123, 0.456, 0.789, wires=0),
+    qml.Toffoli(wires=[0, 1, 2]),
+    qml.PhaseShift(2.133, wires=0),
+    qml.ControlledPhaseShift(1.777, wires=[0, 2]),
+    qml.CPhase(1.777, wires=[0, 2]),
+    qml.MultiRZ(0.112, wires=[1, 2, 3]),
+    qml.CRX(0.836, wires=[2, 3]),
+    qml.CRY(0.721, wires=[2, 3]),
+    qml.CRZ(0.554, wires=[2, 3]),
+    qml.U1(0.123, wires=0),
+    qml.U2(3.556, 2.134, wires=0),
+    qml.U3(2.009, 1.894, 0.7789, wires=0),
+    qml.Hadamard(wires=0),
+    qml.PauliX(wires=0),
+    qml.PauliZ(wires=0),
+    qml.PauliY(wires=0),
+    qml.CRot(0.123, 0.456, 0.789, wires=[0, 1]),
+    qml.QubitUnitary(np.eye(2) * 1j, wires=0),
+    qml.DiagonalQubitUnitary(np.array([1.0, 1.0j]), wires=1),
+    qml.ControlledQubitUnitary(np.eye(2) * 1j, wires=[0], control_wires=[2]),
+    qml.MultiControlledX(control_wires=[0, 1], wires=2, control_values="01"),
+    qml.SingleExcitation(0.123, wires=[0, 3]),
+    qml.SingleExcitationPlus(0.123, wires=[0, 3]),
+    qml.SingleExcitationMinus(0.123, wires=[0, 3]),
+    qml.DoubleExcitation(0.123, wires=[0, 1, 2, 3]),
+    qml.DoubleExcitationPlus(0.123, wires=[0, 1, 2, 3]),
+    qml.DoubleExcitationMinus(0.123, wires=[0, 1, 2, 3]),
+    qml.QubitSum(wires=[0, 1, 2]),
+]
+
 
 class TestOperations:
     """Tests for the operations"""
 
+    @pytest.mark.parametrize("op_cls, mat", NON_PARAMETRIZED_OPERATIONS)
+    def test_nonparametrized_op_copy(self, op_cls, mat, tol):
+        """Tests that copied nonparametrized ops function as expected"""
+        op = op_cls(wires=range(op_cls.num_wires))
+        copied_op = copy.copy(op)
+        np.testing.assert_allclose(op.matrix, copied_op.matrix, atol=tol)
+
+        op._inverse = True
+        copied_op2 = copy.copy(op)
+        np.testing.assert_allclose(op.matrix, copied_op2.matrix, atol=tol)
+
+    @pytest.mark.parametrize("op", PARAMETRIZED_OPERATIONS)
+    def test_parametrized_op_copy(self, op, tol):
+        """Tests that copied parametrized ops function as expected"""
+        copied_op = copy.copy(op)
+        np.testing.assert_allclose(op.matrix, copied_op.matrix, atol=tol)
+
+        op.inv()
+        copied_op2 = copy.copy(op)
+        np.testing.assert_allclose(op.matrix, copied_op2.matrix, atol=tol)
+        op.inv()
+
     @pytest.mark.parametrize("ops, mat", NON_PARAMETRIZED_OPERATIONS)
     def test_matrices(self, ops, mat, tol):
         """Test matrices of non-parametrized operations are correct"""
@@ -418,54 +487,7 @@ class TestOperations:
         res = op.matrix
         assert np.allclose(res, mat, atol=tol, rtol=0)
 
-    @pytest.mark.parametrize(
-        "op",
-        [
-            qml.RX(0.123, wires=0),
-            qml.RY(1.434, wires=0),
-            qml.RZ(2.774, wires=0),
-            qml.S(wires=0),
-            qml.SX(wires=0),
-            qml.T(wires=0),
-            qml.CNOT(wires=[0, 1]),
-            qml.CZ(wires=[0, 1]),
-            qml.CY(wires=[0, 1]),
-            qml.SWAP(wires=[0, 1]),
-            qml.ISWAP(wires=[0, 1]),
-            qml.CSWAP(wires=[0, 1, 2]),
-            qml.PauliRot(0.123, "Y", wires=0),
-            qml.IsingXX(0.123, wires=[0, 1]),
-            qml.IsingZZ(0.123, wires=[0, 1]),
-            qml.Rot(0.123, 0.456, 0.789, wires=0),
-            qml.Toffoli(wires=[0, 1, 2]),
-            qml.PhaseShift(2.133, wires=0),
-            qml.ControlledPhaseShift(1.777, wires=[0, 2]),
-            qml.CPhase(1.777, wires=[0, 2]),
-            qml.MultiRZ(0.112, wires=[1, 2, 3]),
-            qml.CRX(0.836, wires=[2, 3]),
-            qml.CRY(0.721, wires=[2, 3]),
-            qml.CRZ(0.554, wires=[2, 3]),
-            qml.U1(0.123, wires=0),
-            qml.U2(3.556, 2.134, wires=0),
-            qml.U3(2.009, 1.894, 0.7789, wires=0),
-            qml.Hadamard(wires=0),
-            qml.PauliX(wires=0),
-            qml.PauliZ(wires=0),
-            qml.PauliY(wires=0),
-            qml.CRot(0.123, 0.456, 0.789, wires=[0, 1]),
-            qml.QubitUnitary(np.eye(2) * 1j, wires=0),
-            qml.DiagonalQubitUnitary(np.array([1.0, 1.0j]), wires=1),
-            qml.ControlledQubitUnitary(np.eye(2) * 1j, wires=[0], control_wires=[2]),
-            qml.MultiControlledX(control_wires=[0, 1], wires=2, control_values="01"),
-            qml.SingleExcitation(0.123, wires=[0, 3]),
-            qml.SingleExcitationPlus(0.123, wires=[0, 3]),
-            qml.SingleExcitationMinus(0.123, wires=[0, 3]),
-            qml.DoubleExcitation(0.123, wires=[0, 1, 2, 3]),
-            qml.DoubleExcitationPlus(0.123, wires=[0, 1, 2, 3]),
-            qml.DoubleExcitationMinus(0.123, wires=[0, 1, 2, 3]),
-            qml.QubitSum(wires=[0, 1, 2]),
-        ],
-    )
+    @pytest.mark.parametrize("op", PARAMETRIZED_OPERATIONS)
     def test_adjoint_unitaries(self, op, tol):
         op_d = op.adjoint()
         res1 = np.dot(op.matrix, op_d.matrix)