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 flakiness in pulse-optimal UnitarySynthesis test #11307

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

jakelishman
Copy link
Member

Summary

The previous iteration of this test asserted that the sx count for non-optimal synthesis was higher than a certain particular value. This value did not have any fundamental properties, it was just the value that happened to be returned for some time. Recent OpenBLAS support for x86_64 instructions from the AVX512 SkylakeX set meant that supporting processors can now return slightly fewer sx gates in the non-optimal path, despite the pulse-optimal synthesis still not being in use. This caused flaky CI, when we were allocated an Azure VM that had access to the new instructions.

Details and comments

Fix #11287.

Unblocks #11262 and lets us reinstate #11020.

The previous iteration of this test asserted that the `sx` count for
non-optimal synthesis was higher than a certain particular value.  This
value did not have any fundamental properties, it was just the value
that happened to be returned for some time.  Recent OpenBLAS support
for x86_64 instructions from the AVX512 SkylakeX set meant that
supporting processors can now return slightly fewer `sx` gates in the
non-optimal path, despite the pulse-optimal synthesis still not being in
use.  This caused flaky CI, when we were allocated an Azure VM that had
access to the new instructions.
@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 Nov 23, 2023
@jakelishman jakelishman requested a review from a team as a code owner November 23, 2023 12:37
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, this is a more sensible approach to assert that our non-optimal synthesis pass is less optimal than the optimal path. The magic 16 sx gates does seem fragile in retrospect looking at this change now.

@mtreinish mtreinish enabled auto-merge November 23, 2023 12:53
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6969779898

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.004%) to 86.444%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 91.67%
crates/qasm2/src/parse.rs 6 97.13%
Totals Coverage Status
Change from base Build 6966472058: 0.004%
Covered Lines: 66288
Relevant Lines: 76683

💛 - Coveralls

@mtreinish mtreinish added this pull request to the merge queue Nov 23, 2023
Merged via the queue into Qiskit:main with commit c7ecb5f Nov 23, 2023
14 checks passed
@jakelishman jakelishman deleted the fix-pulse-optimize-test branch November 23, 2023 15:19
mergify bot pushed a commit that referenced this pull request Nov 23, 2023
The previous iteration of this test asserted that the `sx` count for
non-optimal synthesis was higher than a certain particular value.  This
value did not have any fundamental properties, it was just the value
that happened to be returned for some time.  Recent OpenBLAS support
for x86_64 instructions from the AVX512 SkylakeX set meant that
supporting processors can now return slightly fewer `sx` gates in the
non-optimal path, despite the pulse-optimal synthesis still not being in
use.  This caused flaky CI, when we were allocated an Azure VM that had
access to the new instructions.

(cherry picked from commit c7ecb5f)
github-merge-queue bot pushed a commit that referenced this pull request Nov 23, 2023
The previous iteration of this test asserted that the `sx` count for
non-optimal synthesis was higher than a certain particular value.  This
value did not have any fundamental properties, it was just the value
that happened to be returned for some time.  Recent OpenBLAS support
for x86_64 instructions from the AVX512 SkylakeX set meant that
supporting processors can now return slightly fewer `sx` gates in the
non-optimal path, despite the pulse-optimal synthesis still not being in
use.  This caused flaky CI, when we were allocated an Azure VM that had
access to the new instructions.

(cherry picked from commit c7ecb5f)

Co-authored-by: Jake Lishman <[email protected]>
FabianBrings pushed a commit to FabianBrings/qiskit that referenced this pull request Nov 27, 2023
The previous iteration of this test asserted that the `sx` count for
non-optimal synthesis was higher than a certain particular value.  This
value did not have any fundamental properties, it was just the value
that happened to be returned for some time.  Recent OpenBLAS support
for x86_64 instructions from the AVX512 SkylakeX set meant that
supporting processors can now return slightly fewer `sx` gates in the
non-optimal path, despite the pulse-optimal synthesis still not being in
use.  This caused flaky CI, when we were allocated an Azure VM that had
access to the new instructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

Flaky pulse-optimal UnitarySynthesis test with Numpy 1.26
4 participants