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

Add common usage explanations and code examples to qiskit.visualization module API page #8569

Merged
merged 27 commits into from
Sep 8, 2022
Merged

Add common usage explanations and code examples to qiskit.visualization module API page #8569

merged 27 commits into from
Sep 8, 2022

Conversation

HuangJunye
Copy link
Collaborator

@HuangJunye HuangJunye commented Aug 17, 2022

Summary

Add common usage explanations and code examples to qiskit.visualization module level API reference page.

Fix #8567

Details and comments

  • Add overview section
  • Add prerequisites section: install qiskit visualization optionals
  • Expand Counts and States Visualization section
    • Add section overview
    • Add APIs section with counts and states visualization sub sections
    • Add Example usage section
      • Counts visualization: use plot_histogram as example
      • State visualization: use plot_state_city as example
  • Add common keyword arguments from inherited from matplotlib
Screenshot

image

@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@coveralls
Copy link

coveralls commented Aug 17, 2022

Pull Request Test Coverage Report for Build 3014449986

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 84.301%

Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/squ.py 2 79.78%
Totals Coverage Status
Change from base Build 3014392914: 0.001%
Covered Lines: 57816
Relevant Lines: 68583

💛 - Coveralls

@HuangJunye HuangJunye marked this pull request as ready for review August 31, 2022 10:35
@Guillermo-Mijares-Vilarino
Copy link
Contributor

In general I think it's very nice. Liked the explanations and how you structured the section. I have some small comments however.

qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
qiskit/visualization/__init__.py Show resolved Hide resolved
qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
@HuangJunye HuangJunye added the documentation Something is not clear or an error documentation label Sep 2, 2022

.. jupyter-execute::

from qiskit.tools.visualization import plot_histogram
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't notice that on my first read, but should we import qiskit.tools.visualization or qiskit.visualization?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Look at the PR migrating the file, I think we should use qiskit.visualization. #1878

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

The text here generally looks good, thanks!

I've left some comments on general structuring of the documentation - mostly about avoiding adding a lot of visual clutter by nesting a lot of headings. Headings are good for separating content, but there's already a fairly natural visual distinction between a lot of the components, because of the splits between tables and prose.

It's also good not to categorise things too precisely (as in the common arguments section) - it can often mean that it's hard to spot relevant documentation when it applies to other functions, and introducing a lot of nested tree structure in the headings just to limit it results in an awkward number of headings with little information to users.

qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
qiskit/visualization/__init__.py Outdated Show resolved Hide resolved
jakelishman
jakelishman previously approved these changes Sep 7, 2022
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

This generally looks good to me, thanks for the changes! If you as a team (you Junye, Guillermo and Luciano if he's involved?) are happy with this, I'll merge it.

@jakelishman jakelishman added the Changelog: None Do not include in changelog label Sep 7, 2022
Copy link
Contributor

@Guillermo-Mijares-Vilarino Guillermo-Mijares-Vilarino left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution!

@HuangJunye
Copy link
Collaborator Author

@jakelishman @Guillermo-Mijares-Vilarino Thanks for the approvals! I just made some last minor changes. I think it's good to merge.

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

1024 was already a round number ;)

