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

VF2PostLayout corrupts final_layout output #10457

Closed
mtreinish opened this issue Jul 20, 2023 · 0 comments · Fixed by #10466
Closed

VF2PostLayout corrupts final_layout output #10457

mtreinish opened this issue Jul 20, 2023 · 0 comments · Fixed by #10466
Assignees
Labels
bug Something isn't working
Milestone

Comments

@mtreinish
Copy link
Member

Environment

  • Qiskit Terra version: 0.24.1 (but it also should affect 0.24.2 and main)
  • Python version: 3.11
  • Operating system: Linux

What is happening?

When running transpile() if the VF2PostLayout pass finds a better letter and re-applies the initial layout the final layout is not adjusted to correct for that change in initial position. This previously was not an issue because the final layout was an internal implementation detail (primarily used only for sabre layout) and we were done using it after layout in the transpiler. But now we're returning the final layout as part of the public interface in a QuantumCircuit.

How can we reproduce the issue?

from qiskit import QuantumCircuit, transpile
from qiskit.providers.fake_provider import FakeVigo
from qiskit_aer import AerSimulator

backend = AerSimulator.from_backend(FakeVigo())
qubits = 3
qc = QuantumCircuit(qubits)
for i in range(5):
    qc.cx(i % qubits, int(i + qubits / 2) % qubits)

tqc = transpile(qc, backend=backend, callback=callback)
print(tqc)
print(tqc.layout)

What should happen?

In that example if VF2PostLayout runs the final layout will potentially point to incorrect qubits. One example incorrect output is:

ancilla_0 -> 0 ───────────────────────────────────────────────────────
                    ┌───┐     ┌───┐┌───┐     ┌───┐     ┌───┐┌───┐     
      q_0 -> 1 ──■──┤ X ├──■──┤ X ├┤ X ├──■──┤ X ├──■──┤ X ├┤ X ├──■──
               ┌─┴─┐└─┬─┘  │  └─┬─┘└─┬─┘  │  └─┬─┘┌─┴─┐└─┬─┘└─┬─┘┌─┴─┐
      q_1 -> 2 ┤ X ├──┼────┼────┼────■────┼────■──┤ X ├──■────┼──┤ X ├
               └───┘  │  ┌─┴─┐  │       ┌─┴─┐     └───┘       │  └───┘
      q_2 -> 3 ───────■──┤ X ├──■───────┤ X ├─────────────────■───────
                         └───┘          └───┘                         
ancilla_1 -> 4 ───────────────────────────────────────────────────────
TranspileLayout(initial_layout=Layout({
1: Qubit(QuantumRegister(3, 'q'), 0),
2: Qubit(QuantumRegister(3, 'q'), 1),
3: Qubit(QuantumRegister(3, 'q'), 2),
0: Qubit(QuantumRegister(2, 'ancilla'), 0),
4: Qubit(QuantumRegister(2, 'ancilla'), 1)
}), input_qubit_mapping={Qubit(QuantumRegister(3, 'q'), 0): 0, Qubit(QuantumRegister(3, 'q'), 1): 1, Qubit(QuantumRegister(3, 'q'), 2): 2, Qubit(QuantumRegister(2, 'ancilla'), 0): 3, Qubit(QuantumRegister(2, 'ancilla'), 1): 4}, final_layout=Layout({
0: Qubit(QuantumRegister(5, 'q'), 0),
3: Qubit(QuantumRegister(5, 'q'), 1),
2: Qubit(QuantumRegister(5, 'q'), 2),
4: Qubit(QuantumRegister(5, 'q'), 3),
1: Qubit(QuantumRegister(5, 'q'), 4)
}))

In this case the final layout is incorrect because it's not taking into account that vf2postlayout changed the initial layout. Instead it should be:

Layout({
0: Qubit(QuantumRegister(5, 'q'), 0),
3: Qubit(QuantumRegister(5, 'q'), 1),
1: Qubit(QuantumRegister(5, 'q'), 2),
2: Qubit(QuantumRegister(5, 'q'), 3),
4: Qubit(QuantumRegister(5, 'q'), 4)
}))

