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.

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

Add Migration Guide for qiskit.algorithms #9557

Merged
merged 25 commits into from
Apr 5, 2023
Merged

Add Migration Guide for qiskit.algorithms #9557

merged 25 commits into from
Apr 5, 2023

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Feb 8, 2023 •
edited
Loading

Summary

Part of algorithms deprecation plan.

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:

@HuangJunye HuangJunye mentioned this pull request Feb 8, 2023
Closed
@coveralls
Copy link

coveralls commented Feb 8, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 4616092703

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1153 unchanged lines in 96 files lost coverage.
  • Overall coverage increased (+0.3%) to 85.425%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/vf2_layout.rs 1 86.44%
qiskit/algorithms/eigen_solvers/eigen_solver.py 1 97.78%
qiskit/algorithms/evolvers/real_evolver.py 1 92.31%
qiskit/algorithms/phase_estimators/phase_estimation_result.py 1 96.36%
qiskit/algorithms/time_evolvers/variational/var_qite.py 1 94.74%
qiskit/circuit/add_control.py 1 97.2%
qiskit/circuit/library/arithmetic/adders/vbe_ripple_carry_adder.py 1 98.55%
qiskit/circuit/library/standard_gates/p.py 1 98.9%
qiskit/opflow/primitive_ops/pauli_sum_op.py 1 86.63%
qiskit/primitives/backend_estimator.py 1 95.37%
Totals Coverage Status
Change from base Build 4533541029: 0.3%
Covered Lines: 67601
Relevant Lines: 79135
💛 - Coveralls

@woodsp-ibm woodsp-ibm added documentation Something is not clear or an error documentation mod: algorithms Related to the Algorithms module Changelog: None Do not include in changelog labels Feb 10, 2023
@ElePT ElePT marked this pull request as ready for review February 23, 2023 10:48
@ElePT ElePT requested a review from a team as a code owner February 23, 2023 10:48
woodsp-ibm
woodsp-ibm reviewed Mar 10, 2023
View reviewed changes
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

I got up to Eigensolvers - will start there next time,

docs/migration_guides/algorithms_migration.rst
the new Sampler-based :class:`qiskit.algorithms.minimum_eigensolvers.SamplingVQE` algorithm. This could formerly
be realized using the legacy :class:`~qiskit.algorithms.minimum_eigen_solvers.VQE` with
:class:`~qiskit.opflow.expectations.CVaRExpectation`.

Copy link
Member

Choose a reason for hiding this comment

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

Should we also note the other parameters that have changed. E.g expectation (and include_custom) is gone since its part of Estimator now, gradient is there but its now primitive gradients. max_evals_grouped is gone but can be set directly on an optimizer

@woodsp-ibm woodsp-ibm mentioned this pull request Mar 21, 2023
Closed
@woodsp-ibm woodsp-ibm added this to the 0.24.0 milestone Mar 27, 2023
declanmillar
declanmillar previously approved these changes Mar 29, 2023
View reviewed changes
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! Just a couple of tiny change suggestions.

@ElePT @declanmillar
Apply suggestions from Declan's code review
701561b 
Co-authored-by: Declan Millar <declan.millar@ibm.com>
woodsp-ibm
woodsp-ibm reviewed Mar 30, 2023
View reviewed changes
docs/migration_guides/algorithms_migration.rst Outdated

[ 0.01765114+0.0e+00j -0.58507654+0.0e+00j -0.15003642-2.8e-17j]

For complete code examples, see the following updated tutorials:
Copy link
Member

Choose a reason for hiding this comment

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

Minor comment: the output formatting seems to to keep have text closer to the drop down example than the bullet list. Maybe it is what it is. I will note there is only one tutorial in this case - plural is used throughout this even in the case where there is one.

image

Also its a complete code example only for the new way. There are some tutorials in older versions of the docs out there showing the old way but maybe we are only interested in a fuller example of the new way.

Copy link
Contributor Author

@ElePT ElePT Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I manually added a line break when this happens, I am not sure if it is the best way to handle these kind of formatting issues.

