Flattened conditional circuits give incorrect results

[chriseclectic]

Information

• Qiskit Terra version:
• Python version:
• Operating system:

What is the current behavior?

Flattening a conditional circuit gives incorrect results. It looks like the issue is probably in how the conditionals are implemented:

Steps to reproduce the problem

Manually adding an additional registers to a teleport circuit works:

def teleport_circuit():
    """Teleport qubit 0 -> 2"""
    qr = QuantumRegister(3)
    c0 = ClassicalRegister(1)
    c1 = ClassicalRegister(1)
    teleport = QuantumCircuit(qr, c0, c1)
    teleport.h(qr[1])
    teleport.cx(qr[1], qr[2])
    teleport.cx(qr[0], qr[1])
    teleport.h(qr[0])
    teleport.measure(qr[0], c0[0])
    teleport.measure(qr[1], c1[0])
    teleport.z(qr[2]).c_if(c0, 1)
    teleport.x(qr[2]).c_if(c1, 1)
    return teleport

# Add extra register for measuring qubit-2
c2 = ClassicalRegister(1)
circ = teleport_circuit()
circ.add_register(c2)
circ.measure(2, c2)

# Run
counts = AerSimulator().run(circ).result().get_counts(0)
marginal_counts(counts, [2])

# Marginal Counts (correct)
{'0': 1024}

But composing and flattening gives wrong answer:

# Compose with larger circuit
circ = QuantumCircuit(3, 3)
circ = circ.compose(teleport_circuit(), range(3), range(2))
circ.measure(2, 2)

# Run
counts = AerSimulator().run(circ).result().get_counts(0)
marginal_counts(counts, [2])

# Marginal Counts (incorrect)
{'1': 228, '0': 796}

What is the expected behavior?

Flattening the conditional circuit should implement the same circuit.

Suggested solutions

From the drawer it looks liek the flattened circuit is handling conditions on the joint circuit wrong:

                    ┌───┐┌─┐              
q_0: ────────────■──┤ H ├┤M├──────────────
     ┌───┐     ┌─┴─┐└┬─┬┘└╥┘              
q_1: ┤ H ├──■──┤ X ├─┤M├──╫───────────────
     └───┘┌─┴─┐└───┘ └╥┘  ║  ┌───┐  ┌───┐ 
q_2: ─────┤ X ├───────╫───╫──┤ Z ├──┤ X ├─
          └───┘       ║   ║  └─╥─┘  └─╥─┘ 
                      ║   ║ ┌──╨──┐┌──╨──┐
c: 3/═════════════════╩═══╩═╡ = 1 ╞╡ = 2 ╞
                      1   0 └─────┘└─────┘

Looks like conditions are triggering on creg values 01 and 10, but nothing on 11 which should trigger both conditional gates.

Looking at the qobj that gets assembled from these circuits we can see the difference:

Multi-register assembled qobj instructions:
assemble(circ).experiments[0].instructions =

[QasmQobjInstruction(name='h', qubits=[1]),
 QasmQobjInstruction(name='cx', qubits=[1, 2]),
 QasmQobjInstruction(name='cx', qubits=[0, 1]),
 QasmQobjInstruction(name='h', qubits=[0]),
 QasmQobjInstruction(name='measure', qubits=[0], register=[0], memory=[0]),
 QasmQobjInstruction(name='measure', qubits=[1], register=[1], memory=[1]),
 QasmQobjInstruction(name='bfunc', register=3, mask="0x1", relation="==", val="0x1"),
 QasmQobjInstruction(name='z', qubits=[2], conditional=3),
 QasmQobjInstruction(name='bfunc', register=4, mask="0x2", relation="==", val="0x2"),
 QasmQobjInstruction(name='x', qubits=[2], conditional=4),
 QasmQobjInstruction(name='measure', qubits=[2], register=[2], memory=[2])]

Flattened circuit assembled qobj instructions:

