From 491f0e28c9240e15faff793e2dce3ee6f3e3b650 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Thu, 1 Aug 2024 18:16:33 -0400 Subject: [PATCH 01/12] absorb `phase` into `coeffs` in `paulis()` setter --- qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py index 91d8d82decae..56e6801e62ad 100644 --- a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py +++ b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py @@ -256,6 +256,8 @@ def paulis(self, value): raise ValueError( f"incorrect number of operators: expected {len(self.paulis)}, got {len(value)}" ) + self.coeffs *= (-1j)**value.phase + value.phase = 0 self._pauli_list = value @property From 2ff4fea1ec2fea823978070c973a0a3f2d90bcdd Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Thu, 1 Aug 2024 18:52:41 -0400 Subject: [PATCH 02/12] do not mutate input array --- qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py index 56e6801e62ad..2bbf7af5530c 100644 --- a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py +++ b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py @@ -257,8 +257,7 @@ def paulis(self, value): f"incorrect number of operators: expected {len(self.paulis)}, got {len(value)}" ) self.coeffs *= (-1j)**value.phase - value.phase = 0 - self._pauli_list = value + self._pauli_list = PauliList.from_symplectic(value.z, value.x, phase=0) @property def coeffs(self): From 8c5a322c0fe425399cf65d7fdb9356be7c221711 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Thu, 1 Aug 2024 18:53:16 -0400 Subject: [PATCH 03/12] add test that paulis setter absorbs phase --- .../operators/symplectic/test_sparse_pauli_op.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py index 4964b0c6a609..b026bd8540c4 100644 --- a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py +++ b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py @@ -1096,6 +1096,19 @@ def test_paulis_setter_rejects_bad_inputs(self): op.paulis = PauliList([Pauli("X"), Pauli("Y")]) with self.assertRaisesRegex(ValueError, "incorrect number of operators"): op.paulis = PauliList([Pauli("XY"), Pauli("ZX"), Pauli("YZ")]) + + def test_paulis_setter_absorbs_phase(self): + """Test that the setter for `paulis` absorbs `paulis.phase` to `self.coeffs`.""" + coeffs_init = np.array([1, 1j]) + op = SparsePauliOp(["XY", "ZX"], coeffs=coeffs_init) + paulis_new = PauliList(["-1jXY", "1jZX"]) + op.paulis = paulis_new + # Paulis attribute should have no phase: + self.assertEqual(op.paulis, PauliList(["XY", "ZX"])) + # Coeffs attribute should now include that phase: + self.assertTrue(np.allclose(op.coeffs, coeffs_init * np.array([-1j, 1j]))) + # Do not mutate the phase of the input array: + self.assertTrue(np.allclose(paulis_new.phase, np.array([1, 3]))) def test_apply_layout_with_transpile(self): """Test the apply_layout method with a transpiler layout.""" From e5aad3c854c262566b2c87213bb4a13ea89aa2e5 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Thu, 1 Aug 2024 18:53:43 -0400 Subject: [PATCH 04/12] lint --- qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py | 2 +- .../quantum_info/operators/symplectic/test_sparse_pauli_op.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py index 2bbf7af5530c..3a75452d90d9 100644 --- a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py +++ b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py @@ -256,7 +256,7 @@ def paulis(self, value): raise ValueError( f"incorrect number of operators: expected {len(self.paulis)}, got {len(value)}" ) - self.coeffs *= (-1j)**value.phase + self.coeffs *= (-1j) ** value.phase self._pauli_list = PauliList.from_symplectic(value.z, value.x, phase=0) @property diff --git a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py index b026bd8540c4..3af7ce58db06 100644 --- a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py +++ b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py @@ -1096,7 +1096,7 @@ def test_paulis_setter_rejects_bad_inputs(self): op.paulis = PauliList([Pauli("X"), Pauli("Y")]) with self.assertRaisesRegex(ValueError, "incorrect number of operators"): op.paulis = PauliList([Pauli("XY"), Pauli("ZX"), Pauli("YZ")]) - + def test_paulis_setter_absorbs_phase(self): """Test that the setter for `paulis` absorbs `paulis.phase` to `self.coeffs`.""" coeffs_init = np.array([1, 1j]) From 836126ece76635cb8591a7a5e0376dbfa676df56 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Fri, 2 Aug 2024 12:56:36 -0400 Subject: [PATCH 05/12] If input paulis have phase, mutate and warn --- .../operators/symplectic/sparse_pauli_op.py | 12 ++++++++++-- .../operators/symplectic/test_sparse_pauli_op.py | 8 +++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py index 3a75452d90d9..3aecbf7e31ef 100644 --- a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py +++ b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py @@ -19,6 +19,7 @@ from collections.abc import Mapping, Sequence, Iterable from numbers import Number from copy import deepcopy +import warnings import numpy as np import rustworkx as rx @@ -256,8 +257,15 @@ def paulis(self, value): raise ValueError( f"incorrect number of operators: expected {len(self.paulis)}, got {len(value)}" ) - self.coeffs *= (-1j) ** value.phase - self._pauli_list = PauliList.from_symplectic(value.z, value.x, phase=0) + if np.any(value.phase): + warnings.warn( + "Assigning SparsePauliOp.paulis to be a PauliList with nonzero phase sets the phase " + "of the given PauliList to zero, absorbing the phase into SparsePauliOp.coeffs.", + UserWarning, + ) + self.coeffs *= (-1j) ** value.phase + value.phase = 0 + self._pauli_list = value @property def coeffs(self): diff --git a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py index 3af7ce58db06..4c7a8677214a 100644 --- a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py +++ b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py @@ -1102,13 +1102,15 @@ def test_paulis_setter_absorbs_phase(self): coeffs_init = np.array([1, 1j]) op = SparsePauliOp(["XY", "ZX"], coeffs=coeffs_init) paulis_new = PauliList(["-1jXY", "1jZX"]) - op.paulis = paulis_new + with self.assertWarns(UserWarning): + # Raise a warning that the RHS is mutated: + op.paulis = paulis_new # Paulis attribute should have no phase: self.assertEqual(op.paulis, PauliList(["XY", "ZX"])) # Coeffs attribute should now include that phase: self.assertTrue(np.allclose(op.coeffs, coeffs_init * np.array([-1j, 1j]))) - # Do not mutate the phase of the input array: - self.assertTrue(np.allclose(paulis_new.phase, np.array([1, 3]))) + # The phase of the input array is now zero: + self.assertTrue(np.allclose(paulis_new.phase, np.array([0, 0]))) def test_apply_layout_with_transpile(self): """Test the apply_layout method with a transpiler layout.""" From 8dd74d9d6528e5cd1cf177442fa2a81ca8555c93 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Fri, 2 Aug 2024 13:56:20 -0400 Subject: [PATCH 06/12] add release note --- .../fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml diff --git a/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml b/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml new file mode 100644 index 000000000000..d6049f89525d --- /dev/null +++ b/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixed a bug when SparsePauliOp.paulis is set to be a PauliList with nonzero phase, where + subsequent calls to several SparsePauliOp methods would produce incorrect results. Now when + SparsePauliOp.paulis is set to a PauliList with nonzero phase, the phase is absorbed into + SparsePauliOp.coeffs, the phase of the input PauliList is set to zero, and a warning is + raised that the input PauliList has been modified. From 50636fa808ad9e9234e55f5ec20e89cefc869bca Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Fri, 2 Aug 2024 13:59:50 -0400 Subject: [PATCH 07/12] release-note formatting --- .../fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml b/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml index d6049f89525d..8e952ead127f 100644 --- a/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml +++ b/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml @@ -1,8 +1,9 @@ --- fixes: - | - Fixed a bug when SparsePauliOp.paulis is set to be a PauliList with nonzero phase, where - subsequent calls to several SparsePauliOp methods would produce incorrect results. Now when - SparsePauliOp.paulis is set to a PauliList with nonzero phase, the phase is absorbed into - SparsePauliOp.coeffs, the phase of the input PauliList is set to zero, and a warning is - raised that the input PauliList has been modified. + Fixed a bug when :attr:`.SparsePauliOp.paulis` is set to be a :class:`.PauliList` with nonzero + phase, where subsequent calls to several :class:`.SparsePauliOp` methods would produce + incorrect results. Now when :attr:`.SparsePauliOp.paulis` is set to a :class:`.PauliList` with + nonzero phase, the phase is absorbed into :attr:`.SparsePauliOp.coeffs`, the phase of the + input :class:`.PauliList` is set to zero, and a warning is raised that the input + :class:`.PauliList` has been modified. From 616e376522298811cb213ab1013c6173f1fffd57 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Wed, 7 Aug 2024 10:29:15 -0400 Subject: [PATCH 08/12] add test with `simplify()` --- .../operators/symplectic/test_sparse_pauli_op.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py index 4c7a8677214a..c11975dd6ffb 100644 --- a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py +++ b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py @@ -1111,6 +1111,12 @@ def test_paulis_setter_absorbs_phase(self): self.assertTrue(np.allclose(op.coeffs, coeffs_init * np.array([-1j, 1j]))) # The phase of the input array is now zero: self.assertTrue(np.allclose(paulis_new.phase, np.array([0, 0]))) + + def test_paulis_setter_absorbs_phase_2(self): + """Test that `paulis` setter followed by `simplify()` handle phase OK.""" + spo = SparsePauliOp(['X', 'X']) + spo.paulis = ['X','-X'] + self.assertEqual(spo.simplify(), SparsePauliOp(['I'], coeffs=[0.+0.j])) def test_apply_layout_with_transpile(self): """Test the apply_layout method with a transpiler layout.""" From 71a752a80fd02f21b280ad5814599807f5cf8213 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Wed, 7 Aug 2024 10:31:07 -0400 Subject: [PATCH 09/12] remove phase-warning from paulis setter --- .../operators/symplectic/sparse_pauli_op.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py index 3aecbf7e31ef..f40192111a66 100644 --- a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py +++ b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py @@ -257,14 +257,8 @@ def paulis(self, value): raise ValueError( f"incorrect number of operators: expected {len(self.paulis)}, got {len(value)}" ) - if np.any(value.phase): - warnings.warn( - "Assigning SparsePauliOp.paulis to be a PauliList with nonzero phase sets the phase " - "of the given PauliList to zero, absorbing the phase into SparsePauliOp.coeffs.", - UserWarning, - ) - self.coeffs *= (-1j) ** value.phase - value.phase = 0 + self.coeffs *= (-1j) ** value.phase + value.phase = 0 self._pauli_list = value @property From cdb74c9a8d363014d3350b01747a736e9db83d5b Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Wed, 7 Aug 2024 10:32:05 -0400 Subject: [PATCH 10/12] lint --- .../operators/symplectic/test_sparse_pauli_op.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py index c11975dd6ffb..dedd84279a8d 100644 --- a/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py +++ b/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py @@ -1102,21 +1102,19 @@ def test_paulis_setter_absorbs_phase(self): coeffs_init = np.array([1, 1j]) op = SparsePauliOp(["XY", "ZX"], coeffs=coeffs_init) paulis_new = PauliList(["-1jXY", "1jZX"]) - with self.assertWarns(UserWarning): - # Raise a warning that the RHS is mutated: - op.paulis = paulis_new + op.paulis = paulis_new # Paulis attribute should have no phase: self.assertEqual(op.paulis, PauliList(["XY", "ZX"])) # Coeffs attribute should now include that phase: self.assertTrue(np.allclose(op.coeffs, coeffs_init * np.array([-1j, 1j]))) # The phase of the input array is now zero: self.assertTrue(np.allclose(paulis_new.phase, np.array([0, 0]))) - + def test_paulis_setter_absorbs_phase_2(self): """Test that `paulis` setter followed by `simplify()` handle phase OK.""" - spo = SparsePauliOp(['X', 'X']) - spo.paulis = ['X','-X'] - self.assertEqual(spo.simplify(), SparsePauliOp(['I'], coeffs=[0.+0.j])) + spo = SparsePauliOp(["X", "X"]) + spo.paulis = ["X", "-X"] + self.assertEqual(spo.simplify(), SparsePauliOp(["I"], coeffs=[0.0 + 0.0j])) def test_apply_layout_with_transpile(self): """Test the apply_layout method with a transpiler layout.""" From 058ba094e2fdc06f9ad802347c3a7a219b063136 Mon Sep 17 00:00:00 2001 From: Andrew Eddins Date: Wed, 7 Aug 2024 11:02:16 -0400 Subject: [PATCH 11/12] remove unused import --- qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py index f40192111a66..f105fe2b87d7 100644 --- a/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py +++ b/qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py @@ -19,7 +19,6 @@ from collections.abc import Mapping, Sequence, Iterable from numbers import Number from copy import deepcopy -import warnings import numpy as np import rustworkx as rx From 3e913866db4f695e1f5b91a430bb4a6cc3a90a5f Mon Sep 17 00:00:00 2001 From: aeddins-ibm <60495383+aeddins-ibm@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:32:54 -0400 Subject: [PATCH 12/12] update reno --- .../notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml b/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml index 8e952ead127f..ff1cf96dd711 100644 --- a/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml +++ b/releasenotes/notes/fix-sparsepauliop-phase-bug-2b24f4b775ca564f.yaml @@ -4,6 +4,5 @@ fixes: Fixed a bug when :attr:`.SparsePauliOp.paulis` is set to be a :class:`.PauliList` with nonzero phase, where subsequent calls to several :class:`.SparsePauliOp` methods would produce incorrect results. Now when :attr:`.SparsePauliOp.paulis` is set to a :class:`.PauliList` with - nonzero phase, the phase is absorbed into :attr:`.SparsePauliOp.coeffs`, the phase of the - input :class:`.PauliList` is set to zero, and a warning is raised that the input - :class:`.PauliList` has been modified. + nonzero phase, the phase is absorbed into :attr:`.SparsePauliOp.coeffs`, and the phase of the + input :class:`.PauliList` is set to zero.