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.23k stars 2.36k forks source link

`quantum_info.Chi` documentation has incorrect normalization convention #10854

Closed zlatko-minev closed 1 year ago

zlatko-minev commented 1 year ago

Environment

What is happening?

Gives wrong answer and not consistent with the doc definition. Seems to not account for normalization correctly, esp. in view of the way I read and understand the documentation.

How can we reproduce the issue?

from qiskit.quantum_info import SuperOp, Chi

A = SuperOp(np.eye(4))
Chi(A)

Yields Chi([[2.+0.j, 0.+0.j, 0.+0.j, 0.+0.j], [0.+0.j, 0.+0.j, 0.+0.j, 0.+0.j], [0.+0.j, 0.+0.j, 0.+0.j, 0.+0.j], [0.+0.j, 0.+0.j, 0.+0.j, 0.+0.j]], input_dims=(2,), output_dims=(2,))

For 2 qubits identity, gives 4 instead of 1. For 3 qubits gives 8, etc.

What should happen?

Should be 1 not 2: Chi([[1.+0.j, 0.+0.j, 0.+0.j, 0.+0.j], [0.+0.j, 0.+0.j, 0.+0.j, 0.+0.j], [0.+0.j, 0.+0.j, 0.+0.j, 0.+0.j], [0.+0.j, 0.+0.j, 0.+0.j, 0.+0.j]], input_dims=(2,), output_dims=(2,))

Any suggestions?

No response

### Tasks
jakelishman commented 1 year ago

@chriseclectic: can I ask you to check what normalisation $\chi$ matrices should have? Fwiw, QuTiP gives this same 1q process matrix a 4 in the $II$ position, but then claims that it's not TP despite it being happy that both the column-stacked super and intermediate Choi matrices (which match Qiskit) are. Qiskit is happy that what it's spitting out is TP, except that's a fairly meaningless statement because it evaluates that by converting back to Choi; if there's a normalisation problem, it'll get wiped out by that.

edit: fwiw, my understanding of process matrices agrees with what Zlatko's saying as well, but my knowledge of the formalism is sketchy at best.

jakelishman commented 1 year ago

To illustrate a bit more completely, let's say I manually set up a process matrix $M$, and build a column-stacked superoperator manually, where left-multiplication of np.kron(pre, i) for identity matrix i to a column-stacked vectorisation of a matrix represents matrix multiplication from the right by pre (i.e. it's the matrix $\text{Pre}(A)$ such that $\text{Pre}(A)\lvert\rho\rangle\rangle$ is the action $\rho A$, etc for np.kron(i, post)). Now the $\chi$ form of my operator should be exactly $M$ (as I understand things), but Qiskit reckons it's $2M$ (and QuTiP has it as $4M$):

In [1]: import numpy as np
   ...: import itertools
   ...: from qiskit import quantum_info
   ...:
   ...: paulis = [
   ...:     np.eye(2, dtype=complex),
   ...:     np.array([[0, 1], [1, 0]], dtype=complex),
   ...:     np.array([[0, -1j], [1j, 0]]),
   ...:     np.array([[1, 0], [0, -1]], dtype=complex),
   ...: ]
   ...:
   ...: m = np.random.rand(4, 4).astype(complex)
   ...: i = np.eye(2, dtype=complex)
   ...:
   ...: liouville = np.zeros((4, 4), dtype=complex)
   ...: for m_ij, (post, pre) in zip(m.flat, itertools.product(paulis, paulis)):
   ...:     liouville += m_ij * np.kron(pre, i) @ np.kron(i, post)
   ...: chi = quantum_info.Chi(quantum_info.SuperOp(liouville))

In [2]: m
Out[2]:
array([[0.58197743+0.j, 0.87432162+0.j, 0.57324181+0.j, 0.65149797+0.j],
       [0.71808147+0.j, 0.24261409+0.j, 0.10546793+0.j, 0.16137622+0.j],
       [0.41655807+0.j, 0.77593943+0.j, 0.51476106+0.j, 0.6784138 +0.j],
       [0.07718234+0.j, 0.59883245+0.j, 0.80980647+0.j, 0.5287559 +0.j]])

