Bug fix in ElidePermutations #13186
Bug fix in ElidePermutations #13186
Conversation
alexanderivrii
added
the
Changelog: Bugfix
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 10941526894Details
💛 - Coveralls |
| expected.cx(4, 3) | ||
|
|
||
| res = self.swap_pass(qc) | ||
| self.assertEqual(res, expected) |
There was a problem hiding this comment.
Should we add some assertions on the permutation tracking too to make sure it's correct and reverses to the original circuit?
There was a problem hiding this comment.
This is a good idea. I have extended the test in 6791002. Do you think it's a good idea to add more "random circuits" tests, with swaps and permutations?
There was a problem hiding this comment.
Adding more tests wouldn't hurt. I think the one you added here provides sufficient coverage to prevent a regression here. But if you wanted to add more that would give us more coverage against potential future issues.
| perm = pass_.property_set["virtual_permutation_layout"].to_permutation(qc.qubits) | ||
| res.append(PermutationGate(perm), [0, 1, 2, 3, 4]) | ||
| self.assertEqual(Operator(res), Operator(qc)) |
There was a problem hiding this comment.
TBH, I'm surprised you didn't just use Operator.from_circuit here, but adding a permutation gate like this works fine too.
mtreinish
added
the
stable backport potential
* elide permutations fix * extending the test to check the final permutation (cherry picked from commit adf55e1)
Summary
A bug fix for the
ElidePermutationspass where the internal qubit mapping was not updated correctly after aPermutationGate. This fix was already present in #13094, however we have decided to separate it to an independent PR as to allow backport.