Use nalgebra matrices for uncommon paths in consolidate blocks#15881
Use nalgebra matrices for uncommon paths in consolidate blocks#15881Cryoris merged 1 commit intoQiskit:mainfrom
Conversation
In the recently merged Qiskit#15871 we updated the consolidate blocks pass to use Matrix4 in the common path of a 2q block being consolidated. This is like > 90% of what the pass does when run in the preset pass manager. However, there were uncommon cases in the pass around the handling of blocks of a single gate that are outside of the target which were not updated to use nalgebra arrays if it's a fixed size 1q or 2q gate. This commit updates these uncommon paths so that we're always returning an nalgebra matrix in the output UnitaryGate if the block being consolidated is a single qubit or two qubits.
mtreinish
added
Changelog: None
|
One or more of the following people are relevant to this code:
|
| // Runs are necessarily single qubit gates so if it has a matrix then it | ||
| // must have a single qubit matrix or it's not a run and the blocks handling above | ||
| // would have covered it | ||
| let matrix = match get_1q_matrix_from_inst(first_inst) { |
There was a problem hiding this comment.
Is it properly documented somewhere that runs are 1-q runs only? I can see it from digging in the code, but from looking at this function only it wasn't obvious -- in case it's not documented, could we add this to this function?
There was a problem hiding this comment.
It's behavior from the passes that populate the fields in the property sets. Collect1qRuns collects "runs" which are 1q, while Collect2qBlocks collected "blocks" which are 2q. Functionally in practice we don't use either of these passes anymore as if neither is set ConsolidateBlocks just finds the 2q "blocks" itself and we don't run a separate analysis pass. This code path is just for backwards compatibility for people running the analysis passes first in a custom pass manager.
I expect it's not documented explicitly though. I can push up a separate documentation PR for this, but I don't view it as in scope for this PR which is only touching the pass's deep internals.
Cryoris
left a comment
Cryoris
left a comment
There was a problem hiding this comment.
Just one comment on documentation, otherwise LGTM 🙂
…t#15881) In the recently merged Qiskit#15871 we updated the consolidate blocks pass to use Matrix4 in the common path of a 2q block being consolidated. This is like > 90% of what the pass does when run in the preset pass manager. However, there were uncommon cases in the pass around the handling of blocks of a single gate that are outside of the target which were not updated to use nalgebra arrays if it's a fixed size 1q or 2q gate. This commit updates these uncommon paths so that we're always returning an nalgebra matrix in the output UnitaryGate if the block being consolidated is a single qubit or two qubits.
4 participants
Summary
In the recently merged #15871 we updated the consolidate blocks pass to use Matrix4 in the common path of a 2q block being consolidated. This is like > 90% of what the pass does when run in the preset pass manager. However, there were uncommon cases in the pass around the handling of blocks of a single gate that are outside of the target which were not updated to use nalgebra arrays if it's a fixed size 1q or 2q gate. This commit updates these uncommon paths so that we're always returning an nalgebra matrix in the output UnitaryGate if the block being consolidated is a single qubit or two qubits.
Details and comments