-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fixing Operator.from_circuit for circuits with final layouts and a non-trivial initial layout #12057
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8454898984Details
💛 - Coveralls |
this should pass CI after merging Qiskit#12057
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this LGTM, thanks for catching this and fixing it. I just had an inline question and a suggestion on the release notes.
if layout is None: | ||
perm_pattern = [final_layout._v2p[v] for v in circuit.qubits] | ||
else: | ||
perm_pattern = [ | ||
layout.input_qubit_mapping[layout.initial_layout._p2v[final_layout._v2p[v]]] | ||
for v in circuit.qubits | ||
] | ||
perm_pattern = [perm_pattern[i] for i in layout.initial_layout.get_physical_bits()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can use final_index_layout()
(https://docs.quantum.ibm.com/api/qiskit/qiskit.transpiler.TranspileLayout#final_index_layout) here to simplify this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the *_*_layout()
functions in TranspileLayout
appear to yield the correct permutation pattern... :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be the inverse of final_index_layout
, because final_index_layout
returns the final permutation of each qubit but we need the inverse of that to reverse the permutation for the operator construction. Does something like this work?:
final_index_layout = layout.final_index_layout()
perm_pattern = [None]*len(final_index_layout)
for index, pos = enumerate(final_index_layout):
perm_pattern[pos] = index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not appear to be the case... :-/ One explicit example is
[5, 1, 0, 2, 4, 3]
- correct[5, 3, 0, 4, 1, 2]
- inversefinal_index_layout
[2, 4, 5, 1, 3, 0]
-final_index_layout
[5, 4, 1, 0, 3, 2]
-initial_index_layout
My impression was that this code might become obsolete with #11399 which is why I did not spend a terrible amount of time on it. :-)
Do you want me to formulate this fix as a function of final_index_layout
and initial_index_layout
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure the index layout functions are correct in their current form. Or at least at the time we added them we did a fair amount of testing around that. But I have zero confidence in any statements about handling permutations, so either my assertion that the inverse of final_index_layout
is what we want here is incorrect, or final_index_layout()
is not correctly written. I'm more inclined the believe the former, but I could see either being possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither initial_index_layout
nor final_index_layout
are direct permutations within the same Hilbert space, nor are their inverses. They're both layouts - it's correct and expected that neither alone is a suitable permutation within the same space here, because both are
When we're comparing the untranspiled qc
to the result of a transpilation out
, you can make a new circuit with the same virtual indices, then use initial_index_layout
as an inverse permutation at the start (to convert the indices to physical indices of the transpiled one), and final_index_layout
as a forwards permutation at the end (to convert back to the virtual indices of the original):
from qiskit import QuantumCircuit
from qiskit.circuit.library import PermutationGate
def undo(transpiled):
initial = transpiled.layout.initial_index_layout()
final = transpiled.layout.final_index_layout()
out = QuantumCircuit(transpiled.num_qubits)
out.append(PermutationGate(initial).inverse(), out.qubits)
# After that permutation, the physical-qubit indices of `transpiled`
# match the virtual-qubit indices of the current circuit.
out.compose(transpiled, out.qubits, inplace=True)
# Now map the virtual qubits back to their starting locations.
out.append(PermutationGate(final), out.qubits)
return out
Now:
import itertools
import math
import numpy.random
from qiskit import QuantumCircuit, transpile
from qiskit.transpiler import CouplingMap
from qiskit.quantum_info import Operator
def rand_circ(qubits, gates):
rng = numpy.random.default_rng()
links = list(itertools.permutations(range(qubits), 2))
qc = QuantumCircuit(qubits)
for _ in range(gates):
qc.rzx(rng.random()*2*math.pi, *rng.choice(links))
return qc
qc = rand_circ(8, 100)
transpiled = transpile(qc, coupling_map=CouplingMap.from_line(qc.num_qubits))
assert Operator(qc) == Operator(undo(transpiled))
(note that neither qc
nor undo(transpiled)
have a set layout
). I've just used the #10835 methods on TranspileLayout
(which already included all the information to make all the correct permutations) and our existing PermutationGate
conventions, rather than the #11399 methods on Layout
that need extra information and have a different set of conventions.
I haven't done the ancilla suppression here - there's a couple more tricks in doing that, and not doing it lets us treat the index layouts as a type of permutation - but I don't remember off the top of my head how we decided to handle that in Operator.from_circuit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear: I'm not necessarily suggesting building the new circuit the way I did in order to power Operator.from_circuit
- if nothing else, I don't think that'd work too well with ancilla suppression. I was just using my undo
as an example to show how the #10835 TranspileLayout
API integrates with the existing PermutationGate
and the existing Operator
matrix expansions.
My aim with the example above was to show that the only valid use-cases for the #11399 method Layout.to_permutation
are already covered by TranspileLayout
. Then, in #9523 (comment), I also put in a couple of simple functions just illustrating that the #11399 Layout.compose
and Layout.inverse
are much simpler if they're only defined in terms of permutations in the already existing sense of PermutationGate
.
releasenotes/notes/operator-from-circuit-bugfix-5dab5993526a2b0a.yaml
Outdated
Show resolved
Hide resolved
…0a.yaml Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Thanks for your suggestions, I still need to figure out the correct point of view when writing PRs and your feedback is very valuable for that! :-) |
After several discussions with @sbrandhsn we have figured out a simpler way to implement the One way to think about the This should also work when there are ancilla qubits, however |
) | ||
final_layout = Layout({circuit.qubits[0]: 1, circuit.qubits[1]: 2, circuit.qubits[2]: 0}) | ||
final_layout = Layout({circuit.qubits[0]: 2, circuit.qubits[1]: 0, circuit.qubits[2]: 1}) | ||
circuit.swap(0, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that the test was also wrong, this is why the behavior is changed after the bug got fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I agree the behavior was incorrect before and this fixes it.
* Add ElideSwaps transpiler pass This commit adds a new transpiler pass ElideSwaps which is a logical optimization pass designed to run prior to layout or any other physical embedding steps in the transpiler pipeline. It traverses the DAG in topological order and when a swap gate is encountered it removes that gate and instead permutes the logical qubits for any subsequent gates in the DAG. This will eliminate any swaps in a circuit not caused by routing. Additionally, this pass is added to the preset pass manager for optimization level 3, we can consider adding it to other levels too if we think it makes sense (there is little overhead, and almost 0 if there are no swaps). One thing to note is that this pass does not recurse into control flow blocks at all, it treats them as black box operations. So if any control flow blocks contain swap gates those will not be optimized away. This change was made because mapping the permutation outside and inside any control flow block was larger in scope than what the intent for this pass was. That type of work is better suited for the routing passes which already have to reason about this. * Update tests with optimization level 3 * Pass final layout from ElideSwap to output The new ElideSwap pass is causing an output permutation just as a routing pass would. This information needs to be passed through to the output in case there is no layout or routing run. In those cases the information about the output permutation caused by the swap elision will be lost and doing layout aware operations like Operator.from_circuit() will not be able to reason about the permutation. This commit fixes this by inserting the original layout and qubit mapping into the property set along with the final layout. Then the base pass class and pass manager class are updated to use the original layout as the initial layout if one isn't set. In cases where we run layout and routing the output metadata from those passes will superscede these new metadata fields. * Move pass in opt level 3 earlier in stage and skip with explicit layout This commit fixes 2 issues in the execution of the new ElideSwaps pass as part of optimization level 3. First we were running it too late in the process both after high level synthesis (which isn't relavant yet, but will be soon when this is expanded to elide all permutations not just swaps) and also after reset diagonal before measurement. The second issue is that if the user is specifying to run with a manual layout set we should not run this pass, as it will interfere with the user intent. * Doc and copy paste fixes * Expand test coverage * Update permutation tracking There were 2 issues with the permutation tracking done in an earlier commit. First, it conflicted with the final_layout property set via routing (or internally by the routing done in the combined sabre layout). This was breaking conditional checks in the preset pass manager around embedding. To fix this a different property is used and set as the output final layout if no final layout is set. The second issue was the output layout object was not taking into account a set initial layout which will permute the qubits and cause the output to not be up to date. This is fixed by updating apply layout to apply the initial layout to the elision_final_layout in the property set. * Generalize pass to support PermutationGate too This commit generalizes the pass from just considering swap gates to all permutations (via the PermutationGate class). This enables the pass to elide additional circuit permutations, not just the special case of a swap gate. The pass and module are renamed accordingly to ElidePermutations and elide_permutations respectively. * Fix permutation handling This commit fixes the recently added handling of the PermutationGate so that it correctly is mapping the qubits. The previous iteration of this messed up the mapping logic so it wasn't valid. * Fix formatting * Fix final layout handling for no initial layout * Improve documentation and log a warning if run post layout * Fix final layout handling with no ElideSwaps being run * Fix docs build * Fix release note * Fix typo * Add test for routing and elide permutations * Apply suggestions from code review Co-authored-by: Jim Garrison <jim@garrison.cc> * Rename test file to test_elide_permutations.py * Apply suggestions from code review Co-authored-by: Kevin Hartman <kevin@hart.mn> * Fix test import after rebase * fixing failing test cases this should pass CI after merging #12057 * addresses kehas comments - thx * Adding FinalyzeLayouts pass to pull the virtual circuit permutation from ElidePermutations to the final layout * formatting * partial rebase on top of 12057 + tests * also need test_operator for partial rebase * removing from transpiler flow for now; reworking elide tests * also adding permutation gate to the example * also temporarily reverting test_transpiler.py * update to release notes * minor fixes * Apply suggestions from code review * Fix lint * Update qiskit/transpiler/passes/optimization/elide_permutations.py * Add test to test we skip after layout * Integrate FinalizeLayouts into the PassManager harness This commit integrates the function that finalize layouts was performing into the passmanager harnesss. We'll always need to run the equivalent of finalize layout if any passes are setting a virtual permutation so using a standalone pass that can be forgotten is potentially error prone. This inlines the logic as part of the passmanager's output preparation stage so we always finalize the layout. * Compose a potential existing virtual_permutation_layout * Remove unused import --------- Co-authored-by: Jim Garrison <jim@garrison.cc> Co-authored-by: Kevin Hartman <kevin@hart.mn> Co-authored-by: Sebastian Brandhofer <148463728+sbrandhsn@users.noreply.github.com> Co-authored-by: AlexanderIvrii <alexi@il.ibm.com> Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Add ElideSwaps transpiler pass This commit adds a new transpiler pass ElideSwaps which is a logical optimization pass designed to run prior to layout or any other physical embedding steps in the transpiler pipeline. It traverses the DAG in topological order and when a swap gate is encountered it removes that gate and instead permutes the logical qubits for any subsequent gates in the DAG. This will eliminate any swaps in a circuit not caused by routing. Additionally, this pass is added to the preset pass manager for optimization level 3, we can consider adding it to other levels too if we think it makes sense (there is little overhead, and almost 0 if there are no swaps). One thing to note is that this pass does not recurse into control flow blocks at all, it treats them as black box operations. So if any control flow blocks contain swap gates those will not be optimized away. This change was made because mapping the permutation outside and inside any control flow block was larger in scope than what the intent for this pass was. That type of work is better suited for the routing passes which already have to reason about this. * Update tests with optimization level 3 * Pass final layout from ElideSwap to output The new ElideSwap pass is causing an output permutation just as a routing pass would. This information needs to be passed through to the output in case there is no layout or routing run. In those cases the information about the output permutation caused by the swap elision will be lost and doing layout aware operations like Operator.from_circuit() will not be able to reason about the permutation. This commit fixes this by inserting the original layout and qubit mapping into the property set along with the final layout. Then the base pass class and pass manager class are updated to use the original layout as the initial layout if one isn't set. In cases where we run layout and routing the output metadata from those passes will superscede these new metadata fields. * Move pass in opt level 3 earlier in stage and skip with explicit layout This commit fixes 2 issues in the execution of the new ElideSwaps pass as part of optimization level 3. First we were running it too late in the process both after high level synthesis (which isn't relavant yet, but will be soon when this is expanded to elide all permutations not just swaps) and also after reset diagonal before measurement. The second issue is that if the user is specifying to run with a manual layout set we should not run this pass, as it will interfere with the user intent. * Doc and copy paste fixes * Expand test coverage * Update permutation tracking There were 2 issues with the permutation tracking done in an earlier commit. First, it conflicted with the final_layout property set via routing (or internally by the routing done in the combined sabre layout). This was breaking conditional checks in the preset pass manager around embedding. To fix this a different property is used and set as the output final layout if no final layout is set. The second issue was the output layout object was not taking into account a set initial layout which will permute the qubits and cause the output to not be up to date. This is fixed by updating apply layout to apply the initial layout to the elision_final_layout in the property set. * Generalize pass to support PermutationGate too This commit generalizes the pass from just considering swap gates to all permutations (via the PermutationGate class). This enables the pass to elide additional circuit permutations, not just the special case of a swap gate. The pass and module are renamed accordingly to ElidePermutations and elide_permutations respectively. * Fix permutation handling This commit fixes the recently added handling of the PermutationGate so that it correctly is mapping the qubits. The previous iteration of this messed up the mapping logic so it wasn't valid. * Fix formatting * Fix final layout handling for no initial layout * Improve documentation and log a warning if run post layout * Fix final layout handling with no ElideSwaps being run * Fix docs build * Fix release note * Fix typo * Add test for routing and elide permutations * Apply suggestions from code review Co-authored-by: Jim Garrison <jim@garrison.cc> * Rename test file to test_elide_permutations.py * Apply suggestions from code review Co-authored-by: Kevin Hartman <kevin@hart.mn> * Fix test import after rebase * fixing failing test cases this should pass CI after merging #12057 * addresses kehas comments - thx * Adding FinalyzeLayouts pass to pull the virtual circuit permutation from ElidePermutations to the final layout * formatting * partial rebase on top of 12057 + tests * also need test_operator for partial rebase * removing from transpiler flow for now; reworking elide tests * also adding permutation gate to the example * also temporarily reverting test_transpiler.py * update to release notes * minor fixes * adding ElidePermutations and FinalizeLayouts to level 3 * fixing tests * release notes * properly merging with main (now that ElidePermutations is merged) * and also deleting finalize_layouts * Update releasenotes/notes/add-elide-permutations-to-pipeline-077dad03bd55ab9c.yaml * updating code comment * Fixing failing test: now that ElidePermutations pass runs with optimization level 3, it does remove the SWAP gate. So we need to consider optimization_level=1 to assert that the extra pass does not remove SWAP gates --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Jim Garrison <jim@garrison.cc> Co-authored-by: Kevin Hartman <kevin@hart.mn> Co-authored-by: Sebastian Brandhofer <148463728+sbrandhsn@users.noreply.github.com>
…n-trivial initial layout (Qiskit#12057) * reno, format, test changes * fix Operator.from_circuit and add failing test case * Update test_operator.py * Update operator.py * Update releasenotes/notes/operator-from-circuit-bugfix-5dab5993526a2b0a.yaml Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * selective merge of 11399 * reimplementing from_circuit --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: AlexanderIvrii <alexi@il.ibm.com>
* Add ElideSwaps transpiler pass This commit adds a new transpiler pass ElideSwaps which is a logical optimization pass designed to run prior to layout or any other physical embedding steps in the transpiler pipeline. It traverses the DAG in topological order and when a swap gate is encountered it removes that gate and instead permutes the logical qubits for any subsequent gates in the DAG. This will eliminate any swaps in a circuit not caused by routing. Additionally, this pass is added to the preset pass manager for optimization level 3, we can consider adding it to other levels too if we think it makes sense (there is little overhead, and almost 0 if there are no swaps). One thing to note is that this pass does not recurse into control flow blocks at all, it treats them as black box operations. So if any control flow blocks contain swap gates those will not be optimized away. This change was made because mapping the permutation outside and inside any control flow block was larger in scope than what the intent for this pass was. That type of work is better suited for the routing passes which already have to reason about this. * Update tests with optimization level 3 * Pass final layout from ElideSwap to output The new ElideSwap pass is causing an output permutation just as a routing pass would. This information needs to be passed through to the output in case there is no layout or routing run. In those cases the information about the output permutation caused by the swap elision will be lost and doing layout aware operations like Operator.from_circuit() will not be able to reason about the permutation. This commit fixes this by inserting the original layout and qubit mapping into the property set along with the final layout. Then the base pass class and pass manager class are updated to use the original layout as the initial layout if one isn't set. In cases where we run layout and routing the output metadata from those passes will superscede these new metadata fields. * Move pass in opt level 3 earlier in stage and skip with explicit layout This commit fixes 2 issues in the execution of the new ElideSwaps pass as part of optimization level 3. First we were running it too late in the process both after high level synthesis (which isn't relavant yet, but will be soon when this is expanded to elide all permutations not just swaps) and also after reset diagonal before measurement. The second issue is that if the user is specifying to run with a manual layout set we should not run this pass, as it will interfere with the user intent. * Doc and copy paste fixes * Expand test coverage * Update permutation tracking There were 2 issues with the permutation tracking done in an earlier commit. First, it conflicted with the final_layout property set via routing (or internally by the routing done in the combined sabre layout). This was breaking conditional checks in the preset pass manager around embedding. To fix this a different property is used and set as the output final layout if no final layout is set. The second issue was the output layout object was not taking into account a set initial layout which will permute the qubits and cause the output to not be up to date. This is fixed by updating apply layout to apply the initial layout to the elision_final_layout in the property set. * Generalize pass to support PermutationGate too This commit generalizes the pass from just considering swap gates to all permutations (via the PermutationGate class). This enables the pass to elide additional circuit permutations, not just the special case of a swap gate. The pass and module are renamed accordingly to ElidePermutations and elide_permutations respectively. * Fix permutation handling This commit fixes the recently added handling of the PermutationGate so that it correctly is mapping the qubits. The previous iteration of this messed up the mapping logic so it wasn't valid. * Fix formatting * Fix final layout handling for no initial layout * Improve documentation and log a warning if run post layout * Fix final layout handling with no ElideSwaps being run * Fix docs build * Fix release note * Fix typo * Add test for routing and elide permutations * Apply suggestions from code review Co-authored-by: Jim Garrison <jim@garrison.cc> * Rename test file to test_elide_permutations.py * Apply suggestions from code review Co-authored-by: Kevin Hartman <kevin@hart.mn> * Fix test import after rebase * fixing failing test cases this should pass CI after merging Qiskit#12057 * addresses kehas comments - thx * Adding FinalyzeLayouts pass to pull the virtual circuit permutation from ElidePermutations to the final layout * formatting * partial rebase on top of 12057 + tests * also need test_operator for partial rebase * removing from transpiler flow for now; reworking elide tests * also adding permutation gate to the example * also temporarily reverting test_transpiler.py * update to release notes * minor fixes * Apply suggestions from code review * Fix lint * Update qiskit/transpiler/passes/optimization/elide_permutations.py * Add test to test we skip after layout * Integrate FinalizeLayouts into the PassManager harness This commit integrates the function that finalize layouts was performing into the passmanager harnesss. We'll always need to run the equivalent of finalize layout if any passes are setting a virtual permutation so using a standalone pass that can be forgotten is potentially error prone. This inlines the logic as part of the passmanager's output preparation stage so we always finalize the layout. * Compose a potential existing virtual_permutation_layout * Remove unused import --------- Co-authored-by: Jim Garrison <jim@garrison.cc> Co-authored-by: Kevin Hartman <kevin@hart.mn> Co-authored-by: Sebastian Brandhofer <148463728+sbrandhsn@users.noreply.github.com> Co-authored-by: AlexanderIvrii <alexi@il.ibm.com> Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Add ElideSwaps transpiler pass This commit adds a new transpiler pass ElideSwaps which is a logical optimization pass designed to run prior to layout or any other physical embedding steps in the transpiler pipeline. It traverses the DAG in topological order and when a swap gate is encountered it removes that gate and instead permutes the logical qubits for any subsequent gates in the DAG. This will eliminate any swaps in a circuit not caused by routing. Additionally, this pass is added to the preset pass manager for optimization level 3, we can consider adding it to other levels too if we think it makes sense (there is little overhead, and almost 0 if there are no swaps). One thing to note is that this pass does not recurse into control flow blocks at all, it treats them as black box operations. So if any control flow blocks contain swap gates those will not be optimized away. This change was made because mapping the permutation outside and inside any control flow block was larger in scope than what the intent for this pass was. That type of work is better suited for the routing passes which already have to reason about this. * Update tests with optimization level 3 * Pass final layout from ElideSwap to output The new ElideSwap pass is causing an output permutation just as a routing pass would. This information needs to be passed through to the output in case there is no layout or routing run. In those cases the information about the output permutation caused by the swap elision will be lost and doing layout aware operations like Operator.from_circuit() will not be able to reason about the permutation. This commit fixes this by inserting the original layout and qubit mapping into the property set along with the final layout. Then the base pass class and pass manager class are updated to use the original layout as the initial layout if one isn't set. In cases where we run layout and routing the output metadata from those passes will superscede these new metadata fields. * Move pass in opt level 3 earlier in stage and skip with explicit layout This commit fixes 2 issues in the execution of the new ElideSwaps pass as part of optimization level 3. First we were running it too late in the process both after high level synthesis (which isn't relavant yet, but will be soon when this is expanded to elide all permutations not just swaps) and also after reset diagonal before measurement. The second issue is that if the user is specifying to run with a manual layout set we should not run this pass, as it will interfere with the user intent. * Doc and copy paste fixes * Expand test coverage * Update permutation tracking There were 2 issues with the permutation tracking done in an earlier commit. First, it conflicted with the final_layout property set via routing (or internally by the routing done in the combined sabre layout). This was breaking conditional checks in the preset pass manager around embedding. To fix this a different property is used and set as the output final layout if no final layout is set. The second issue was the output layout object was not taking into account a set initial layout which will permute the qubits and cause the output to not be up to date. This is fixed by updating apply layout to apply the initial layout to the elision_final_layout in the property set. * Generalize pass to support PermutationGate too This commit generalizes the pass from just considering swap gates to all permutations (via the PermutationGate class). This enables the pass to elide additional circuit permutations, not just the special case of a swap gate. The pass and module are renamed accordingly to ElidePermutations and elide_permutations respectively. * Fix permutation handling This commit fixes the recently added handling of the PermutationGate so that it correctly is mapping the qubits. The previous iteration of this messed up the mapping logic so it wasn't valid. * Fix formatting * Fix final layout handling for no initial layout * Improve documentation and log a warning if run post layout * Fix final layout handling with no ElideSwaps being run * Fix docs build * Fix release note * Fix typo * Add test for routing and elide permutations * Apply suggestions from code review Co-authored-by: Jim Garrison <jim@garrison.cc> * Rename test file to test_elide_permutations.py * Apply suggestions from code review Co-authored-by: Kevin Hartman <kevin@hart.mn> * Fix test import after rebase * fixing failing test cases this should pass CI after merging Qiskit#12057 * addresses kehas comments - thx * Adding FinalyzeLayouts pass to pull the virtual circuit permutation from ElidePermutations to the final layout * formatting * partial rebase on top of 12057 + tests * also need test_operator for partial rebase * removing from transpiler flow for now; reworking elide tests * also adding permutation gate to the example * also temporarily reverting test_transpiler.py * update to release notes * minor fixes * adding ElidePermutations and FinalizeLayouts to level 3 * fixing tests * release notes * properly merging with main (now that ElidePermutations is merged) * and also deleting finalize_layouts * Update releasenotes/notes/add-elide-permutations-to-pipeline-077dad03bd55ab9c.yaml * updating code comment * Fixing failing test: now that ElidePermutations pass runs with optimization level 3, it does remove the SWAP gate. So we need to consider optimization_level=1 to assert that the extra pass does not remove SWAP gates --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Jim Garrison <jim@garrison.cc> Co-authored-by: Kevin Hartman <kevin@hart.mn> Co-authored-by: Sebastian Brandhofer <148463728+sbrandhsn@users.noreply.github.com>
Summary
Operator.from_circuit
generates a wrong operator when a final layout is specified with a non-trivial initial layout via thetranspile
function. This behavior is fixed in this PR while adding new tests to check the correct operator initialization usingOperator.from_circuit
. This PR also modifies the documentation on theOperator
constructor slightly, i.e. there is a hint for usingOperator.from_circuit
when a non-canonical qubit order/permutation should be assumed for the initialization of the operator.Details and comments
For example,
Operator.from_circuit(transpile(qc, cm, initial_layout=[2, 3, 4, 0, 1])).equiv(qc)
would incorrectly return false whileOperator.from_circuit(transpile(qc, cm, initial_layout=[0, 1, 2, 3, 4])).equiv(qc)
would correctly return true (with the only change being the initial layout).This is error occurs because the specification of
final_layout
inOperator.from_circuit
does not account for a permutation introduced by theinitial_layout
of a quantum circuit. However, this permutation is introduced by:qiskit/qiskit/quantum_info/operators/operator.py
Line 422 in dd2570b
In this PR, the permutation given by
final_layout
is first combined with the permutation given by theinitial_layout
before the permutation is applied to the operator.