Add ContractIdleWiresInControlFlow optimisation pass
#13779
Conversation
jakelishman
added
on hold
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 13530225985Details
💛 - Coveralls |
This transpiler pass removes data dependencies on idle qubits from
control-flow operations. For example, given a circuit such as::
from qiskit.circuit import QuantumCircuit
qc = QuantumCircuit(1, 1)
qc.x(0)
with qc.if_test((qc.clbits[0], True)):
qc.x(0)
qc.x(0)
qc.x(0)
the current optimisation passes will collapse the inner control-flow
block to the identity, but the qubit dependency will remain, preventing
the outer two X gates from being cancelled. This pass removes the
now-spurious dependency, making it possible to detect and remove the two
X gates in a follow-up loop iteration.
As an accidental side-effect of their algorithms, the control-flow-aware
routing passes currently do this when they run. This aims to move the
logic into a suitable place to run before routing (so the spurious
dependency never arises in routing in the first place) and in the
low-level optimisation stage.
The aim of this pass is also to centralise the logic, so when the
addition of the new `box` scope with different semantics around whether
a wire is truly idle in the box or not, the routers aren't accidentally
breaking them, and it's clearer when the modifications happen.
jakelishman
force-pushed
the
control-flow/strip-data-dependency
branch
from
c3ba4bf to
ff133cf
Compare
|
Rebased over main and no longer on hold. |
|
|
||
| def run(self, dag): | ||
| # `control_flow_op_nodes` is eager and doesn't borrow; we're mutating the DAG in the loop. | ||
| for node in dag.control_flow_op_nodes() or []: |
There was a problem hiding this comment.
You'll probably remember, but adding a note here to remove the or [] once it is no longer needed.
There was a problem hiding this comment.
Good to highlight thanks, because it's now been long enough that I probably would have forgotten to do it haha.
| replacement.add_qubits(list(qubits)) | ||
| replacement.add_clbits(list(clbits)) | ||
| for var in vars_: | ||
| replacement.add_captured_var(var) |
There was a problem hiding this comment.
Do we need to add local variables too?
There was a problem hiding this comment.
In the replacement they're all "capturing" from the circuit they're about to be inlined into - the outer circuit defines the true "type" of them.
| replacement._apply_op_node_back(DAGOpNode.from_instruction(new_inst)) | ||
| # The replacement DAG is defined over all the same qubits, but with the correct | ||
| # qubits now explicitly marked as idle, so everything gets linked up correctly. | ||
| dag.substitute_node_with_dag( |
There was a problem hiding this comment.
I was a bit confused at first reading through this until I realized that replacement is a temporary DAG that gets its contents unwrapped / flattened into dag. Do we not have a simpler way to replace a node with another node in DAGCircuit? Presumably there's more going on here than I'm understanding!
There was a problem hiding this comment.
The trick is around the node contraction - substitute_node_with_dag has enough information available to it that it can link the now-unused wires together. We have substitute_node, but it requires that the two nodes act on the same set of wires.
There was a problem hiding this comment.
Ah, thanks for clarifying 🙂.
| qc = QuantumCircuit(3, 1) | ||
| with qc.while_loop((qc.clbits[0], False)): | ||
| qc.cx(0, 1) | ||
| qc.noop(2) |
There was a problem hiding this comment.
Certainly beyond the scope of this PR, but I find it a bit strange that a noop doesn't add an operation (I did not see the introductory PR for it). I was surprised to see that there's no noop in the expected circuit of this test, and thought I may have missed something in this new pass where they get removed.
There was a problem hiding this comment.
The entire point of noop is not to add an operation haha. id is the mathematical expression of "unitary identity", whereas noop is "mark this qubit as used, but do explicitly do not do anything with it".
There was a problem hiding this comment.
Heh, I suppose I'm thinking of it more like a noop instruction in classical computing, which does end up getting executed by the CPU and takes some number of cycles to do nothing.
There was a problem hiding this comment.
It kind of does here too, but qubit operations are all inherently super parallel. Putting a noop on a qubit to bring it into a box causes it to idle for the duration of the box (unless you apply other things to it).
|
I did this in Python because it was faster and cleaner to write for the initial implementation, and there won't be any appreciable slowdown because all the logic is Python-space anyway - control-flow ops are still represented by Python-space objects. The loop |
qiskit/transpiler/passes/optimization/contract_idle_wires_in_control_flow.py
Outdated
Show resolved
Hide resolved
Summary
This transpiler pass removes data dependencies on idle qubits from control-flow operations. For example, given a circuit such as::
the current optimisation passes will collapse the inner control-flow block to the identity, but the qubit dependency will remain, preventing the outer two X gates from being cancelled. This pass removes the now-spurious dependency, making it possible to detect and remove the two X gates in a follow-up loop iteration.
As an accidental side-effect of their algorithms, the control-flow-aware routing passes currently do this when they run. This aims to move the logic into a suitable place to run before routing (so the spurious dependency never arises in routing in the first place) and in the low-level optimisation stage.
The aim of this pass is also to centralise the logic, so when the addition of the new
boxscope with different semantics around whether a wire is truly idle in the box or not, the routers aren't accidentally breaking them, and it's clearer when the modifications happen.Details and comments
Built on top of #13774, since it's very convenient to have that method for writing the tests. This is one part of #13768.
I haven't hooked this up to the preset pass-managers in this commit; I wanted to do that after #13768 is fixed properly and potentially once
boxitself has landed.