Repository: PennyLaneAI/pennylane
Pull request #1376 (2021-05-31 17:45:44)

Fixes a bug where `qml.ctrl` would fail to transform gates that had no control defined and no decomposition defined.
**Context:**

The current behaviour of `qml.ctrl` is as follows:

- Call the `Operation._controlled()` method
- If not defined, call `Operation.expand()`, and then call `_controlled()` on each gate in the decomposition.

However, if there is a gate with no decomposition defined, such as `CX`, `CY`, and `CZ`, then a `NotImplementedError` is raised which is **ignored further in the logic**. As a result, an obscure error message is produced:

```python
import pennylane as qml

dev = qml.device("default.qubit", wires=3)

@qml.qnode(dev)
def circuit():
    qml.ctrl(qml.CZ, control=0)(wires=[1, 2])
    return qml.probs(wires=2)

>>> circuit()
pennylane._device.DeviceError: Gate ControlledOperation not supported on device default.qubit.autograd
```

**Description of the Change:**

Add a try-except branch, which allows `qml.ctrl` to deal with the lack of a decomposition by simply calling `qml.ControlledQubitUnitary` with the dense matrix representation of the operator to control.

**Benefits:**

Any gate that does not have the expansion nor control specified will now work on any device that supports `ControlledQubitUnitary`.

```python
>>> circuit()
[1., 0.]
>>> print(qml.draw(circuit)())
 0: ──╭C───┤
 1: ──├U0──┤
 2: ──╰U0──┤ Probs
U0 =
[[ 1  0  0  0]
 [ 0  1  0  0]
 [ 0  0  1  0]
 [ 0  0  0 -1]]
```

**Possible Drawbacks:**

Since this fallback uses a dense matrix representation, it is _not_ efficient. Instead, it makes more sense to consider the device during the control expansion:

- If the device supports arbitrary controls, never expand `ControlledOperation`, instead, translate it directly to the devices arbitrary control logic.

- If the device does not support arbitrary controls, but natively supports the controlled version of the _specific_ gate, e.g., `ControlledOperation` contains a `CZ` and the underlying device supports `CCZ`, this should be automatically used.

- Finally, if the device does not support either of the above, we _could_ fall back to `ControlledQubitUnitary`, or we could raise an error.

**Related GitHub Issues:** n/a


