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

Deprecate Algorithms Factorizers and Linear Solvers #8617

Merged
merged 6 commits into from
Aug 31, 2022
Merged

Deprecate Algorithms Factorizers and Linear Solvers #8617

merged 6 commits into from
Aug 31, 2022

Conversation

manoelmarques
Copy link
Member

@manoelmarques manoelmarques commented Aug 25, 2022 •
edited by woodsp-ibm
Loading

Summary

Factorizers and Linear Solvers deprecated as part of the primitives refactoring.

Closes #8502
Closes #8503

Details and comments

@manoelmarques manoelmarques self-assigned this Aug 25, 2022
@manoelmarques manoelmarques requested review from a team and woodsp-ibm as code owners August 25, 2022 14:57
@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:

@manoelmarques manoelmarques added the Changelog: Deprecation Include in "Deprecated" section of changelog label Aug 25, 2022
@coveralls
Copy link

coveralls commented Aug 25, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2963851033

  • 32 of 33 (96.97%) changed or added relevant lines in 10 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.006%) to 84.15%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/algorithms/linear_solvers/numpy_linear_solver.py 3 4 75.0%
Totals Coverage Status
Change from base Build 2961130318: 0.006%
Covered Lines: 57024
Relevant Lines: 67765
💛 - Coveralls

@manoelmarques manoelmarques changed the title Deprecate HHL and Shor Deprecate Algorithms Factorizers and Linear Solvers Aug 25, 2022
@woodsp-ibm woodsp-ibm added the mod: algorithms Related to the Algorithms module label Aug 25, 2022
Cryoris
Cryoris reviewed Aug 29, 2022
View reviewed changes
qiskit/algorithms/factorizers/shor.py
@@ -50,6 +56,10 @@ class Shor:
See also https://arxiv.org/abs/quant-ph/0205095
"""

@deprecate_function(
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now this will trigger a DeprecationWarning, should we use a PendingDeprecationWarning first and remove these algorithms in 6 months instead of 3 since this is quite a substantial removal?

Copy link
Member

Choose a reason for hiding this comment

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

They get removed after a min of 3 months or two releases right. So they will be around for 6 months roughly if deprecated now. The other algorithms such as VQE etc, I would have expected these to be deprecated now favoring the new primitive based ones that will be introduced shortly so I would have though things consistent with that.

Copy link
Member

Choose a reason for hiding this comment

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

For this where there isn't an alternative it's a deprecation towards removal since we're saying they're no longer in scope for terra deprecation up front is fine. For things like vqe though @Cryoris is correct since we're replacing that functionality with a new thing we have to have the new thing in a release prior to emitting a deprecation warning.

For more details you can refer to https://qiskit.org/documentation/deprecation_policy.html

woodsp-ibm
woodsp-ibm approved these changes Aug 30, 2022
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.

LGTM

@mergify mergify bot merged commit 9195ec1 into Qiskit:main Aug 31, 2022
@manoelmarques manoelmarques deleted the hhl branch August 31, 2022 13:43
@1ucian0 1ucian0 mentioned this pull request Oct 4, 2022
Closed
@HuangJunye HuangJunye mentioned this pull request Nov 24, 2022
Merged
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
@manoelmarques
Deprecate Algorithms Factorizers and Linear Solvers (Qiskit#8617)
16066e5 
* Deprecate HHL and Shor

* Deprecate factorizers and linear solvers

* Fix docstrings
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
@manoelmarques
Deprecate Algorithms Factorizers and Linear Solvers (Qiskit/qiskit#8617)
925910e 
* Deprecate HHL and Shor

* Deprecate factorizers and linear solvers

* Fix docstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cryoris Cryoris Cryoris left review comments

@woodsp-ibm woodsp-ibm woodsp-ibm approved these changes

@mtreinish mtreinish Awaiting requested review from mtreinish

Assignees

@manoelmarques manoelmarques

Labels
Changelog: Deprecation Include in "Deprecated" section of changelog mod: algorithms Related to the Algorithms module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate Shor's algorithm. Deprecate HHL algorithm.
6 participants
@manoelmarques @qiskit-bot @coveralls @mtreinish @Cryoris @woodsp-ibm