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

BarrierBeforeFinalMeasurement should be removed from transpiled circuit #10321

Closed
chriseclectic opened this issue Jun 22, 2023 · 4 comments · Fixed by #10323
Closed

BarrierBeforeFinalMeasurement should be removed from transpiled circuit #10321

chriseclectic opened this issue Jun 22, 2023 · 4 comments · Fixed by #10323
Assignees
Labels
bug Something isn't working

Comments

@chriseclectic
Copy link
Member

chriseclectic commented Jun 22, 2023

Environment

  • Qiskit Terra version: 0.42.1
  • Python version: 3.9
  • Operating system: MacOS

What is happening?

When the transpiler does its routing pass, in some cases it inserts a barrier before final measurement. This barrier is not necessarily in the input circuit, and in cases where it is, it is expanded to span the full width of hte device rather than only the qubits of the input qubit.

If this barrier is not in the input circuit it should be removed from the transpiled circuit. If it is in the input circuit it should be only on the appropriately mapped qubits in the output circuit.

How can we reproduce the issue?

Example Circuit 1

First example is for a circuit with no barriers:

from qiskit import QuantumCircuit, transpile

qc = QuantumCircuit(2, 2)
qc.cx(0, 1)
qc.measure(range(2), range(2))
          ┌─┐   
q_0: ──■──┤M├───
     ┌─┴─┐└╥┘┌─┐
q_1: ┤ X ├─╫─┤M├
     └───┘ ║ └╥┘
c: 2/══════╩══╩═
           0  1 

Example Circuit 2

If I use measure_all which inserts a barrier, it gets expanded to all ancilla qubits:

              ░ ┌─┐   
   q_0: ──■───░─┤M├───
        ┌─┴─┐ ░ └╥┘┌─┐
   q_1: ┤ X ├─░──╫─┤M├
        └───┘ ░  ║ └╥┘
meas: 2/═════════╩══╩═
                 0  1 

Transpiled Circuits

If i force some complicated layout, in both cases the transpiled circuit has a full width barrier in it

transpile(qc, initial_layout=[1, 4], coupling_map=[[1, 2], [2, 3], [3, 4]]).draw()
                        ░       
ancilla_0 -> 0 ─────────░───────
                        ░       
      q_0 -> 1 ─X───────░───────
                │       ░ ┌─┐   
ancilla_1 -> 2 ─X───■───░─┤M├───
                  ┌─┴─┐ ░ └╥┘┌─┐
ancilla_2 -> 3 ─X─┤ X ├─░──╫─┤M├
                │ └───┘ ░  ║ └╥┘
      q_1 -> 4 ─X───────░──╫──╫─
                        ░  ║  ║ 
       meas: 2/════════════╩══╩═
                           0  1 

What should happen?

Expected transpiled circuit 1

                               
ancilla_0 -> 0 ────────────────
                               
      q_0 -> 1 ─X──────────────
                │        ┌─┐   
ancilla_1 -> 2 ─X───■────┤M├───
                  ┌─┴─┐  └╥┘┌─┐
ancilla_2 -> 3 ─X─┤ X ├───╫─┤M├
                │ └───┘   ║ └╥┘
      q_1 -> 4 ─X─────────╫──╫─
                          ║  ║ 
       meas: 2/═══════════╩══╩═
                           0  1 

Expected transpiled circuit 2

                               
ancilla_0 -> 0 ────────────────
                               
      q_0 -> 1 ─X──────────────
                │       ░ ┌─┐   
ancilla_1 -> 2 ─X───■───░─┤M├───
                  ┌─┴─┐ ░ └╥┘┌─┐
ancilla_2 -> 3 ─X─┤ X ├─░──╫─┤M├
                │ └───┘ ░  ║ └╥┘
      q_1 -> 4 ─X──────────╫──╫─
                           ║  ║ 
       meas: 2/════════════╩══╩═
                           0  1 

Any suggestions?

Add another transpiler pass to the generate_routing_passmanager that removes the barrier inserted by BarrierBeforeFinalMeasurement, and in the case where the original circuit has a barrier, preserve that barrier mapped to the appropriate qubits in the transpiled circuit.

@chriseclectic chriseclectic added the bug Something isn't working label Jun 22, 2023
@mtreinish mtreinish self-assigned this Jun 22, 2023
@mtreinish
Copy link
Member

Yeah I think ideally we should have an option to label the barrier BarrierBeforeFinalMeasurement inserts and later remove the labeled barriers before optimization. The barriers are only inserted for routing purposes so there isn't a reason for them to persist outside of the transpiler. This shouldn't be too hard to implement.

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jun 22, 2023
This commit adds a new transpiler pass RemoveLabeledOps which is used to
remove any op nodes that match a given label. This is paired with a new
label option for BarrierBeforeFinalMeasurements. These are combined in
the preset pass managers to ensure we're not always adding a barrier
before output measurements in the output of the transpiler.

