-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing the tools for plotting Pauli vec #10619
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 5866932109
💛 - Coveralls |
@mtreinish I see this is beyond my ability. The test image for matplotlib is incorrect and as such we would need to update the file qiskit-terra/test/visual/mpl/graph/references/paulivec.png it should be +1 on the "I" and -1 on "Z". I don't know how you generate a png which can be used for comparison in the unit test infrastructure. |
The image tests actually upload the results in a tarball you can download in this case. The image file it generated was: you can just drop that image in https://github.com/Qiskit/qiskit-terra/tree/main/test/visual/mpl/graph/references and that should fix the failure. |
It seems the visualization uses the definition of the Pauli vector which implies that the Pauli vector coefficients are The example in the test is constructing plotting the Pauli vector for the Could this additional factor of 1/2 in the inner product could cause the discrepancy in what you were expecting? |
The standard definition is \rho = 1/d \sum_I x_I P_I so that x_I = trace[P_I rho] and for a single qubit the coefficients are x, y and z which are the Bloch vectors and for stabilizers states the coefficients have a value of 1. I think we defined it for the first time in this paper 0908.1955. If you include the d factor I the coefficients they your y axis will decay to zero exp with the number of qubits and to get anything meaningful you need to carry around a multiplication of d for each coefficient. |
Yeah that makes sense, it might be nice to add that equation to the docstring and clarify what's expected -- I can also add that 🙂 |
please feel free to add. I also think it might be better to add this as a method to statevector rather than a hidden function if you want to use it. It is exponential but useful. |
Docstring is updated in bdfd094 👍🏻 It's actually already possible to call the paulivec plot from the statevector class via |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for fixing this.
* fixing the plot for pauli_vec to scaled to pm 1 * testing * linting * ref figure * Update docstring to define Paulivector * add reno --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> (cherry picked from commit 213580d)
* fixing the plot for pauli_vec to scaled to pm 1 * testing * linting * ref figure * Update docstring to define Paulivector * add reno --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> (cherry picked from commit 213580d) Co-authored-by: Jay Gambetta <jay.gambetta@us.ibm.com>
* fixing the plot for pauli_vec to scaled to pm 1 * testing * linting * ref figure * Update docstring to define Paulivector * add reno --------- Co-authored-by: Julien Gacon <gaconju@gmail.com>
Summary
I am not sure where we lost a dimensional factor in the plotting of the state in the Pauli vec. Added it back in and made a quick test for the future.
Details and comments