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 circuit diagrams to source code #7350

Merged
merged 30 commits into from
Feb 4, 2022
Merged

Add circuit diagrams to source code #7350

merged 30 commits into from
Feb 4, 2022

Conversation

yjt98765
Copy link
Contributor

@yjt98765 yjt98765 commented Dec 3, 2021

Summary

This PR finishes the task proposed by #7283.

When reading source code that is related to some circuits, it is often useful to be able to see the circuit. This PR adds circuit diagrams to source files that did not provide them.

Details and comments

I use the QuantumCircuit.draw function to generate the diagrams and copy them to the source code. Sometimes, I edit the diagram a little to make it more concise.

Other than the two files mentioned in the issue, this PR also modifies many other files. Nevertheless, there must be some files that have not been covered by this PR. If you find such a case, please let me know.

If the related function is included in the documentation (i.e., not test functions or internal functions), I will try to put the circuit diagram in the docstring under a .. parsed-literal:: directive. In this case, the diagram will be included in the generated documentation. For test functions or internal functions, I usually annotate the circuit diagram as line comments (start with #). It is closer to the corresponding code lines compared with the docstring style, so probably easier to be understood.

I did not try to add circuit diagram to the following types of source code:

In these cases, I think the circuit diagram would be too complex that it cannot help the reader to better understand the circuit.

In addition, I think there is no need to add a circuit diagram to

  • Jupyter Notebook output, e.g., controlledgate.py. Circuit diagram will be automatically generated in the documentation, e.g. this.
  • Trivial cases, e.g., test_basicaer_integration.py. When there are only a few gates or all the gates are applied on a qubit, it is easy to imagine the circuit diagram.

close #7283

@coveralls
Copy link

coveralls commented Dec 3, 2021

Pull Request Test Coverage Report for Build 1796262884

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.38%

Totals Coverage Status
Change from base Build 1795794116: 0.0%
Covered Lines: 52237
Relevant Lines: 62649

💛 - Coveralls

@1ucian0
Copy link
Member

1ucian0 commented Dec 3, 2021

Not fully sure (I did not check the generated documentation by this PR), but shouldn't they be .. code-block::ed ?

@yjt98765
Copy link
Contributor Author

yjt98765 commented Dec 3, 2021

Not fully sure (I did not check the generated documentation by this PR), but shouldn't they be .. code-block::ed ?

I see your point. You mean the style like this, right? The circuit will keep its format in the generated document. I was following the styles of test_dynamical_decoupling.py and test_instruction_alignments.py. Maybe it is also OK?

@yjt98765
Copy link
Contributor Author

yjt98765 commented Dec 3, 2021

I was thinking about helping people read the source code. So I put some circuit diagrams in the middle of the program, close to the lines of the implementation. Should we consider more for the generated documentation? In that case, I will move more circuits to the docstring.

@yjt98765
Copy link
Contributor Author

yjt98765 commented Dec 7, 2021

I see the problem. The following docstring

https://github.com/Qiskit/qiskit-terra/blob/1a9f93804f2696e3a6c119ecbebd3724df008dfa/qiskit/circuit/quantumcircuit.py#L434-L440

generates this document, which is not nice.

As a comparison, this one

https://github.com/Qiskit/qiskit-terra/blob/8c9e01d33f56da076b2a294698b43d59566fa3d0/qiskit/circuit/library/n_local/pauli_two_design.py#L40-L50

produces the right format.

I will change the docstrings that will be included in the documentation. However, for those that will not (e.g., test files and functions start with _), I will leave them as the current state.

@1ucian0
Copy link
Member

1ucian0 commented Dec 9, 2021

Sometimes .. parsed-literal:: is used:
https://github.com/Qiskit/qiskit-terra/blob/d9ab98f2b3663b42ba354ed1cd2bc18b3f2cd19d/qiskit/circuit/library/standard_gates/rx.py#L29-L36

I'm not sure about the difference with .. code-block:: . Maybe a good moment to unify them all?

@yjt98765
Copy link
Contributor Author

yjt98765 commented Dec 9, 2021

Yes, I also noticed that. I searched .. parsed-literal:: in my local repository and found about 170 results. Most of them are circuit diagrams. For .. code-block:: searching results, most of them are Python or C code. So I think parsed-literal is a better choice for circuit diagrams, leaving code-block really for code blocks. Do we have to unify all the usages? Many release notes use code-block to add circuit diagrams. Do we also need to change them to parsed-literal?

@1ucian0
Copy link
Member

1ucian0 commented Dec 9, 2021

Do we have to unify all the usages? Many release notes use code-block to add circuit diagrams. Do we also need to change them to parsed-literal?

I think is make sense to have all the circuit diagrams as parsed-literal. But if it gets too long, we can do that in a follow up PR. As you prefer.

@yjt98765
Copy link
Contributor Author

I have changed related code-block to parsed-literal. It turns out there are only a few of them. Now, the PR is ready for review.

@1ucian0 1ucian0 requested a review from ikkoham as a code owner January 31, 2022 21:07
@1ucian0 1ucian0 added automerge Changelog: None Do not include in changelog labels Jan 31, 2022
Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

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

This is great work! Thanks a lot for taking your time to add all these diagrams.

@mergify mergify bot merged commit 1337034 into Qiskit:main Feb 4, 2022
@yjt98765 yjt98765 deleted the diagram branch February 5, 2022 04:56
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* Add circuit diagrams to test_hoare_opt.py

* Add circuit diagram to equivalence_library.py

* Add a TODO in equivalence_library.py

* Add circuit diagram to echo_rzx_weyl_decomposition

* Add circuit diagram to merge_adjacent_barriers.py

* Add circuit diagram to test/python/circuit/library

* Add circuit diagram to test/python/circuit

* Add circuit diagram to test/python/compiler

* Add circuit diagram to test/python/dagcircuit

* Add circuit diagram to test/python

* Format source code with black

* Chang block comment from string to line annotation

* Add circuit diagram to standard_gates

* Add parsed-literal before circuit diagram

* fix a lint problem

* Update circuit diagram in test

* Fix a lint problem

* Add blank lines before and after parsed-literal

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
* Add circuit diagrams to test_hoare_opt.py

* Add circuit diagram to equivalence_library.py

* Add a TODO in equivalence_library.py

* Add circuit diagram to echo_rzx_weyl_decomposition

* Add circuit diagram to merge_adjacent_barriers.py

* Add circuit diagram to test/python/circuit/library

* Add circuit diagram to test/python/circuit

* Add circuit diagram to test/python/compiler

* Add circuit diagram to test/python/dagcircuit

* Add circuit diagram to test/python

* Format source code with black

* Chang block comment from string to line annotation

* Add circuit diagram to standard_gates

* Add parsed-literal before circuit diagram

* fix a lint problem

* Update circuit diagram in test

* Fix a lint problem

* Add blank lines before and after parsed-literal

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add circuit diagrams to source code when some claim about a circuit is made
3 participants