Repository: PennyLaneAI/pennylane
Pull request #5041 (2024-01-11 12:44:04)

Fix issue 5040 for wires in PauliSentence
Quick fix for https://github.com/PennyLaneAI/pennylane/issues/5040

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.
Qottmann: Noticed that some qchem tests have that behavior baked in, is there a logic behind the (seemingly arbitrary) wire ordering that I am missing 😬 ?
Qottmann: There are some new, non-trivial failures in the tapering tests. I tried to check if it just simple wire re-orderings but there seem to be some genuine discrepancies between what is expected and returned, in particular for the cases where new opmath is enabled 😬 @soranjh tbh its very hard to figure out what is going wrong (and tested in the first place) for me not being deep in the qchem module. Could you (or someone else) take another look at this?

We did no see those earlier because the test suite did not run that "far" in the tests. I ran all qchem tests locally and get that short summary (so main problem seems in the tapering tests indeed)

```
======================================================== short test summary info ========================================================
FAILED tests/qchem/test_tapering.py::test_optimal_sector[symbols0-geometry0-0-generators0-2-result0] - assert [-1, 1, -1] == [1, -1, -1]
FAILED tests/qchem/test_tapering.py::test_optimal_sector[symbols2-geometry2-1-generators2-2-result2] - assert [1, 1] == [-1, -1]
FAILED tests/qchem/test_tapering.py::test_transform_hf[generators1-paulixops1-paulix_sector1-2-6-result1] - assert tensor(False, requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_transform_hf[generators2-paulixops2-paulix_sector2-4-12-result2] - assert tensor(False, requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols0-geometry0-0-True] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols1-geometry1-1-False] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols1-geometry1-1-True] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols2-geometry2-1-False] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols2-geometry2-1-True] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols3-geometry3-0-False] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_obs[symbols3-geometry3-0-True] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_excitations[symbols1-geometry1-1-generators1-paulixops1-paulix_sector1-num_commuting1] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_taper_excitations[symbols2-geometry2-1-generators2-paulixops2-paulix_sector2-num_commuting2] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_consistent_taper_ops[operation2-op_gen2] - assert tensor([[False]], requires_grad=True)
FAILED tests/qchem/test_tapering.py::test_consistent_taper_ops[operation3-op_gen3] - assert False
======================================== 15 failed, 299 passed, 86 skipped, 1 xfailed in 29.24s =========================================
```
soranjh: > There are some new, non-trivial failures in the tapering tests. I tried to check if it just simple wire re-orderings but there seem to be some genuine discrepancies between what is expected and returned, in particular for the cases where new opmath is enabled 😬 @soranjh tbh its very hard to figure out what is going wrong (and tested in the first place) for me not being deep in the qchem module. Could you (or someone else) take another look at this?
> 
> We did no see those earlier because the test suite did not run that "far" in the tests. I ran all qchem tests locally and get that short summary (so main problem seems in the tapering tests indeed)
> 

There are two places in tapering that assume ascending wire orders:

