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

Option for skipping OptimizeSwapBeforeMeasure when not all the wires are measured. #5890

Closed
wants to merge 14 commits into from

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Feb 23, 2021

Summary

This PR extends OptimizeSwapBeforeMeasure to only remove swaps when both wires are measured. Instead of changing the default behaviour, I added a parameter for backwards compatibility (and in case we want to change it back for some transpilation cases). When OptimizeSwapBeforeMeasure is used in level 3, the option is activated. This fixes #4911

Details and comments

When the SWAP should not be remove

circ = QuantumCircuit(2, 1)
circ.swap(0, 1)
circ.measure(0, 0)

display(circ.draw())
display(OptimizeSwapBeforeMeasure(all_measurement=True)(circ).draw())
        ┌─┐
q_0: ─X─┤M├
      │ └╥┘
q_1: ─X──╫─
         ║ 
c: 1/════╩═
         0 
        ┌─┐
q_0: ─X─┤M├
      │ └╥┘
q_1: ─X──╫─
         ║ 
c: 1/════╩═
         0 

When the SWAP can be removed:

from qiskit import QuantumCircuit
from qiskit.transpiler.passes import OptimizeSwapBeforeMeasure

circ = QuantumCircuit(2, 2)
circ.swap(0, 1)
circ.measure(range(2), range(2))

display(circ.draw())
display(OptimizeSwapBeforeMeasure(all_measurement=True)(circ).draw())
        ┌─┐   
q_0: ─X─┤M├───
      │ └╥┘┌─┐
q_1: ─X──╫─┤M├
         ║ └╥┘
c: 2/════╩══╩═
         0  1 
        ┌─┐
q_0: ───┤M├
     ┌─┐└╥┘
q_1: ┤M├─╫─
     └╥┘ ║ 
c: 2/═╩══╩═
      0  1 

@1ucian0 1ucian0 marked this pull request as ready for review February 23, 2021 20:13
@1ucian0 1ucian0 requested a review from a team as a code owner February 23, 2021 20:13
Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good. One question generally. Would it be more inline with the direction of #4608 to have this pass switch the order of the swap and measure instructions, instead of removing the swap? That would preserve the the layout in all cases and still allow the optimization in cases where only some of the qubits are measured.

@1ucian0
Copy link
Member Author

1ucian0 commented Feb 24, 2021

#4608 transpiles a circuit without measurement and then measures. Therefore, this PR already solves #4608 since this pass has no effect if no measurement is found.

As swapping the swap and the measure, I have no strong opinion (maybe @ajavadia?). Probably makes sense when intermediate measurements.

@1ucian0
Copy link
Member Author

1ucian0 commented Feb 24, 2021

For example:

from qiskit import QuantumCircuit
from qiskit.transpiler.passes import OptimizeSwapBeforeMeasure

circ = QuantumCircuit(2, 2)
circ.h(0)
circ.x(0)
circ.swap(0, 1)
circ.measure(range(2), range(2))
circ.h(0)
circ.x(0)
circ.swap(0, 1)
circ.measure(range(2), range(2))

print(circ)
print(OptimizeSwapBeforeMeasure(all_measurement=True, move_swap=True)(circ))
     ┌───┐┌───┐   ┌─┐┌───┐┌───┐   ┌─┐
q_0: ┤ H ├┤ X ├─X─┤M├┤ H ├┤ X ├─X─┤M├───
     └───┘└───┘ │ └╥┘└┬─┬┘└───┘ │ └╥┘┌─┐
q_1: ───────────X──╫──┤M├───────X──╫─┤M├
                   ║  └╥┘          ║ └╥┘
c: 2/══════════════╩═══╩═══════════╩══╩═
                   0   1           0  1
     ┌───┐┌───┐┌───┐┌───┐┌─┐   ┌─┐
q_0: ┤ H ├┤ X ├┤ H ├┤ X ├┤M├─X─┤M├────X─
     └┬─┬┘└───┘└───┘└───┘└╥┘ │ └╥┘┌─┐ │
q_1: ─┤M├─────────────────╫──X──╫─┤M├─X─
      └╥┘                 ║     ║ └╥┘
c: 2/══╩══════════════════╩═════╩══╩════
       0                  1     1  0

Does this make sense?

@kdk
Copy link
Member

kdk commented Feb 24, 2021

I had been thinking something even simpler, I think. For the circuit above, I'd expect an output (after one iteration) like

     ┌───┐┌───┐┌─┐   ┌───┐┌───┐┌─┐   
q_0: ┤ H ├┤ X ├┤M├─X─┤ H ├┤ X ├┤M├─X─
     └┬─┬┘└───┘└╥┘ │ └┬─┬┘└───┘└╥┘ │ 
q_1: ─┤M├───────╫──X──┤M├───────╫──X─
      └╥┘       ║     └╥┘       ║    
c: 2/══╩════════╩══════╩════════╩════
       0        1      0        1    

But looking at the behavior from master, I wonder if there isn't already a bug here with mid-circuit measurements.

     ┌───┐┌───┐┌───┐┌───┐┌─┐┌─┐
q_0: ┤ H ├┤ X ├┤ H ├┤ X ├┤M├┤M├
     └┬─┬┘└┬─┬┘└───┘└───┘└╥┘└╥┘
q_1: ─┤M├──┤M├────────────╫──╫─
      └╥┘  └╥┘            ║  ║ 
c: 2/══╩════╩═════════════╩══╩═
       0    0             1  1 

doesn't look right to me. I'd expect (regardless of how the optimization is done) for there to be at least one measurement between the first H-X and second H-X.

@1ucian0
Copy link
Member Author

1ucian0 commented Feb 25, 2021

Good catch. Reported in #5905, fixed in #5906

@ajavadia
Copy link
Member

ajavadia commented Mar 3, 2021

I don't understand what all_measurement does. Can you explain?

@jakelishman
Copy link
Member

This appears stale to me now with all its conflicts, and no longer really as in line with how we treat layouts, routings, and the responsibilities of the preset pass managers. I'll close it as staled, but feel free to re-open if there's more to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OptimizeSwapBeforeMeasure doesn't preserve unitary if swap is only operation
4 participants