Repository: PennyLaneAI/pennylane (python)

Issue:
[BUG] `qml.is_commuting` breaks with `Tensor` operators.  
If we try to check commutation of `Tensor` objects with `qml.is_commuting`, we get:

```
>>> qml.is_commuting(qml.PauliX(0) @ qml.PauliX(1), qml.PauliX(0) @ qml.PauliY(2))
----> 1 qml.is_commuting(qml.PauliX(0) @ qml.PauliX(1), qml.PauliX(0) @ qml.PauliY(2))

File pennylane/pennylane/ops/functions/is_commuting.py:356, in is_commuting(operation1, operation2)
    354 # Check if operations are non simplified rotations and return commutation if it is the case.
    355 op_set = {"U2", "U3", "Rot", "CRot"}
--> 356 if operation1.name in op_set and operation2.name in op_set:
    357     return check_commutation_two_non_simplified_rotations(operation1, operation2)
    359 ctrl_base_1 = _get_target_name(operation1)

TypeError: unhashable type: 'list'
```

We should either:
1) Explicitly not support Tensor operations
2) Include the logic from `qml.grouping.is_commuting` and actually determine whether or not the objects commute.


The first would only be an option for the short term if we don't want to spend the time improving `qml.is_commuting` right now.

Pull request:
Move pauli word logic from `qml.grouping.utils.is_commuting` to `qml.is_commuting`
**Context:**
There are currently two different implementations of `is_commuting()`: one at the top-level for a variety of operators, and another in `pennylane/grouping/utils.py` specifically for Pauli words. These should be in the same place to avoid drift, give a simpler user interface and ease future development, so I'm putting them in the same place (`qml.is_commuting`)

**Description of the Change:**
- Move the Pauli word is_commuting logic to `qml.is_commuting()` (thereby removing `qml.grouping.utils.is_commuting()`), and try to check this before other checks
- If it deems one or both operators to not be Pauli words, check if either are Tensors
  - If they are, raise an exception
  - Else, carry on
- Explicitly remove support for Prod and Sum classes (as well as Tensors) since we do not yet have an implementation to support them

**Benefits:**
- Single method for users to check if two operators commute. Distinguishing logic for Pauli words vs. other operators is hidden away.
- Explicitly removes support for (non-Pauli word) Tensors, as well as composite operators

**Possible Drawbacks:**
- `qml.is_commuting()` is becoming large and messy (as hinted by our pylint disabled features 😄 ). Hopefully this gets some love in the future. The code is still organized and legible, just a warning.

**Related GitHub Issues:**
Fixes #3007 

