qiskit
qiskit copied to clipboard
Implement eigensolvers (VQD) with primitives
Summary
This PR addresses #8491 and #8492 and provides an implementation of VQD using the new primitive interfaces. The new implementation comes with a change of namespace for the whole eigensolvers module (from eigen_solvers to eigensolvers), which is more consistent with the naming conventions used throughout the stack.
Details and comments
Blocked by:
- [x] #8528
- [x] #8303
- [x] #8683
- [ ] #8702
Closely related to (but not blocked by):
- [ ] #8755
To ensure consistency with the changes made in VQE (#8702), the following elements have been removed:
- The final
eigenstatethat used to be part of theEigensolverResult. To reproduce this one may use a Sampler and the optimal point from VQD outside the algorithm. - The setting
propertyandprint_settingsmethod. - All trivial getters and setters, just use public attributes instead (expect for
initial_point, which requires it for theVariationalAlgorithminterface). expectationandinclude_custom. These are not required anymore when using anEstimator.
Additionally, the following changes have been applied:
- Removed the option to send a custom
Gradientthrough the VQD intialization. This option will be on hold untilFidelityGradientsusing primitives are enabled. - Created a new
NumPyEigensolverResultclass that allows theNumPyEigensolverto still return theeigenstate. - Enforced "non-essential" arguments to be keyword-only.
This PR also fixes a bug found in qiskit/algorithms/observables_evaluator.py where the variance/shots are not correctly extracted from the estimator metadata, and adds a corresponding unit test to verify the fix.
TO-DO:
- [x] Add attributes to doctstring
- [x] Add support and unit tests for BaseOperator inputs in numpy eigensolver
- [x] Create property defaults in compute instead of init
- [ ] Import _validate_bounds, _validate_initial_point from utils (blocked by #8702)
- [ ] Modify VQE import (blocked by #8702)
TO-AGREE-ON:
- [x] Keyword vs. positional args
- [x] Require optimizer??
- [ ] Including metadata in
EigensolverResult, adding variance/shots from aux. ops, changing aux ops return type (only eigenvalue).
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
- @manoelmarques
- @woodsp-ibm
Pull Request Test Coverage Report for Build 3150026480
- 372 of 398 (93.47%) changed or added relevant lines in 8 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.05%) to 84.551%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| qiskit/algorithms/eigensolvers/eigensolver.py | 32 | 33 | 96.97% |
| qiskit/algorithms/eigensolvers/vqd.py | 184 | 194 | 94.85% |
| qiskit/algorithms/eigensolvers/numpy_eigensolver.py | 145 | 160 | 90.63% |
| <!-- | Total: | 372 | 398 |
| Totals | |
|---|---|
| Change from base Build 3150022884: | 0.05% |
| Covered Lines: | 60756 |
| Relevant Lines: | 71857 |
💛 - Coveralls
In #8702, since the algorithms are now in a subfolder and need to be imported from there to avoid name collisions, the unit tests for the new algorithms were added in a like-named subfolder. That maybe a better bet here too - leave the old NumPy/VQD tests be where they were testing the pending deprecated existing versions and add tests for new algos in a new folder