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 Migration Guide for QuantumInstance #9554

Merged
merged 28 commits into from
Apr 13, 2023
Merged

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Feb 8, 2023

Summary

Closes #9517.

Details and comments

Blocked by #9549

@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:

@1ucian0 1ucian0 added the on hold Can not fix yet label Feb 8, 2023
@coveralls
Copy link

coveralls commented Feb 8, 2023

Pull Request Test Coverage Report for Build 4691111819

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 478 unchanged lines in 40 files lost coverage.
  • Overall coverage increased (+0.3%) to 85.738%

Files with Coverage Reduction New Missed Lines %
qiskit/primitives/backend_sampler.py 1 98.81%
qiskit/synthesis/evolution/matrix_synthesis.py 1 93.75%
qiskit/transpiler/passes/layout/full_ancilla_allocation.py 1 97.73%
qiskit/transpiler/passes/optimization/template_matching/template_substitution.py 1 93.2%
qiskit/transpiler/passes/utils/check_map.py 1 97.83%
qiskit/algorithms/amplitude_estimators/estimation_problem.py 2 93.67%
qiskit/primitives/sampler.py 2 97.1%
qiskit/qasm/init.py 2 71.43%
qiskit/transpiler/passes/layout/csp_layout.py 2 96.67%
qiskit/transpiler/passes/layout/trivial_layout.py 2 91.3%
Totals Coverage Status
Change from base Build 4627350272: 0.3%
Covered Lines: 70162
Relevant Lines: 81833

💛 - Coveralls

@ElePT ElePT marked this pull request as ready for review February 17, 2023 17:59
@ElePT ElePT requested a review from a team as a code owner February 17, 2023 17:59
@ElePT ElePT added this to the 0.24.0 milestone Feb 21, 2023
@ElePT ElePT added documentation Something is not clear or an error documentation mod: opflow Related to the Opflow module labels Feb 21, 2023
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
@HuangJunye HuangJunye mentioned this pull request Feb 27, 2023
7 tasks
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved

The functionality of :class:`~qiskit.utils.QuantumInstance.execute` has
now been delegated to the different implementations of the :mod:`~qiskit.primitives` base classes,
while the explicit transpilation has been left to the :mod:`~qiskit.transpiler` module (see table below).
Copy link
Member

Choose a reason for hiding this comment

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

This is more a comment, but since its talking about transpilation its done by default now by the primitives, so algorithms no longer need to manage that aspect along with binding parameters. Transpilation can be turned off - but since operator and circuit size need to match for Estimator, doing your own transpilation needs to meet that constraint - if say it adds swaps, which increase overall circuit width, then it will fail as there is no mechanism to say identify the active qubits it should consider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added that algorithms no longer need to take care of transpilation, as it has been moved to the primitive level. I will see where I can mention the transpilation + Estimator challenges.

* - QuantumInstance method
- Alternative
* - ``QuantumInstance.execute``
- ``Sampler.run`` or ``Estimator.run``
Copy link
Member

Choose a reason for hiding this comment

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

This might also be an alternative https://qiskit.org/documentation/apidoc/execute.html - though the QI one had the mitigation, job splitting/combining and reliability which you may now want/need to do yourself if using this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We're probably going to deprecate execute, so that would not need to be listed as alternative 🙂 See also #7892

docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
circuit.x(1)
circuit.measure_all()

sampler = Sampler(run_options = {"method":"statevector", "shots":200})
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment. I will note while this is ok for Sampler presently, as far as I am aware, the Aer Estimator will override any method you pass it as it sets it internally based on approximation and if noise model is used.