codecov[bot]: # [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
> Merging [#1376](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (26394ba) into [master](https://codecov.io/gh/PennyLaneAI/pennylane/commit/a4d7abdc047869a2d378e54c7b1cc0b036ae7625?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (a4d7abd) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376/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/1376?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)

```diff
@@           Coverage Diff           @@
##           master    #1376   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files         154      154           
  Lines       11548    11552    +4     
=======================================
+ Hits        11336    11340    +4     
  Misses        212      212           
```


| [Impacted Files](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | Coverage Δ | |
|---|---|---|
| [pennylane/transforms/control.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL3RyYW5zZm9ybXMvY29udHJvbC5weQ==) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376?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/1376?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Last update [a4d7abd...26394ba](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1376?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: > how would a device prevent expansion of `qml.ctrl(qml.CZ, control=0)` to `ControlledQubitUnitary` in the event that it is able to support arbitrary control? Would this be a combination of moving tape expansion to the device and the device defining it's own custom stop_at function (which would stop at `ControlledOperation`)?

This is a good question that is not 100% clear. I think this is the case, yes!
josh146: @trbromley great catch, it turns out there weren't any tests testing `qml.ctrl` acting on nested tapes. Have fixed the logic, and added a new test to check exactly this.

Patch:
diff --git a/.github/CHANGELOG.md b/.github/CHANGELOG.md
index 884d572ba..2ef5f6148 100644
--- a/.github/CHANGELOG.md
+++ b/.github/CHANGELOG.md
@@ -351,6 +351,10 @@ random_mat2 = rng.standard_normal(3, requires_grad=False)
 
 <h3>Bug fixes</h3>
 
+* Fixes a bug where `qml.ctrl` would fail to transform gates that had no
+  control defined and no decomposition defined.
+  [(#1376)](https://github.com/PennyLaneAI/pennylane/pull/1376)
+
 * Copying the `JacobianTape` now correctly also copies the `jacobian_options` attribute. This fixes a bug
   allowing the JAX interface to support adjoint differentiation.
   [(#1349)](https://github.com/PennyLaneAI/pennylane/pull/1349)
diff --git a/pennylane/transforms/control.py b/pennylane/transforms/control.py
index 13a799d28..3a583b131 100644
--- a/pennylane/transforms/control.py
+++ b/pennylane/transforms/control.py
@@ -15,6 +15,8 @@
 Contains the control transform.
 """
 from functools import wraps
+
+import pennylane as qml
 from pennylane.tape import QuantumTape, get_active_tape
 from pennylane.operation import Operation, AnyWires
 from pennylane.wires import Wires
@@ -45,9 +47,24 @@ def expand_with_control(tape, control_wire):
                 # pylint: disable=protected-access
                 op._controlled(control_wire)
             else:
-                tmp_tape = op.expand()
+                # Attempt to decompose the operation and apply
+                # controls to each gate in the decomposition.
+                with new_tape.stop_recording():
+                    try:
+                        tmp_tape = op.expand()
+
+                    except NotImplementedError:
+                        # No decomposition is defined. Create a
+                        # ControlledQubitUnitary gate using the operation
+                        # matrix representation.
+                        with QuantumTape() as tmp_tape:
+                            qml.ControlledQubitUnitary(
+                                op.matrix, control_wires=control_wire, wires=op.wires
+                            )
+
                 tmp_tape = expand_with_control(tmp_tape, control_wire)
                 requeue_ops_in_tape(tmp_tape)
+
     return new_tape
 
 


Test patch:
diff --git a/tests/transforms/test_control.py b/tests/transforms/test_control.py
index 641d108c8..6f4181ec2 100644
--- a/tests/transforms/test_control.py
+++ b/tests/transforms/test_control.py
@@ -210,3 +210,48 @@ def test_no_control_defined():
     # Check that all operations are updated to their controlled version.
     for op in tape.operations:
         assert type(op) in {qml.ControlledPhaseShift, qml.Toffoli, qml.CRX, qml.CSWAP}
+
+
+def test_no_decomposition_defined():
+    """Test that a controlled gate that has no control transform defined,
+    as well as no decomposition transformed defined, still works correctly"""
+
+    with QuantumTape() as tape:
+        ctrl(qml.CZ, 0)(wires=[1, 2])
+
+    tape = expand_tape(tape)
+
+    assert len(tape.operations) == 1
+    assert tape.operations[0].name == "ControlledQubitUnitary"
+
+
+def test_controlled_template():
+    """Test that a controlled template correctly expands
+    on a device that doesn't support it"""
+
+    weights = np.ones([3, 2])
+
+    with QuantumTape() as tape:
+        ctrl(qml.templates.BasicEntanglerLayers, 0)(weights, wires=[1, 2])
+
+    tape = expand_tape(tape)
+    assert len(tape.operations) == 9
+    assert all(o.name in {"CRX", "Toffoli"} for o in tape.operations)
+
+
+def test_controlled_template_and_operations():
+    """Test that a combination of controlled templates and operations correctly expands
+    on a device that doesn't support it"""
+
+    weights = np.ones([3, 2])
+
+    def ansatz(weights, wires):
+        qml.PauliX(wires=wires[0])
+        qml.templates.BasicEntanglerLayers(weights, wires=wires)
+
+    with QuantumTape() as tape:
+        ctrl(ansatz, 0)(weights, wires=[1, 2])
+
+    tape = expand_tape(tape)
+    assert len(tape.operations) == 10
+    assert all(o.name in {"CNOT", "CRX", "Toffoli"} for o in tape.operations)