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

SabreSwap can throw away classically conditioned gates and all successors #8040

Closed
jakelishman opened this issue May 10, 2022 · 0 comments · Fixed by #8041
Closed

SabreSwap can throw away classically conditioned gates and all successors #8040

jakelishman opened this issue May 10, 2022 · 0 comments · Fixed by #8041
Labels
bug Something isn't working

Comments

@jakelishman
Copy link
Member

jakelishman commented May 10, 2022

Environment

  • Qiskit Terra version: main @ 7726d47
  • Python version: any
  • Operating system: any

What is happening?

The SabreSwap transpiler pass can sometimes remove classically conditioned gates from the circuit, and all their successors, whether or not the successors have classical conditions.

This is (I believe) the cause of the current failures in the randomised test suite (from #2645 (comment) to #2645 (comment)).

How can we reproduce the issue?

In [1]: from qiskit import QuantumCircuit
   ...: from qiskit.transpiler import PassManager, CouplingMap
   ...: from qiskit.transpiler.passes import SabreSwap, TrivialLayout
   ...:
   ...: qc = QuantumCircuit(2, 1)
   ...: qc.z(0)
   ...: qc.z(0).c_if(qc.cregs[0], 0)
   ...: print(qc.draw())
   ...: print()
   ...: cm = CouplingMap([(0, 1), (1, 0)])
   ...: print(PassManager([TrivialLayout(cm), SabreSwap(cm)]).run(qc).draw())
     ┌───┐ ┌───┐
q_0: ┤ Z ├─┤ Z ├─
     └───┘ └─╥─┘
q_1: ────────╫───
          ┌──╨──┐
c: 1/═════╡ 0x0 ╞
          └─────┘

         ┌───┐
q_0 -> 0Z ├
         └───┘
q_1 -> 1 ─────
              
    c: 1/═════           

What should happen?

The transpiled circuit should not have lost the second classically controlled gate.

Any suggestions?

Conditions currently aren't being accounted for in the "expected number of predecessors" calculation that determines whether a gate is eligible to be added to the circuit. The current _is_resolved check, since #7952, is:

self.applied_predecessors[node] == len(node.qargs) + len(node.cargs)

but conditions are not part of the cargs. Since the condition is equality to the known value, if the relevant node is not in the original front layer, its applied_predecessors (which correctly account for conditions) can be increased beyond the count of its qargs and cargs, so the condition never triggers.

(#7952 didn't cause this issue, it just didn't completely address the issue of all classical arguments.)

The fix is to account for all possible predecessors in SabreSwap._is_resolved.

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