Update plots to work with quantum state classes #4324
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chriseclectic
requested review from
1ucian0,
ajavadia,
maddy-tod,
mtreinish and
nonhermitian
as code owners
jaygambetta
previously approved these changes
Apr 28, 2020
ajavadia
previously approved these changes
Apr 29, 2020
kdk
requested changes
Apr 29, 2020
There was a problem hiding this comment.
Lint failure:
************* Module qiskit.visualization.state_visualization
qiskit/visualization/state_visualization.py:61:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
qiskit/visualization/state_visualization.py:279:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
qiskit/visualization/state_visualization.py:628:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.utils
qiskit/visualization/utils.py:293:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
qiskit/visualization/utils.py:316:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.interactive.iplot_paulivec
qiskit/visualization/interactive/iplot_paulivec.py:84:0: R1721: Unnecessary use of a comprehension (unnecessary-comprehension)
************* Module qiskit.visualization.interactive.iplot_cities
qiskit/visualization/interactive/iplot_cities.py:33:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.interactive.iplot_qsphere
qiskit/visualization/interactive/iplot_qsphere.py:37:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.interactive.iplot_hinton
qiskit/visualization/interactive/iplot_hinton.py:33:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.state_visualization
qiskit/visualization/state_visualization.py:61:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
qiskit/visualization/state_visualization.py:279:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
qiskit/visualization/state_visualization.py:628:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.utils
qiskit/visualization/utils.py:293:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
qiskit/visualization/utils.py:316:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.interactive.iplot_paulivec
qiskit/visualization/interactive/iplot_paulivec.py:84:0: R1721: Unnecessary use of a comprehension (unnecessary-comprehension)
************* Module qiskit.visualization.interactive.iplot_cities
qiskit/visualization/interactive/iplot_cities.py:33:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.interactive.iplot_qsphere
qiskit/visualization/interactive/iplot_qsphere.py:37:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
************* Module qiskit.visualization.interactive.iplot_hinton
qiskit/visualization/interactive/iplot_hinton.py:33:0: W9006: "VisualizationError" not documented as being raised (missing-raises-doc)
and doc failure
QiskitError: 'Cannot apply instruction with classical registers: measure'
5594902
chriseclectic
force-pushed
the
fix/visualizations
branch
from
95050f0 to
5594902
Compare
mtreinish
reviewed
Apr 30, 2020
There was a problem hiding this comment.
This lgtm, tested it locally with some scripts I wrote in late 2018 to fix some bugs and everything still works as expected. My only concern with this is the renaming of rho to state as the first param. I agree state is a better name, but we've considered the interface stable for some time now and a user could (however unlikely) be calling the positional param by name, something like :plot_state_hinton(title='My hinton', rho=my_state) which would be broken by this change. @kdk wrote a handy decorator for these cases: https://github.com/Qiskit/qiskit-terra/blob/master/qiskit/util.py#L102 which can be used like: https://github.com/Qiskit/qiskit-terra/blob/86294337a9255ec7ef04ff666a5df02a18b8c218/qiskit/circuit/quantumcircuit.py#L1600-L1601 once you deprecate rho this should be good to go
kdk
reviewed
May 12, 2020
chriseclectic
force-pushed
the
fix/visualizations
branch
from
17725c7 to
cc2f97b
Compare
chriseclectic
force-pushed
the
fix/visualizations
branch
from
8f3c2a7 to
1921332
Compare
In environments without matplotlib installed the deprecation decorator would not be imported leading to an import error on qiskit because the decorator needs to exist at module import time. This commit corrects this by moving the decorator import outside the HAS_MPL condition so it's always present.
The deprecate arguments decorator is used to enable people to pass a positional parameter by name using a depcrecated old name while we transition to a new one. It works by allowing the old name as a keyword only argument and having the decorator pass that value to the new positional arg. However, for this to work the old name of the positional argument must be set as a kwarg on the function. This commit adjust the function signature to enable this.
…/qiskit-terra into fix/visualizations
|
I've updated this PR to fix the issues and correct the usage of the |
kdk
approved these changes
Jul 17, 2020
kdk
added
automerge
Changelog: API Change
mtreinish
added
Changelog: Deprecation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Updates state plotting functions to work directly with
StatevectorandDensityMatrixobjects (along with anything that can be converted to a density matrix like arrays and lists).Simplified some of the plotting code that works with Pauli basis by using the new PauliTable and SparsePauliOp classes.
Details and comments