Skip to content
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

Updated plot_state_hinton #3093

Merged
merged 2 commits into from
Sep 24, 2019
Merged

Conversation

dennis-liu-1
Copy link
Contributor

@dennis-liu-1 dennis-liu-1 commented Sep 12, 2019

Summary

Fixes #2321

Details and comments

  • Add axes to the image part. It looks like the image axes already exist for non-empty graphs. The change that I made here is to remove if np.any(dataimag != 0): if the desired behavior is to see the axes on graphs that don't contain any elements.
  • replace rho with the latex rho
  • Use standard format for real and imag parts of rho

@Travis-S-IBM
Copy link

Regarding point 4, a Hinton diagram doesn't need a colorbar, as there's a canonical convention about using white/black squares for positive/negative values, and the size of the boxes indicates the magnitude.

@Travis-S-IBM
Copy link

@1ucian0 @nonhermitian @mtreinish Is there anything else @dennis-liu-1 needs to do to enable this PR to be closed? If not, mind reviewing the changes?

@mtreinish
Copy link
Member

I agree with @Travis-S-IBM about the color bar. It's not needed because the size of the box scales with magnitude and the color is binary white/black for postivie/negative. I've seen a hinton with color bar before like what QuTip does: http://qutip.org/docs/latest/images/visualization-rho-ss.png but that's different because they also shade the boxes. So I'm fine as marking this as closing #2321 (I've updated the summary message to indicate that).

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code here LGTM, thanks for fixing this issue.

@mtreinish
Copy link
Member

Just for reference an output image like what's reported in the bug with this change is:

hinton

