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

Update optimizers to allow new primitives-based algorithms #436

Merged
merged 22 commits into from
Nov 30, 2022

Conversation

t-imamichi
Copy link
Collaborator

@t-imamichi t-imamichi commented Nov 11, 2022

Summary

Add support of the new primitive-based algorithms to optimizers

  • MinimumEigenOptimizer
  • GroverOptimizer
  • WarmStartQAOAOptimizer

addresses #425

  • update codes
  • update tests
  • update docs
  • add reno
    • add prelude

Note that the update of tutorials will be a separate PR to minimize the size of PR.

Details and comments

I copied some tests with legacy solvers to test/algorithms/legacy dir so that we can easiliy remove them when the legacy algorithms are deprecated.

@t-imamichi t-imamichi force-pushed the update-meo2 branch 2 times, most recently from 62334fc to 5776fe5 Compare November 11, 2022 10:17
@t-imamichi t-imamichi added this to the 0.5.0 milestone Nov 11, 2022
@t-imamichi t-imamichi force-pushed the update-meo2 branch 2 times, most recently from eb327ef to 2736b59 Compare November 11, 2022 13:25
@coveralls
Copy link

coveralls commented Nov 11, 2022

Pull Request Test Coverage Report for Build 3579787083

  • 111 of 127 (87.4%) changed or added relevant lines in 7 files are covered.
  • 3 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.06%) to 92.253%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_optimization/algorithms/optimization_algorithm.py 18 19 94.74%
qiskit_optimization/algorithms/grover_optimizer.py 48 54 88.89%
qiskit_optimization/applications/optimization_application.py 14 23 60.87%
Files with Coverage Reduction New Missed Lines %
qiskit_optimization/runtime/vqe_program.py 1 96.67%
qiskit_optimization/algorithms/grover_optimizer.py 1 94.26%
qiskit_optimization/algorithms/optimization_algorithm.py 1 92.2%
Totals Coverage Status
Change from base Build 3575196177: 0.06%
Covered Lines: 4394
Relevant Lines: 4763

💛 - Coveralls

@t-imamichi t-imamichi mentioned this pull request Nov 16, 2022
4 tasks
@t-imamichi t-imamichi changed the title [WIP] update MinimumEigenOptimizer to allow new primitive-based algorithms Update optimizers to allow new primitives-based algorithms Nov 18, 2022
@t-imamichi t-imamichi marked this pull request as ready for review November 18, 2022 09:33
@t-imamichi t-imamichi force-pushed the update-meo2 branch 2 times, most recently from ccd9649 to 3c4eb2e Compare November 18, 2022 09:41
@woodsp-ibm
Copy link
Member

The qiskit-terra version in requirements.dev should be updated to >= 0.22.* since this now needs the newly updated primitives.

Also unit tests should be catching deprecated messages so as CI just catches those which we do not expect (or are outside of our code's control). These would be where we still support/test with older now pending deprecated terra algos for instance.

Looking at the messages there are some that are not related to the primitves change that we might want to take a look at to see if we want/need to adjust the optimization code - e.g. some from our use of matplotlib - best done though if needed in a separate PR.

@t-imamichi
Copy link
Collaborator Author

t-imamichi commented Nov 24, 2022

The unit tests fail with the following message. But I'm not sure how to fix it. Do you have any idea, @manoelmarques?

/home/runner/work/qiskit-optimization/qiskit-optimization/docs/release_notes.rst:36: WARNING: Inline literal start-string without end-string.

Update:
The error seems to be fixed by Steve's correction #436 (comment).

@t-imamichi
Copy link
Collaborator Author

While I update the tutorial #442, I notice that OptimizationApplication.sample_most_likely needs to be updated. Since the method does not have a test, I didn't notice it. I will add a unit test of the method too.

@t-imamichi
Copy link
Collaborator Author

t-imamichi commented Nov 24, 2022

I have updated OptimizationApplication.sample_most_likely. It gets ready for review.

@t-imamichi
Copy link
Collaborator Author

I applied Steve's suggestions. So, this PR is about to merge. The option of binary prob or nearest prob #436 (comment) will be a separate PR.

@t-imamichi
Copy link
Collaborator Author

I'm asking @garrison to take a look at this PR to avoid break of https://github.com/qiskit-community/prototype-qrao

@t-imamichi
Copy link
Collaborator Author

t-imamichi commented Nov 30, 2022

According to Jim, this PR works fine with QRAO. So, I merge this PR.
qiskit-community/prototype-qrao#21 (comment)

@t-imamichi t-imamichi added Changelog: New feature Include in the Added section of the changelog automerge labels Nov 30, 2022
@mergify mergify bot merged commit a752fd9 into qiskit-community:main Nov 30, 2022
@t-imamichi t-imamichi deleted the update-meo2 branch November 30, 2022 05:24
garrison added a commit to qiskit-community/prototype-qrao that referenced this pull request Dec 8, 2022
garrison added a commit to qiskit-community/prototype-qrao that referenced this pull request Dec 9, 2022
* WIP: use primitives for solving the relaxed problem

* Use QAOAAnsatz with VQE

The new QAOA implementation in qiskit-terra assumes the problem
Hamiltonian is diagonal so that it can benefit from certain
additional features, like applying CVaR or other aggregation
functions.  However, for us the problem Hamiltonian is not
diagonal.  So, we implement QAOA using the `QAOAAnsatz` together
with `VQE`.

* Use the default mixer in QAOA, rather than the problem Hamiltonian

* The optimizer now works with both the old and new eigen[ ]solvers

* Update optimizer to accept new eigensolvers

Parts of this follow qiskit-community/qiskit-optimization#436

* Restore tests for the other legacy eigen_solvers

(and suppress the `PendingDeprecationWarning`s using pytest)

* Make the code path for `LegacyNumPyMinimumEigensolver` more explicit

This way, it will be more clear when this branch/block can be
removed in the future

* Fix a typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Changelog: New feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants