Fix composition of final_layout#14919
Merged
Merged
Conversation
The previous implementation had it backwards. There are two ways to
think about the `final_layout`, which is a `Layout` object mapping
"virtual" objects (qubits at the start of the circuit) to "physical"
indices (qubits at the end of the circuit):
- It is a "goes to" permutation that you would _prepend_ to the
_unrouted_ circuit to have the same effect as routing.
- It is a "comes from" permutation that you would _append_ to the
_routed_ circuit to undo the effects of routing.
If you have permutation `A` that is already applied, and you're newly
applying permutation `B`, then in the first interpretation, you want to
transform
A and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)"
by standard inverse rules.
In both cases, the logic is implemented as `previous.compose(new)` in
terms of `Layout.compose`.
jakelishman
added
Changelog: Fixed
Collaborator
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 16992684590Details
💛 - Coveralls |
mtreinish
approved these changes
Aug 15, 2025
mtreinish
left a comment
mtreinish
left a comment
Member
There was a problem hiding this comment.
The fix looks good to me, do you think it's worth backporting for 2.1.2?
mtreinish
added
the
stable backport potential
Member
Author
|
Yeah, it's eligible for backport, it just shouldn't really matter in practice in 2.1. We can backport it. |
mergify Bot
pushed a commit
that referenced
this pull request
Aug 15, 2025
The previous implementation had it backwards. There are two ways to
think about the `final_layout`, which is a `Layout` object mapping
"virtual" objects (qubits at the start of the circuit) to "physical"
indices (qubits at the end of the circuit):
- It is a "goes to" permutation that you would _prepend_ to the
_unrouted_ circuit to have the same effect as routing.
- It is a "comes from" permutation that you would _append_ to the
_routed_ circuit to undo the effects of routing.
If you have permutation `A` that is already applied, and you're newly
applying permutation `B`, then in the first interpretation, you want to
transform
A and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)"
by standard inverse rules.
In both cases, the logic is implemented as `previous.compose(new)` in
terms of `Layout.compose`.
(cherry picked from commit dfcc5c6)
# Conflicts:
# qiskit/transpiler/passes/layout/sabre_layout.py
# qiskit/transpiler/passes/routing/sabre_swap.py
jakelishman
added a commit
that referenced
this pull request
Aug 18, 2025
The previous implementation had it backwards. There are two ways to
think about the `final_layout`, which is a `Layout` object mapping
"virtual" objects (qubits at the start of the circuit) to "physical"
indices (qubits at the end of the circuit):
- It is a "goes to" permutation that you would _prepend_ to the
_unrouted_ circuit to have the same effect as routing.
- It is a "comes from" permutation that you would _append_ to the
_routed_ circuit to undo the effects of routing.
If you have permutation `A` that is already applied, and you're newly
applying permutation `B`, then in the first interpretation, you want to
transform
A and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)"
by standard inverse rules.
In both cases, the logic is implemented as `previous.compose(new)` in
terms of `Layout.compose`.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 18, 2025
The previous implementation had it backwards. There are two ways to
think about the `final_layout`, which is a `Layout` object mapping
"virtual" objects (qubits at the start of the circuit) to "physical"
indices (qubits at the end of the circuit):
- It is a "goes to" permutation that you would _prepend_ to the
_unrouted_ circuit to have the same effect as routing.
- It is a "comes from" permutation that you would _append_ to the
_routed_ circuit to undo the effects of routing.
If you have permutation `A` that is already applied, and you're newly
applying permutation `B`, then in the first interpretation, you want to
transform
A and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)"
by standard inverse rules.
In both cases, the logic is implemented as `previous.compose(new)` in
terms of `Layout.compose`.
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
littlebullGit
pushed a commit
to littlebullGit/qiskit
that referenced
this pull request
Sep 5, 2025
The previous implementation had it backwards. There are two ways to
think about the `final_layout`, which is a `Layout` object mapping
"virtual" objects (qubits at the start of the circuit) to "physical"
indices (qubits at the end of the circuit):
- It is a "goes to" permutation that you would _prepend_ to the
_unrouted_ circuit to have the same effect as routing.
- It is a "comes from" permutation that you would _append_ to the
_routed_ circuit to undo the effects of routing.
If you have permutation `A` that is already applied, and you're newly
applying permutation `B`, then in the first interpretation, you want to
transform
A and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)"
by standard inverse rules.
In both cases, the logic is implemented as `previous.compose(new)` in
terms of `Layout.compose`.
aaryav-3
pushed a commit
to aaryav-3/qiskit
that referenced
this pull request
Oct 21, 2025
The previous implementation had it backwards. There are two ways to
think about the `final_layout`, which is a `Layout` object mapping
"virtual" objects (qubits at the start of the circuit) to "physical"
indices (qubits at the end of the circuit):
- It is a "goes to" permutation that you would _prepend_ to the
_unrouted_ circuit to have the same effect as routing.
- It is a "comes from" permutation that you would _append_ to the
_routed_ circuit to undo the effects of routing.
If you have permutation `A` that is already applied, and you're newly
applying permutation `B`, then in the first interpretation, you want to
transform
A and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)"
by standard inverse rules.
In both cases, the logic is implemented as `previous.compose(new)` in
terms of `Layout.compose`.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
The previous implementation had it backwards. There are two ways to think about the
final_layout, which is aLayoutobject mapping "virtual" objects (qubits at the start of the circuit) to "physical" indices (qubits at the end of the circuit):It is a "goes to" permutation that you would prepend to the unrouted circuit to have the same effect as routing.
It is a "comes from" permutation that you would append to the routed circuit to undo the effects of routing.
If you have permutation
Athat is already applied, and you're newly applying permutationB, then in the first interpretation, you want to transformA and then (B and then circuit)
into
(A and then B) and then circuit.
In the second interpretation, you instead want to turn
(circuit then "undo B") then "undo A"
into
circuit then ("undo B" then "undo A")
The composition "undo B then undo A" is the same as "undo (A then B)" by standard inverse rules.
In both cases, the logic is implemented as
previous.compose(new)in terms ofLayout.compose.Details and comments
The commit message is just part of me iterating towards a set of words I'm happy with to describe the full situation. I still don't think I'm quite yet there. Either way, the point is that the test is enforcing an invariant that I think we can agree on (up to whether we like the convention of
PermutationGate, but we don't have freedom to choose that).Technically this is stable for backport if we were to choose to.
This fixes the remaining bug of #14904. The bug certainly exists in Qiskit 2.1 (and probably for as long as
final_layoutexisted), but #14904 causes it to surface because it normalisesvirtual_permutation_layoutintofinal_layoutearlier in the transpiler process, causing layout/routing to now actually have to deal with the update in some cases.