search

Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.26k stars 2.37k forks source link

SparsePauliOp does not respect phase of Pauli product #5603

Closed mrossinek closed 2 years ago

mrossinek commented 3 years ago

Information

What is the current behavior?

The SparsePauliOp class does not obtain the correct phase when constructed from the product of two Paulis. Here are some example calculations (see next section for the code):

direct Pauli multiplications
Pauli('IY') * Pauli('IX') =  -iIZ
Pauli('YZ') * Pauli('XZ') =  -iZI
Pauli('IZ') * Pauli('ZI') =  -ZZ

wrap each Pauli in a SparsePauliOp
SparsePauliOp(Pauli('IY')) * SparsePauliOp(Pauli('IX')) =  [('IZ', -1j)]
SparsePauliOp(Pauli('YZ')) * SparsePauliOp(Pauli('XZ')) =  [('ZI', -1j)]
SparsePauliOp(Pauli('IZ')) * SparsePauliOp(Pauli('ZI')) =  [('ZZ', (-1-0j))]

wrap Pauli products in a SparsePauliOp
SparsePauliOp(Pauli('IY') * Pauli('IX')) =  [('IZ', (1+0j))]
SparsePauliOp(Pauli('YZ') * Pauli('XZ')) =  [('ZI', (1+0j))]
SparsePauliOp(Pauli('IZ') * Pauli('ZI')) =  [('ZZ', (1+0j))]

Steps to reproduce the problem

Here is the Python sample to produce the output above: ``` from qiskit.quantum_info.operators.symplectic import Pauli from qiskit.quantum_info.operators.symplectic import SparsePauliOp # multiplication behavior of Pauli class is as expected print('direct Pauli multiplications') ii = Pauli('II') iy = Pauli('IY') ix = Pauli('IX') iz = iy * ix print("Pauli('IY') * Pauli('IX') = ", iz) yz = Pauli('YZ') xz = Pauli('XZ') zi = yz * xz print("Pauli('YZ') * Pauli('XZ') = ", zi) zz = iz * zi print("Pauli('IZ') * Pauli('ZI') = ", zz) # multiplication behaves normally after wrapping each Pauli with a SparsePauliOp print('\nwrap each Pauli in a SparsePauliOp') ii_s = SparsePauliOp(Pauli('II')) iy_s = SparsePauliOp(Pauli('IY')) ix_s = SparsePauliOp(Pauli('IX')) iz_s = iy_s * ix_s print("SparsePauliOp(Pauli('IY')) * SparsePauliOp(Pauli('IX')) = ", iz_s.to_list()) yz_s = SparsePauliOp(Pauli('YZ')) xz_s = SparsePauliOp(Pauli('XZ')) zi_s = yz_s * xz_s print("SparsePauliOp(Pauli('YZ')) * SparsePauliOp(Pauli('XZ')) = ", zi_s.to_list()) zz_s = iz_s * zi_s print("SparsePauliOp(Pauli('IZ')) * SparsePauliOp(Pauli('ZI')) = ", zz_s.to_list()) # BUT: multiplication breaks when the SparsePauliOp is constructed from a product of Pauli's print('\nwrap Pauli products in a SparsePauliOp') prod = Pauli('IY') * Pauli('IX') iz_s_p = SparsePauliOp(prod) # iz_s_p = SparsePauliOp(prod, coeffs=[(-1j) ** prod.phase]) print("SparsePauliOp(Pauli('IY') * Pauli('IX')) = ", iz_s_p.to_list()) prod = Pauli('YZ') * Pauli('XZ') zi_s_p = SparsePauliOp(prod) # zi_s_p = SparsePauliOp(prod, coeffs=[(-1j) ** prod.phase]) print("SparsePauliOp(Pauli('YZ') * Pauli('XZ')) = ", zi_s_p.to_list()) zz_s_p = iz_s_p * zi_s_p print("SparsePauliOp(Pauli('IZ') * Pauli('ZI')) = ", zz_s_p.to_list()) ```

What is the expected behavior?

I would expect that the two last sections of the above example would produce identical results. I.e. the following:

direct Pauli multiplications
Pauli('IY') * Pauli('IX') =  -iIZ
Pauli('YZ') * Pauli('XZ') =  -iZI
Pauli('IZ') * Pauli('ZI') =  -ZZ

