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

Improve debugability of TwoQubitWeylDecomposition assertRoundTrip failures #11797

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Improve debugability of TwoQubitWeylDecomposition assertRoundTrip failures #11797

merged 1 commit into from
Feb 14, 2024

Conversation

levbishop
Copy link
Member

Move requested_fidelity assertion earlier in sequence, before making assertions that are downstream of that.
Might make debugging issues like #11676 easier.

@levbishop
TwoQubitWeylDecomposition assertRoundTrip
c5ab51f 
Move requested_fidelity assertion earlier in  sequence, before making assertions that are downstream of that.
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

jakelishman
jakelishman approved these changes Feb 14, 2024
View reviewed changes
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

This seems fine to do, but I'm not sure it's going to help the debugging - the test failures in the linked PR show equal requested_fidelity in the two reprs.

@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: None Do not include in changelog labels Feb 14, 2024
@levbishop
Copy link
Member Author

Yeah, it's rather grasping at straws but I have fixed issues where numpy floats are not displayed with correct rounding
numpy/numpy#18609
At least its one fewer things to worry about when tracking things down

@jakelishman
Copy link
Member

Sure yeah, it makes sense.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7904465022

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.003%) to 89.217%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 6 91.69%
Totals Coverage Status
Change from base Build 7904240889: 0.003%
Covered Lines: 58837
Relevant Lines: 65948
💛 - Coveralls

@Chewypewy

This comment was marked as spam.

Sign in to view
@jakelishman jakelishman added this pull request to the merge queue Feb 14, 2024
Merged via the queue into Qiskit:main with commit 12847e8 Feb 14, 2024
12 checks passed
mergify bot pushed a commit that referenced this pull request Feb 14, 2024
@levbishop @mergify
TwoQubitWeylDecomposition assertRoundTrip (#11797)
9ba1842 
Move requested_fidelity assertion earlier in  sequence, before making assertions that are downstream of that.

(cherry picked from commit 12847e8)
@mergify mergify bot mentioned this pull request Feb 14, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Feb 14, 2024
@mergify @levbishop
TwoQubitWeylDecomposition assertRoundTrip (#11797) (#11799)
e72493d 
Move requested_fidelity assertion earlier in  sequence, before making assertions that are downstream of that.

(cherry picked from commit 12847e8)

Co-authored-by: Lev Bishop <18673315+levbishop@users.noreply.github.com>
@levbishop levbishop mentioned this pull request Feb 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakelishman jakelishman jakelishman approved these changes

@alexanderivrii alexanderivrii Awaiting requested review from alexanderivrii alexanderivrii is a code owner

@ShellyGarion ShellyGarion Awaiting requested review from ShellyGarion ShellyGarion is a code owner

Assignees
No one assigned
Labels
Changelog: None Do not include in changelog stable backport potential The bug might be minimal and/or import enough to be port to stable type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@levbishop @qiskit-bot @jakelishman @coveralls @Chewypewy