Suggestions for improvements to GatesInBasis

[yaelbh]

qiskit-ibm-runtime contains code to check if a circuit fits hardware requirements (aka layout and basis gates), very similar to qiskit's GatesInBasis pass.

By comparing the two pieces of code, the following suggestions emerge: 1. There is no need to update wire_map inside _visit_target. In fact wire_map doesn't have to be passed at all to _visit_target. Define qubit_map in the beginning of run and use it inside _visit_target.

2. Skip gates with calibrations (@jakelishman recommends to use dag.has_calibration_for).
3. Verify that the number of qubits in the circuit is at most the number of qubits in the device.

[jakelishman]

What makes you say it's not necessary to update wire_map inside _visit_target? That should be a fairly critical part of ensuring that control-flow operations are handled correctly.

[yaelbh]

The critical part is to define qubit_map. The updating only restricts the keys in the wire_map dictionary to the qubits inside the block.

[jakelishman]

That's not really the case - the updating does "restrict" it, but it also handles remapping from the Qubit instances that are used within a control flow's block back to the outer circuit. The Qubit instances used within a block are not guaranteed to be the same as the qubit instances used in the outermost circuit, especially if the circuit underwent layout selection earlier in the transpiler.

[yaelbh]

I see. In the circuit (not dag) implementation that we have in qiskit-ibm-runtime, the Qubit instances do seem to be stable throughout.

[jakelishman]

To illustrate, here's a valid QuantumCircuit:

from qiskit.circuit import QuantumCircuit, QuantumRegister, IfElseOp, library
from qiskit.transpiler import Target
from qiskit.transpiler.passes import GatesInBasis

true_body = QuantumCircuit(QuantumRegister(2, "inner"))
true_body.cx(0, 1)

circuit = QuantumCircuit(3, 3)
circuit.h(1)
circuit.measure(1, 1)
circuit.if_test((circuit.clbits[1], True), true_body, [0, 2], [])

target = Target()
target.add_instruction(library.HGate(), {(0,): None, (1,): None, (2,): None})
target.add_instruction(library.Measure(), {(0,): None, (1,): None, (2,): None})
target.add_instruction(library.CXGate(), {(0, 1): None, (1, 2): None})
target.add_instruction(IfElseOp, name="if_else")

pass_ = GatesInBasis(target=target)
pass_(circuit)
pass_.property_set["all_gates_in_basis"]

Note that the Qubit instances that the control-flow op is created with are not the same Qubit instances that are in the outer circuit. Here I'm doing things very manually to minimise the amount of things happening, but you can end up in a very similar situation if we run a layout pass before as well - actually, it's more insidious, because the instances might look like they're valid (if you're unlucky), but actually they refer to something totally different.

If we resolve the circuit qubits that true_body is applied to, it's asking for a cx on (outer) qubits (0, 2), which aren't in the Target. My GatesInBasis-based code will correctly detect that, but qiskit-ibm-runtime's is_isa_circuit will raise an exception. Worse, if I modify the circuit construction to:

true_body = QuantumCircuit(3)
true_body.cx(0, 1)

circuit = QuantumCircuit(3, 3)
circuit.h(1)
circuit.measure(1, 1)
circuit.if_test((circuit.clbits[1], True), true_body, [0, 2, 1], [])

(note that the if_test is not applied to ordered qubits in circuit) then GatesInBasis will correctly report that this is not all in the basis, but is_isa_circuit will say that it is, despite the circuit still calling for a cx on (0, 2), which isn't in the Target.