In [3]: chi.data
Out[3]:
array([[ 1.16395486+0.j,  1.74864324+0.j, -1.14648362+0.j,  1.30299595+0.j],
       [ 1.43616294+0.j,  0.48522818+0.j, -0.21093586+0.j,  0.32275243+0.j],
       [ 0.83311613+0.j,  1.55187887+0.j, -1.02952211+0.j,  1.35682759+0.j],
       [ 0.15436468+0.j,  1.1976649 +0.j, -1.61961295+0.j,  1.05751181+0.j]])
jakelishman commented 1 year ago

If so, the suspicious lines to me look like in qiskit.quantum_info.operators.channels.transforms._transform_{to,from}_pauli, which normalise by 2 ** num_qubits, but it feels like they maybe ought to be 4 ** num_qubits to account for the squared size of superoperators?

kevinsung commented 1 year ago

I discussed this with Zlatko and we concluded that the normalization is indeed incorrect according to the docs and standard convention. From the docs: image If the P_i are standard Pauli matrices, then the chi matrix of the identity channel must have a 1 in the upper left corner for all system sizes. We found the following references to support this convention:

If so, the suspicious lines to me look like in qiskit.quantum_info.operators.channels.transforms._transform_{to,from}_pauli, which normalise by 2 ** num_qubits, but it feels like they maybe ought to be 4 ** num_qubits to account for the squared size of superoperators?

I tried making this change, but it seems to result in operators that are not CPTP and it also breaks all conversions. I can investigate this issue further to find a proper fix.

kevinsung commented 1 year ago

@chriseclectic I just wanted to check that you agree with the assessment above. Since it appears you wrote the operator implementations, any tips on the best way to fix it would be appreciated.

kevinsung commented 1 year ago

In https://arxiv.org/abs/1111.6950, which is cited in the documentation, the chi matrix is defined using an orthonormal operator basis (which translates to dividing the Pauli matrices by a normalization factor), but this convention seems rare and is not what is actually documented.

chriseclectic commented 1 year ago

As @kevinsung notes this is the difference to whether you define Chi wrt to the orthonormal Pauli basis {\sigma_j / \sqrt{2^n}} or un-normalized Pauli basis {\sigmaj}. The definition of Chi in qiskit is wrt to the former which would make the documentation incorrect: It should say $\mathcal{E}(\rho) = \sum{ij} \frac{1}{2^n}\chi_{ij} P_i \rho Pj$. Similarly the documentation for PTM is missing some factors of $1/2^n$ and $\sqrt{1 / 2^n}$ for $R{ij}$ and $|A>>_p$

kevinsung commented 1 year ago

Ok, I've submitted #10909 to fix the PTM documentation.

As for the Chi matrix, we're faced with a choice:

  1. Keep the convention that is currently implemented in the code, and fix the documentation as @chriseclectic has described. As discussed above, this convention is not without precedent, and it comes from using an orthonormal operator basis.
  2. Change the code to implement the convention that @zlatko-minev expected. This convention does seem to be more widespread, but this would be a breaking change.

@jakelishman @zlatko-minev @chriseclectic Do you have thoughts? I'm partial to to (1) because it avoids breaking things, but I don't have a good sense of how surprising the current convention is likely to be to most users.

jakelishman commented 1 year ago

I agree, 1 sounds like the best choice to me too - there's nothing inherently wrong with the code, so it would be painful to make a breaking change to it. In #10909, should we update the docs for Chi as well, or were you intending that to be separate?

jakelishman commented 1 year ago

Also: thanks a lot for digging into the literature to both Kevin and Zlatko, and to Chris for replying here - that's a huge help in getting this sorted!

zlatko-minev commented 1 year ago

Yeah, I agree, thanks everyone. Given the precedent throughout the code base, option one sounds good to keep it as it is and document it heavily.

I would suggest maybe using some of those caution boxes in sphinx, I think I've seen elsewhere in the documentation.

That is since what we mean by a Pauli now varies in different parts of the qiskit stack. Here it is normalized and in other parts it's unnormalized, such as when creating Pauli operators.

kevinsung commented 1 year ago

Thanks everyone. I'll update #10909 to also fix the Chi documentation.

That is since what we mean by a Pauli now varies in different parts of the qiskit stack. Here it is normalized and in other parts it's unnormalized, such as when creating Pauli operators.

I'll try to make this clear. I think we can stick with a single meaning for the Paulis by carefully explaining the normalization factors.