Deprecate Instruction.condition in favor of IfElseOp

[kdk]

What should we add?

Instruction.condition is Qiskit's way of representing classical execution of instructions based on the values in specified classical registers. However, due to it's location as a property on Instruction (but outside params) has historically lead to it being overlooked, resulting in a handful of bugs where this condition information is either lost or not considered e.g. when evaluating instruction order (Some examples: #6475, #6810, #8553, #8040, #7950, #7247, #7006, #6994, #6583, #6192, #6016, #4727, #4672, #3164, #3215, #2667, #2571 .) Additionally, it is one of the pieces of mutable state that is currently preventing us from moving toward more lightweight Operation instances ( to resolve e.g. #3800, #5895, #7624, and #9372 ).

IfElseOp can currently represent a superset of what can be expressed with Instruction.condition and is more inline with how we expect to handle conditional and control flow operations going forward, so we should plan to deprecate Instruction.condition with equivalent usage of IfElseOp. This issue is to discuss and plan that process.

Deprecating Instruction.conditionwhile maintaining full backwards compatibility seems difficult (due to both a change in Operation type from Instruction to IfElseOp, and due to .condition being a property of the Instruction without context of the QuantumCircuit in which it is included). The following are two use cases to consider:

>>> qc = qk.QuantumCircuit(2,1)
>>> qc.h(0)
>>> qc.measure(0,0)
>>> qc.x(1).c_if(0, 0)

qc = qk.QuantumCircuit(1, 1)
qc.x(0)
qc.data[0][0].condition = (qc.clbits[0], 1)

Additionally, we should:
1)Updating existing visualization to inspect IfElseOp operations and, when possible, draw as an equivalent condition
2)Updating qiskit.assemble, similarly to visualization, to replace applicable IfElseOp s with their equivalent condition representation

[mtreinish]

This depends on #9417 and is blocked until it is implemented.

[jakelishman]

In practice, the routing performance for conditioned single 1q and 2q blocks is currently very different compared to the .condition path as well, because of the way we do routing. Explicitly: for a .condition gate, we will apply layout transformations and insert swaps as if the 1q/2q gate is a completely standard gate, but in the IfElseOp form, any routing required for the layout will happen conditionally inside the If and then need to be undone within the same block. Given that the chance that a condition is true (by a very naive estimate based on us doing exact numeric equality) is less than 50%, this might be the preferred way, but the eager "stop the world and consider only this gate" approach we take right now is likely compromising parallel routing quality.

[mtreinish]

TBH, I'm personally less concerned about that for this issue at least. It's an optimization/fix we can work on in parallel and also do post-deprecation. For me, to deprecate c_if/condition we need to be able to functionally do all the same stuff with IfElseOp (which offers a superset of functionality). The only big missing piece is #9417 I think.

[jakelishman]

Agreed - I'm mostly trying to get these thoughts written down. Kevin and I had a meeting where that routing quality was a concern, so it'll affect some users, but it's likely lower priority than getting the Instruction.condition mutable field gone.

[kdk]

I think this can be taken off 'on hold' since the work for #9417 is in flight, and doesn't strictly block the development of this issue, only its release.

[mtreinish]

I looked at this today and I think it's too soon for 0.25 especially with the proposal freeze so close. Besides being blocked on #9417 actually merging (which there are enough moving pieces in progress still that it'll be tight to fit in the release). The actual mechanics of this are more involved than I think is reasonable in the time frame for 0.25, mostly around testing. There are 7685 tests that fail locally because of the deprecation warnings. These will all need to be updated to catch the deprecation warnings. If we want to do this I think the better time frame is to do this early in 0.26/0.45 to ensure we have enough debug time for any issues that will inevitably come up from the deprecation especially for something as core as c_if/condition.