Skip to content

Update top level provider documentation for BackendV2 #7323

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Dec 1, 2021
Merged

Update top level provider documentation for BackendV2 #7323

merged 5 commits into from
Dec 1, 2021

Conversation

mtreinish
Copy link
Member

Summary

As a follow-on to #5885 this commit updates the top level
qiskit.providers module documentation for BackendV2. This primarily
consists of two things, updating the writing a provider section to use
BackendV2 instead of BackendV1 and adding a migration section on user
facing changes between BackendV1 and BackendV2.

Details and comments

@mtreinish mtreinish added on hold Can not fix yet documentation Something is not clear or an error documentation priority: high Changelog: None Do not include in changelog labels Nov 29, 2021
@mtreinish mtreinish added this to the 0.19 milestone Nov 29, 2021
@mtreinish mtreinish requested review from chriseclectic, jyu00 and a team as code owners November 29, 2021 18:00
@mtreinish
Copy link
Member Author

mtreinish commented Nov 29, 2021 •
edited
Loading

This PR is based on #5885's branch and is on hold until #5885 merges. After #5885 merges I'll rebase this on main and remove the on hold label. To see the changes in this PR you can look at the last commit: 4e4b4d1

@coveralls
Copy link

coveralls commented Nov 29, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 1527872187

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 82.963%
Totals Coverage Status
Change from base Build 1527703798: 0.008%
Covered Lines: 50767
Relevant Lines: 61192
💛 - Coveralls

@mtreinish mtreinish force-pushed the backend-v2-toplevel-docs branch from 38c9418 to 4e4b4d1 Compare November 29, 2021 18:53
@mtreinish mtreinish mentioned this pull request Nov 29, 2021
Merged
8 tasks
@mtreinish mtreinish changed the title [WIP] Update top level provider documentation for BackendV2 Update top level provider documentation for BackendV2 Nov 29, 2021
jakelishman
jakelishman reviewed Nov 30, 2021
View reviewed changes
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.

Just a quick read through. I don't know enough about what's happened in the main BackendV2 PR to properly review the content, but the bits I do know looked correct to me.

@mtreinish mtreinish force-pushed the backend-v2-toplevel-docs branch 3 times, most recently from 12673dd to b445912 Compare November 30, 2021 15:00
@mtreinish
Update top level provider documentation for BackendV2
4be51fb 
As a follow-on to Qiskit#5885 this commit updates the top level
qiskit.providers module documentation for BackendV2. This primarily
consists of two things, updating the writing a provider section to use
BackendV2 instead of BackendV1 and adding a migration section on user
facing changes between BackendV1 and BackendV2.
@mtreinish mtreinish force-pushed the backend-v2-toplevel-docs branch from b445912 to 4be51fb Compare December 1, 2021 14:37
@mtreinish mtreinish removed the on hold Can not fix yet label Dec 1, 2021
@mtreinish
Copy link
Member Author

Ok, now that #5885 has merged I've rebased this on the merged version and it's no longer blocked

1ucian0
1ucian0 reviewed Dec 1, 2021
View reviewed changes
@1ucian0
Copy link
Member

1ucian0 commented Dec 1, 2021

The following sentence sounds repetitive:

For a simple example of a provider the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package is worth using as an example.

I suggest:

For a simple example of a provider, see the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package.

1ucian0
1ucian0 reviewed Dec 1, 2021
View reviewed changes
qiskit/providers/__init__.py
Comment on lines +136 to 139
:meth:`~~qiskit.providers.BackendV2.run` method.

* Add any custom gates for the backend's basis to the session
:class:`~qiskit.circuit.EquivalenceLibrary` instance.
Copy link
Member

Choose a reason for hiding this comment

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

Is this spacing intentional?

Screenshot 2021-12-01 at 21 44 55

Suggested change
:meth:`~~qiskit.providers.BackendV2.run` method.
* Add any custom gates for the backend's basis to the session
:class:`~qiskit.circuit.EquivalenceLibrary` instance.
:meth:`~~qiskit.providers.BackendV2.run` method.
* Add any custom gates for the backend's basis to the session
:class:`~qiskit.circuit.EquivalenceLibrary` instance.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, the custom gates thing is a sub-bullet of building a backend (as it's an optionally a part of building a backend class) see where @jakelishman asked the same thing: #7323 (comment)

1ucian0
1ucian0 reviewed Dec 1, 2021
View reviewed changes
1ucian0
1ucian0 reviewed Dec 1, 2021
View reviewed changes
1ucian0
1ucian0 reviewed Dec 1, 2021
View reviewed changes
1ucian0
1ucian0 reviewed Dec 1, 2021
View reviewed changes
@mtreinish @1ucian0
Apply suggestions from code review
a682ff1 
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
@mtreinish
Copy link
Member Author

The following sentence sounds repetitive:

For a simple example of a provider the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package is worth using as an example.

I suggest:

For a simple example of a provider, see the
`qiskit-aqt-provider <https://github.com/qiskit-community/qiskit-aqt-provider>`__
package.

Done in: 254270d I also updated the link to the repo's current home

1ucian0
1ucian0 previously approved these changes Dec 1, 2021
View reviewed changes
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.

Thanks!

@mergify mergify bot merged commit b68b712 into Qiskit:main Dec 1, 2021
@mtreinish mtreinish deleted the backend-v2-toplevel-docs branch December 2, 2021 18:28
@rathishcholarajan rathishcholarajan mentioned this pull request Jan 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakelishman jakelishman jakelishman left review comments

@1ucian0 1ucian0 1ucian0 approved these changes

@chriseclectic chriseclectic Awaiting requested review from chriseclectic

@jyu00 jyu00 Awaiting requested review from jyu00

Assignees

@1ucian0 1ucian0

Labels
Changelog: None Do not include in changelog documentation Something is not clear or an error documentation priority: high
Projects
None yet
Milestone
0.19
Development

Successfully merging this pull request may close these issues.
4 participants
@mtreinish @coveralls @1ucian0 @jakelishman