@mergify mergify bot merged commit e9913e8 into Qiskit:main Sep 8, 2022
ewinston pushed a commit to ewinston/qiskit that referenced this pull request Sep 8, 2022
…on module API page (Qiskit#8569)

* add draft

* revert unintended changes

* revert unintended changes

* add example usage

* add common keyword arguments section

* add sections to apis

* fix internal links

* remove generated hist figure

* lint

* use matplotlib instead of Matplotlib in class reference

* change to a valid denisty matrix in examples

* import from qiskit.visualization instead of qiskit.tools.visualization

* remove unintended changes

* split counts and state visualizations

* remove overview and prerequisite headings

* move common kwargs sections to the top level

* remove link to api table

* remove apis headings

* remove extra blank lines

* remove extra blank lines

* minor twig on the counts to make them 1000 shots in total

* add an intro sentence before the example for common kwargs

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ewinston added a commit to ewinston/qiskit that referenced this pull request Oct 5, 2022
commit a3076a50524a72f1bc91cb633ec5c67a765ef937
Merge: 06dfe40 b7e6329
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Wed Oct 5 09:02:28 2022 -0400

    Merge branch 'main' into controlflow/stochastic_swap

commit 06dfe40
Author: ewinston <ewinston@us.ibm.com>
Date:   Tue Oct 4 23:34:42 2022 -0400

    Update qiskit/transpiler/passes/routing/stochastic_swap.py

    Co-authored-by: Jake Lishman <jake@binhbar.com>

commit 1487b80
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 23:33:34 2022 -0400

    simplify function and naming of control flow layer transpilation
    function

commit 5477c50
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 21:44:18 2022 -0400

    replace "maxind" with "deepest_index"

commit 02e704f
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 21:40:06 2022 -0400

    simplify determining idle qubits

commit 753a637
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 20:39:11 2022 -0400

    fix layer bug

    this fixes a bug where gates were dropped if they shared a layer with a
    control flow op.

commit 4dd68a9
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 11:48:01 2022 -0400

    remove _idle_wires function

commit d74b6ee
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 11:40:22 2022 -0400

    remove utils function for getting qubit order

commit 56029b1
Merge: 9d16884 53e215c
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 09:30:58 2022 -0400

    Merge branch 'main' into controlflow/stochastic_swap

commit 9d16884
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Oct 4 03:00:22 2022 -0400

    make sure stochastic swap child instances get new seeds

commit dc5b23c
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 30 04:39:29 2022 -0400

    linting. fix merge error.

commit 302320d
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 30 04:32:39 2022 -0400

    merged main and renabled checkmap tests

commit 8ec9259
Merge: cb32908 3c9d8d5
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 30 04:14:53 2022 -0400

    Merge branch 'main' into controlflow/stochastic_swap

commit cb32908
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 30 02:28:01 2022 -0400

    updated to allow cf blocks with different registers than containing
    circuit.

commit fb0496e
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 29 15:38:28 2022 -0400

    fix while loop test

commit 463a466
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Wed Sep 28 14:26:14 2022 -0400

    simplify dadnode handling of swap and cz

commit 4f1af5b
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 8 13:16:15 2022 -0400

    remove merge artifact

commit 3133949
Merge: 3c85677 53231fb
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 8 13:15:18 2022 -0400

    Merge branch 'test_merge' into controlflow/stochastic_swap

commit 53231fb
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 8 12:24:55 2022 -0400

    remove redefiing coupling map in control flow context

commit 0161913
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Wed Jul 20 09:56:28 2022 -0400

    add controlflow handling to stochastic swap pass

commit 072c550
Author: Junye Huang <h.jun.ye@gmail.com>
Date:   Thu Sep 8 14:22:12 2022 +0200

    Add common usage explanations and code examples to qiskit.visualization module API page (Qiskit#8569)

    * add draft

    * revert unintended changes

    * revert unintended changes

    * add example usage

    * add common keyword arguments section

    * add sections to apis

    * fix internal links

    * remove generated hist figure

    * lint

    * use matplotlib instead of Matplotlib in class reference

    * change to a valid denisty matrix in examples

    * import from qiskit.visualization instead of qiskit.tools.visualization

    * remove unintended changes

    * split counts and state visualizations

    * remove overview and prerequisite headings

    * move common kwargs sections to the top level

    * remove link to api table

    * remove apis headings

    * remove extra blank lines

    * remove extra blank lines

    * minor twig on the counts to make them 1000 shots in total

    * add an intro sentence before the example for common kwargs

    Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit 64bbeff
Author: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Date:   Thu Sep 8 19:38:53 2022 +0900

    Add an error check for Sampler (Qiskit#8678)

    * add an error check for sampler

    * update

    * Update qiskit/primitives/base_sampler.py

    Co-authored-by: Julien Gacon <gaconju@gmail.com>

    * remove a duplicate check

    * update an error messgage

    Co-authored-by: Julien Gacon <gaconju@gmail.com>

commit bb41815
Author: Luciano Bello <bel@zurich.ibm.com>
Date:   Thu Sep 8 00:52:46 2022 +0200

    Remove deprecated pulse-builder contexts (Qiskit#8697)

    * Remove qiskit.pulse.builder.inline as deprecated in 0.18.0 (2021-07-13)

    * remove imports

    * remove test

    * Remove qiskit.pulse.builder.pad as deprecated in 0.18.0 (2021-07-13)

    * reno

    * Reword removal note

    Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

commit ee0a4f1
Author: Manoel Marques <Manoel.Marques@ibm.com>
Date:   Wed Sep 7 14:35:47 2022 -0400

    Improve HHL/Shor deprecated messages (Qiskit#8699)

commit ea4ebed
Author: a-matsuo <47442626+a-matsuo@users.noreply.github.com>
Date:   Wed Sep 7 17:33:53 2022 +0900

    Include primitive's run_options (Qiskit#8694)

    * include primitive's run_options

    * rename to get_local_run_options

commit a842f40
Author: Edwin Navarro <enavarro@comcast.net>
Date:   Tue Sep 6 18:12:00 2022 -0700

    Move circuit drawer files to `qiskit.visualization.circuit` (Qiskit#8306)

    * Add graph and circuit dirs

    * Move files to new folders

    * Finishing transition to circuit and graph dirs

    * Finish import changes

    * Positioning files and setting init entries

    * Final tweaks to compatibility and lint

    * Reduce to circuit dir only

    * Cleanup

    * Add qcstyle stub for docs

    * Merge main conflicts fix

    * Lint

    * Change test message

    * Fix _directive change

    * Fix op.condition reference

    * Change to _utils and cleanup

    * Lint

    * Fix _trim and dag_drawer test

    * Allow direct import of text, etc.

    * Add comment explaining backwards-compatibility imports

    * Add release note

    Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

commit f4a5241
Author: Ikko Hamamura <ikkoham@users.noreply.github.com>
Date:   Tue Sep 6 00:37:43 2022 +0900

    Default run_options for Primitives (Qiskit#8513)

    * Add run_options to Primitives

    * rm unnecessary comments

    * initial commit of Settings dataclass

    * Revert "initial commit of Settings dataclass"

    This reverts commit 96b8479.

    * fix lint, improve docs, don't return self

    Co-authored-by: Julien Gacon <gaconju@gmail.com>
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

commit a8e553c
Author: a-matsuo <47442626+a-matsuo@users.noreply.github.com>
Date:   Mon Sep 5 21:05:35 2022 +0900

    Gradients with the primitives (Qiskit#8528)

    * added the gradients with the primitives

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * add run_options and supported gate

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * added unittests

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * lint

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix based on the comments

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * add spsa gradient

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * simplify + async

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * added gradient variance

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * lint

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * added the run_options field

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix lint

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix based on comments

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * wip fix2

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * lint

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix epsilon and doc

    * lint

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix

    * Update qiskit/algorithms/gradients/base_sampler_gradient.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/base_sampler_gradient.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/base_estimator_gradient.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/base_sampler_gradient.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/base_estimator_gradient.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/base_estimator_gradient.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * change epsilon error

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/estimator_gradient_result.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * Update qiskit/algorithms/gradients/sampler_gradient_result.py

    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * add gradient test

    * added batch size in spsa gradients

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * fix

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * lint

    * Update qiskit/algorithms/gradients/lin_comb_estimator_gradient.py

    Co-authored-by: Julien Gacon <gaconju@gmail.com>

    * add operator tests

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * consistent name

    * rewrite spsa

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>

    * use algorithm job

    Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
    Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
    Co-authored-by: Julien Gacon <gaconju@gmail.com>
    Co-authored-by: Takashi Imamichi <imamichi@jp.ibm.com>

commit 879f7ba
Author: dalin27 <77298980+dalin27@users.noreply.github.com>
Date:   Mon Sep 5 05:02:11 2022 +0300

    fixed issue 8670 by inverting qubits_coordinates and coupling_map (Qiskit#8671)

commit 3c85677
Merge: 33e0007 e9913e8
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 8 12:30:39 2022 -0400

    Merge branch 'main' into controlflow/stochastic_swap

commit 33e0007
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 8 12:24:55 2022 -0400

    remove redefiing coupling map in control flow context

commit cd9c5cf
Merge: ca4f477 aca01eb
Author: ewinston <ewinston@us.ibm.com>
Date:   Sat Sep 3 02:15:59 2022 -0400

    Merge branch 'main' into controlflow/stochastic_swap

commit ca4f477
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 2 16:15:08 2022 -0400

    remove breakpoint

commit ee78880
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 2 16:13:38 2022 -0400

    fix check_map

commit ef35278
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Fri Sep 2 01:13:12 2022 -0400

    add check_map to tests. tests passing

commit 349dca4
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Thu Sep 1 11:29:47 2022 -0400

    add check_map

commit cb1398d
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Wed Aug 31 22:29:37 2022 -0400

    convert up to for loop

commit 3169afd
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Tue Aug 30 15:54:53 2022 -0400

    lint

commit 2b8d5bb
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Mon Aug 29 16:02:12 2022 -0400

    rmove `continue` handling. raise error on unsupported control flow op.

commit 12fb4bc
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Mon Aug 29 15:55:44 2022 -0400

    remove DAGCircuit.control_flow_ops

commit 9a0ad5c
Merge: 09f8490 82e38d1
Author: ewinston <ewinston@us.ibm.com>
Date:   Fri Aug 12 10:46:41 2022 -0400

    Merge branch 'main' into controlflow/stochastic_swap

commit 09f8490
Author: Erick Winston <ewinston@us.ibm.com>
Date:   Wed Jul 20 09:56:28 2022 -0400

    add controlflow handling to stochastic swap pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog documentation Something is not clear or an error documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add common usage explanations and code examples to qiskit.visualization module level API reference page.
6 participants