qiskit
qiskit copied to clipboard
Qiskit
Migrating AdaptVQE to Qiskit Terra
Summary
Implements https://github.com/qiskit-advocate/qamp-spring-22/issues/3
This migrates AdaptVQE from Qiskit Nature to Qiskit Terra to make it more widely available.
Details and comments
This is a part of the Qiskit Mentorship Program 2022 spring cohort.
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:
- @Qiskit/terra-core
- @kevinhartman
- @manoelmarques
- @mtreinish
- @woodsp-ibm
Great, looking forward to having this a more generic algorithm here. As a general comment, here in Terra there should no longer be any references/use of Qiskit Nature. I think VQEAdapt here can be neutral and take an EvolvedOpAnsatz (plus sub-class thereof) and perform a MinimumEigenSolver function using that. If we want additional information in the result, beyond what standard VQE has, then we can extend VQEResult. Upper layer algorithms, like the ground state solvers in Nature, give access via their result to the 'raw' underlying algorithm result, so these extra fields would be available there if wanted. But in general, any place VQE plus UCC/UVVC is used in Nature VQEAdapt, when implemented here, should be able to be seamlessly swapped in.
Also make sure the files here are formatted with the latest version of black. CI checks if they would be reformatted and fails if they would be (no reformatting is done by CI at all so they have to be formatted correctly in the PR) - the files here are failing this check.
The above suggestions are purely ensuring that the code works as intended and that the unittest passes. I will do a more thorough review to actually make the code pretty later this week or early next week :+1:
Pull Request Test Coverage Report for Build 3145410868
- 112 of 120 (93.33%) changed or added relevant lines in 2 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.03%) to 84.489%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| qiskit/algorithms/minimum_eigensolvers/adapt_vqe.py | 111 | 119 | 93.28% |
| <!-- | Total: | 112 | 120 |
| Totals | |
|---|---|
| Change from base Build 3141343504: | 0.03% |
| Covered Lines: | 60197 |
| Relevant Lines: | 71248 |
💛 - Coveralls
AdaptVQE is still not compatible with QAOA as it builds the ansatz using QAOAAnsatz. We need EvolvedOperatorAnsatz in AdaptVQE and QAOAAnsatz has its parent class as EvolvedOperatorAnsatz . But the ansatz is decomposed in QAOA due to issue #6647. Once decompose gets removed hopefully it can be compatible with AdaptVQE.
Putting this on hold until the primitive-based VQE is added to Terra next week, as discussed with @mrossinek!
