Skip to content

Conversation

@abbycross
Copy link
Collaborator

@abbycross abbycross commented Oct 8, 2024

Summary

Add code examples where useful, and copyedit if necessary; closes #13299
Add examples for

  • QuantumCircuit
  • data
  • global_phase
  • name
  • metadata
  • qregs
  • cregs
  • qubits
  • ancillas
  • clbits
  • layout
  • op_start_times
  • num_ancillas
  • num_clbits

Moving the rest to a new PR to break up the work in more manageable pieces!

abbycross and others added 15 commits December 4, 2023 17:06
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
@abbycross abbycross requested a review from a team as a code owner October 8, 2024 21:18
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@abbycross abbycross marked this pull request as draft October 8, 2024 21:18
@abbycross abbycross self-assigned this Oct 8, 2024
@abbycross abbycross added the documentation Something is not clear or an error documentation label Oct 8, 2024
@coveralls
Copy link

coveralls commented Oct 8, 2024

Pull Request Test Coverage Report for Build 14000514726

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 88.063%

Totals Coverage Status
Change from base Build 13816498497: -0.05%
Covered Lines: 72601
Relevant Lines: 82442

💛 - Coveralls

@abbycross abbycross requested a review from ElePT January 21, 2025 17:23
@jakelishman jakelishman self-assigned this Jan 28, 2025
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.

Thanks for looking at this Abby, and sorry it took me a while to get back to you with the review! Overall it's good to get more examples into the docs for the circuit, and mostly they look solid.

I left a couple of questions, most of which I think are for the docs team - I have my opinions, but you guys are more in charge of how you want the docs to be presented.

Comment on lines 1355 to 1366
.. code-block:: text
TranspileLayout(initial_layout=Layout({
0: Qubit(QuantumRegister(3, 'q'), 0),
1: Qubit(QuantumRegister(3, 'q'), 1),
2: Qubit(QuantumRegister(3, 'q'), 2)
}), input_qubit_mapping={Qubit(QuantumRegister(3, 'q'), 0): 0,
Qubit(QuantumRegister(3, 'q'), 1): 1, Qubit(QuantumRegister(3,
'q'), 2): 2}, final_layout=None, _input_qubit_count=3,
_output_qubit_list=[Qubit(QuantumRegister(3, 'q'), 0), Qubit
(QuantumRegister(3, 'q'), 1), Qubit(QuantumRegister(3, 'q'),
2)])
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should show a derived property from the TranspileLayout, rather than printing it out - this big block of text isn't super useful, but I think showing the output of a method indicates to the reader "look, here's an example of what you actually want to do with this", then they can go to the TranspileLayout docs page to read more.

It's probably also good to choose a circuit for which something will actually happen in transpilation, such as:

qc = QuantumCircuit(3, 3)
qc.h(0)
qc.cx(0, 1)
qc.swap(1, 2)
qc.cx(0, 1)
qc.measure([0, 1, 2], [0, 1, 2])

transpiled = pass_manager.run(qc)
print(transpiled.routing_permutation())

# it'll be something like `[0, 2, 1]`, if I remember right.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With a little help from Kaelyn I think I've got this one figured out :-) Please do check my work in 40df969

Co-authored-by: Jake Lishman <jake@binhbar.com>
@abbycross
Copy link
Collaborator Author

@jakelishman thank you so much for your review, and it has been well over a month so I apologize - various large projects got in the way 😱 I will bring your questions to the docs team! This has been a super helpful learning exercise.

@abbycross abbycross requested a review from jakelishman March 24, 2025 13:50
@abbycross
Copy link
Collaborator Author

Ready for another pass-through @jakelishman - many thanks

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.

Sorry I was holding up the final merge on this for so long - I'm fine to go along with what the docs team generally wanted for the level of detail, and to leave things like MPL drawings for a follow-up.