Patch:
diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md
index c03562076..00b0bdecc 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -418,6 +418,9 @@
   `~/math/matrix_manipulation.py`
   [(#3008)](https://github.com/PennyLaneAI/pennylane/pull/3008)
 
+* `qml.grouping.utils.is_commuting` is removed, and its Pauli word logic is now part of `qml.is_commuting`.
+  [(#3033)](https://github.com/PennyLaneAI/pennylane/pull/3033)
+
 <h3>Deprecations</h3>
 
 * In-place inversion is now deprecated. This includes `op.inv()` and `op.inverse=value`. Please
diff --git a/pennylane/grouping/__init__.py b/pennylane/grouping/__init__.py
index da2cfa290..f034a9fa4 100644
--- a/pennylane/grouping/__init__.py
+++ b/pennylane/grouping/__init__.py
@@ -33,7 +33,6 @@ from .utils import (
     pauli_word_to_string,
     string_to_pauli_word,
     pauli_word_to_matrix,
-    is_commuting,
     is_qwc,
     are_pauli_words_qwc,
     observables_to_binary_matrix,
diff --git a/pennylane/grouping/utils.py b/pennylane/grouping/utils.py
index dbc94eff3..74626e605 100644
--- a/pennylane/grouping/utils.py
+++ b/pennylane/grouping/utils.py
@@ -60,9 +60,6 @@ def is_pauli_word(observable):
     Returns:
         bool: true if the input observable is a Pauli word, false otherwise.
 
-    Raises:
-        TypeError: if input observable is not an Observable instance.
-
     **Example**
 
     >>> is_pauli_word(qml.Identity(0))
@@ -532,56 +529,6 @@ def pauli_word_to_matrix(pauli_word, wire_map=None):
     return reduce(np.kron, pauli_mats)
 
 
-def is_commuting(pauli_word_1, pauli_word_2, wire_map=None):
-    r"""Checks if two Pauli words commute.
-
-    To determine if two Pauli words commute, we can check the value of the
-    symplectic inner product of their binary vector representations.
-    For two binary vectors representing Pauli words, :math:`p_1 = [x_1, z_1]`
-    and :math:`p_2 = [x_2, z_2],` the symplectic inner product is defined as
-    :math:`\langle p_1, p_2 \rangle_{symp} = z_1 x_2^T + z_2 x_1^T`. If the symplectic
-    product is :math:`0` they commute, while if it is :math:`1`, they don't commute.
-
-    Args:
-        pauli_word_1 (Observable): first Pauli word in commutator
-        pauli_word_2 (Observable): second Pauli word in commutator
-        wire_map (dict[Union[str, int], int]): dictionary containing all wire labels used in
-            the Pauli word as keys, and unique integer labels as their values
-
-    Returns:
-        bool: returns True if the input Pauli commute, False otherwise
-
-    Raises:
-        TypeError: if either of the Pauli words is not valid.
-
-    **Example**
-
-    >>> wire_map = {'a' : 0, 'b' : 1, 'c' : 2}
-    >>> pauli_word_1 = qml.PauliX('a') @ qml.PauliY('b')
-    >>> pauli_word_2 = qml.PauliZ('a') @ qml.PauliZ('c')
-    >>> is_commuting(pauli_word_1, pauli_word_2, wire_map=wire_map)
-    False
-    """
-
-    if not (is_pauli_word(pauli_word_1) and is_pauli_word(pauli_word_2)):
-        raise TypeError(
-            f"Expected Pauli word observables, instead got {pauli_word_1} and {pauli_word_2}"
-        )
-
-    if wire_map is None:
-        wire_map = _wire_map_from_pauli_pair(pauli_word_1, pauli_word_2)
-
-    n_qubits = len(wire_map)
-
-    pauli_vec_1 = pauli_to_binary(pauli_word_1, n_qubits=n_qubits, wire_map=wire_map)
-    pauli_vec_2 = pauli_to_binary(pauli_word_2, n_qubits=n_qubits, wire_map=wire_map)
-
-    x1, z1 = pauli_vec_1[:n_qubits], pauli_vec_1[n_qubits:]
-    x2, z2 = pauli_vec_2[:n_qubits], pauli_vec_2[n_qubits:]
-
-    return (np.dot(z1, x2) + np.dot(z2, x1)) % 2 == 0
-
-
 def is_qwc(pauli_vec_1, pauli_vec_2):
     """Checks if two Pauli words in the binary vector representation are qubit-wise commutative.
 
diff --git a/pennylane/ops/functions/is_commuting.py b/pennylane/ops/functions/is_commuting.py
index bface69a3..59e010c00 100644
--- a/pennylane/ops/functions/is_commuting.py
+++ b/pennylane/ops/functions/is_commuting.py
@@ -17,6 +17,49 @@ Defines `is_commuting`, an function for determining if two functions commute.
 
 import pennylane as qml
 from pennylane import numpy as np
+from pennylane.grouping.utils import is_pauli_word, pauli_to_binary, _wire_map_from_pauli_pair
+
+
+def _pword_is_commuting(pauli_word_1, pauli_word_2, wire_map=None):
+    r"""Checks if two Pauli words commute.
+
+    To determine if two Pauli words commute, we can check the value of the
+    symplectic inner product of their binary vector representations.
+    For two binary vectors representing Pauli words, :math:`p_1 = [x_1, z_1]`
+    and :math:`p_2 = [x_2, z_2],` the symplectic inner product is defined as
+    :math:`\langle p_1, p_2 \rangle_{symp} = z_1 x_2^T + z_2 x_1^T`. If the symplectic
+    product is :math:`0` they commute, while if it is :math:`1`, they don't commute.
+
+    Args:
+        pauli_word_1 (Observable): first Pauli word in commutator
+        pauli_word_2 (Observable): second Pauli word in commutator
+        wire_map (dict[Union[str, int], int]): dictionary containing all wire labels used in
+            the Pauli word as keys, and unique integer labels as their values
+
+    Returns:
+        bool: returns True if the input Pauli commute, False otherwise
+
+    **Example**
+
+    >>> wire_map = {'a' : 0, 'b' : 1, 'c' : 2}
+    >>> pauli_word_1 = qml.PauliX('a') @ qml.PauliY('b')
+    >>> pauli_word_2 = qml.PauliZ('a') @ qml.PauliZ('c')
+    >>> is_commuting(pauli_word_1, pauli_word_2, wire_map=wire_map)
+    False
+    """
+
+    if wire_map is None:
+        wire_map = _wire_map_from_pauli_pair(pauli_word_1, pauli_word_2)
+
+    n_qubits = len(wire_map)
+
+    pauli_vec_1 = pauli_to_binary(pauli_word_1, n_qubits=n_qubits, wire_map=wire_map)
+    pauli_vec_2 = pauli_to_binary(pauli_word_2, n_qubits=n_qubits, wire_map=wire_map)
+
+    x1, z1 = pauli_vec_1[:n_qubits], pauli_vec_1[n_qubits:]
+    x2, z2 = pauli_vec_2[:n_qubits], pauli_vec_2[n_qubits:]
+
+    return (np.dot(z1, x2) + np.dot(z2, x1)) % 2 == 0
 
 
 def _get_target_name(op):
@@ -211,6 +254,8 @@ unsupported_operations = [
     "CommutingEvolution",
     "DisplacementEmbedding",
     "SqueezingEmbedding",
+    "Prod",
+    "Sum",
 ]
 non_commuting_operations = [
     # State Prep
@@ -259,7 +304,7 @@ non_commuting_operations = [
 ]
 
 
-def is_commuting(operation1, operation2):
+def is_commuting(operation1, operation2, wire_map=None):
     r"""Check if two operations are commuting using a lookup table.
 
     A lookup table is used to check the commutation between the
@@ -279,6 +324,8 @@ def is_commuting(operation1, operation2):
     Args:
         operation1 (.Operation): A first quantum operation.
         operation2 (.Operation): A second quantum operation.
+        wire_map (dict[Union[str, int], int]): dictionary for Pauli word commutation containing all
+            wire labels used in the Pauli word as keys, and unique integer labels as their values
 
     Returns:
          bool: True if the operations commute, False otherwise.
@@ -301,7 +348,15 @@ def is_commuting(operation1, operation2):
     ):
         raise qml.QuantumFunctionError(f"Operation {operation2.name} not supported.")
 
-    # Case 1 operations are disjoints
+    if is_pauli_word(operation1) and is_pauli_word(operation2):
+        return _pword_is_commuting(operation1, operation2, wire_map)
+
+    # aside from Pauli words, Tensor commutation evaluation is not supported
+    if isinstance(operation1, qml.operation.Tensor) or isinstance(operation2, qml.operation.Tensor):
+        # pylint: disable=raise-missing-from
+        raise qml.QuantumFunctionError("Tensor operations are not supported.")
+
+    # operations are disjoints
     if not intersection(operation1.wires, operation2.wires):
         return True
 


Test patch:
diff --git a/tests/grouping/test_grouping_utils.py b/tests/grouping/test_grouping_utils.py
index ab0ff27d8..fca020cb0 100644
--- a/tests/grouping/test_grouping_utils.py
+++ b/tests/grouping/test_grouping_utils.py
@@ -28,7 +28,6 @@ from pennylane.grouping.utils import (
     pauli_word_to_string,
     string_to_pauli_word,
     pauli_word_to_matrix,
-    is_commuting,
     is_qwc,
     are_pauli_words_qwc,
     observables_to_binary_matrix,
@@ -477,38 +476,3 @@ class TestGroupingUtils:
         """Ensure invalid inputs are handled properly when converting Pauli words to matrices."""
         with pytest.raises(TypeError):
             pauli_word_to_matrix(non_pauli_word)
-
-    @pytest.mark.parametrize(
-        "pauli_word_1,pauli_word_2,wire_map,commute_status",
-        [
-            (Identity(0), PauliZ(0), {0: 0}, True),
-            (PauliY(0), PauliZ(0), {0: 0}, False),
-            (PauliX(0), PauliX(1), {0: 0, 1: 1}, True),
-            (PauliY("x"), PauliX("y"), None, True),
-            (
-                PauliZ("a") @ PauliY("b") @ PauliZ("d"),
-                PauliX("a") @ PauliZ("c") @ PauliY("d"),
-                {"a": 0, "b": 1, "c": 2, "d": 3},
-                True,
-            ),
-            (
-                PauliX("a") @ PauliY("b") @ PauliZ("d"),
-                PauliX("a") @ PauliZ("c") @ PauliY("d"),
-                {"a": 0, "b": 1, "c": 2, "d": 3},
-                False,
-            ),
-        ],
-    )
-    def test_is_commuting(self, pauli_word_1, pauli_word_2, wire_map, commute_status):
-        """Test that (non)-commuting Pauli words are correctly identified."""
-        do_they_commute = is_commuting(pauli_word_1, pauli_word_2, wire_map=wire_map)
-        assert do_they_commute == commute_status
-
-    @pytest.mark.parametrize(
-        "pauli_word_1,pauli_word_2",
-        [(non_pauli_words[0], PauliX(0) @ PauliY(2)), (PauliX(0) @ PauliY(2), non_pauli_words[0])],
-    )
-    def test_is_commuting_invalid_input(self, pauli_word_1, pauli_word_2):
-        """Ensure invalid inputs are handled properly when determining commutativity."""
-        with pytest.raises(TypeError):
-            is_commuting(pauli_word_1, pauli_word_2)
diff --git a/tests/grouping/test_pauli_group.py b/tests/grouping/test_pauli_group.py
index 3c02ac63b..0d4085539 100644
--- a/tests/grouping/test_pauli_group.py
+++ b/tests/grouping/test_pauli_group.py
@@ -16,8 +16,8 @@ Unit tests for the :mod:`pauli_group`  functions in ``grouping/pauli.py``.
 """
 import pytest
 
-from pennylane import Identity, PauliX, PauliY, PauliZ
-from pennylane.grouping import is_commuting, string_to_pauli_word
+from pennylane import Identity, PauliX, PauliY, PauliZ, is_commuting
+from pennylane.grouping import string_to_pauli_word
 from pennylane.grouping.pauli import (
     partition_pauli_group,
     pauli_group,
diff --git a/tests/ops/functions/test_is_commuting.py b/tests/ops/functions/test_is_commuting.py
index 2da9d88d5..32cccceca 100644
--- a/tests/ops/functions/test_is_commuting.py
+++ b/tests/ops/functions/test_is_commuting.py
@@ -21,7 +21,6 @@ import pennylane as qml
 from pennylane.ops.functions.is_commuting import (
     _get_target_name,
     _check_mat_commutation,
-    is_commuting,
 )
 
 control_base_map_data = [
@@ -770,13 +769,51 @@ class TestCommutingFunction:
             ([[0], [1]], True),
         ],
     )
-    def test_u3_identity_barrier(self, wires, res):
+    def test_barrier_u3_identity(self, wires, res):
         """Commutation between Barrier and U3(0.0, 0.0, 0.0)."""
         commutation = qml.is_commuting(
             qml.Barrier(wires=wires[1]), qml.U3(0.0, 0.0, 0.0, wires=wires[0])
         )
         assert commutation == res
 
+    @pytest.mark.parametrize(
+        "pauli_word_1,pauli_word_2,wire_map,commute_status",
+        [
+            (qml.Identity(0), qml.PauliZ(0), {0: 0}, True),
+            (qml.PauliY(0), qml.PauliZ(0), {0: 0}, False),
+            (qml.PauliX(0), qml.PauliX(1), {0: 0, 1: 1}, True),
+            (qml.PauliY("x"), qml.PauliX("y"), None, True),
+            (
+                qml.PauliZ("a") @ qml.PauliY("b") @ qml.PauliZ("d"),
+                qml.PauliX("a") @ qml.PauliZ("c") @ qml.PauliY("d"),
+                {"a": 0, "b": 1, "c": 2, "d": 3},
+                True,
+            ),
+            (
+                qml.PauliX("a") @ qml.PauliY("b") @ qml.PauliZ("d"),
+                qml.PauliX("a") @ qml.PauliZ("c") @ qml.PauliY("d"),
+                {"a": 0, "b": 1, "c": 2, "d": 3},
+                False,
+            ),
+        ],
+    )
+    def test_pauli_words(self, pauli_word_1, pauli_word_2, wire_map, commute_status):
+        """Test that (non)-commuting Pauli words are correctly identified."""
+        do_they_commute = qml.is_commuting(pauli_word_1, pauli_word_2, wire_map=wire_map)
+        assert do_they_commute == commute_status
+
+    @pytest.mark.parametrize(
+        "pauli_word_1,pauli_word_2",
+        [
+            (qml.PauliX(0) @ qml.Hadamard(1) @ qml.Identity(2), qml.PauliX(0) @ qml.PauliY(2)),
+            (qml.PauliX(0) @ qml.PauliY(2), qml.PauliX(0) @ qml.Hadamard(1) @ qml.Identity(2)),
+        ],
+    )
+    def test_non_pauli_word_tensors_not_supported(self, pauli_word_1, pauli_word_2):
+        """Ensure invalid inputs are handled properly when determining commutativity."""
+        with pytest.raises(qml.QuantumFunctionError, match="Tensor operations are not supported."):
+            qml.is_commuting(pauli_word_1, pauli_word_2)
+
     def test_operation_1_not_supported(self):
         """Test that giving a non supported operation raises an error."""
         rho = np.zeros((2**1, 2**1), dtype=np.complex128)
diff --git a/tests/test_qaoa.py b/tests/test_qaoa.py
index dd68208c8..bb78cec2b 100644
--- a/tests/test_qaoa.py
+++ b/tests/test_qaoa.py
@@ -163,10 +163,7 @@ class TestMixerHamiltonians:
         mixer_hamiltonian = qaoa.x_mixer(wires)
 
         # check that all observables commute
-        assert all(
-            qml.grouping.is_commuting(o, mixer_hamiltonian.ops[0])
-            for o in mixer_hamiltonian.ops[1:]
-        )
+        assert all(qml.is_commuting(o, mixer_hamiltonian.ops[0]) for o in mixer_hamiltonian.ops[1:])
         # check that the 1-group grouping information was set
         assert mixer_hamiltonian.grouping_indices is not None
         assert mixer_hamiltonian.grouping_indices == [[0, 1, 2, 3]]
@@ -974,7 +971,7 @@ class TestCostHamiltonians:
         cost_h, _ = qaoa.maxcut(graph)
 
         # check that all observables commute
-        assert all(qml.grouping.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
+        assert all(qml.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
         # check that the 1-group grouping information was set
         assert cost_h.grouping_indices is not None
         assert cost_h.grouping_indices == [list(range(len(cost_h.ops)))]
@@ -996,7 +993,7 @@ class TestCostHamiltonians:
         cost_h, _ = qaoa.max_independent_set(graph)
 
         # check that all observables commute
-        assert all(qml.grouping.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
+        assert all(qml.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
         # check that the 1-group grouping information was set
         assert cost_h.grouping_indices is not None
         assert cost_h.grouping_indices == [list(range(len(cost_h.ops)))]
@@ -1018,7 +1015,7 @@ class TestCostHamiltonians:
         cost_h, _ = qaoa.min_vertex_cover(graph)
 
         # check that all observables commute
-        assert all(qml.grouping.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
+        assert all(qml.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
         # check that the 1-group grouping information was set
         assert cost_h.grouping_indices is not None
         assert cost_h.grouping_indices == [list(range(len(cost_h.ops)))]
@@ -1042,7 +1039,7 @@ class TestCostHamiltonians:
         cost_h, _ = qaoa.max_clique(graph)
 
         # check that all observables commute
-        assert all(qml.grouping.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
+        assert all(qml.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
         # check that the 1-group grouping information was set
         assert cost_h.grouping_indices is not None
         assert cost_h.grouping_indices == [list(range(len(cost_h.ops)))]
@@ -1077,7 +1074,7 @@ class TestCostHamiltonians:
         cost_h, _, _ = qaoa.max_weight_cycle(graph)
 
         # check that all observables commute
-        assert all(qml.grouping.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
+        assert all(qml.is_commuting(o, cost_h.ops[0]) for o in cost_h.ops[1:])
         # check that the 1-group grouping information was set
         assert cost_h.grouping_indices is not None
         assert cost_h.grouping_indices == [list(range(len(cost_h.ops)))]