Add test for 6350 #6369
Merged
Add test for 6350 #6369
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mtreinish
reviewed
May 6, 2021
|
|
||
| def test_skip_target_basis_equivalences_1(self): | ||
| """Test that BasisTranslator skips gates in the target_basis - #6085""" | ||
| from qiskit.test.mock import FakeAthens |
There was a problem hiding this comment.
Is there a reason to do this as a runtime import vs just putting it at the top and importing it with the module?
There was a problem hiding this comment.
Not really. ;-) Done in 0cb1626.
mtreinish
approved these changes
May 6, 2021
| circ_transpiled = transpile( | ||
| circ, | ||
| backend=FakeAthens(), | ||
| basis_gates=["id", "rz", "sx", "x", "cx"], |
There was a problem hiding this comment.
This line is a bit redundant because FakeAthens already has this basis set, but it doesn't hurt anything.
|
Thanks. I think I got the formatting done. Just getting used to using black. |
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Jun 8, 2021
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.
mergify bot
added a commit
that referenced
this pull request
Jun 11, 2021
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. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Details and comments
This PR adds a test to the changes in #6350 which was merged as part of 0.17.2.