[QasmQobjInstruction(name='h', qubits=[1]),
 QasmQobjInstruction(name='cx', qubits=[1, 2]),
 QasmQobjInstruction(name='cx', qubits=[0, 1]),
 QasmQobjInstruction(name='h', qubits=[0]),
 QasmQobjInstruction(name='measure', qubits=[0], register=[0], memory=[0]),
 QasmQobjInstruction(name='measure', qubits=[1], register=[1], memory=[1]),
 QasmQobjInstruction(name='bfunc', register=3, mask="0x7", relation="==", val="0x1"),
 QasmQobjInstruction(name='z', qubits=[2], conditional=3),
 QasmQobjInstruction(name='bfunc', register=4, mask="0x7", relation="==", val="0x2"),
 QasmQobjInstruction(name='x', qubits=[2], conditional=4),
 QasmQobjInstruction(name='measure', qubits=[2], register=[2], memory=[2])]

You can see that in the second case the mask value is being set incorrectly to 0x7 = 0b111 in both case to the full 3-bit register rather than the the single bit values.

[chriseclectic]

Follow up building with a flat circuit this way looks to give correct result:

teleport = QuantumCircuit(3, 2)
teleport.h(1)
teleport.cx(1, 2)
teleport.cx(0, 1)
teleport.h(0)
teleport.measure(0, 0)
teleport.measure(1, 1)
creg = teleport.cregs[0]
teleport.z(2).c_if(creg[0], 1)
teleport.x(2).c_if(creg[1], 1)

# Compose with another circuit
circ = QuantumCircuit(3, 3)
circ = circ.compose(teleport, range(3), range(2))
circ.measure(2, 2)

assembled qobj

[QasmQobjInstruction(name='h', qubits=[1]),
 QasmQobjInstruction(name='cx', qubits=[1, 2]),
 QasmQobjInstruction(name='cx', qubits=[0, 1]),
 QasmQobjInstruction(name='h', qubits=[0]),
 QasmQobjInstruction(name='measure', qubits=[0], register=[0], memory=[0]),
 QasmQobjInstruction(name='measure', qubits=[1], register=[1], memory=[1]),
 QasmQobjInstruction(name='bfunc', register=3, mask="0x1", relation="==", val="0x1"),
 QasmQobjInstruction(name='z', qubits=[2], conditional=3),
 QasmQobjInstruction(name='bfunc', register=4, mask="0x2", relation="==", val="0x2"),
 QasmQobjInstruction(name='x', qubits=[2], conditional=4),
 QasmQobjInstruction(name='measure', qubits=[2], register=[2], memory=[2])]

However this circuit breaks the drawer:

circ.draw()

Stack trace:

------------------------------------------------
KeyError       Traceback (most recent call last)
<ipython-input-74-d2648e69e6a7> in <module>
----> 1 transpile(circ).draw()

~/git/qiskit/qiskit-terra/qiskit/circuit/quantumcircuit.py in draw(self, output, scale, filename, style, interactive, plot_barriers, reverse_bits, justify, vertical_compression, idle_wires, with_layout, fold, ax, initial_state, cregbundle)
   1663             ax=ax,
   1664             initial_state=initial_state,
-> 1665             cregbundle=cregbundle,
   1666         )
   1667 

~/git/qiskit/qiskit-terra/qiskit/visualization/circuit_visualization.py in circuit_drawer(circuit, scale, filename, style, output, interactive, plot_barriers, reverse_bits, justify, vertical_compression, idle_wires, with_layout, fold, ax, initial_state, cregbundle)
    205             fold=fold,
    206             initial_state=initial_state,
--> 207             cregbundle=cregbundle,
    208         )
    209     elif output == "latex":