(I was running without seeds set so it was random failure based on sabre's output)

Any suggestions?

We should update the ApplyLayout pass to also update the final_layout in the property set if it's present, just like it does for the layout field.

@mtreinish mtreinish added the bug Something isn't working label Jul 20, 2023
@mtreinish mtreinish added this to the 0.25.0 milestone Jul 20, 2023
@mtreinish mtreinish self-assigned this Jul 20, 2023
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jul 20, 2023
This commit fixes a bug in the preset pass managers when VF2PostLayout
is run and finds a better layout to use. In these cases the ApplyLayout
was updating the layout but we never updated the final layout to reflect
these changes. This would result in an incorrect final layout because
the input positions of the qubits were incorrect after re-applying the
layout. This commit fixes this by adding code to ApplyLayout to also
update final_layout, if one is set, to reflect the new initial layout
found by VF2PostLayout.

Fixes Qiskit#10457
github-merge-queue bot pushed a commit that referenced this issue Jul 24, 2023
* Fix final_layout when VF2PostLayout finds a better layout

This commit fixes a bug in the preset pass managers when VF2PostLayout
is run and finds a better layout to use. In these cases the ApplyLayout
was updating the layout but we never updated the final layout to reflect
these changes. This would result in an incorrect final layout because
the input positions of the qubits were incorrect after re-applying the
layout. This commit fixes this by adding code to ApplyLayout to also
update final_layout, if one is set, to reflect the new initial layout
found by VF2PostLayout.

Fixes #10457

* Remove stray debug print

* Use a list instead of a dict

* Add assertion on vf2postlayout being used in compiler.transpile tests

* Actually assert a post layout is set
mergify bot pushed a commit that referenced this issue Jul 24, 2023
* Fix final_layout when VF2PostLayout finds a better layout

This commit fixes a bug in the preset pass managers when VF2PostLayout
is run and finds a better layout to use. In these cases the ApplyLayout
was updating the layout but we never updated the final layout to reflect
these changes. This would result in an incorrect final layout because
the input positions of the qubits were incorrect after re-applying the
layout. This commit fixes this by adding code to ApplyLayout to also
update final_layout, if one is set, to reflect the new initial layout
found by VF2PostLayout.

Fixes #10457

* Remove stray debug print

* Use a list instead of a dict

* Add assertion on vf2postlayout being used in compiler.transpile tests

* Actually assert a post layout is set

(cherry picked from commit 42a0ee8)
github-merge-queue bot pushed a commit that referenced this issue Jul 25, 2023
…10493)

* Fix final_layout when VF2PostLayout finds a better layout

This commit fixes a bug in the preset pass managers when VF2PostLayout
is run and finds a better layout to use. In these cases the ApplyLayout
was updating the layout but we never updated the final layout to reflect
these changes. This would result in an incorrect final layout because
the input positions of the qubits were incorrect after re-applying the
layout. This commit fixes this by adding code to ApplyLayout to also
update final_layout, if one is set, to reflect the new initial layout
found by VF2PostLayout.

Fixes #10457

* Remove stray debug print

* Use a list instead of a dict

* Add assertion on vf2postlayout being used in compiler.transpile tests

* Actually assert a post layout is set

(cherry picked from commit 42a0ee8)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
to24toro pushed a commit to to24toro/qiskit-terra that referenced this issue Aug 3, 2023
* Fix final_layout when VF2PostLayout finds a better layout

This commit fixes a bug in the preset pass managers when VF2PostLayout
is run and finds a better layout to use. In these cases the ApplyLayout
was updating the layout but we never updated the final layout to reflect
these changes. This would result in an incorrect final layout because
the input positions of the qubits were incorrect after re-applying the
layout. This commit fixes this by adding code to ApplyLayout to also
update final_layout, if one is set, to reflect the new initial layout
found by VF2PostLayout.

Fixes Qiskit#10457

* Remove stray debug print

* Use a list instead of a dict

* Add assertion on vf2postlayout being used in compiler.transpile tests

* Actually assert a post layout is set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant