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

Fix the calculation for identity operator's expectation value #13345

Merged
merged 6 commits into from
Nov 14, 2024

Conversation

yjh-bill
Copy link
Contributor

Summary

Fix a bug in qiskit.quantum_info in the calculation of expectation value of the identity operator.

Fixes #13029

Details and comments

Currently, qiskit.quantum_info yields incorrect results for the expectation value of an identity operator for a given state vector. It returns the norm of the state vector; however, it ought to return the square of the norm.

Therefore, I am changing the following line

if x_mask + z_mask == 0:
return pauli_phase * np.linalg.norm(self.data)

to

   if x_mask + z_mask == 0: 
       return pauli_phase * np.linalg.norm(self.data) ** 2

@yjh-bill yjh-bill requested a review from a team as a code owner October 19, 2024 00:02
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Oct 19, 2024
@CLAassistant
Copy link

CLAassistant commented Oct 19, 2024

CLA assistant check
All committers have signed the CLA.

@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 following people are relevant to this code:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Oct 19, 2024

Pull Request Test Coverage Report for Build 11826979058

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 2294 unchanged lines in 122 files lost coverage.
  • Overall coverage increased (+0.4%) to 88.933%

Files with Coverage Reduction New Missed Lines %
qiskit/pulse/instructions/directives.py 1 97.14%
qiskit/circuit/library/boolean_logic/inner_product.py 1 96.0%
qiskit/transpiler/passes/scheduling/alignments/pulse_gate_validation.py 1 96.3%
qiskit/pulse/instructions/play.py 1 97.5%
crates/qasm2/src/expr.rs 1 94.02%
qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py 1 94.54%
qiskit/circuit/store.py 1 97.06%
qiskit/circuit/library/iqp.py 1 96.15%
qiskit/transpiler/passes/calibration/pulse_gate.py 1 95.45%
qiskit/circuit/library/boolean_logic/quantum_or.py 1 98.08%
Totals Coverage Status
Change from base Build 11411236768: 0.4%
Covered Lines: 79336
Relevant Lines: 89209

💛 - Coveralls

@Cryoris
Copy link
Contributor

Cryoris commented Oct 21, 2024

Thanks for opening a PR to fix this! Could you add a release note and a test, to verify it now works as expected? (See e.g. this PR for an example on how a bugfix PR should look like 🙂)

@yjh-bill
Copy link
Contributor Author

Sure, I just added a release note and a test.

@@ -1152,6 +1152,37 @@ def test_expval_pauli_qargs(self, qubits):
expval = state.expectation_value(op, qubits)
self.assertAlmostEqual(expval, target)


def test_expval_identity(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you need to run the formatter here, you can do that e.g. by calling make black (you might want to make sure the right version is installed before doing so, by running pip install -r requirements-dev.txt -c constraints.txt before)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks for the instruction.

Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

Thanks for this fix!

@Cryoris Cryoris added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Nov 13, 2024
@Cryoris Cryoris enabled auto-merge November 13, 2024 22:33
@Cryoris Cryoris added this pull request to the merge queue Nov 13, 2024
Merged via the queue into Qiskit:main with commit edad499 Nov 14, 2024
17 checks passed
mergify bot pushed a commit that referenced this pull request Nov 14, 2024
* Fix the calculation for identity operator's expectation value

* Add release note and test

* reformatted `test_statevector.py` with black

* Update releasenotes/notes/fix_identity_operator_9e2ec9770ac046a6.yaml

Co-authored-by: Julien Gacon <[email protected]>

* add a docstring to pass the pylint test

* Fix leading `.`

---------

Co-authored-by: Julien Gacon <[email protected]>
(cherry picked from commit edad499)
github-merge-queue bot pushed a commit that referenced this pull request Nov 14, 2024
#13436)

* Fix the calculation for identity operator's expectation value

* Add release note and test

* reformatted `test_statevector.py` with black

* Update releasenotes/notes/fix_identity_operator_9e2ec9770ac046a6.yaml

Co-authored-by: Julien Gacon <[email protected]>

* add a docstring to pass the pylint test

* Fix leading `.`

---------

Co-authored-by: Julien Gacon <[email protected]>
(cherry picked from commit edad499)

Co-authored-by: Jinghong Yang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Statevector._expectation_value_pauli returns incorrect result in certain cases
5 participants