Visualization: fix histograms (#6692)#6698
Closed
drewrisinger wants to merge 3 commits intoQiskit:mainfrom
Closed
Conversation
drewrisinger
requested review from
a team,
nkanazawa1989 and
nonhermitian
as code owners
drewrisinger
force-pushed
the
dr-pr-fix-issue-6692
branch
from
a3a2595 to
f62b091
Compare
plot_histogram() was having trouble producing figures when number_to_keep was set. There were two semi-related symptoms: * the bars for a given state were not being paired when passed two data * setting the X tick marks was failing
The base issue was that the most common elements was being computed for each dictionary separately when ``number_to_keep != None``, which led to more labels than expected and misaligned data sets. This changes how the dictionaries & keys to keep are calculated, giving equal weighting to each input data, and only returning the correct number of overall states to keep.
drewrisinger
force-pushed
the
dr-pr-fix-issue-6692
branch
from
f62b091 to
caca666
Compare
Contributor
Author
|
CI fails on installing dependencies for Docs, which is unrelated to this PR. |
HuangJunye
added
the
Community PR
Contributor
|
Hi @drewrisinger are you still working on this? |
Contributor
Author
|
No, I am not. Feel free to either merge it, close it, or fork it as desired. |
Contributor
Author
|
I haven't checked recently to see if the underlying issue is still present, or would be improved by this code. |
Contributor
|
Ok thanks for letting us know, I'll close this PR but if you would like to come back to it at a later date just let us know and we can reopen it 😄 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Fixes #6692.
The issue was caused by calculating the most-common elements for the data separately for each dictionary, which caused different labels to be chosen for different dictionaries, and thus the output didn't align.
This resolves it by calculating the states to keep together, and getting values for all of those kept states for each dictionary.
Details and comments
Adds tests for setting
plot_histogram(..., number_to_keep=int). Note that the other arguments toplot_histogram()appear untested still.