wrap each Pauli in a SparsePauliOp
SparsePauliOp(Pauli('IY')) * SparsePauliOp(Pauli('IX')) =  [('IZ', -1j)]
SparsePauliOp(Pauli('YZ')) * SparsePauliOp(Pauli('XZ')) =  [('ZI', -1j)]
SparsePauliOp(Pauli('IZ')) * SparsePauliOp(Pauli('ZI')) =  [('ZZ', (-1-0j))]

wrap Pauli products in a SparsePauliOp
SparsePauliOp(Pauli('IY') * Pauli('IX')) =  [('IZ', -1j)]
SparsePauliOp(Pauli('YZ') * Pauli('XZ')) =  [('ZI', -1j)]
SparsePauliOp(Pauli('IZ') * Pauli('ZI')) =  [('ZZ', (-1-0j))]

Suggested solutions

I have a patch (see below) which can achieve the expected behavior. However, while starting to write a unittest for this, I noticed that SparsePauliOp is mainly compared against PauliTable and, thus, I wanted to integrate the fix into that class. But then I noticed that the docstring of PauliTable says the following:

    **Group Product**

    The Pauli's in the Pauli table do not represent the full Pauli as they are
    restricted to having `+1` phase. The dot-product for the Pauli's is defined
    to discard any phase obtained from matrix multiplication so that we have
    :math:`X.Z = Z.X = Y`, etc. This means that for the PauliTable class the
    operator methods :meth:`compose` and :meth:`dot` are equivalent.

Thus, my question is the following: why was the PauliTable designed like this? If this should not be changed, the user should definitely be warned about this limitation of the SparsePauliOp construction unless we choose to apply the following patch and design a unittest which does not compare with the PauliTable directly.