ElePT and others added 3 commits March 31, 2023 12:22
@ElePT @woodsp-ibm
Apply suggestions from Steve's code review
cd1c143 
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
@ElePT
Apply review
0d562c1
@ElePT
Merge branch 'algos-migration-guide' of https://github.com/ElePT/qisk…
93ae38b 
…it-terra into algos-migration-guide
@woodsp-ibm woodsp-ibm mentioned this pull request Apr 1, 2023
Closed
Cryoris
Cryoris approved these changes Apr 5, 2023
View reviewed changes
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.

Thanks for all the work, this looks great 💯

@Cryoris Cryoris enabled auto-merge April 5, 2023 07:49
@Cryoris Cryoris added this pull request to the merge queue Apr 5, 2023
Merged via the queue into Qiskit:main with commit ef2aaf6 Apr 5, 2023
giacomoRanieri pushed a commit to giacomoRanieri/qiskit-terra that referenced this pull request Apr 16, 2023
@ElePT @woodsp-ibm
@declanmillar @giacomoRanieri
Add Migration Guide for qiskit.algorithms (Qiskit#9557)
cf1b48a 
* Add draft

* Remove qiskit

* Apply review

* Add algos guide

* Add skeleton

* Add content

* Fix sampling VQE example

* Apply suggestions from Steve's code review

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

* Apply review, update code examples up to Eigensolvers

* Add doctest to sphinx config

* Add tests up to eigensolvers

* Review code examples from Eigensolvers

* Update heading style

* Apply suggestions from Declan's code review

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

* Apply suggestions from Steve's code review

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

* Apply review

* Final changes, fix docs

* Apply Julien's comments

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
@ElePT @woodsp-ibm
@declanmillar @king-p3nguin
Add Migration Guide for qiskit.algorithms (Qiskit#9557)
c5ef3c4 
* Add draft

* Remove qiskit

* Apply review

* Add algos guide

* Add skeleton

* Add content

* Fix sampling VQE example

* Apply suggestions from Steve's code review

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

* Apply review, update code examples up to Eigensolvers

* Add doctest to sphinx config

* Add tests up to eigensolvers

* Review code examples from Eigensolvers

* Update heading style

* Apply suggestions from Declan's code review

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

* Apply suggestions from Steve's code review

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

* Apply review

* Final changes, fix docs

* Apply Julien's comments

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 6, 2023
@ElePT @woodsp-ibm @declanmillar
Add Migration Guide for qiskit.algorithms (Qiskit/qiskit#9557)
0de4635 
* Add draft

* Remove qiskit

* Apply review

* Add algos guide

* Add skeleton

* Add content

* Fix sampling VQE example

* Apply suggestions from Steve's code review

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

* Apply review, update code examples up to Eigensolvers

* Add doctest to sphinx config

* Add tests up to eigensolvers

* Review code examples from Eigensolvers

* Update heading style

* Apply suggestions from Declan's code review

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

* Apply suggestions from Steve's code review

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

* Apply review

* Final changes, fix docs

* Apply Julien's comments

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
@ElePT @woodsp-ibm
@declanmillar @Eric-Arellano
Add Migration Guide for qiskit.algorithms (Qiskit/qiskit#9557)
82ed935 
* Add draft

* Remove qiskit

* Apply review

* Add algos guide

* Add skeleton

* Add content

* Fix sampling VQE example

* Apply suggestions from Steve's code review

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

* Apply review, update code examples up to Eigensolvers

* Add doctest to sphinx config

* Add tests up to eigensolvers

* Review code examples from Eigensolvers

* Update heading style

* Apply suggestions from Declan's code review

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

* Apply suggestions from Steve's code review

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

* Apply review

* Final changes, fix docs

* Apply Julien's comments

---------

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Declan Millar <declan.millar@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@declanmillar declanmillar declanmillar left review comments

@woodsp-ibm woodsp-ibm woodsp-ibm left review comments

@Cryoris Cryoris Cryoris approved these changes

Assignees
No one assigned
Labels
Changelog: None Do not include in changelog documentation Something is not clear or an error documentation mod: algorithms Related to the Algorithms module
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ElePT @qiskit-bot @coveralls @Cryoris @declanmillar @woodsp-ibm