estimator = Estimator(
backend_options={
"method": "density_matrix",
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned above this will be overidden in any case - yes, you have what it chooses internally but there is no choice at this level.

docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
On the other hand, when using the primitives:

* You cannot explicitly access their transpilation routine.
* The mechanism to apply custom transpilation passes to the Aer, Runtime and Backend primitives is to pre-transpile
Copy link
Member

Choose a reason for hiding this comment

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

Care is needed here - as I mentioned in one of the earlier comments at the head of the file. In opflow the ansatz would always have the basis change and measurement gates added before transpilation so if the circuit ended up on more qubits it did not matter. Transpiling it first before giving it to the Estimator is a different flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a warning

Aer simulation following the statevector method. This would be the direct 1-1 replacement of the ``QuantumInstance``
example, as they are both accessing the same simulator. For this reason, the output metadata is
closer to the Quantum Instance's output. Please note that
the resulting quasi-probability distribution does not use bitstrings but **integers** to identify the states.
Copy link
Member

Choose a reason for hiding this comment

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

You could note the QuasiProb does have methods binary_probabilities and hex_probabilities if the integer keys are not to your liking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added note and example

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
@HuangJunye HuangJunye self-assigned this Mar 24, 2023
declanmillar
declanmillar previously approved these changes Mar 31, 2023
Copy link
Member

@declanmillar declanmillar left a comment

Choose a reason for hiding this comment

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

LGTM! Again, just minor grammar tweaks.

docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Elena. The content looks great!

However I have some comments about the Sphinx links.

docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
The only alternative for local simulations using the quantum instance was using an Aer Simulator backend.
Please note that ``QuantumInstance.execute()`` returned the counts bitstrings in hexadecimal format.

.. testcode::
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't plan to run the tests (I see that you used the SKIP flag) you can use .. code-block:: python and .. code-block:: text for input and output respectively so you don't depend on the Sphinx doctest extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know about .. code-block:: text, thanks!

Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

Some minor points and one comment on the unbound PMs, otherwise LGTM 👍🏻

docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
Quasi dists: [{2: 0.0008492371522941081, 3: 0.9968874384378738, 0: -0.0003921227905920063,
1: 0.002655447200424097}]

.. dropdown:: Example 4: Circuit Sampling with Custom Bound and Unbound Pass Managers
Copy link
Contributor

Choose a reason for hiding this comment

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

The power of unbound and bound pass managers in the quantum instance is actually coming from the combination with the CircuitSampler 🙂 If you have an unbound and bound pass manager (for example, the unbound pass manager does the routing and high level optimization and the bound does pulse optimization), then you could do

qi = QuantumInstance(pass_manager=unbound, bound_pass_manager=bound, ....)
sampler = CircuitSampler(qi)

circuits = .... # circuit with *free* parameters
sampler.convert(circuits, param_values=....)  # sampler will first do the unbound, then bind, then the bound for you

This workflow is (afaik) not possible anymore with the primitives. Should we update this example?

Co-authored-by: Julien Gacon <gaconju@gmail.com>
@ElePT ElePT removed the on hold Can not fix yet label Apr 13, 2023
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
docs/migration_guides/qi_migration.rst Outdated Show resolved Hide resolved
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Copy link
Contributor

@Cryoris Cryoris 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 all the detailed work on the migration guides!

@Cryoris Cryoris enabled auto-merge April 13, 2023 15:39
@Cryoris Cryoris added this pull request to the merge queue Apr 13, 2023
Merged via the queue into Qiskit:main with commit 9157b40 Apr 13, 2023
giacomoRanieri pushed a commit to giacomoRanieri/qiskit-terra that referenced this pull request Apr 16, 2023
* Add draft

* Remove qiskit

* Add qi migration file

* Add content

* Finish content

* Update provider syntax

* Finish content

* Apply auto-suggestions

* Add doctests

* Change to dropdowns

* Add EM results

* Apply suggestions from code review

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Apply review comments

* Apply suggestions from Declan's code review

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Apply suggestions from Guillermo's code review

Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>

* Change from doctest to codeblocks

* Review follow-up edits

* Replace statevector with quantum info

* Reorder index

* Add assemble alternative

* Apply suggestions from Julien's code review

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

* Change transpilation example

* Apply suggestions from Juliencode review

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

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
* Add draft

* Remove qiskit

* Add qi migration file

* Add content

* Finish content

* Update provider syntax

* Finish content

* Apply auto-suggestions

* Add doctests

* Change to dropdowns

* Add EM results

* Apply suggestions from code review

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Apply review comments

* Apply suggestions from Declan's code review

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Apply suggestions from Guillermo's code review

Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>

* Change from doctest to codeblocks

* Review follow-up edits

* Replace statevector with quantum info

* Reorder index

* Add assemble alternative

* Apply suggestions from Julien's code review

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

* Change transpilation example

* Apply suggestions from Juliencode review

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

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 6, 2023
* Add draft

* Remove qiskit

* Add qi migration file

* Add content

* Finish content

* Update provider syntax

* Finish content

* Apply auto-suggestions

* Add doctests

* Change to dropdowns

* Add EM results

* Apply suggestions from code review

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Apply review comments

* Apply suggestions from Declan's code review

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Apply suggestions from Guillermo's code review

Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>

* Change from doctest to codeblocks

* Review follow-up edits

* Replace statevector with quantum info

* Reorder index

* Add assemble alternative

* Apply suggestions from Julien's code review

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

* Change transpilation example

* Apply suggestions from Juliencode review

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

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
* Add draft

* Remove qiskit

* Add qi migration file

* Add content

* Finish content

* Update provider syntax

* Finish content

* Apply auto-suggestions

* Add doctests

* Change to dropdowns

* Add EM results

* Apply suggestions from code review

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Apply review comments

* Apply suggestions from Declan's code review

Co-authored-by: Declan Millar <declan.millar@ibm.com>

* Apply suggestions from Guillermo's code review

Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>

* Change from doctest to codeblocks

* Review follow-up edits

* Replace statevector with quantum info

* Reorder index

* Add assemble alternative

* Apply suggestions from Julien's code review

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

* Change transpilation example

* Apply suggestions from Juliencode review

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

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Something is not clear or an error documentation mod: opflow Related to the Opflow module priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Quantum Instance Migration Guide
10 participants