From b67eea1578b2faf0f34f38697f2dff88f474f6ce Mon Sep 17 00:00:00 2001 From: lillian542 Date: Wed, 24 Jul 2024 18:26:01 -0400 Subject: [PATCH 01/15] update assert_valid to catch CNOT decomposition --- pennylane/ops/functions/assert_valid.py | 3 +++ tests/ops/functions/test_assert_valid.py | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py index bc92514a4ce..4befa9f35c5 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" diff --git a/tests/ops/functions/test_assert_valid.py b/tests/ops/functions/test_assert_valid.py index b9a6cf45368..bbbda986b2d 100644 --- a/tests/ops/functions/test_assert_valid.py +++ b/tests/ops/functions/test_assert_valid.py @@ -80,6 +80,17 @@ def compute_decomposition(wires): 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 decomposition op 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.""" From 5d169856ef9ca179692dfa449c81eb11ef40d020 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 10:32:01 -0400 Subject: [PATCH 02/15] explictly give CNOT no decomposition --- pennylane/ops/op_math/controlled_ops.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pennylane/ops/op_math/controlled_ops.py b/pennylane/ops/op_math/controlled_ops.py index d2c5d3afb4a..bbc2405fe28 100644 --- a/pennylane/ops/op_math/controlled_ops.py +++ b/pennylane/ops/op_math/controlled_ops.py @@ -791,6 +791,27 @@ def _primitive_bind_call(cls, wires, id=None): 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 + Returns: + list[Operator]: decomposition of the operator + """ + raise qml.operation.DecompositionUndefinedError + def __repr__(self): return f"CNOT(wires={self.wires.tolist()})" From 9f93751247deb8a366af3164eafdcb4a40404a77 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 10:34:53 -0400 Subject: [PATCH 03/15] update changelog --- doc/releases/changelog-dev.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md index f67840f549b..cda764943a6 100644 --- a/doc/releases/changelog-dev.md +++ b/doc/releases/changelog-dev.md @@ -105,6 +105,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) +

Community contributions 🥳

* `DefaultQutritMixed` readout error has been added using parameters `readout_relaxation_probs` and From 51a3c10496b6ceb67cce03ccfcbd5b07c0d60063 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 14:03:26 -0400 Subject: [PATCH 04/15] fix legacy custom decomp when operator has no default decomp --- pennylane/transforms/tape_expand.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pennylane/transforms/tape_expand.py b/pennylane/transforms/tape_expand.py index f3e9b0d26e5..d9d555966cc 100644 --- a/pennylane/transforms/tape_expand.py +++ b/pennylane/transforms/tape_expand.py @@ -297,14 +297,17 @@ def _custom_decomposition(obj, fn): obj = getattr(qml, obj) original_decomp_method = obj.compute_decomposition + original_has_decomp_propety = 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_propety # Loop through the decomposition dictionary and create all the contexts try: From 800da7660dde38cd1632134098c5dbe96271cbd5 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 15:07:02 -0400 Subject: [PATCH 05/15] hopefully fix docs build --- pennylane/ops/op_math/controlled_ops.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pennylane/ops/op_math/controlled_ops.py b/pennylane/ops/op_math/controlled_ops.py index bbc2405fe28..f2ad72e77ae 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**. From b4c56338dc7cb1f24f7ea2bf31eb413c8d8195ea Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 15:15:46 -0400 Subject: [PATCH 06/15] update assert_valid test --- pennylane/ops/functions/assert_valid.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py index 4befa9f35c5..73936c56241 100644 --- a/pennylane/ops/functions/assert_valid.py +++ b/pennylane/ops/functions/assert_valid.py @@ -70,12 +70,14 @@ 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." + # I don't think this is a good solution, try to think of a better one + 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( From 02564d9101edd0adbdf8504c64d198f3bb482366 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 15:43:23 -0400 Subject: [PATCH 07/15] has_decomposition is a property --- pennylane/ops/functions/assert_valid.py | 2 +- pennylane/transforms/tape_expand.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py index 73936c56241..d539c55984f 100644 --- a/pennylane/ops/functions/assert_valid.py +++ b/pennylane/ops/functions/assert_valid.py @@ -71,7 +71,7 @@ def _check_decomposition(op, skip_wire_mapping): wire_map = {w: ascii_lowercase[i] for i, w in enumerate(op.wires)} mapped_op = op.map_wires(wire_map) # I don't think this is a good solution, try to think of a better one - if mapped_op.has_decomposition(): + 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): diff --git a/pennylane/transforms/tape_expand.py b/pennylane/transforms/tape_expand.py index d9d555966cc..a290a068e2b 100644 --- a/pennylane/transforms/tape_expand.py +++ b/pennylane/transforms/tape_expand.py @@ -307,7 +307,7 @@ def _custom_decomposition(obj, fn): finally: obj.compute_decomposition = staticmethod(original_decomp_method) - obj.has_decomposition = original_has_decomp_propety + obj.has_decomposition = property(original_has_decomp_propety) # Loop through the decomposition dictionary and create all the contexts try: From c197ace64e82b7a810b53f3fedf8669a6f4e2922 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 16:40:40 -0400 Subject: [PATCH 08/15] perhaps this might work --- pennylane/transforms/tape_expand.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pennylane/transforms/tape_expand.py b/pennylane/transforms/tape_expand.py index a290a068e2b..d9d555966cc 100644 --- a/pennylane/transforms/tape_expand.py +++ b/pennylane/transforms/tape_expand.py @@ -307,7 +307,7 @@ def _custom_decomposition(obj, fn): finally: obj.compute_decomposition = staticmethod(original_decomp_method) - obj.has_decomposition = property(original_has_decomp_propety) + obj.has_decomposition = original_has_decomp_propety # Loop through the decomposition dictionary and create all the contexts try: From 1e2184153119bffa133f9dce538d8919f55bb012 Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 16:43:00 -0400 Subject: [PATCH 09/15] doc indentation fix --- pennylane/ops/op_math/controlled_ops.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pennylane/ops/op_math/controlled_ops.py b/pennylane/ops/op_math/controlled_ops.py index f2ad72e77ae..a80c3cf8c9b 100644 --- a/pennylane/ops/op_math/controlled_ops.py +++ b/pennylane/ops/op_math/controlled_ops.py @@ -798,15 +798,20 @@ def has_decomposition(self): @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 + Returns: list[Operator]: decomposition of the operator """ From e45e9e79d06f2d9aa708040d7aec085ddc4b33f4 Mon Sep 17 00:00:00 2001 From: lillian542 <38584660+lillian542@users.noreply.github.com> Date: Thu, 25 Jul 2024 17:39:45 -0400 Subject: [PATCH 10/15] Apply suggestions from code review Co-authored-by: David Wierichs --- pennylane/transforms/tape_expand.py | 4 ++-- tests/ops/functions/test_assert_valid.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pennylane/transforms/tape_expand.py b/pennylane/transforms/tape_expand.py index d9d555966cc..60580eb3f67 100644 --- a/pennylane/transforms/tape_expand.py +++ b/pennylane/transforms/tape_expand.py @@ -297,7 +297,7 @@ def _custom_decomposition(obj, fn): obj = getattr(qml, obj) original_decomp_method = obj.compute_decomposition - original_has_decomp_propety = obj.has_decomposition + original_has_decomp_property = obj.has_decomposition try: # Explicitly set the new compute_decomposition method @@ -307,7 +307,7 @@ def _custom_decomposition(obj, fn): finally: obj.compute_decomposition = staticmethod(original_decomp_method) - obj.has_decomposition = original_has_decomp_propety + obj.has_decomposition = original_has_decomp_property # Loop through the decomposition dictionary and create all the contexts try: diff --git a/tests/ops/functions/test_assert_valid.py b/tests/ops/functions/test_assert_valid.py index bbbda986b2d..9fff7758338 100644 --- a/tests/ops/functions/test_assert_valid.py +++ b/tests/ops/functions/test_assert_valid.py @@ -81,7 +81,7 @@ def compute_decomposition(wires): assert_valid(BadDecomp(wires=0), skip_pickle=True) def test_decomposition_must_not_contain_op(self): - """Test that decomposition op an operator doesn't include the operator itself""" + """Test that the decomposition of an operator doesn't include the operator itself""" class BadDecomp(Operator): @staticmethod From e7b36927e0cfc3c167d21b8cad68537f4c7bd21e Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 17:40:14 -0400 Subject: [PATCH 11/15] update docstring --- pennylane/ops/op_math/controlled_ops.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pennylane/ops/op_math/controlled_ops.py b/pennylane/ops/op_math/controlled_ops.py index a80c3cf8c9b..4d2a8475f6b 100644 --- a/pennylane/ops/op_math/controlled_ops.py +++ b/pennylane/ops/op_math/controlled_ops.py @@ -812,8 +812,8 @@ def compute_decomposition(*params, wires=None, **hyperparameters): # -> List["O wires (Iterable[Any], Wires): wires that the operator acts on **hyperparams (dict): non-trainable hyperparameters of the operator, as stored in the ``hyperparameters`` attribute - Returns: - list[Operator]: decomposition of the operator + Raises: + qml.DecompositionUndefinedError """ raise qml.operation.DecompositionUndefinedError From 2e9b6db260e0813df7349e2f5f5d678ffcbaeb9e Mon Sep 17 00:00:00 2001 From: lillian542 Date: Thu, 25 Jul 2024 18:06:11 -0400 Subject: [PATCH 12/15] add specific CNOT decomposition test --- tests/ops/op_math/test_controlled_ops.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/ops/op_math/test_controlled_ops.py b/tests/ops/op_math/test_controlled_ops.py index d82308daf65..44ab653299a 100644 --- a/tests/ops/op_math/test_controlled_ops.py +++ b/tests/ops/op_math/test_controlled_ops.py @@ -763,3 +763,14 @@ 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""" + + with pytest.raises(qml.operation.DecompositionUndefinedError): + qml.CNOT.compute_decomposition() + + with pytest.raises(qml.operation.DecompositionUndefinedError): + qml.CNOT([0, 1]).decomposition() From 262732148515553a5eb1f7310359c539a4ab107f Mon Sep 17 00:00:00 2001 From: lillian542 <38584660+lillian542@users.noreply.github.com> Date: Fri, 26 Jul 2024 12:26:21 -0400 Subject: [PATCH 13/15] Update comment --- pennylane/ops/functions/assert_valid.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py index d539c55984f..5323ff5c45f 100644 --- a/pennylane/ops/functions/assert_valid.py +++ b/pennylane/ops/functions/assert_valid.py @@ -70,7 +70,10 @@ 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) - # I don't think this is a good solution, try to think of a better one + # 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() From 148fd242c6dc2916a63e0e4e6d350588b1a46e6d Mon Sep 17 00:00:00 2001 From: lillian542 <38584660+lillian542@users.noreply.github.com> Date: Fri, 26 Jul 2024 14:08:01 -0400 Subject: [PATCH 14/15] Apply suggestions from code review Co-authored-by: Christina Lee --- pennylane/ops/functions/assert_valid.py | 2 +- tests/ops/op_math/test_controlled_ops.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pennylane/ops/functions/assert_valid.py b/pennylane/ops/functions/assert_valid.py index 5323ff5c45f..e3795ff6608 100644 --- a/pennylane/ops/functions/assert_valid.py +++ b/pennylane/ops/functions/assert_valid.py @@ -71,7 +71,7 @@ def _check_decomposition(op, skip_wire_mapping): wire_map = {w: ascii_lowercase[i] for i, w in enumerate(op.wires)} mapped_op = op.map_wires(wire_map) # 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 + # 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: diff --git a/tests/ops/op_math/test_controlled_ops.py b/tests/ops/op_math/test_controlled_ops.py index 44ab653299a..46221f2f555 100644 --- a/tests/ops/op_math/test_controlled_ops.py +++ b/tests/ops/op_math/test_controlled_ops.py @@ -768,6 +768,7 @@ def test_tuple_control_wires_parametric_ops(op_type): 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() From f491a61e213c672ca2301847319de90d98f346fe Mon Sep 17 00:00:00 2001 From: lillian542 Date: Fri, 26 Jul 2024 14:39:04 -0400 Subject: [PATCH 15/15] black formatting --- tests/ops/op_math/test_controlled_ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ops/op_math/test_controlled_ops.py b/tests/ops/op_math/test_controlled_ops.py index 46221f2f555..7a002cccda4 100644 --- a/tests/ops/op_math/test_controlled_ops.py +++ b/tests/ops/op_math/test_controlled_ops.py @@ -768,7 +768,7 @@ def test_tuple_control_wires_parametric_ops(op_type): 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 + assert not qml.CNOT((0, 1)).has_decomposition with pytest.raises(qml.operation.DecompositionUndefinedError): qml.CNOT.compute_decomposition()