Unroll3qOrMore before layout allocation stage in the levels 0, 1, and 2 #7251
Conversation
| # 1. Decompose so only 1-qubit and 2-qubit gates remain | ||
| _unroll3q = [ | ||
| # Use unitary synthesis for basis aware decomposition of UnitaryGates | ||
| UnitarySynthesis( | ||
| basis_gates, | ||
| approximation_degree=approximation_degree, | ||
| coupling_map=coupling_map, |
There was a problem hiding this comment.
If we haven't laid out the qubits yet, what does the coupling map mean for the plugin? We could omit it, but then it starts to feel like we'll not following the calling conventions we laid out in the definition of the synthesis plugin.
There was a problem hiding this comment.
Yeah, this is a good point. If the circuit doesn't have a layout set this will be doing the wrong thing, because the synthesis pass assumes it's going to have the bits in physical order in the dag. I think we should set this to None if run prior to layout. None is still a valid value here to indicate there is no target coupling map set.
There was a problem hiding this comment.
Oh yeah, I forgot we still passed through explicit None on that one. Yeah, that seems like the best solution for now.
There was a problem hiding this comment.
If the circuit doesn't have a layout set this will be doing the wrong thing,
Any suggested test to add about this?
There was a problem hiding this comment.
It's a bit tricky to test for because it doesn't come up with the default/built-in unitary synthesis mechanism since for >=3q it uses isometry which isn't coupling map aware. For the default plugin this only comes up with <=2q unitaries. It's only for potential plugins like AQC or BQSKit where this would come up, since the synthesis pass sends bit indices to the plugin assuming their in physical order: https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/transpiler/passes/synthesis/unitary_synthesis.py#L272-L276 (ie post routing)
There was a problem hiding this comment.
You could set up a unittest.mock.MagicMock wrapper around the default synthesis plugin to catch all calls, then run the transpiler pass and assert that the first call has min_qubits=3 and coupling_map=None, perhaps? There's some examples of this ~~sort of test in test_unitary_synthesis_plugin.py, like here: https://github.com/Qiskit/qiskit-terra/blob/7e118e81344cb05aada5082e3cc71c1c239baaa5/test/python/transpiler/test_unitary_synthesis_plugin.py#L220-L235
There was a problem hiding this comment.
The testing is a bit artificial. Is it worthy?
Pull Request Test Coverage Report for Build 1521847541
💛 - Coveralls |
There was a problem hiding this comment.
This LGTM, we can skip a test since yeah we'd be just validating that we're passing None before layout and a coupling map after layout to the UnitarySynthesis pass and we already have test coverage that validates from inside the pass propagates to plugins so we're probably fine on coverage
Fixes #7156
Summary
Many layout methods ignore 3-or-more qubit gates resulting in expected layout allocation decisions. This PR runs
Unroll3qOrMorebefore any layout pass.