Repository: PennyLaneAI/pennylane
Pull request #3745 (2023-02-09 17:01:47)

Fix bug related to labeling of nested controlled operations in drawer
**Context:** When drawing operations where controls are nested (e.g., a controlled controlled operation, or nested use of `qml.ctrl`, not all the control wires were identified as such, and would be drawn using the label of the operation being applied under the control.

**Description of the Change:** Updates the tape text drawer to handle the special case of controlled operations when drawing

**Benefits:** Tapes are now drawn correctly :tada:

**Possible Drawbacks:** Matplotlib case was also affected; have visually inspected that the results are now correct, but not sure how to write the tests.

**Related GitHub Issues:** #3702 


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

```diff
@@           Coverage Diff           @@
##           master    #3745   +/-   ##
=======================================
  Coverage   99.82%   99.82%           
=======================================
  Files         329      329           
  Lines       28848    28860   +12     
=======================================
+ Hits        28797    28809   +12     
  Misses         51       51           
```


| [Impacted Files](https://codecov.io/gh/PennyLaneAI/pennylane/pull/3745?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | Coverage Δ | |
|---|---|---|
| [pennylane/drawer/tape\_mpl.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/3745?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2RyYXdlci90YXBlX21wbC5weQ==) | `98.85% <100.00%> (-0.02%)` | :arrow_down: |
| [pennylane/drawer/tape\_text.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/3745?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2RyYXdlci90YXBlX3RleHQucHk=) | `100.00% <100.00%> (ø)` | |
| [pennylane/drawer/utils.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/3745?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL2RyYXdlci91dGlscy5weQ==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/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 e4697f97d..ecc5a7940 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -662,6 +662,10 @@
 
 <h3>Bug fixes</h3>
 
+* Fixed a bug in the drawer where nested controlled operations would output
+  the label of the operation being controlled, rather than the control values.
+  [(#3745)](https://github.com/PennyLaneAI/pennylane/pull/3745)
+
 * Fixed a bug in `qml.transforms.metric_tensor` where prefactors of operation generators were taken
   into account multiple times, leading to wrong outputs for non-standard operations.
   [(#3579)](https://github.com/PennyLaneAI/pennylane/pull/3579)
@@ -718,6 +722,7 @@ Utkarsh Azad
 Cristian Boghiu
 Astral Cai
 Isaac De Vlugt
+Olivia Di Matteo
 Lillian M. A. Frederiksen
 Soran Jahangiri
 Christina Lee
diff --git a/pennylane/drawer/tape_mpl.py b/pennylane/drawer/tape_mpl.py
index 7417e0350..8fa5fcc88 100644
--- a/pennylane/drawer/tape_mpl.py
+++ b/pennylane/drawer/tape_mpl.py
@@ -23,7 +23,7 @@ from pennylane import ops
 from pennylane.wires import Wires
 from .mpldrawer import MPLDrawer
 from .drawable_layers import drawable_layers
-from .utils import convert_wire_order
+from .utils import convert_wire_order, unwrap_controls
 from .style import use_style
 
 has_mpl = True
@@ -131,15 +131,14 @@ def _tape_mpl(tape, wire_order=None, show_all_wires=False, decimals=None, **kwar
                 specialfunc(drawer, layer, mapped_wires, op)
 
             else:
-                op_control_wires = getattr(op, "control_wires", [])
+                op_control_wires, control_values = unwrap_controls(op)
+
                 control_wires = [wire_map[w] for w in op_control_wires]
                 target_wires = (
                     [wire_map[w] for w in op.wires if w not in op_control_wires]
                     if len(op.wires) != 0
                     else wire_map.values()
                 )
-                # It is assumed that if `op.wires == 0`, `op` acts on all wires
-                control_values = op.hyperparameters.get("control_values", None)
 
                 if control_values is None:
                     control_values = [True for _ in control_wires]
diff --git a/pennylane/drawer/tape_text.py b/pennylane/drawer/tape_text.py
index fa5276081..6d9f6fe37 100644
--- a/pennylane/drawer/tape_text.py
+++ b/pennylane/drawer/tape_text.py
@@ -19,7 +19,7 @@ import pennylane as qml
 from pennylane.measurements import Expectation, Probability, Sample, Variance, State
 
 from .drawable_layers import drawable_layers
-from .utils import convert_wire_order
+from .utils import convert_wire_order, unwrap_controls
 
 
 def _add_grouping_symbols(op, layer_str, wire_map):
@@ -45,8 +45,8 @@ def _add_op(op, layer_str, wire_map, decimals, cache):
     """Updates ``layer_str`` with ``op`` operation."""
     layer_str = _add_grouping_symbols(op, layer_str, wire_map)
 
-    control_wires = getattr(op, "control_wires", [])
-    control_values = op.hyperparameters.get("control_values", None)
+    control_wires, control_values = unwrap_controls(op)
+
     if control_values:
         for w, val in zip(control_wires, control_values):
             layer_str[wire_map[w]] += "●" if _bool_control_value(val) else "○"
diff --git a/pennylane/drawer/utils.py b/pennylane/drawer/utils.py
index f75561a74..5ba888eb8 100644
--- a/pennylane/drawer/utils.py
+++ b/pennylane/drawer/utils.py
@@ -14,6 +14,7 @@
 """
 This module contains some useful utility functions for circuit drawing.
 """
+from pennylane.ops import Controlled
 
 
 def default_wire_map(tape):
@@ -57,3 +58,44 @@ def convert_wire_order(tape, wire_order=None, show_all_wires=False):
         wire_order = [wire for wire in wire_order if wire in used_wires]
 
     return {wire: ind for ind, wire in enumerate(wire_order)}
+
+
+def unwrap_controls(op):
+    """Unwraps nested controlled operations for drawing.
+
+    Controlled operations may themselves contain controlled operations; check
+    for any nesting of operators when drawing so that we correctly identify
+    and label _all_ control and target qubits.
+
+    Args:
+        op (.Operation): A PennyLane operation.
+
+    Returns:
+        Wires, List: The control wires of the operation, along with any associated
+        control values.
+    """
+    # Get wires and control values of base operation; need to make a copy of
+    # control values, otherwise it will modify the list in the operation itself.
+    control_wires = getattr(op, "control_wires", [])
+    control_values = op.hyperparameters.get("control_values", None)
+
+    if isinstance(control_values, list):
+        control_values = control_values.copy()
+
+    if isinstance(op, Controlled):
+        next_ctrl = op
+
+        while hasattr(next_ctrl, "base"):
+            base_control_wires = getattr(next_ctrl.base, "control_wires", [])
+            control_wires += base_control_wires
+
+            base_control_values = next_ctrl.base.hyperparameters.get(
+                "control_values", [True] * len(base_control_wires)
+            )
+
+            if control_values is not None:
+                control_values.extend(base_control_values)
+
+            next_ctrl = next_ctrl.base
+
+    return control_wires, control_values


Test patch:
diff --git a/tests/drawer/test_drawer_utils.py b/tests/drawer/test_drawer_utils.py
index da1517436..6fe0123e6 100644
--- a/tests/drawer/test_drawer_utils.py
+++ b/tests/drawer/test_drawer_utils.py
@@ -17,7 +17,8 @@ Unit tests for the pennylane.drawer.utils` module.
 
 import pytest
 import pennylane as qml
-from pennylane.drawer.utils import default_wire_map, convert_wire_order
+from pennylane.drawer.utils import default_wire_map, convert_wire_order, unwrap_controls
+from pennylane.wires import Wires
 
 
 class TestDefaultWireMap:
@@ -93,3 +94,64 @@ class TestConvertWireOrder:
 
         wire_map = convert_wire_order(ops, wire_order, show_all_wires=True)
         assert wire_map == {"a": 0, "b": 1, "c": 2, "d": 3}
+
+
+class TestUnwrapControls:
+    """Tests the ``unwrap_controls`` utility function."""
+
+    @pytest.mark.parametrize(
+        "op,expected_control_wires,expected_control_values",
+        [
+            (qml.PauliX(wires="a"), Wires([]), None),
+            (qml.CNOT(wires=["a", "b"]), Wires("a"), None),
+            (qml.ctrl(qml.PauliX(wires="b"), control="a"), Wires("a"), [True]),
+            (
+                qml.ctrl(qml.PauliX(wires="b"), control=["a", "c", "d"]),
+                Wires(["a", "c", "d"]),
+                [True, True, True],
+            ),
+            (
+                qml.ctrl(qml.PauliZ(wires="c"), control=["a", "d"], control_values=[True, False]),
+                Wires(["a", "d"]),
+                [True, False],
+            ),
+            (
+                qml.ctrl(
+                    qml.CRX(0.3, wires=["c", "e"]),
+                    control=["a", "b", "d"],
+                    control_values=[True, False, False],
+                ),
+                Wires(["a", "b", "d", "c"]),
+                [True, False, False, True],
+            ),
+            (
+                qml.ctrl(qml.CNOT(wires=["c", "d"]), control=["a", "b"]),
+                Wires(["a", "b", "c"]),
+                [True, True, True],
+            ),
+            (
+                qml.ctrl(qml.ctrl(qml.CNOT(wires=["c", "d"]), control=["a", "b"]), control=["e"]),
+                Wires(["e", "a", "b", "c"]),
+                [True, True, True, True],
+            ),
+            (
+                qml.ctrl(
+                    qml.ctrl(
+                        qml.CNOT(wires=["c", "d"]), control=["a", "b"], control_values=[False, True]
+                    ),
+                    control=["e"],
+                    control_values=[False],
+                ),
+                Wires(["e", "a", "b", "c"]),
+                [False, False, True, True],
+            ),
+        ],
+    )
+    def test_multi_defined_control_values(
+        self, op, expected_control_wires, expected_control_values
+    ):
+        """Test a multi-controlled single-qubit operation with defined control values."""
+        control_wires, control_values = unwrap_controls(op)
+
+        assert control_wires == expected_control_wires
+        assert control_values == expected_control_values
diff --git a/tests/drawer/test_tape_mpl.py b/tests/drawer/test_tape_mpl.py
index 87bfa0410..75a85ee76 100644
--- a/tests/drawer/test_tape_mpl.py
+++ b/tests/drawer/test_tape_mpl.py
@@ -463,6 +463,20 @@ class TestControlledGates:
         tape = QuantumScript.from_queue(q_tape)
         self.check_tape_controlled_qubit_unitary(tape)
 
+    def test_nested_control_values_bool(self):
+        """Test control_values get displayed correctly for nested controlled operations
+        when they are provided as a list of bools."""
+
+        with qml.queuing.AnnotatedQueue() as q_tape:
+            qml.ctrl(
+                qml.ctrl(qml.PauliX(wires=4), control=[2, 3], control_values=[1, 0]),
+                control=[0, 1],
+                control_values=[1, 0],
+            )
+
+        tape = QuantumScript.from_queue(q_tape)
+        self.check_tape_controlled_qubit_unitary(tape)
+
     def check_tape_controlled_qubit_unitary(self, tape):
         """Checks the control symbols for a tape with some version of a controlled qubit unitary."""
         _, ax = tape_mpl(tape, style=None)  # set style to None to use plt.rcParams values
diff --git a/tests/drawer/test_tape_text.py b/tests/drawer/test_tape_text.py
index be1c3ceb1..404021683 100644
--- a/tests/drawer/test_tape_text.py
+++ b/tests/drawer/test_tape_text.py
@@ -298,6 +298,25 @@ single_op_tests_data = [
         ),
         "0: ───┤ ╭<𝓗>\n1: ───┤ ╰<𝓗>",
     ),
+    # Operations (both regular and controlled) and nested multi-valued controls
+    (qml.ctrl(qml.PauliX(wires=2), control=[0, 1]), "0: ─╭●─┤  \n1: ─├●─┤  \n2: ─╰X─┤  "),
+    (qml.ctrl(qml.CNOT(wires=[1, 2]), control=0), "0: ─╭●─┤  \n1: ─├●─┤  \n2: ─╰X─┤  "),
+    (
+        qml.ctrl(qml.CRZ(0.2, wires=[1, 2]), control=[3, 0]),
+        "3: ─╭●────────┤  \n0: ─├●────────┤  \n1: ─├●────────┤  \n2: ─╰RZ(0.20)─┤  ",
+    ),
+    (
+        qml.ctrl(qml.CH(wires=[0, 3]), control=[2, 1], control_values=[False, True]),
+        "2: ─╭○─┤  \n1: ─├●─┤  \n0: ─├●─┤  \n3: ─╰H─┤  ",
+    ),
+    (
+        qml.ctrl(
+            qml.ctrl(qml.CY(wires=[3, 4]), control=[1, 2], control_values=[True, False]),
+            control=0,
+            control_values=[False],
+        ),
+        "0: ─╭○─┤  \n1: ─├●─┤  \n2: ─├○─┤  \n3: ─├●─┤  \n4: ─╰Y─┤  ",
+    ),
 ]