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

Simplify UnitarySynthesis pass test test_qv_natural #9359

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

mtreinish
Copy link
Member

Summary

The test_qv_natural UnitarySynthesis test is designed to test that when the pulse efficient flag is set on the default unitary synthesis plugin for the UnitarySynthesis pass the output from synthesis is only emitting entangling gates in the natural direction implemented on the target. However, when running on Windows with Python 3.11 and numpy 1.24 we're seeing a non-zero failure rate (in CI it's ~5% across all jobs) of this test likely do to fp precision. When this failure occurs it appears as the failure from detailed in issues #5832, #9177, and others as the test was creating a pass manager that mirrors the optimization loop used in the preset pass managers. The optimization loop is failing to converge on a fixed depth as the unitary synthesis is oscillating between multiple equivalent outputs. However, in this case the test doesn't need the full complexity of the optimization loop as we're just trying to verify the output of unitary synthesis is equivalent with different options and that the output is valid given different options. This can be done with a single iteration of the loop and doesn't require trying to fully optimize the circuit. This commit reduces the complexity of the test to remove the loop from the optimization stage in the custom pass manager so the test is now just running a single iteration of UnitarySynthesis and comparing the output. This will hopefully make the test stable in CI moving forward avoiding this failure mode.

Details and comments

The test_qv_natural UnitarySynthesis test is designed to test that when
the pulse efficient flag is set on the default unitary synthesis plugin
for the UnitarySynthesis pass the output from synthesis is only emitting
entangling gates in the natural direction implemented on the target.
However, when running on Windows with Python 3.11 and numpy 1.24 we're
seeing a non-zero failure rate (in CI it's ~5% across all jobs) of this
test likely do to fp precision. When this failure occurs it appears as
the failure from detailed in issues Qiskit#5832 and Qiskit#9177 as the test was
creating a pass manager that mirrors the optimization loop used in the
preset pass managers. The optimization loop is failing to converge on a
fixed depth as the unitary synthesis is oscillating between multiple
equivalent outputs. However, in this case the test doesn't need the
full complexity of the optimization loop as we're just trying to verify
the output of unitary synthesis is equivalent with different options and
that the output is valid given different options. This can be done with
a single iteration of the loop and doesn't require trying to fully
optimize the circuit. This commit reduces the complexity of the test to
remove the loop from the optimization stage in the custom pass manager
so the test is now just running a single iteration of UnitarySynthesis
and comparing the output. This will hopefully make the test stable in CI
moving forward avoiding this failure mode.
@mtreinish mtreinish requested a review from a team as a code owner January 10, 2023 17:14
@mtreinish mtreinish added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in changelog labels Jan 10, 2023
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.

I was able to reproduce the failures from this test for various seeds of QuantumVolume on both Windows and macOS with Python 3.10 and 3.11, and Numpy 1.23 and 1.24. The failures we have in CI are just because the particular combination of Windows, Python 3.11 and Numpy 1.24 happen to trigger the bug for seed=15, but the same failure mode is triggered on other combinations with different seeds; the failure is somewhere in the pulse_optimize=True path.

All that said, this test doesn't need a complete loop, and it's blocking CI at the moment, so let's merge.

@jakelishman
Copy link
Member

I pushed an empty commit up because the GitHub webhooks had got stuck and failed to trigger the Azure, GHA or CLA runs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3889078584

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.008%) to 84.632%

Files with Coverage Reduction New Missed Lines %
src/vf2_layout.rs 3 94.74%
Totals Coverage Status
Change from base Build 3885047843: 0.008%
Covered Lines: 64329
Relevant Lines: 76010

💛 - Coveralls

@mergify mergify bot merged commit 6fa7ae2 into Qiskit:main Jan 11, 2023
@mtreinish mtreinish deleted the simplify-qv_natural-test branch January 11, 2023 15:15
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 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.

3 participants