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.

Sign up for GitHub

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

Jump to bottom

API Regression: VQE aux_op_eigenvalues return type #7142

Closed
mrossinek opened this issue Oct 15, 2021 · 0 comments · Fixed by #7144
Closed

API Regression: VQE aux_op_eigenvalues return type #7142

mrossinek opened this issue Oct 15, 2021 · 0 comments · Fixed by #7144
Assignees
@mrossinek
Labels
bug Something isn't working mod: algorithms Related to the Algorithms module
Milestone
0.19

Comments

@mrossinek
Copy link
Member

Information

  • Qiskit Terra version: main
  • Python version: 3.9.7
  • Operating system: Linux

What is the current behavior?

#6870 changed the returned type of aux_op_eigenvalues in the VQE such that it mismatches that of the NumPyEigensolver.

Steps to reproduce the problem

See any of the VQE-based unittests in the Qiskit Nature CI: https://github.com/Qiskit/qiskit-nature/actions/runs/1344253728

What is the expected behavior?

The API needs to be fixed.

Suggested solutions

I outlined some observations in an issue comment over at Nature: qiskit-community/qiskit-nature#396 (comment).

I suggest that we:

  • fix the VQE to again wrap each aux_op_eigenvalue
    • this time, into a tuple rather than a list
    • setting the second entry of the tuple to 0
    • this is more aligned with what the NumPyEigensolver does
  • adding documentation of the expected return type of aux_op_eigenvalues (into the _eval_aux_ops method and the Result objects which expose these eigenvalues as properties)

I will create a PR later today 👍
@mrossinek mrossinek added the bug Something isn't working label Oct 15, 2021
@Cryoris Cryoris added this to the 0.19 milestone Oct 15, 2021
@Cryoris Cryoris added the mod: algorithms Related to the Algorithms module label Oct 15, 2021
mrossinek added a commit to mrossinek/qiskit that referenced this issue Oct 15, 2021
@mrossinek
Fix Qiskit#7142
0961daf 
Ensures a unified API of the `aux_operator_eigenvalues`.
Prior to this commit, the returned types of the `NumPyEigensolver` and
`VQE` were mismatched. Furthermore, the documented type hint in the
`EigensolverResult` was yet another value.
This commit unifies the API, documents the expected type properly and
adds unittests which assert that this bug remains fixed.
@mrossinek mrossinek mentioned this issue Oct 18, 2021
Merged
@mergify mergify bot closed this as completed in #7144 Oct 21, 2021
mergify bot added a commit that referenced this issue Oct 21, 2021
@mrossinek @mergify
Fix #7142 (#7144)
4278ba7 
* Fix #7142

Ensures a unified API of the `aux_operator_eigenvalues`.
Prior to this commit, the returned types of the `NumPyEigensolver` and
`VQE` were mismatched. Furthermore, the documented type hint in the
`EigensolverResult` was yet another value.
This commit unifies the API, documents the expected type properly and
adds unittests which assert that this bug remains fixed.

* Do not map expectation value to real

* Compute aux operator standard deviations during VQE

* Assert standard deviations in EigensolverResult objects

* Improve aux_operator_eigenvalues docstring

* Fix NumPyEigensolver threshold check

* Run black

* Update release note

* Avoid ambiguous truth value of numpy arrays

* Add aux_op test for qasm-based VQE (non-trivial std dev)

* Remove erroneous nesting

This was accidentally copied from the `EigensolverResult` case.

* Remove release note

This fixes a bug in an unreleased feature so this release note is
actually not needed (in fact its more confusing than helpful).

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit that referenced this issue Jun 27, 2023
@mrossinek @mergify
Fix Qiskit#7142 (Qiskit#7144)
f41747f 
* Fix Qiskit#7142

Ensures a unified API of the `aux_operator_eigenvalues`.
Prior to this commit, the returned types of the `NumPyEigensolver` and
`VQE` were mismatched. Furthermore, the documented type hint in the
`EigensolverResult` was yet another value.
This commit unifies the API, documents the expected type properly and
adds unittests which assert that this bug remains fixed.

* Do not map expectation value to real

* Compute aux operator standard deviations during VQE

* Assert standard deviations in EigensolverResult objects

* Improve aux_operator_eigenvalues docstring

* Fix NumPyEigensolver threshold check

* Run black

* Update release note

* Avoid ambiguous truth value of numpy arrays

* Add aux_op test for qasm-based VQE (non-trivial std dev)

* Remove erroneous nesting

This was accidentally copied from the `EigensolverResult` case.

* Remove release note

This fixes a bug in an unreleased feature so this release note is
actually not needed (in fact its more confusing than helpful).

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this issue Jul 17, 2023
@mrossinek @mergify
Fix Qiskit/qiskit#7142 (Qiskit/qiskit#7144)
fa88ef5 
* Fix Qiskit/qiskit#7142

Ensures a unified API of the `aux_operator_eigenvalues`.
Prior to this commit, the returned types of the `NumPyEigensolver` and
`VQE` were mismatched. Furthermore, the documented type hint in the
`EigensolverResult` was yet another value.
This commit unifies the API, documents the expected type properly and
adds unittests which assert that this bug remains fixed.

* Do not map expectation value to real

* Compute aux operator standard deviations during VQE

* Assert standard deviations in EigensolverResult objects

* Improve aux_operator_eigenvalues docstring

* Fix NumPyEigensolver threshold check

* Run black

* Update release note

* Avoid ambiguous truth value of numpy arrays

* Add aux_op test for qasm-based VQE (non-trivial std dev)

* Remove erroneous nesting

This was accidentally copied from the `EigensolverResult` case.

* Remove release note

This fixes a bug in an unreleased feature so this release note is
actually not needed (in fact its more confusing than helpful).

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mrossinek mrossinek

Labels
bug Something isn't working mod: algorithms Related to the Algorithms module
Projects
None yet
Milestone
0.19
Development

Successfully merging a pull request may close this issue.

Fix #7142 mrossinek/qiskit
2 participants
@Cryoris @mrossinek