qiskit icon indicating copy to clipboard operation
qiskit copied to clipboard

Appending circuit with ControlFlowOp fails

Open chriseclectic opened this issue 1 year ago • 1 comments

Environment

  • Qiskit version: 1.0.1
  • Python version: 3.11
  • Operating system:

What is happening?

Using the same teleport example in #12083 but now trying to flatten it into another circuit with append instead of compose gives different errors.

How can we reproduce the issue?

If I take the IfElseOp teleport circuit in #12083 and try and append it to another circuit like

qc2_flat = QuantumCircuit(3, 3)
qc2_flat.append(qc2, qc2_flat.qubits, qc2_flat.clbits)
qc2_flat

I get the error

NotImplementedError: IfElseOp cannot be classically controlled through Instruction.c_if. Please nest it in an IfElseOp instead.

If I use the SwitchOp circuit and decompose I get:

qc3_flat = QuantumCircuit(3, 3)
qc3_flat.append(qc3, qc3_flat.qubits, qc3_flat.clbits)
qc3_flat_dec = qc3_flat.decompose()
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[16], line 3
      1 qc3_flat = QuantumCircuit(3, 3)
      2 qc3_flat.append(qc3, qc3_flat.qubits, qc3_flat.clbits)
----> 3 qc3_flat_dec = qc3_flat.decompose()

File ~/mambaforge/envs/qiskit1/lib/python3.11/site-packages/qiskit/circuit/quantumcircuit.py:1651, in QuantumCircuit.decompose(self, gates_to_decompose, reps)
   1649 pass_ = Decompose(gates_to_decompose)
   1650 for _ in range(reps):
-> 1651     dag = pass_.run(dag)
   1652 # do not copy operations, this is done in the conversion with circuit_to_dag
   1653 return dag_to_circuit(dag, copy_operations=False)

File ~/mambaforge/envs/qiskit1/lib/python3.11/site-packages/qiskit/transpiler/passes/basis/decompose.py:64, in Decompose.run(self, dag)
     62             dag.substitute_node(node, rule[0].operation, inplace=True)
     63         else:
---> 64             decomposition = circuit_to_dag(node.op.definition)
     65             dag.substitute_node_with_dag(node, decomposition)
     67 return dag

File ~/mambaforge/envs/qiskit1/lib/python3.11/site-packages/qiskit/converters/circuit_to_dag.py:92, in circuit_to_dag(circuit, copy_operations, qubit_order, clbit_order)
     90     if copy_operations:
     91         op = copy.deepcopy(op)
---> 92     dagcircuit.apply_operation_back(op, instruction.qubits, instruction.clbits, check=False)
     94 dagcircuit.duration = circuit.duration
     95 dagcircuit.unit = circuit.unit

File ~/mambaforge/envs/qiskit1/lib/python3.11/site-packages/qiskit/dagcircuit/dagcircuit.py:682, in DAGCircuit.apply_operation_back(self, op, qargs, cargs, check)
    675 self._increment_op(op)
    677 # Add new in-edges from predecessors of the output nodes to the
    678 # operation node while deleting the old in-edges of the output nodes
    679 # and adding new edges from the operation node to each output node
    680 self._multi_graph.insert_node_on_in_edges_multiple(
    681     node._node_id,
--> 682     [self.output_map[bit]._node_id for bits in (qargs, all_cbits) for bit in bits],
    683 )
    684 return node

File ~/mambaforge/envs/qiskit1/lib/python3.11/site-packages/qiskit/dagcircuit/dagcircuit.py:682, in <listcomp>(.0)
    675 self._increment_op(op)
    677 # Add new in-edges from predecessors of the output nodes to the
    678 # operation node while deleting the old in-edges of the output nodes
    679 # and adding new edges from the operation node to each output node
    680 self._multi_graph.insert_node_on_in_edges_multiple(
    681     node._node_id,
--> 682     [self.output_map[bit]._node_id for bits in (qargs, all_cbits) for bit in bits],
    683 )
    684 return node

KeyError: Clbit(ClassicalRegister(2, 'aux'), 1)

What should happen?

The append should be able to add the instructions to the new circuit with correctly remapped registers.

Any suggestions?

No response

chriseclectic avatar Mar 26 '24 17:03 chriseclectic

So the main underlying reason for an error here is that the Instruction interface cannot represent control-flow operations, and for the time being is not supposed to be able to that. This is because the CircuitInstruction context object of an atom of QuantumCircuit does not pass well-typed classical data through any sort of "function call"; it only acts on qubits and clbits, not any sort of structure. We absolutely want to add the concept of "function call" to Qiskit, it's just a large body of work that we've not got all the pieces in a position to do yet. The most major one is that we need to move away from ClassicalRegister being used as typed classical data, because it's way too overloaded in meaning throughout hardware for us to be able to safely change it and how it's passed between instructions, and yet it also cannot represent near enough for everything that's useful for conditioning.

In the more immediate term, the way to move control-flow operations from one QuantumCircuit to another will only be by the instruction-inlining operation compose. Obviously with #12083, there's still a problem with that in this case - I'll try and go there and comment too.

Fundamentally, the problem here is not that circuit_to_instruction is failing, it's that it occasionally appears to work despite having silently broken Qiskit's data model. The correct thing for us to do is to loudly error and point users towards what is supported and its limitations. We ought to be raising a CircuitError if there are control-flow constructs in a circuit to be converted to an Instruction that involve registers, because they really just don't work. The only times they might appear to are when they're able to silently "close over" a register in the outer circuit, and something unrolls them directly without ever checking.

The Instruction.definition model is not properly able to support this, because there's no concept of "calling" an Instruction with a register, and the bits in the definition can be mapped to arbitrary bits on the outer circuits, which may already be in registers. Registers are de facto defined to be the output containers of a circuit on all hardware, so while Qiskit added support for register aliasing in the OpenQASM 3 model, this can't ever practically be used, because there's no support for it, so even if we were to change our data model to try and make it work, it'd not work for submission to hardware without user intervention anyway.


Next steps on this:

  • circuit_to_instruction should raise a proper error for attempts to do this, like it does for use of what will (eventually) replace registers completely, Var variables, but as written, the code in this error is supposed to raise an error.
  • we need better documentation on the circuit data model and how Instruction and control flow fit into it. We merged the first bits of that in #11904, and I'm writing a follow up with more detail at the moment, which will go live in 1.1
  • we need to make compose work better in cases where it should, and give clearer errors explaining user action needed when it doesn't.

jakelishman avatar May 13 '24 16:05 jakelishman