1. The wiremap [here](https://github.com/PennyLaneAI/pennylane/blob/5cfdeddbb59a7147146a0c550b8e1099de0cc022/pennylane/qchem/tapering.py#L302). To fix the issue, you can just sort the wires:

wiremap = dict(zip(list(wireset.toset()), range(len(wireset) + 1)))

2. The wire check [here](https://github.com/PennyLaneAI/pennylane/blob/5cfdeddbb59a7147146a0c550b8e1099de0cc022/pennylane/qchem/tapering.py#L438). This can be fixed in the same way with:

symmstr = np.array([1 if wire in tau.wires else 0 for wire in qubit_op.wires.toset()])

I believe these changes are enough to pass the tests and I do not think they will cause any issues elsewhere. But we should be careful. 



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5041?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 [(`5cfdedd`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/commit/5cfdeddbb59a7147146a0c550b8e1099de0cc022?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.36% compared to head [(`afa078a`)](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5041?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) 99.67%.


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


```diff
@@            Coverage Diff             @@
##           master    #5041      +/-   ##
==========================================
+ Coverage   99.36%   99.67%   +0.30%     
==========================================
  Files         392      392              
  Lines       35850    35578     -272     
==========================================
- Hits        35623    35463     -160     
+ Misses        227      115     -112     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PennyLaneAI/pennylane/pull/5041?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).

Qottmann: > Mind adding a test that would fail under master to assert that the bug is "fixed"?

Good point, thanks! Done ✅ 

Patch:
diff --git a/doc/releases/changelog-dev.md b/doc/releases/changelog-dev.md
index 60ed26fbf..9c569ea18 100644
--- a/doc/releases/changelog-dev.md
+++ b/doc/releases/changelog-dev.md
@@ -269,6 +269,9 @@
 * Ensure `tf.function` works with `TensorSpec(shape=None)` by skipping batch size computation.
   [(#5089)](https://github.com/PennyLaneAI/pennylane/pull/5089)
 
+* `PauliSentence.wires` no longer imposes a false order.
+  [(#5041)](https://github.com/PennyLaneAI/pennylane/pull/5041)
+
 <h3>Contributors ✍️</h3>
 
 This release contains contributions from (in alphabetical order):
diff --git a/pennylane/pauli/pauli_arithmetic.py b/pennylane/pauli/pauli_arithmetic.py
index 5d375c67f..888590656 100644
--- a/pennylane/pauli/pauli_arithmetic.py
+++ b/pennylane/pauli/pauli_arithmetic.py
@@ -646,7 +646,7 @@ class PauliSentence(dict):
     @property
     def wires(self):
         """Track wires of the PauliSentence."""
-        return Wires(set().union(*(pw.wires for pw in self.keys())))
+        return Wires.all_wires((pw.wires for pw in self.keys()))
 
     def to_mat(self, wire_order=None, format="dense", buffer_size=None):
         """Returns the matrix representation.
diff --git a/pennylane/qchem/tapering.py b/pennylane/qchem/tapering.py
index 2a76c60f0..0274ed63a 100644
--- a/pennylane/qchem/tapering.py
+++ b/pennylane/qchem/tapering.py
@@ -299,7 +299,7 @@ def _taper_pauli_sentence(ps_h, generators, paulixops, paulix_sector):
         ts_ps += ps_u * ps * ps_u  # helps restrict the peak memory usage for u @ h @ u
 
     wireset = ps_u.wires + ps_h.wires
-    wiremap = dict(zip(wireset, range(len(wireset) + 1)))
+    wiremap = dict(zip(list(wireset.toset()), range(len(wireset) + 1)))
     paulix_wires = [x.wires[0] for x in paulixops]
 
     o = []
@@ -435,7 +435,7 @@ def optimal_sector(qubit_op, generators, active_electrons):
 
     perm = []
     for tau in generators:
-        symmstr = np.array([1 if wire in tau.wires else 0 for wire in qubit_op.wires])
+        symmstr = np.array([1 if wire in tau.wires else 0 for wire in qubit_op.wires.toset()])
         coeff = -1 if np.logical_xor.reduce(np.logical_and(symmstr, hf_str)) else 1
         perm.append(coeff)
 


Test patch:
diff --git a/tests/gradients/finite_diff/test_spsa_gradient_shot_vec.py b/tests/gradients/finite_diff/test_spsa_gradient_shot_vec.py
index fe1493fcc..16979e203 100644
--- a/tests/gradients/finite_diff/test_spsa_gradient_shot_vec.py
+++ b/tests/gradients/finite_diff/test_spsa_gradient_shot_vec.py
@@ -25,6 +25,8 @@ from pennylane.devices import DefaultQubitLegacy
 from pennylane.measurements import Shots
 from pennylane.operation import Observable, AnyWires
 
+np.random.seed(0)
+
 h_val = 0.1
 spsa_shot_vec_tol = 0.31
 
diff --git a/tests/pauli/test_pauli_arithmetic.py b/tests/pauli/test_pauli_arithmetic.py
index 9c10d80fc..70e2691aa 100644
--- a/tests/pauli/test_pauli_arithmetic.py
+++ b/tests/pauli/test_pauli_arithmetic.py
@@ -501,6 +501,13 @@ class TestPauliSentence:
         assert new_pw not in ps.keys()
         assert ps[new_pw] == 0.0
 
+    @pytest.mark.parametrize("pw", words)
+    def test_wires_not_reordered(self, pw):
+        """Test that wires are set correctly and not reshuffled when put in a PS"""
+        true_wires = pw.wires
+        ps = PauliSentence({pw: 1.0})
+        assert ps.wires == true_wires
+
     def test_set_items(self):
         """Test that we can add to a PauliSentence"""
         pw = PauliWord({0: X})
diff --git a/tests/qchem/of_tests/test_molecular_hamiltonian.py b/tests/qchem/of_tests/test_molecular_hamiltonian.py
index 588a84452..18c42f2a0 100644
--- a/tests/qchem/of_tests/test_molecular_hamiltonian.py
+++ b/tests/qchem/of_tests/test_molecular_hamiltonian.py
@@ -321,9 +321,19 @@ def test_custom_wiremap_hamiltonian_pyscf(
 )
 def test_custom_wiremap_hamiltonian_dhf(symbols, geometry, wiremap, args, tmpdir, op_arithmetic):
     r"""Test that the generated Hamiltonian has the correct wire labels given by a custom wiremap."""
+
+    wiremap_dict = dict(zip(range(len(wiremap)), wiremap))
+
     if op_arithmetic:
         enable_new_opmath()
 
+    hamiltonian_ref, _ = qchem.molecular_hamiltonian(
+        symbols=symbols,
+        coordinates=geometry,
+        args=args,
+        outpath=tmpdir.strpath,
+    )
+
     hamiltonian, _ = qchem.molecular_hamiltonian(
         symbols=symbols,
         coordinates=geometry,
@@ -332,7 +342,9 @@ def test_custom_wiremap_hamiltonian_dhf(symbols, geometry, wiremap, args, tmpdir
         outpath=tmpdir.strpath,
     )
 
-    assert list(hamiltonian.wires) == list(wiremap)
+    wiremap_calc = dict(zip(list(hamiltonian_ref.wires), list(hamiltonian.wires)))
+
+    assert wiremap_calc == wiremap_dict
 
     if op_arithmetic:
         disable_new_opmath()
diff --git a/tests/test_hermitian_edge_cases.py b/tests/test_hermitian_edge_cases.py
index c27b3158f..b3f2365eb 100644
--- a/tests/test_hermitian_edge_cases.py
+++ b/tests/test_hermitian_edge_cases.py
@@ -20,6 +20,8 @@ import numpy as np
 
 import pennylane as qml
 
+np.random.seed(0)
+
 
 THETA = np.linspace(0.11, 1, 3)
 PHI = np.linspace(0.32, 1, 3)