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

QGT is hermitian, not symmetric #9374

Closed
wants to merge 1 commit into from
Closed

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Jan 13, 2023

Summary

Fix a minor bug where the complex part of the LinCombQFI was assumed to be symmetric, when it is actually hermitian.

Details and comments

For a quantum state \phi, the complex parts of the QFI is 4 times the following equation
image

which is hermitian, i.e. G_{ij} = conj(G_{ji}). The LinCombQFI however implemented G_{ij} = G_{ji} which led to wrong results for derivative_types being any other than DerivativeType.REAL.

This does not include a reno since the QFIs are not yet released.

@Cryoris Cryoris added the bug Something isn't working label Jan 13, 2023
@Cryoris Cryoris added this to the 0.23.0 milestone Jan 13, 2023
@qiskit-bot
Copy link
Collaborator

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:

@Cryoris
Copy link
Contributor Author

Cryoris commented Jan 13, 2023

Tagging @a-matsuo and @Zoufalc 🙂

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless @Zoufalc or @a-matsuo have anything to object, LGTM.

@woodsp-ibm woodsp-ibm added Changelog: None Do not include in changelog mod: algorithms Related to the Algorithms module labels Jan 13, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3912843816

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0004%) to 84.846%

Totals Coverage Status
Change from base Build 3908401991: 0.0004%
Covered Lines: 65699
Relevant Lines: 77433

💛 - Coveralls

@Cryoris
Copy link
Contributor Author

Cryoris commented Jan 16, 2023

This is separately fixed in #9085.

@Cryoris Cryoris closed this Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Changelog: None Do not include in changelog mod: algorithms Related to the Algorithms module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants