Repository: PennyLaneAI/pennylane
Pull request #5037 (2024-01-10 20:14:43)

[Bug] `qml.equal` fails with Sum() when wires are strings
**Context:** 
Sorting wires fails when some of the wires are strings and the others are integers. It also fails due to how sum operands wires are sorted and compared.

**Description of the Change:**
- Cast all wire labels to string and then sort
- Add a "shortcut" check which uses the PauliRep to quickly check equality. 

resolves this issue https://github.com/PennyLaneAI/pennylane/issues/5033


github-actions[bot]: Hello. You may have forgotten to update the changelog!
Please edit [`doc/releases/changelog-dev.md`](/PennyLaneAI/pennylane/blob/master/doc/releases/changelog-dev.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.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5037?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`9a6419c`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/9a6419c6c017e0c104a4c110b1ad956806044e6d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.67% compared to head [(`d9d32e6`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5037?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.66%.
> Report is 7 commits behind head on master.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5037      +/-   ##
==========================================
- Coverage   99.67%   99.66%   -0.02%     
==========================================
  Files         394      394              
  Lines       35582    35402     -180     
==========================================
- Hits        35467    35283     -184     
- Misses        115      119       +4     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5037?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).

timmysilv: can we have this test case fixed before merging?
```python
H1 = qml.prod(qml.PauliX(0), qml.PauliX(1))
H2 = qml.s_prod(1., qml.sum(qml.PauliY(0), qml.PauliY(1)))
true_res = qml.sum(qml.s_prod(2j, qml.prod(qml.PauliZ(0), qml.PauliX(1))), qml.s_prod(2j, qml.prod(qml.PauliX(0), qml.PauliZ(1))))
res = qml.prod(H1, H2) - qml.prod(H2, H1)
res = res.simplify()

assert true_res == res
```
Jaybsoni: > can we have this test case fixed before merging?
> 
> ```python
> H1 = qml.prod(qml.PauliX(0), qml.PauliX(1))
> H2 = qml.s_prod(1., qml.sum(qml.PauliY(0), qml.PauliY(1)))
> true_res = qml.sum(qml.s_prod(2j, qml.prod(qml.PauliZ(0), qml.PauliX(1))), qml.s_prod(2j, qml.prod(qml.PauliX(0), qml.PauliZ(1))))
> res = qml.prod(H1, H2) - qml.prod(H2, H1)
> res = res.simplify()
> 
> assert true_res == res
> ```

Added! 
Jaybsoni: This fix does NOT solve the more general problem of sums of operations which are mathematically identical but not programmatically identical. The consensus was to solve this particular instance of the problem (with the pauli rep shortcut) and continue with the assumption that `qml.equal` checks programmatic equality.

Patch:
diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md
index 0cf92d86b..cfe3800f6 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -250,6 +250,9 @@
 * `StatePrep` operations expanded onto more wires are now compatible with backprop.
   [(#5028)](https://github.com/PennyLaneAI/pennylane/pull/5028)
 
+* `qml.equal` works well with `qml.Sum` operators when wire labels are a mix of integers and strings.
+  [(#5037)](https://github.com/PennyLaneAI/pennylane/pull/5037)
+
 * The return value of `Controlled.generator` now contains a projector that projects onto the correct subspace based on the control value specified.
   [(#5068)](https://github.com/PennyLaneAI/pennylane/pull/5068)
 
@@ -277,3 +280,4 @@ Lee J. O'Riordan,
 Mudit Pandey,
 Alex Preciado,
 Matthew Silverman.
+Jay Soni,
diff --git a/pennylane/ops/functions/equal.py b/pennylane/ops/functions/equal.py
index efa99f304..d0bc56e41 100644
--- a/pennylane/ops/functions/equal.py
+++ b/pennylane/ops/functions/equal.py
@@ -272,6 +272,9 @@ def _equal_prod_and_sum(op1: CompositeOp, op2: CompositeOp, **kwargs):
     if len(op1.operands) != len(op2.operands):
         return False
 
+    if op1.pauli_rep is not None and (op1.pauli_rep == op2.pauli_rep):  # shortcut check
+        return True
+
     # organizes by wire indicies while respecting commutation relations
     sorted_ops1 = op1._sort(op1.operands)
     sorted_ops2 = op2._sort(op2.operands)
@@ -344,6 +347,9 @@ def _equal_sprod(op1: SProd, op2: SProd, **kwargs):
     """Determine whether two SProd objects are equal"""
     rtol, atol = (kwargs["rtol"], kwargs["atol"])
 
+    if op1.pauli_rep is not None and (op1.pauli_rep == op2.pauli_rep):  # shortcut check
+        return True
+
     if not qml.math.allclose(op1.scalar, op2.scalar, rtol=rtol, atol=atol):
         return False
     return qml.equal(op1.base, op2.base)
diff --git a/pennylane/ops/op_math/sum.py b/pennylane/ops/op_math/sum.py
index 7816d08e8..5ea056088 100644
--- a/pennylane/ops/op_math/sum.py
+++ b/pennylane/ops/op_math/sum.py
@@ -19,8 +19,6 @@ import itertools
 from copy import copy
 from typing import List
 
-import numpy as np
-
 import pennylane as qml
 from pennylane import math
 from pennylane.operation import Operator
@@ -330,7 +328,7 @@ class Sum(CompositeOp):
             wires = op.wires
             if wire_map is not None:
                 wires = wires.map(wire_map)
-            return np.min(wires), len(wires), str(op)
+            return sorted(list(map(str, wires)))[0], len(wires), str(op)
 
         return sorted(op_list, key=_sort_key)
 


Test patch:
diff --git a/tests/ops/functions/test_commutator.py b/tests/ops/functions/test_commutator.py
index c0ff2a64f..bff5e3af0 100644
--- a/tests/ops/functions/test_commutator.py
+++ b/tests/ops/functions/test_commutator.py
@@ -58,7 +58,6 @@ class TestLegacySupport:
         assert isinstance(res, SProd)
         assert true_res == res
 
-    @pytest.mark.xfail
     def test_Hamiltonian_sum(self):
         """Test that Hamiltonians with Tensors and sums get transformed to new operator classes and return the correct result"""
         H1 = qml.Hamiltonian([1.0], [qml.PauliX(0) @ qml.PauliX(1)])
diff --git a/tests/ops/functions/test_equal.py b/tests/ops/functions/test_equal.py
index a25c20224..79067bca1 100644
--- a/tests/ops/functions/test_equal.py
+++ b/tests/ops/functions/test_equal.py
@@ -1588,7 +1588,14 @@ class TestSymbolicOpComparison:
         assert qml.equal(op1, op2, atol=1e-3, rtol=1e-2)
         assert not qml.equal(op1, op2, atol=1e-5, rtol=1e-4)
 
-    @pytest.mark.parametrize("bases_bases_match", BASES)
+    additional_cases = [
+        (qml.sum(qml.PauliX(0), qml.PauliY(0)), qml.sum(qml.PauliY(0), qml.PauliX(0)), True),
+        (qml.sum(qml.PauliX(0), qml.PauliY(1)), qml.sum(qml.PauliX(1), qml.PauliY(0)), False),
+        (qml.prod(qml.PauliX(0), qml.PauliY(1)), qml.prod(qml.PauliY(1), qml.PauliX(0)), True),
+        (qml.prod(qml.PauliX(0), qml.PauliY(1)), qml.prod(qml.PauliX(1), qml.PauliY(0)), False),
+    ]
+
+    @pytest.mark.parametrize("bases_bases_match", BASES + additional_cases)
     @pytest.mark.parametrize("params_params_match", PARAMS)
     def test_s_prod_comparison(self, bases_bases_match, params_params_match):
         """Test that equal compares two objects of the SProd class"""
@@ -1772,6 +1779,22 @@ class TestSumComparisons:
         op2 = qml.sum(*base_list2)
         assert qml.equal(op1, op2) == res
 
+    def test_sum_equal_order_invarient(self):
+        """Test that the order of operations doesn't affect equality"""
+        H1 = qml.prod(qml.PauliX(0), qml.PauliX(1))
+        H2 = qml.s_prod(1.0, qml.sum(qml.PauliY(0), qml.PauliY(1)))
+
+        true_res = qml.sum(
+            qml.s_prod(2j, qml.prod(qml.PauliZ(0), qml.PauliX(1))),
+            qml.s_prod(2j, qml.prod(qml.PauliX(0), qml.PauliZ(1))),
+        )
+        true_res = true_res.simplify()
+
+        res = qml.prod(H1, H2) - qml.prod(H2, H1)
+        res = res.simplify()
+
+        assert true_res == res
+
 
 def f1(p, t):
     return np.polyval(p, t)
diff --git a/tests/ops/op_math/test_sum.py b/tests/ops/op_math/test_sum.py
index 850c72df0..c53fd5a8f 100644
--- a/tests/ops/op_math/test_sum.py
+++ b/tests/ops/op_math/test_sum.py
@@ -23,9 +23,9 @@ import pytest
 import pennylane as qml
 import pennylane.numpy as qnp
 from pennylane import math
+from pennylane.wires import Wires
 from pennylane.operation import AnyWires, MatrixUndefinedError, Operator
 from pennylane.ops.op_math import Prod, Sum
-from pennylane.wires import Wires
 
 no_mat_ops = (
     qml.Barrier,
@@ -800,11 +800,13 @@ class TestSortWires:
         op_list = [
             qml.PauliX(3),
             qml.PauliZ(2),
+            qml.PauliY("a"),
             qml.RX(1, 5),
             qml.PauliY(0),
             qml.PauliY(1),
-            qml.PauliZ(3),
+            qml.PauliZ("c"),
             qml.PauliX(5),
+            qml.PauliZ("ba"),
         ]
         sorted_list = Sum._sort(op_list)  # pylint: disable=protected-access
         final_list = [
@@ -812,9 +814,11 @@ class TestSortWires:
             qml.PauliY(1),
             qml.PauliZ(2),
             qml.PauliX(3),
-            qml.PauliZ(3),
             qml.PauliX(5),
             qml.RX(1, 5),
+            qml.PauliY("a"),
+            qml.PauliZ("ba"),
+            qml.PauliZ("c"),
         ]
 
         for op1, op2 in zip(final_list, sorted_list):
@@ -827,23 +831,29 @@ class TestSortWires:
             qml.PauliX(5),
             qml.Toffoli([2, 3, 4]),
             qml.CNOT([2, 5]),
+            qml.PauliZ("ba"),
             qml.RX(1, 5),
             qml.PauliY(0),
             qml.CRX(1, [0, 2]),
             qml.PauliZ(3),
+            qml.Toffoli([1, "c", "ab"]),
             qml.CRY(1, [1, 2]),
+            qml.PauliX("d"),
         )
         sorted_list = Sum._sort(op_tuple)  # pylint: disable=protected-access
         final_list = [
             qml.PauliY(0),
             qml.CRX(1, [0, 2]),
             qml.CRY(1, [1, 2]),
+            qml.Toffoli([1, "c", "ab"]),
             qml.CNOT([2, 5]),
             qml.Toffoli([2, 3, 4]),
             qml.PauliX(3),
             qml.PauliZ(3),
             qml.PauliX(5),
             qml.RX(1, 5),
+            qml.PauliZ("ba"),
+            qml.PauliX("d"),
         ]
 
         for op1, op2 in zip(final_list, sorted_list):