@mtreinish mtreinish merged commit 3617304 into Qiskit:master Sep 24, 2019
kdk pushed a commit that referenced this pull request Sep 26, 2019
* Remove changelog and document automated creation (#3109)

* Remove changelog and document automated creation

With the introduction of the qiskit-bot release automation for terra we
no longer need to manually maintain or keep a changelog in repo. The
github release pages (which we were manually copy and pasting the
changelog into already) take the place of this file. For all future
releases the creation of this page is completely automatic and handled
by the bot. This works by using the git log and PR tags to figure out
which commits belong in the changelog summary. The commit msg summary
(the first line) is used for the changelog based on the tags. If a
longer explanation is necessary that should be handled in the reno
release notes.

Fixes #3077

* Add README section on changelog

* Make tweaks to readme for clarity

* The option fig.tight_layout is triggering warnings in some situations so it is being removed (#3123)

* no fig.tight_layout

* removing the rest of fig.tight_layout()

* check casteable types in Layout  (#3100)

* test

* cast in insinstance

* style

* layouts are already layouts :)

* Travis CI: The sudo: tag is deprecated on Travis (#3134)

https://blog.travis-ci.com/2018-11-19-required-linux-infrastructure-migration

* Fix a syntax error in QuantumCircuit.mirror. (#3138)

* Fix for #3106 gatefacecolor key on circuit diagrams (#3137)

* Issue #3106 gatefacecolor key has no effect on circuit diagrams

If the style property "gatefacecolor" is set to a different value than the default, this color is applied.

Fixes #3106

* Solved style problem

space before : eliminated

* Update plot_state_hinton() formatting (#3093)

Add axes to the image part. It looks like the image axes already exist for non-empty graphs. The change that I made here is to remove if np.any(dataimag != 0): if the desired behavior is to see the axes on graphs that don't contain any elements.

Replace rho with the latex rho

Use standard format for real and imag parts of rho

Fixes #2321

* Set barrier across multi-qubits correctly (#3042)

* Previously it was assumed that specified barrier has
  a single contiguous span across qubits. Qubits for the
  barrier need not be adjacent in the diagram. This commit
  handles this case by adding barrier across correct qubits.

* compute multiple short spans.

Fixes #2918

* Test pulse clipping fix Osx #1. (#3144)

* Remove superfluous reference to timeslots from assemble_schedules (#3139)

* Extend custom multiqubit gates over classical bits for mpl (#3062)

* Got it to extend over the clasical bits

* Fixed a small bug

* Removed a file accidentally modified

* Fix linter

* Deprecate allowing extra keys in circuit drawer style dict (#3105)

* test

* cast in insinstance

* style

* layouts are already layouts :)

* style['plotbarrier'] is being deprecated

* internal param for text drawer renamed to plot_barriers

* error when style contains unknown options

* better error message

* remove #3100, since it was not my intention to be on it

* style

* remove plotbarrier

* unused import

* replace error with warning

* release note

* rochester_layout (#3125)

* Use _append when args already converted and broadcast. (#2936)

* Remove line in init which had been commented out (#3150)

* Fix circuit drawing justification (#3061)

Fixes #2802

qiskit/visualization/utils.py _get_layered_instructions() was erroneous and did not justify left nor right correctly. I have reworked that code which underlies many visual representations of circuits to the user.

* refactored, needs more work on right justification

* Looks like this works for qiskit-terra issue #2802

* May fix first test failure for this pull request, while causing another?

* Draws conditionals on ClassicalRegister correctly now.

* - Occurs check for gate in gates leads to good test results.
- Added a few more self.maxDiff = None while debugging. Should they be pulled out?

* tabs and trailing spaces ;)

* removed the maxDiff's because they make the Style and lint test in Python 3.5 fail

* added tests
modified changelog

* Test case extended to include original issue reported in #2802

* oops, left a maxDiff=None in from testing. Fixed

* changes to utils.py requested by reviewer maddy-tod

* test cases split as requested by reviewer maddy-tod

* added circuit diagrams in comments to new tests

* removed note from changelog and made it a release note per @mtreinish

* made _LayerSpooler a private class

* Revert "Remove changelog and document automated creation (#3109)"

This reverts commit effad12.

* Make _LayerSpooler a subclass of list

The _LayerSpooler class was only ever used as list. While the class was
an independent type it basically just wrapped an internal list. THen
there were helper functions to access and manipulate that list. These
methods just duplicated what python already provides and the list class
itself already implements. This commit fixes this so the _LayerSpooler
itself is a subclass of list so when it's initialized you can just treat
it as the output list it was generating.

* Clean up release note, fix only, not prelude

* un reverted my revert of CONTRIBUTING.md

* refactored, needs more work on right justification

* Looks like this works for qiskit-terra issue #2802

* May fix first test failure for this pull request, while causing another?

* Draws conditionals on ClassicalRegister correctly now.

* - Occurs check for gate in gates leads to good test results.
- Added a few more self.maxDiff = None while debugging. Should they be pulled out?

* tabs and trailing spaces ;)

* removed the maxDiff's because they make the Style and lint test in Python 3.5 fail

* added tests
modified changelog

* Test case extended to include original issue reported in #2802

* oops, left a maxDiff=None in from testing. Fixed

* changes to utils.py requested by reviewer maddy-tod

* test cases split as requested by reviewer maddy-tod

* added circuit diagrams in comments to new tests

* removed note from changelog and made it a release note per @mtreinish

* made _LayerSpooler a private class

* Make _LayerSpooler a subclass of list

The _LayerSpooler class was only ever used as list. While the class was
an independent type it basically just wrapped an internal list. THen
there were helper functions to access and manipulate that list. These
methods just duplicated what python already provides and the list class
itself already implements. This commit fixes this so the _LayerSpooler
itself is a subclass of list so when it's initialized you can just treat
it as the output list it was generating.

* Clean up release note, fix only, not prelude

* put back missing section

* Add partial transpiler support for targeting ion trap backends. (#3122)

* handle reset/barrier/measure (#3130)

* Do not unroll an Rx,Ry,Rxx circuit to U3,CX because of measure. (#3140)
kdk added a commit that referenced this pull request Oct 11, 2019
* Add partial transpiler support for targeting ion trap backends. (#3122)

* handle reset/barrier/measure (#3130)

* Do not unroll an Rx,Ry,Rxx circuit to U3,CX because of measure. (#3140)

* rebase ion-trap branch (#3153)

* Remove changelog and document automated creation (#3109)

* Remove changelog and document automated creation

With the introduction of the qiskit-bot release automation for terra we
no longer need to manually maintain or keep a changelog in repo. The
github release pages (which we were manually copy and pasting the
changelog into already) take the place of this file. For all future
releases the creation of this page is completely automatic and handled
by the bot. This works by using the git log and PR tags to figure out
which commits belong in the changelog summary. The commit msg summary
(the first line) is used for the changelog based on the tags. If a
longer explanation is necessary that should be handled in the reno
release notes.

Fixes #3077

* Add README section on changelog

* Make tweaks to readme for clarity

* The option fig.tight_layout is triggering warnings in some situations so it is being removed (#3123)

* no fig.tight_layout

* removing the rest of fig.tight_layout()

* check casteable types in Layout  (#3100)

* test

* cast in insinstance

* style

* layouts are already layouts :)

* Travis CI: The sudo: tag is deprecated on Travis (#3134)

https://blog.travis-ci.com/2018-11-19-required-linux-infrastructure-migration

* Fix a syntax error in QuantumCircuit.mirror. (#3138)

* Fix for #3106 gatefacecolor key on circuit diagrams (#3137)

* Issue #3106 gatefacecolor key has no effect on circuit diagrams

If the style property "gatefacecolor" is set to a different value than the default, this color is applied.

Fixes #3106

* Solved style problem

space before : eliminated

* Update plot_state_hinton() formatting (#3093)

Add axes to the image part. It looks like the image axes already exist for non-empty graphs. The change that I made here is to remove if np.any(dataimag != 0): if the desired behavior is to see the axes on graphs that don't contain any elements.

Replace rho with the latex rho

Use standard format for real and imag parts of rho

Fixes #2321

* Set barrier across multi-qubits correctly (#3042)

* Previously it was assumed that specified barrier has
  a single contiguous span across qubits. Qubits for the
  barrier need not be adjacent in the diagram. This commit
  handles this case by adding barrier across correct qubits.

* compute multiple short spans.

Fixes #2918

* Test pulse clipping fix Osx #1. (#3144)

* Remove superfluous reference to timeslots from assemble_schedules (#3139)

* Extend custom multiqubit gates over classical bits for mpl (#3062)

* Got it to extend over the clasical bits

* Fixed a small bug

* Removed a file accidentally modified

* Fix linter

* Deprecate allowing extra keys in circuit drawer style dict (#3105)

* test

* cast in insinstance

* style

* layouts are already layouts :)

* style['plotbarrier'] is being deprecated

* internal param for text drawer renamed to plot_barriers

* error when style contains unknown options

* better error message

* remove #3100, since it was not my intention to be on it

* style

* remove plotbarrier

* unused import

* replace error with warning

* release note

* rochester_layout (#3125)

* Use _append when args already converted and broadcast. (#2936)

* Remove line in init which had been commented out (#3150)

* Fix circuit drawing justification (#3061)

Fixes #2802

qiskit/visualization/utils.py _get_layered_instructions() was erroneous and did not justify left nor right correctly. I have reworked that code which underlies many visual representations of circuits to the user.

* refactored, needs more work on right justification

* Looks like this works for qiskit-terra issue #2802

* May fix first test failure for this pull request, while causing another?

* Draws conditionals on ClassicalRegister correctly now.

* - Occurs check for gate in gates leads to good test results.
- Added a few more self.maxDiff = None while debugging. Should they be pulled out?

* tabs and trailing spaces ;)

* removed the maxDiff's because they make the Style and lint test in Python 3.5 fail

* added tests
modified changelog

* Test case extended to include original issue reported in #2802

* oops, left a maxDiff=None in from testing. Fixed

* changes to utils.py requested by reviewer maddy-tod

* test cases split as requested by reviewer maddy-tod

* added circuit diagrams in comments to new tests

* removed note from changelog and made it a release note per @mtreinish

* made _LayerSpooler a private class

* Revert "Remove changelog and document automated creation (#3109)"

This reverts commit effad12.

* Make _LayerSpooler a subclass of list

The _LayerSpooler class was only ever used as list. While the class was
an independent type it basically just wrapped an internal list. THen
there were helper functions to access and manipulate that list. These
methods just duplicated what python already provides and the list class
itself already implements. This commit fixes this so the _LayerSpooler
itself is a subclass of list so when it's initialized you can just treat
it as the output list it was generating.

* Clean up release note, fix only, not prelude

* un reverted my revert of CONTRIBUTING.md

* refactored, needs more work on right justification

* Looks like this works for qiskit-terra issue #2802

* May fix first test failure for this pull request, while causing another?

* Draws conditionals on ClassicalRegister correctly now.

* - Occurs check for gate in gates leads to good test results.
- Added a few more self.maxDiff = None while debugging. Should they be pulled out?

* tabs and trailing spaces ;)

* removed the maxDiff's because they make the Style and lint test in Python 3.5 fail

* added tests
modified changelog

* Test case extended to include original issue reported in #2802

* oops, left a maxDiff=None in from testing. Fixed

* changes to utils.py requested by reviewer maddy-tod

* test cases split as requested by reviewer maddy-tod

* added circuit diagrams in comments to new tests

* removed note from changelog and made it a release note per @mtreinish

* made _LayerSpooler a private class

* Make _LayerSpooler a subclass of list

The _LayerSpooler class was only ever used as list. While the class was
an independent type it basically just wrapped an internal list. THen
there were helper functions to access and manipulate that list. These
methods just duplicated what python already provides and the list class
itself already implements. This commit fixes this so the _LayerSpooler
itself is a subclass of list so when it's initialized you can just treat
it as the output list it was generating.

* Clean up release note, fix only, not prelude

* put back missing section

* Add partial transpiler support for targeting ion trap backends. (#3122)

* handle reset/barrier/measure (#3130)

* Do not unroll an Rx,Ry,Rxx circuit to U3,CX because of measure. (#3140)

* Add global Molmer-Sorenson gate (#3149)

* add global ms gate

* clean up

* Update qiskit/extensions/standard/ms.py

Co-Authored-By: Kevin Krsulich <[email protected]>

* Update qiskit/extensions/standard/ms.py

Co-Authored-By: Kevin Krsulich <[email protected]>

* clean up

* Update qiskit/extensions/standard/ms.py

Co-Authored-By: Lauren Capelluto <[email protected]>

* Update MS docstring
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
Add axes to the image part. It looks like the image axes already exist for non-empty graphs. The change that I made here is to remove if np.any(dataimag != 0): if the desired behavior is to see the axes on graphs that don't contain any elements.

Replace rho with the latex rho

Use standard format for real and imag parts of rho

Fixes Qiskit#2321
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
* Add partial transpiler support for targeting ion trap backends. (Qiskit#3122)

* handle reset/barrier/measure (Qiskit#3130)

* Do not unroll an Rx,Ry,Rxx circuit to U3,CX because of measure. (Qiskit#3140)

* rebase ion-trap branch (Qiskit#3153)

* Remove changelog and document automated creation (Qiskit#3109)

* Remove changelog and document automated creation

With the introduction of the qiskit-bot release automation for terra we
no longer need to manually maintain or keep a changelog in repo. The
github release pages (which we were manually copy and pasting the
changelog into already) take the place of this file. For all future
releases the creation of this page is completely automatic and handled
by the bot. This works by using the git log and PR tags to figure out
which commits belong in the changelog summary. The commit msg summary
(the first line) is used for the changelog based on the tags. If a
longer explanation is necessary that should be handled in the reno
release notes.

Fixes Qiskit#3077

* Add README section on changelog

* Make tweaks to readme for clarity

* The option fig.tight_layout is triggering warnings in some situations so it is being removed (Qiskit#3123)

* no fig.tight_layout

* removing the rest of fig.tight_layout()

* check casteable types in Layout  (Qiskit#3100)

* test

* cast in insinstance

* style

* layouts are already layouts :)

* Travis CI: The sudo: tag is deprecated on Travis (Qiskit#3134)

https://blog.travis-ci.com/2018-11-19-required-linux-infrastructure-migration

* Fix a syntax error in QuantumCircuit.mirror. (Qiskit#3138)

* Fix for Qiskit#3106 gatefacecolor key on circuit diagrams (Qiskit#3137)

* Issue Qiskit#3106 gatefacecolor key has no effect on circuit diagrams

If the style property "gatefacecolor" is set to a different value than the default, this color is applied.

Fixes Qiskit#3106

* Solved style problem

space before : eliminated

* Update plot_state_hinton() formatting (Qiskit#3093)

Add axes to the image part. It looks like the image axes already exist for non-empty graphs. The change that I made here is to remove if np.any(dataimag != 0): if the desired behavior is to see the axes on graphs that don't contain any elements.

Replace rho with the latex rho

Use standard format for real and imag parts of rho

Fixes Qiskit#2321

* Set barrier across multi-qubits correctly (Qiskit#3042)

* Previously it was assumed that specified barrier has
  a single contiguous span across qubits. Qubits for the
  barrier need not be adjacent in the diagram. This commit
  handles this case by adding barrier across correct qubits.

* compute multiple short spans.

Fixes Qiskit#2918

* Test pulse clipping fix Osx Qiskit#1. (Qiskit#3144)

* Remove superfluous reference to timeslots from assemble_schedules (Qiskit#3139)

* Extend custom multiqubit gates over classical bits for mpl (Qiskit#3062)

* Got it to extend over the clasical bits

* Fixed a small bug

* Removed a file accidentally modified

* Fix linter

* Deprecate allowing extra keys in circuit drawer style dict (Qiskit#3105)

* test

* cast in insinstance

* style

* layouts are already layouts :)

* style['plotbarrier'] is being deprecated

* internal param for text drawer renamed to plot_barriers

* error when style contains unknown options

* better error message

* remove Qiskit#3100, since it was not my intention to be on it

* style

* remove plotbarrier

* unused import

* replace error with warning

* release note

* rochester_layout (Qiskit#3125)

* Use _append when args already converted and broadcast. (Qiskit#2936)

* Remove line in init which had been commented out (Qiskit#3150)

* Fix circuit drawing justification (Qiskit#3061)

Fixes Qiskit#2802

qiskit/visualization/utils.py _get_layered_instructions() was erroneous and did not justify left nor right correctly. I have reworked that code which underlies many visual representations of circuits to the user.

* refactored, needs more work on right justification

* Looks like this works for qiskit-terra issue Qiskit#2802

* May fix first test failure for this pull request, while causing another?

* Draws conditionals on ClassicalRegister correctly now.

* - Occurs check for gate in gates leads to good test results.
- Added a few more self.maxDiff = None while debugging. Should they be pulled out?

* tabs and trailing spaces ;)

* removed the maxDiff's because they make the Style and lint test in Python 3.5 fail

* added tests
modified changelog

* Test case extended to include original issue reported in Qiskit#2802

* oops, left a maxDiff=None in from testing. Fixed

* changes to utils.py requested by reviewer maddy-tod

* test cases split as requested by reviewer maddy-tod

* added circuit diagrams in comments to new tests

* removed note from changelog and made it a release note per @mtreinish

* made _LayerSpooler a private class

* Revert "Remove changelog and document automated creation (Qiskit#3109)"

This reverts commit effad12.

* Make _LayerSpooler a subclass of list

The _LayerSpooler class was only ever used as list. While the class was
an independent type it basically just wrapped an internal list. THen
there were helper functions to access and manipulate that list. These
methods just duplicated what python already provides and the list class
itself already implements. This commit fixes this so the _LayerSpooler
itself is a subclass of list so when it's initialized you can just treat
it as the output list it was generating.

* Clean up release note, fix only, not prelude

* un reverted my revert of CONTRIBUTING.md

* refactored, needs more work on right justification

* Looks like this works for qiskit-terra issue Qiskit#2802

* May fix first test failure for this pull request, while causing another?

* Draws conditionals on ClassicalRegister correctly now.

* - Occurs check for gate in gates leads to good test results.
- Added a few more self.maxDiff = None while debugging. Should they be pulled out?

* tabs and trailing spaces ;)

* removed the maxDiff's because they make the Style and lint test in Python 3.5 fail

* added tests
modified changelog

* Test case extended to include original issue reported in Qiskit#2802

* oops, left a maxDiff=None in from testing. Fixed

* changes to utils.py requested by reviewer maddy-tod

* test cases split as requested by reviewer maddy-tod

* added circuit diagrams in comments to new tests

* removed note from changelog and made it a release note per @mtreinish

* made _LayerSpooler a private class

* Make _LayerSpooler a subclass of list

The _LayerSpooler class was only ever used as list. While the class was
an independent type it basically just wrapped an internal list. THen
there were helper functions to access and manipulate that list. These
methods just duplicated what python already provides and the list class
itself already implements. This commit fixes this so the _LayerSpooler
itself is a subclass of list so when it's initialized you can just treat
it as the output list it was generating.

* Clean up release note, fix only, not prelude

* put back missing section

* Add partial transpiler support for targeting ion trap backends. (Qiskit#3122)

* handle reset/barrier/measure (Qiskit#3130)

* Do not unroll an Rx,Ry,Rxx circuit to U3,CX because of measure. (Qiskit#3140)

* Add global Molmer-Sorenson gate (Qiskit#3149)

* add global ms gate

* clean up

* Update qiskit/extensions/standard/ms.py

Co-Authored-By: Kevin Krsulich <[email protected]>

* Update qiskit/extensions/standard/ms.py

Co-Authored-By: Kevin Krsulich <[email protected]>

* clean up

* Update qiskit/extensions/standard/ms.py

Co-Authored-By: Lauren Capelluto <[email protected]>

* Update MS docstring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

making plot_hinton look nicer
3 participants