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 bug in qpy with PauliFeatureMap class #13727

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

Fix bug in qpy with PauliFeatureMap class #13727

wants to merge 5 commits into from
+28 −5

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jan 23, 2025 •
edited
Loading

Summary

Fixes #13720.

Details and comments

@coveralls
Copy link

coveralls commented Jan 23, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 13014887166

Details

  • 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
  • 12 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.006%) to 88.926%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/qpy/binary_io/value.py 4 5 80.0%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/two_qubit_decompose.rs 1 92.08%
crates/accelerate/src/unitary_synthesis.rs 1 92.97%
crates/qasm2/src/lex.rs 4 91.73%
crates/qasm2/src/parse.rs 6 96.68%
Totals Coverage Status
Change from base Build 13010967771: -0.006%
Covered Lines: 79673
Relevant Lines: 89595
💛 - Coveralls

@ElePT
Don't assume parameter vector names are unique

Verified

This commit was signed with the committer’s verified signature.
erikmd Erik Martin-Dorel
GPG key ID: 9ED87DA01733593F
Verified
Learn about vigilant mode
457068f
@ElePT ElePT force-pushed the fix-qpy-blueprint branch from 0de998b to 457068f Compare January 24, 2025 10:44
@ElePT ElePT mentioned this pull request Jan 27, 2025
Merged
@ElePT ElePT changed the title Fix bug in QPY with PauliFeatureMap class Fix bug in qpy with PauliFeatureMap class Jan 27, 2025
@ElePT ElePT added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Jan 28, 2025
@ElePT ElePT force-pushed the fix-qpy-blueprint branch from 934fa78 to 36835f7 Compare January 28, 2025 13:45
@ElePT ElePT added this to the 2.0.0 milestone Jan 28, 2025
@ElePT ElePT marked this pull request as ready for review January 28, 2025 13:45
@ElePT ElePT requested a review from a team as a code owner January 28, 2025 13:45
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core
  • @mtreinish
  • @nkanazawa1989

@ElePT ElePT added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Jan 28, 2025
@Cryoris
Copy link
Contributor

Cryoris commented Jan 28, 2025

It seems like this should also be reproducible outside of the NLocal, or do you think this only happens in that context? 🤔

@ElePT
Copy link
Contributor Author

ElePT commented Jan 28, 2025

I know what you mean, the parameter replay sequence should be reproducible, but I have not been able to reproduce it outside of NLocal. I did stop trying after a while so it's something I guess could be retried in the future.

@Cryoris
Copy link
Contributor

Cryoris commented Feb 14, 2025

I don't fully understand how this bug is triggered and why we weren't able to reproduce it outside of the PauliFeatureMap, which makes me a bit worried that we don't 100% understand what's going on and that we might fix a symptom but not the full bug. E.g. could this also happen for Parameters instead of ParameterVectors, if we used these in PauliFeatureMap? I think I'd slightly prefer holding off a merge until we really understood this since it's touching a quite fundamental bit, but I'm happy to be overvoted if people think otherwise 🙂

@mtreinish mtreinish modified the milestones: 2.0.0, 1.4.1 Mar 3, 2025
@1ucian0 1ucian0 modified the milestones: 1.4.1, 1.4.2 Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cryoris Cryoris

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

Assignees

@Cryoris Cryoris

@ElePT ElePT

Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Milestone
1.4.2
Development

Successfully merging this pull request may close these issues.

Parameterized circuits can no longer be loaded with qpy.
6 participants
@ElePT @coveralls @qiskit-bot @Cryoris @1ucian0 @mtreinish