~/git/qiskit/qiskit-terra/qiskit/visualization/circuit_visualization.py in _text_circuit_drawer(circuit, filename, reverse_bits, plot_barriers, justify, vertical_compression, idle_wires, with_layout, fold, initial_state, cregbundle, encoding)
    308     """
    309     qubits, clbits, ops = utils._get_layered_instructions(
--> 310         circuit, reverse_bits=reverse_bits, justify=justify, idle_wires=idle_wires
    311     )
    312 

~/git/qiskit/qiskit-terra/qiskit/visualization/utils.py in _get_layered_instructions(circuit, reverse_bits, justify, idle_wires)
    122             ops.append([node])
    123     else:
--> 124         ops = _LayerSpooler(dag, justify, measure_map, reverse_bits)
    125 
    126     if reverse_bits:

~/git/qiskit/qiskit-terra/qiskit/visualization/utils.py in __init__(self, dag, justification, measure_map, reverse_bits)
    207                 dag_nodes = _sorted_nodes(dag_layer)
    208                 for node in dag_nodes:
--> 209                     self.add(node, current_index)
    210         else:
    211             dag_layers = []

~/git/qiskit/qiskit-terra/qiskit/visualization/utils.py in add(self, node, index)
    325         """Add 'node' where it belongs, starting the try at 'index'."""
    326         if self.justification == "left":
--> 327             self.slide_from_left(node, index)
    328         else:
    329             self.slide_from_right(node, index)

~/git/qiskit/qiskit-terra/qiskit/visualization/utils.py in slide_from_left(self, node, index)
    249             index_stop = -1
    250             if node.op.condition:
--> 251                 index_stop = self.measure_map[node.op.condition[0]]
    252             elif node.cargs:
    253                 for carg in node.cargs:

KeyError: Clbit(ClassicalRegister(3, 'c'), 0)

[kdk]

I believe this is an issue in QuantumCircuit.compose failing to map the condition correctly from the two single-bit cregs in the input circuit onto the one larger creg on the output circuit. Before #6018 , it wouldn't have been possible to map these conditions and I would've expected .compose to raise similar to how circuit_to_instruction(teleport_circuit()) currently raises: "Cannot convert condition in circuit with multiple classical registers to instruction".

After #6018 , this should at least be possible for the case where gates are conditioned on a single Clbit. @TharrmashasthaPV , do you have an idea how this might look?

The drawing error I believe is fixed by #6261 .

[TharrmashasthaPV]

I think that this trouble is not just with single-bit cregs. For instance take the following circuit

q = QuantumRegister(2)
c = ClassicalRegister(2)
qc = QuantumCircuit(q, c)
qc.h(0).c_if(c, 1)

circ = QuantumCircuit(3,3)
circ = circ.compose(qc, [0,1], [1,2])

This circuit outputs assemble(circ).experiments[0].instructions as

[QasmQobjInstruction(name='bfunc', register=3, mask="0x7", relation="==", val="0x2"), 
QasmQobjInstruction(name='h', qubits=[0], conditional=3)]

This error seems to happen whenever a circuit qc containing a creg, say c, is composed to a circuit that contains a single creg whose size is larger than that of c.

As for the conditioning on single Clbit, currently a type check is performed to see if the condition is on a ClassicalRegister or Clbit before the condition is mapped to the new_cregs. I guess that's why teleport.z(2).c_if(c0, 1) returns a circuit with an incorrect mapping while teleport.z(2).c_if(creg[0], 1) outputs a correct circuit.

However, we could extend the type checking to also check if the conditioned register is of length 1 and then if the length was 1, then mapping of condition could proceed similar to that done for a Clbit condition. Something like

if isinstance(condition[0], Clbit):
    cond_creg = [condition[0]]
elif len(list(condition[0]))==1:
    cond_creg = list(condition[0])
else:
    cond_creg = condition[0]
    is_reg = True
...

in line 584 in the _map_condition() in DAGCircuit class should do the job I guess.

[jakelishman]

The drawer issue is now fixed by #7285 (#6261 didn't appear to get us all the way when I tested it just now).

For the rest: the conditions issue could now be solved by adding additional classical registers to the circuit after composition, that cover the specific bits we need - now that a bit is the fundamental data unit, and registers are just aliases to collections of bits, we can add what we need to make the condition work. In practice, though, assemble won't be able to cope with the output of that (because QasmQobj can't represent it at all), so it's something we might want to add a flag for in compose. We could have it throw an error by default if it detects a situation that can't be represented by single-bit conditions or non-overlapping classical registers, but add a flag which enables the behaviour to simply add aliasing classical registers whenever required to make all conditions work.

[chriseclectic]

Bumping this again because its still an issue (see qiskit-community/qiskit-experiments#943 for more details).

The core issue seems to be if I have circuit1 with multiple registers and conditionals on them, and then i initialize circuit2 with a single flat classical register and compose circuit1 into it, the conditionals get expanded to conditionals of the full classical register of circuit2, but using only the bit value of the original register expanded with zeros across the other bits. What should happen is it should trigger regardless of values on the other bits, not just when they are zero.

Eg to use the teleport example:

from qiskit import QuantumCircuit, QuantumRegister, ClassicalRegister
qr = QuantumRegister(3, 'qr')
c0 = ClassicalRegister(1, 'c0')
c1 = ClassicalRegister(1, 'c1')
circuit1 = QuantumCircuit(qr, c0, c1)
circuit1.h(1)
circuit1.cx(1, 2)
circuit1.cx(0, 1)
circuit1.h(0)
circuit1.measure(0, c0)
circuit1.measure(1, c1)
circuit1.z(2).c_if(c0, 1)
circuit1.x(2).c_if(c1, 1)
circuit1.draw()

                     ┌───┐┌─┐          
qr_0: ────────────■──┤ H ├┤M├──────────
      ┌───┐     ┌─┴─┐└┬─┬┘└╥┘          
qr_1: ┤ H ├──■──┤ X ├─┤M├──╫───────────
      └───┘┌─┴─┐└───┘ └╥┘  ║ ┌───┐┌───┐
qr_2: ─────┤ X ├───────╫───╫─┤ Z ├┤ X ├
           └───┘       ║   ║ └─╥─┘└─╥─┘
  c0: ═════════════════╬═══╩═══■════╬══
                       ║      0x1   ║  
  c1: ═════════════════╩════════════■══
                                   0x1 

Composing with a flat register

circuit2 = QuantumCircuit(3, 2)
circuit2.compose(circuit1, inplace=True)
circuit2.draw()

gives

circuit2 = QuantumCircuit(3, 2)

circuit2.compose(circuit1, inplace=True)

circuit2.draw()

                    ┌───┐┌─┐          
q_0: ────────────■──┤ H ├┤M├──────────
     ┌───┐     ┌─┴─┐└┬─┬┘└╥┘          
q_1: ┤ H ├──■──┤ X ├─┤M├──╫───────────
     └───┘┌─┴─┐└───┘ └╥┘  ║ ┌───┐┌───┐
q_2: ─────┤ X ├───────╫───╫─┤ Z ├┤ X ├
          └───┘       ║   ║ └─╥─┘└─╥─┘
c_0: ═════════════════╬═══╩═══■════o══
                      ║       ║    ║  
c_1: ═════════════════╩═══════o════■══
                             0x1  0x2 

Composing with an enlarged register

circuit3 = QuantumCircuit(3, 4)
circuit3.compose(circuit1, clbits=[0, 1], inplace=True)
circuit3.draw()

gives

                    ┌───┐┌─┐          
q_0: ────────────■──┤ H ├┤M├──────────
     ┌───┐     ┌─┴─┐└┬─┬┘└╥┘          
q_1: ┤ H ├──■──┤ X ├─┤M├──╫───────────
     └───┘┌─┴─┐└───┘ └╥┘  ║ ┌───┐┌───┐
q_2: ─────┤ X ├───────╫───╫─┤ Z ├┤ X ├
          └───┘       ║   ║ └─╥─┘└─╥─┘
c_0: ═════════════════╬═══╩═══■════o══
                      ║       ║    ║  
c_1: ═════════════════╩═══════o════■══
                              ║    ║  
c_2: ═════════════════════════o════o══
                              ║    ║  
c_3: ═════════════════════════o════o══
                             0x1  0x2 

[jakelishman]

I will prepare a fix for this for Terra 0.23. There's other stuff in compose that's likely to cause issues as well, and I'll try and get it all in one thump.

[jakelishman]

I've made #9206 that should fix this. Just to note: the drawers don't handle overlapping registers correctly, so it may well still look like there's an issue, but if you inspect the objects to look in their conditions, registers and the bits in those conditions, you should find it's correct.

I've checked that the Aer simulations at the top work as expected with them, but I can't make any promises about how well it'll serialise to actual backends - I'm not sure how well the aliasing register model will transport.