qiskit
qiskit copied to clipboard
VQE implementation with estimator primitive
Summary
Adds a standard VQE
implementation using an Estimator
primitive. Closes #8471.
A SamplingVQE
implementation that takes a Sampler
primitive is introduced in PR #8669.
Details and comments
This fresh implementation lives in qiskit/algorithms/minimum_eigensolvers
. The prior implementation may still be found in qiskit/algorithms/minimum_eigen_solvers
.
The following elements have been removed:
- The final "eigenstate". To reproduce this one may use a
Sampler
and the optimal point from VQE outside the algorithm. - ~~The callback from the energy evaluation. Instead the callback should be attached to the optimizer which can allow to additionally evaluate the energy at the current point.~~ After discussing with @woodsp-ibm, I decided to add back the callback on VQE as this may be simpler for users. Callbacks are not universally supplied by all optimizers.
- The
setting
property andprint_settings
method. No other algorithm has these. - All trivial getters and setters, just use public attributes instead (expect for
initial_point
, which requires it for theVariationalAlgorithm
interface. -
expectation
andinclude_custom
. These are not required anymore when using anEstimator
.
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 3135497477
- 257 of 266 (96.62%) changed or added relevant lines in 11 files are covered.
- 9 unchanged lines in 2 files lost coverage.
- Overall coverage increased (+0.08%) to 84.451%
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
---|---|---|---|
qiskit/algorithms/minimum_eigensolvers/minimum_eigensolver.py | 31 | 32 | 96.88% |
qiskit/algorithms/minimum_eigensolvers/numpy_minimum_eigensolver.py | 42 | 45 | 93.33% |
qiskit/algorithms/minimum_eigensolvers/vqe.py | 110 | 115 | 95.65% |
<!-- | Total: | 257 | 266 |
Files with Coverage Reduction | New Missed Lines | % |
---|---|---|
qiskit/extensions/quantum_initializer/squ.py | 2 | 79.78% |
qiskit/algorithms/optimizers/qnspsa.py | 7 | 87.3% |
<!-- | Total: | 9 |
Totals | |
---|---|
Change from base Build 3132502212: | 0.08% |
Covered Lines: | 59852 |
Relevant Lines: | 70872 |
💛 - Coveralls
I note that estimate_observables
now returns the variances and shots if obtained from metadata (and sets these to 0.0 otherwise). Should I add this for the main eigenvalue too? It seems odd to have this data for the aux operators and not the main one. I guess there are three ways of doing this:
- Make
eigenvalue: tuple[complex, tuple[complex, int]]
, which will require a lot of downstream changes. - Add a
metadata
property to the result class with the metadata pulled straight from the job. - Add
shots
andvariance
properties to the result class. Any thoughts on this @woodsp-ibm, @mrossinek, @Cryoris?
I note that estimate_observables now returns the variances and shots if obtained from metadata (and sets these to 0.0 otherwise). Should I add this for the main eigenvalue too?
It seems sensible/consistent to have access to other information for the main operator too. I had suggested in the estimate_observables passing back the whole metadata https://github.com/Qiskit/qiskit-terra/pull/8683#discussion_r967314224 while perhaps keeping the extracted variance as a first class field.
#8105 may change things up.
I note that
estimate_observables
now returns the variances and shots if obtained from metadata (and sets these to 0.0 otherwise). Should I add this for the main eigenvalue too? It seems odd to have this data for the aux operators and not the main one. I guess there are three ways of doing this:
- Make
eigenvalue: tuple[complex, tuple[complex, int]]
, which will require a lot of downstream changes.- Add a
metadata
property to the result class with the metadata pulled straight from the job.- Add
shots
andvariance
properties to the result class. Any thoughts on this @woodsp-ibm, @mrossinek, @Cryoris?
I think that adding the metadata
property (option 2) sounds like the most consistent option. I am not sure about the usefulness of also including the variance
as a first class field, I think it would complicate things. Or @woodsp-ibm, were you referring to the aux_ops_eigenvalues
that already return the variance?? I think it would make sense for them to mirror the main eigenvalues, and also return the auxiliary variance/shots as a metadata entry.
About https://github.com/Qiskit/qiskit-terra/pull/8105, it looks like the discussion hasn't progressee in a while, so it might change things in the future, but I don't think we should worry about it for this release.
were you referring to the aux_ops_eigenvalues that already return the variance??
Yes, there the variance is already taken out of the metadata - the resultant tuple of (variance, shots) is always present with either defaulting to 0 if its not in the metadata. All I meant by first class field was extracting it out of the metadata like the estimate observables does. It would be nice to see some of what is in the metdata more directly as fields estimator result etc you can know what is in the result, IDE auto-completion and so on. Having it "hidden" in metadata seems less ideal which is an aspects #8105 was seeking to address too I think.
In thinking about the comments in a different PR #8695 I am wondering whether it would be a good idea to add OptimizerResult to the VQEResult. Generally as we have gone up the stack we added the the result of the algorithm being used to the result at that level. So a nature ground state result or an optimization you can look in and find the underlying result this was based on and any other info on that result. I am thinking we could do the same here - it would give access to optimizer results, from whatever optimizer was run, and you can access it or not as you choose. Maybe the place to add this would be in VariationalResult since that contains other optimizer values. @ElePT since this would be for VQD as well.
It looks good overall though I wrote some minor comments.