Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
4.9k stars 2.3k forks source link

The transpiler does not re-map quantum registers for control-flow blocks #9332

Open taalexander opened 1 year ago

taalexander commented 1 year ago

Environment

What is happening?

The transpiler is not remapping virtual register to physical registers correctly in the transpiler.

How can we reproduce the issue?

    from qiskit import transpile
    from qiskit.dagcircuit import DAGCircuit, DAGOpNode
    from qiskit.circuit import ClassicalRegister, QuantumCircuit, QuantumRegister, ControlFlowOp
    from qiskit.transpiler.passmanager import PassManager
    from qiskit.converters.circuit_to_dag import circuit_to_dag
    from qiskit.providers.fake_provider.backends.jakarta.fake_jakarta import FakeJakarta

    backend = FakeJakarta()

    # Temporary workaround for mock backends. For real backends this is not required.
    backend.configuration().basis_gates.append("if_else")

    qr = QuantumRegister(3)
    crz = ClassicalRegister(1, name="crz")
    crx = ClassicalRegister(1, name="crx")
    result = ClassicalRegister(1, name="result")

    teleport = QuantumCircuit(qr, crz, crx, result, name="Teleport")

    teleport.h(qr[1])
    teleport.cx(qr[1], qr[2])
    teleport.cx(qr[0], qr[1])
    teleport.h(qr[0])
    teleport.measure(qr[0], crz)
    teleport.measure(qr[1], crx)
    with teleport.if_test((crz, 1)):
        teleport.z(qr[2])
    with teleport.if_test((crx, 1)):
        teleport.x(qr[2])
    teleport.measure(qr[2], result)

    teleport = transpile(teleport, backend)
    dag = circuit_to_dag(teleport)
    for node in dag.nodes():
        if isinstance(node, DAGOpNode) and isinstance(node.op, ControlFlowOp):
            for node in circuit_to_dag(node.op.blocks[0]).nodes():
                try:
                    print(node.qargs)
                except Exception:
                    pass

Yields

(Qubit(QuantumRegister(3, 'q63'), 2),)
(Qubit(QuantumRegister(3, 'q63'), 2),)

What should happen?

I would expect the physical qubits to be used (which are used in the top-level nodes)

(Qubit(QuantumRegister(7, 'q'), 2),)
(Qubit(QuantumRegister(7, 'q'), 2),)

Any suggestions?

Propagate the register updates to child blocks of control flow operations.

jakelishman commented 1 year ago

Copying over some offline discussion on this topic:

The current model is that consumers of ControlFlowOp need to track and apply the qubit mappings themselves as they recurse in, so there is a workaround - for example, here in the VF2Layout passes: https://github.com/Qiskit/qiskit-terra/blob/396b06c8a1fbb37692dd9b12f7b7bb6c322f1e93/qiskit/transpiler/passes/layout/vf2_utils.py#L45-L52.

At the moment, the form is consistent with how gate definitions have to be handled within the transpiler (the definition uses its own qubits, and it’s up to the pass with context to apply the binding of circuit qubits to gate arguments). Control flow is very shoe-horned into the existing “instruction” structure, so by default it’s just subject to the same rules.

Within the current structure, if we do normalise the qubits inside the control-flow ops, there’s a data redundancy between the qubits and clbits arguments of the CircuitInstruction context object, and the qubits and clbits fields of the internal structure, which increases the possibility of way more subtle bugs where some passes use one source and some use the other (and they’re out-of-sync in the ordering). A minor positive of the current state of affairs is that things fail noisily if a pass doesn’t done the binding correctly.

That said, I fully agree that the current form sucks for actual use in the transpiler.

Kevin and I have played around with alternate data structures for the transpiler a couple of times late last year, because there’s a need to empower DAGCircuit with something more now that there’s control flow present as well, but it’s hard to match the data-flow requirements we want for efficient qubit routing with the control-flow requirements we need for the classical components.

If we apply the split into a control-flow basic-block structure too early, we potentially make it near-impossible to perform a good qubit layout+routing (which is a massive chunk of getting good performance at a high level), but the later we leave it, the more components need to be explicitly aware of all the complexities of the implementation of the classical control flow (which is the current situation).