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

Update basis equivalence test to be more specific #6532

Merged
merged 3 commits into from
Jun 11, 2021

Conversation

mtreinish
Copy link
Member

Summary

This commit updates the basis translator test:
test_skip_target_basis_equivalences_1 to be more speific to the case it
was designed to test. It was originally added in #6369 as a dedicated
reproduce test to validate we fixed issue #6350 where the basis
translator was trying to translate a gate already in the basis set which
resulted in a less than ideal (but still valid) output circuit. However,
this test was written to just mirror the original issue which was part
of a larger workflow and is too broad for a dedicated test case. This is
primarily because it was running a full transpile against a backend with
the sabre swap routing pass instead of only running the basis
translator (which is what we're testing). To fix this issue with the
test this commit extracts the qasm from the transpilation into the basis
translator when running the full reproduce from the original issue and
uses that as the input circuit, then the test is updated to only run
basis translation and optimization instead of the full transpilation
against a backend. This makes the test more targetted to what it's
specifically for and stable against unrelated changes to the sabre swap
pass.

Details and comments

This was extracted from #6302 because the same issue I hit with this test in that PR @boschmitt also encountered in #6498

This commit updates the basis translator test:
test_skip_target_basis_equivalences_1 to be more speific to the case it
was designed to test. It was originally added in Qiskit#6369 as a dedicated
reproduce test to validate we fixed issue Qiskit#6350 where the basis
translator was trying to translate a gate already in the basis set which
resulted in a less than ideal (but still valid) output circuit. However,
this test was written to just mirror the original issue which was part
of a larger workflow and is too broad for a dedicated test case. This is
primarily because it was running a full transpile against a backend with
the sabre swap routing pass instead of only running the basis
translator (which is what we're testing). To fix this issue with the
test this commit extracts the qasm from the transpilation into the basis
translator when running the full reproduce from the original issue and
uses that as the input circuit, then the test is updated to only run
basis translation and optimization instead of the full transpilation
against a backend. This makes the test more targetted to what it's
specifically for and stable against unrelated changes to the sabre swap
pass.
@mtreinish mtreinish added the Changelog: None Do not include in changelog label Jun 8, 2021
@mtreinish mtreinish requested a review from a team as a code owner June 8, 2021 13:17
@mergify mergify bot merged commit 4b27c63 into Qiskit:main Jun 11, 2021
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants