OptimizeSwapBeforeMeasure only when last measurements #5906
Conversation
There was a problem hiding this comment.
I can't find any problem with this. Pulled the branch and played with it, verified that it solves the issue, and tests pass. But, someone more familiar with the transpiler should review, as well.
You might consider adding a comment or two. Especially since the method was completely rewritten and a couple of comments were lost.
There was a problem hiding this comment.
I agree with @jlapeyre that added comments would be helpful here. Also, the pass now rebuilds the entire circuit, rather than scanning and substituting only around swaps and measures, which would be more costly for large circuits. Can this be avoided?
| new_dag = DAGCircuit() | ||
| new_dag.metadata = dag.metadata | ||
| new_dag._global_phase = dag._global_phase | ||
| for creg in dag.cregs.values(): | ||
| new_dag.add_creg(creg) | ||
| for qreg in dag.qregs.values(): | ||
| new_dag.add_qreg(qreg) |
There was a problem hiding this comment.
| new_dag = DAGCircuit() | |
| new_dag.metadata = dag.metadata | |
| new_dag._global_phase = dag._global_phase | |
| for creg in dag.cregs.values(): | |
| new_dag.add_creg(creg) | |
| for qreg in dag.qregs.values(): | |
| new_dag.add_qreg(qreg) | |
| new_dag = dag._copy_circuit_metadata() |
There was a problem hiding this comment.
nice one. Why is _copy_circuit_metadata a private method?
There was a problem hiding this comment.
Initially because it was used internally, but then was useful to passes that needed to rebuild the dag (but preserve metadata) like the layout routing passes. Making it public seems like a good idea.
| for node in dag.topological_op_nodes(): | ||
| if node.type == 'op': | ||
| qargs = [_trivial_layout[_layout[qarg]] for qarg in node.qargs] | ||
| if isinstance(node.op, SwapGate): |
There was a problem hiding this comment.
Does this apply for all swaps or only those added by routing?
There was a problem hiding this comment.
does not distinguish from where is the SWAP coming from.
The logic of a local replacement was getting hard with more "looking ahead" conditions. Now conditions are more complex like a swap followed by a measurement (in both wires), followed by an out node. Things get even harder with #5890 on top of this one. Because I'm trying to partially preserve previous behaviour as an option in #5890, it will be possible to remove a swap mid circuit. This breaks the locality of the transformation (ie, following gates need to be moved to a different wire). I could try to keep the structure of the pass as it was, but that will result in an incompatible pass wrt to the current |
|
This appears to have been obsoleted by #11413, which fixed the final-measurement checker a bit more completely. I'll close this PR as obsolete now. |
Fixes #5905