Skip to content

Remove deprecated logic from algorithms#18

Merged
mergify[bot] merged 4 commits into
qiskit-community:mainfrom
ElePT:the-time-has-come
Jul 25, 2023
Merged

Remove deprecated logic from algorithms#18
mergify[bot] merged 4 commits into
qiskit-community:mainfrom
ElePT:the-time-has-come

Conversation

@ElePT
Copy link
Copy Markdown
Collaborator

@ElePT ElePT commented Jul 25, 2023

Summary

Fixes #12

Details and comments

Missing reno.

@ElePT ElePT requested review from Cryoris and woodsp-ibm as code owners July 25, 2023 12:03
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 25, 2023

Pull Request Test Coverage Report for Build 5659622531

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 90.544%

Totals Coverage Status
Change from base Build 5658864852: -0.004%
Covered Lines: 6741
Relevant Lines: 7445

💛 - Coveralls

Comment thread qiskit_algorithms/optimizers/adam_amsgrad.py Outdated
Comment on lines 76 to +77
with self.assertWarns(DeprecationWarning):
result = spsa.optimize(circuit.num_parameters, objective, initial_point=initial_point)
result = spsa.minimize(objective, x0=initial_point)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would have thought the assertWarns line should have gone, as it was to check that the optimize method indeed raised a DeprecationWarning as was intended. I am not sure how this is passing now - nothing in the minimize path should be deprecated I would have thought!

Copy link
Copy Markdown
Collaborator Author

@ElePT ElePT Jul 25, 2023

Choose a reason for hiding this comment

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

A deprecation warning is raised from the evolved operator ansatz, I believe, because it still uses opflow in some methods. This also happens in some of the tests from #17, where the assertion is still needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, this more that we should be changing the code to avoid that deprecation if possible. Its not really something that we are raising in our code and we want to make sure it is and gets seen by the end-user. For this case that assertWarns should not be there - I can remove it (outdenting the call to minimize of course) and it passes fine.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since its passing CI lets go with this - it is likely that once the opflow/QI logic use here is all removed that this will need to be revisited.

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
@ElePT ElePT force-pushed the the-time-has-come branch from 9290397 to cd1c596 Compare July 25, 2023 15:35
@mergify mergify Bot merged commit 92cceb9 into qiskit-community:main Jul 25, 2023
ElePT added a commit to ElePT/qiskit-algorithms that referenced this pull request Jul 26, 2023
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
@ElePT ElePT added this to the 0.1.0 milestone Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove deprecated logic from algorithms whose time has come

4 participants