Proposed patch. I had to rename the file to a `txt` in order to be able to upload it here. [patch.txt](https://github.com/Qiskit/qiskit-terra/files/5794919/patch.txt)
ikkoham commented 3 years ago

PauliTable will be deprecated and @chriseclectic is improving SparsePauliOp.

mrossinek commented 3 years ago

Thanks @ikkoham! That's is good to know. Shall we leave this open then, so that it doesn't get lost in the process of these changes?

stfnmangini commented 2 years ago

Hi everyone 👋 Is there any update on this issue? As for now, the problem persists and you incur in loosing signs depending on how you implement the composition (* or &) of Paulis, and if you use SparsePauliOp or just Pauli.

For example, all the following commands should yield the same result , but they don't:

from qiskit.quantum_info import SparsePauliOp
from qiskit.quantum_info.operators import Pauli

print("Direct Pauli multiplications with *")
print(Pauli('IY') * Pauli('IX')) #=  -iIZ
print(Pauli('YZ') * Pauli('XZ')) #=  -iZI
print(Pauli('IZ') * Pauli('ZI'), "\n") #=  ZZ

print("Direct Pauli multiplications with &")
print(Pauli('IY') & Pauli('IX')) #=  iIZ
print(Pauli('YZ') & Pauli('XZ')) #=  iZI
print(Pauli('IZ') & Pauli('ZI'), "\n") #=  ZZ

print("Wrap each Pauli in a SparsePauliOp and multiply with &")
print((SparsePauliOp(Pauli('IY')) & SparsePauliOp(Pauli('IX'))).to_list()) #=  [('IZ', 1j)]
print((SparsePauliOp(Pauli('YZ')) & SparsePauliOp(Pauli('XZ'))).to_list()) #=  [('ZI', 1j)]
print((SparsePauliOp(Pauli('IZ')) & SparsePauliOp(Pauli('ZI'))).to_list(), "\n") #=  [('ZZ', (1+0j))]

print("Wrap each Pauli in a SparsePauliOp and multiply with *")
print((SparsePauliOp(Pauli('IY')) * SparsePauliOp(Pauli('IX'))).to_list()) #=  [('IZ', -1j)]
print((SparsePauliOp(Pauli('YZ')) * SparsePauliOp(Pauli('XZ'))).to_list()) #=  [('ZI', -1j)]
print((SparsePauliOp(Pauli('IZ')) * SparsePauliOp(Pauli('ZI'))).to_list(), "\n") #=  [('ZZ', (1+0j))]

print("Wrap Pauli products in a SparsePauliOp")
print((SparsePauliOp(Pauli('IY') * Pauli('IX'))).to_list()) #=  [('IZ', (1+0j))]
print((SparsePauliOp(Pauli('YZ') * Pauli('XZ'))).to_list()) #=  [('ZI', (1+0j))]
print((SparsePauliOp(Pauli('IZ') * Pauli('ZI'))).to_list()) #=  [('ZZ', (1+0j))]

Version Information

Qiskit Software Version
qiskit-terra 0.18.3
qiskit-aer 0.9.1
qiskit-ignis 0.6.0
qiskit-ibmq-provider 0.18.1
qiskit-aqua 0.9.5
qiskit 0.32.1
qiskit-nature 0.2.2

Doing some experimentations with the developer version of Qiskit (qiskit-terra=0.19.0 and qiskit-nature=0.3.0) the problem seems to be solve though. Thanks 😃

ikkoham commented 2 years ago

Thank you for checking this. I think this is resolved in main branch (dev-version). (By the way, * operator has been deprecated and will be removed in Terra 0.19.0.) So closable

jakelishman commented 2 years ago

Fixed by #6826.

I wrote a test case _bisect.py:

import sys
from qiskit.quantum_info import SparsePauliOp, Pauli
sys.exit(int(SparsePauliOp(Pauli("IY") & Pauli("IX")).coeffs[0] == 1j))

which exits with code 0 if the bug is fixed and 1 if it's there. Then:

$ git bisect start main 5f609e5
Bisecting: 362 revisions left to test after this (roughly 9 steps)
[f95b1afd13e224b1d6b0f6fed7253a442477e7be] Changing HHL to allow python list inputs and associatedd tests (#6463)

$ git bisect run python _bisect.py
running  'python' '_bisect.py'
Bisecting: 181 revisions left to test after this (roughly 8 steps)
[91dfa547acb542d7b374da0c4f600511cd40cf0d] Merge adjacent strings (#6908)
running  'python' '_bisect.py'
Bisecting: 90 revisions left to test after this (roughly 7 steps)
[322f5689d0dad86a66d1d511810fdc2ac60a01c7] Fix initialization of SabreSwap pass with coupling_map=None (#7111)
running  'python' '_bisect.py'
Bisecting: 45 revisions left to test after this (roughly 6 steps)
[70d2ab5954a28419f206c108c2ff4291efe757f8] add random_pauli_list to __init__.py (#7012)
running  'python' '_bisect.py'
Bisecting: 22 revisions left to test after this (roughly 5 steps)
[132341d964ae7617df0c8145e01b924433764fdf] Fix PiecewiseChebyshev for constant functions (#6707)
running  'python' '_bisect.py'
Bisecting: 10 revisions left to test after this (roughly 4 steps)
[ed3ceea6e0e4b9046c9b8e852c47e492a1581ddf] Modify copy of instruction in .qasm() method (fix #6687) (#6952)
running  'python' '_bisect.py'
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[40deaf108b12ec7ef8c9d9ddb414a88a969f834e] Add pulse gate pass (#6759)
running  'python' '_bisect.py'
Bisecting: 2 revisions left to test after this (roughly 1 step)
[32f5216df6e356ae66311f2c74f04b81de009dec] More performance improvements for Collect2qBlocks (#6680)
running  'python' '_bisect.py'
Bisecting: 0 revisions left to test after this (roughly 1 step)
[75e06dc915f764ea4e5c1a57097e980e9d01b119] Switch PauliTable to PauliList in SparsePauliOp (#6826)
running  'python' '_bisect.py'
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[90574723b5b61fb9965530a28855e026d579dce5] Remove stray test file accidently commited (#6957)
running  'python' '_bisect.py'
75e06dc915f764ea4e5c1a57097e980e9d01b119 is the first bad commit
commit 75e06dc915f764ea4e5c1a57097e980e9d01b119
Author: Ikko Hamamura <ikkoham@users.noreply.github.com>
Date:   Sat Aug 28 22:20:16 2021 +0900

    Switch PauliTable to PauliList in SparsePauliOp (#6826)

is the commit that fixed things.