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

Fix SparsePauliOp initialization with dense Y labels #13580

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Fix SparsePauliOp initialization with dense Y labels #13580

wants to merge 14 commits into from
+22 −1

Conversation

trigpolynom
Copy link
Contributor

@trigpolynom trigpolynom commented Dec 18, 2024 •
edited
Loading

Summary

Fixes #13522

"When a SparsePauliOp object is instantiated with a long string of Y's (greater than 100), the imaginary component of the coefficients is non-zero (but trivially small, ~1e-15). This results in errors when submitting jobs using qiskit-ibm-runtime which throws an error when using the SparsePauliOp as the expectation value to estimate."

The fix to the above problem involved using modular arithmetic to calculate coeffs for the SparsePauliOp when coeffs is None. This avoids the floating point error that occurs for instantiating heavy-weight SparsePauliOps.

Details and comments

Added a test method test_sparse_pauli_op_init_long_ys to ensure that the error causing PauliList ('Y"*100) results with the same coeffs as PauliList ("Y"*99)

@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Dec 18, 2024
@trigpolynom trigpolynom mentioned this pull request Dec 18, 2024
Open
@coveralls
Copy link

coveralls commented Dec 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 12581628855

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 88.955%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.02%
crates/qasm2/src/lex.rs 4 92.48%
Totals Coverage Status
Change from base Build 12535289542: 0.02%
Covered Lines: 79433
Relevant Lines: 89296
💛 - Coveralls

@ShellyGarion ShellyGarion added mod: quantum info Related to the Quantum Info module (States & Operators) Changelog: Bugfix Include in the "Fixed" section of the changelog labels Dec 19, 2024
@ShellyGarion
Copy link
Member

Hi @trigpolynom - thanks for fixing this bug!
After you complete the tests and fix the lint error, could you please add release notes?

@trigpolynom
Copy link
Contributor Author

Hi @ShellyGarion and @Cryoris thanks for taking a look at this! I'll make the updates and requested changes tonight EST.

@trigpolynom trigpolynom marked this pull request as ready for review December 20, 2024 03:27
@trigpolynom trigpolynom requested a review from a team as a code owner December 20, 2024 03:27
@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

@Cryoris Cryoris changed the title Fix sparse pauli op amps Fix SparsePauliOp initialization with dense Y labels Dec 20, 2024
@ShellyGarion
Copy link
Member

there are some formatting issues. other than that, I think this PR is ready to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cryoris Cryoris Awaiting requested review from Cryoris

@ShellyGarion ShellyGarion Awaiting requested review from ShellyGarion

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: quantum info Related to the Quantum Info module (States & Operators)
Projects
Contributor Monitoring
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SparsePauliOp incorrectly sets non-zero imaginary component
5 participants
@trigpolynom @coveralls @ShellyGarion @qiskit-bot @Cryoris