Fixes Qiskit#10321
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jun 22, 2023
This commit adds a new transpiler pass RemoveLabeledOps which is used to
remove any op nodes that match a given label. This is paired with a new
label option for BarrierBeforeFinalMeasurements. These are combined in
the preset pass managers to ensure we're not always adding a barrier
before output measurements in the output of the transpiler.

Fixes Qiskit#10321
@kdk
Copy link
Member

kdk commented Jul 5, 2023

Are these barriers still necessary for routing? I don't believe they are needed for the backends anymore, so maybe BarrierBeforeFinalMeasurement can be removed from the default levels.

@mtreinish
Copy link
Member

mtreinish commented Jul 5, 2023

Yeah, they are still needed for routing. Without it sabre will potentially route past a measurement so we still rely on the inserted barrier for routing to work. To change this we'd have to give sabre more context about what a measurement is, it just treats it as a 1q operation right now.

@jakelishman
Copy link
Member

jakelishman commented Jul 5, 2023

I think they're not needed for backends, but a mid-circuit measurement (which these measurements become if swaps appear after them) have different error rates and performance characteristics than final measurements, so it's probably not ideal to be routing gates after them. It's also better for parallelisation of readout on basically any hardware architecture that can parallelise readout, I think?

Fwiw, I want to update Sabre with more context about every operation as a stepping stone to getting it to support mirror-gate synthesis and optimising the final circuit construction in Python space, and that would naturally tie into this as well. (edit: and I know what I'd do to do that update - I just haven't had time to actually do it.)

github-merge-queue bot pushed a commit that referenced this issue Nov 22, 2023
* Add pass to remove labeled ops and label inserted barriers

This commit adds a new transpiler pass RemoveLabeledOps which is used to
remove any op nodes that match a given label. This is paired with a new
label option for BarrierBeforeFinalMeasurements. These are combined in
the preset pass managers to ensure we're not always adding a barrier
before output measurements in the output of the transpiler.

Fixes #10321

* Fix rebase issues

* Handle pre-existing adjacent barriers

In the case when a label is set to trigger the removal of the labelled
barrier the merge step would lose the context around which barrier
instruction was transpiler inserted and which was user provided. To
address this issue this commit skips the MergeAdjacentBarrier step so
that the transpiler barrier is kept separate from any user inserted
barriers which need to be preserved.

* Generalize RemoveLabeledOps to FilterOpNodes

This commit generalizes the new RemoveLabeledOps pass to be a generic
node filtering pass that given a filter function it will remove any
matching op nodes in the circuit.

* Add release note

* Add tests for new pass

* Add release note for new barrier pass kwarg

* Fix type hint

* Fix docs typo

* Invert predicate usage

This commit inverts the predicate usage to be consistent with Python's
built in filter() function. Now if the predicate returns True the dag node
is retained and if it returns false it is removed. This is also
explicitly documented to make it clear how the pass is to be used.

* Update pass manager drawer reference images
FabianBrings pushed a commit to FabianBrings/qiskit that referenced this issue Nov 27, 2023
* Add pass to remove labeled ops and label inserted barriers

This commit adds a new transpiler pass RemoveLabeledOps which is used to
remove any op nodes that match a given label. This is paired with a new
label option for BarrierBeforeFinalMeasurements. These are combined in
the preset pass managers to ensure we're not always adding a barrier
before output measurements in the output of the transpiler.

Fixes Qiskit#10321

* Fix rebase issues

* Handle pre-existing adjacent barriers

In the case when a label is set to trigger the removal of the labelled
barrier the merge step would lose the context around which barrier
instruction was transpiler inserted and which was user provided. To
address this issue this commit skips the MergeAdjacentBarrier step so
that the transpiler barrier is kept separate from any user inserted
barriers which need to be preserved.

* Generalize RemoveLabeledOps to FilterOpNodes

This commit generalizes the new RemoveLabeledOps pass to be a generic
node filtering pass that given a filter function it will remove any
matching op nodes in the circuit.

* Add release note

* Add tests for new pass

* Add release note for new barrier pass kwarg

* Fix type hint

* Fix docs typo

* Invert predicate usage

This commit inverts the predicate usage to be consistent with Python's
built in filter() function. Now if the predicate returns True the dag node
is retained and if it returns false it is removed. This is also
explicitly documented to make it clear how the pass is to be used.

* Update pass manager drawer reference images
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.

4 participants