The layout example looks cleaner now, thanks. We perhaps wanted to use an example that induces a non-trivial routing_permutation if that's what we're showing, but that's not a big deal - it's mostly the responsibility of the TranspileLayout class to document that. I have a half-done commit locally from a couple of months ago that does that, which I need to find some time to dust off and publish.

@jakelishman jakelishman added this pull request to the merge queue Apr 29, 2025
@jakelishman jakelishman added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: None Do not include in changelog labels Apr 29, 2025
@jakelishman jakelishman added this to the 2.0.1 milestone Apr 29, 2025
Merged via the queue into Qiskit:main with commit a3214a7 Apr 29, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Docs Planning Apr 29, 2025
mergify bot pushed a commit that referenced this pull request Apr 29, 2025
* Create config.yml

* Update CONTRIBUTING.md

* Update contributing.md

* Update .github/ISSUE_TEMPLATE/config.yml

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>

* Update CONTRIBUTING.md

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>

* code review

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>

* code review

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>

* add migration guide link, add "issues" to header

* add to toc at top, update anchor tag

* Update CONTRIBUTING.md

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>

* Add code example to class description

* add data example

* first attempt to add global_phase examples

* add :context:

* tox -e black

* shorten line

* example for name attr

* example of metadata attr

* clarify it's a dictionary, copyedit

* ex. for qregs cregs qubits ancilla clbits

* tox -e black fix

* relocate code examples

* example for layout

* shorten line

* op_start_times example

Co-Authored-By: Kaelyn Ferris <43348706+kaelynj@users.noreply.github.com>

* tox -e black

* fix linting error

* examples for num_ancillas and num_clbits

* try to fix lint errors

* Apply suggestions from code review

Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>

* simplify metadata example

* Update quantumcircuit.py

* Apply suggestions from code review

Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>

* Apply suggestions from code review

Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>

* simplify circuit

Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>

* tox -e black

* hopefully fix the failed check

* fix error

* Apply suggestions from code review

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

* code review

* code review

* trim down example

* tox -e black

* address lint error

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Kaelyn Ferris <43348706+kaelynj@users.noreply.github.com>
Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>
Co-authored-by: Jake Lishman <jake@binhbar.com>
(cherry picked from commit a3214a7)
github-merge-queue bot pushed a commit that referenced this pull request Apr 29, 2025
* Create config.yml

* Update CONTRIBUTING.md

* Update contributing.md

* Update .github/ISSUE_TEMPLATE/config.yml



* Update CONTRIBUTING.md



* code review



* code review



* add migration guide link, add "issues" to header

* add to toc at top, update anchor tag

* Update CONTRIBUTING.md



* Add code example to class description

* add data example

* first attempt to add global_phase examples

* add :context:

* tox -e black

* shorten line

* example for name attr

* example of metadata attr

* clarify it's a dictionary, copyedit

* ex. for qregs cregs qubits ancilla clbits

* tox -e black fix

* relocate code examples

* example for layout

* shorten line

* op_start_times example



* tox -e black

* fix linting error

* examples for num_ancillas and num_clbits

* try to fix lint errors

* Apply suggestions from code review



* simplify metadata example

* Update quantumcircuit.py

* Apply suggestions from code review



* Apply suggestions from code review



* simplify circuit



* tox -e black

* hopefully fix the failed check

* fix error

* Apply suggestions from code review



* code review

* code review

* trim down example

* tox -e black

* address lint error

---------






(cherry picked from commit a3214a7)

Co-authored-by: abbycross <across@us.ibm.com>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Kaelyn Ferris <43348706+kaelynj@users.noreply.github.com>
Co-authored-by: Frank Harkins <frankharkins@hotmail.co.uk>
Co-authored-by: Jake Lishman <jake@binhbar.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 documentation Something is not clear or an error documentation stable backport potential The bug might be minimal and/or import enough to be port to stable

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Improve qiskit/qiskit.circuit.QuantumCircuit page

6 participants