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

Fix lack of alternative for Optimizer in AQC #11099

Merged
merged 9 commits into from
Oct 30, 2023
Merged

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Oct 24, 2023

Summary

While working on the removal of qiskit.algorithms I noticed that the use of the Optimizer class in AQC did not have a proper alternative path, nor did it raise a deprecation warning. This got overlooked during the deprecation process last release, and is necessary to be able to remove the use of algorithms for 1.0. This fix introduces an alternative on 0.45.0 to lay the ground for the future removal.

Details and comments

This is PR # 1 of the "Opflow & Algorithms deprecation series", it blocks #11086.

@ElePT ElePT requested review from alexanderivrii, ShellyGarion and a team as code owners October 24, 2023 12:17
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@ElePT ElePT added this to the 0.45.0 milestone Oct 24, 2023
@ElePT ElePT changed the title Fix deprecation warning for use of Optimizer in AQC Fix alternative for Optimizer in AQC Oct 24, 2023
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.

Overall LGTM, just some minor comments 🙂

@ElePT ElePT changed the title Fix alternative for Optimizer in AQC [opfl-alg-depr-1] Fix lack of alternative for Optimizer in AQC Oct 25, 2023
@ElePT ElePT changed the title [opfl-alg-depr-1] Fix lack of alternative for Optimizer in AQC Fix lack of alternative for Optimizer in AQC Oct 25, 2023
@@ -0,0 +1,21 @@
---
fixes:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that the introduction of the alternative should have happened before, I am considering this a "bugfix", and showing the corresponding release note, but it is a bit of a tricky PR because I could also see it as being a new feature.

@mtreinish mtreinish added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Oct 25, 2023
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!

@Cryoris Cryoris added this pull request to the merge queue Oct 30, 2023
Merged via the queue into Qiskit:main with commit f0d3937 Oct 30, 2023
mergify bot pushed a commit that referenced this pull request Oct 30, 2023
* Add alternative and deprecation warning

* Add deprecation test

* Apply Julien's suggestion

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

* Apply suggestions from Julien's code review

* Move optimizer setting to init

* Fix ddt import

* Fix lint

* Add reno

---------

Co-authored-by: Julien Gacon <gaconju@gmail.com>
(cherry picked from commit f0d3937)
github-merge-queue bot pushed a commit that referenced this pull request Oct 30, 2023
* Add alternative and deprecation warning

* Add deprecation test

* Apply Julien's suggestion

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

* Apply suggestions from Julien's code review

* Move optimizer setting to init

* Fix ddt import

* Fix lint

* Add reno

---------

Co-authored-by: Julien Gacon <gaconju@gmail.com>
(cherry picked from commit f0d3937)

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants