Port ConsolidateBlocks to Rust
#12250
Assignees
Labels
mod: transpiler
Issues and PRs related to Transpiler
performance
Rust
This PR or issue is related to Rust code in the repository
Milestone
Comments
mtreinish
added
performance
Rust
|
Related to #11659. |
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this issue
Oct 24, 2024
This commit ports the consolidate blocks pass to rust. The logic remains the same and this is just a straight porting. One optimization is that to remove the amount of python processing the Collect2qBlocks pass is no longer run as part of the preset pass managers and this is called directly in rust. This speeds up the pass because it avoids 3 crossing of the language boundary and also the intermediate creation of DAGNode python objects. The pass still supports running with Collect2qBlocks for backwards compatibility and will skip running the pass equivalent internally the field is present in the property set. There are potential improvements that can be investigated here such as avoiding in place dag contraction and moving to rebuilding the dag iteratively. Also changing the logic around estimated error (see Qiskit#11659) to be more robust. But these can be left for follow up PRs as they change the logic. Realistically we should look at combining ConsolidateBlocks for it's current two usages with Split2qUnitaries and UnitarySynthesis into those passes for more efficiency. We can improve the performance and logic as part of that refactor. See Qiskit#12007 for more details on this for UnitarySynthesis. Closes Qiskit#12250
3 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 6, 2024
* Oxidize the ConsolidateBlocks pass This commit ports the consolidate blocks pass to rust. The logic remains the same and this is just a straight porting. One optimization is that to remove the amount of python processing the Collect2qBlocks pass is no longer run as part of the preset pass managers and this is called directly in rust. This speeds up the pass because it avoids 3 crossing of the language boundary and also the intermediate creation of DAGNode python objects. The pass still supports running with Collect2qBlocks for backwards compatibility and will skip running the pass equivalent internally the field is present in the property set. There are potential improvements that can be investigated here such as avoiding in place dag contraction and moving to rebuilding the dag iteratively. Also changing the logic around estimated error (see #11659) to be more robust. But these can be left for follow up PRs as they change the logic. Realistically we should look at combining ConsolidateBlocks for it's current two usages with Split2qUnitaries and UnitarySynthesis into those passes for more efficiency. We can improve the performance and logic as part of that refactor. See #12007 for more details on this for UnitarySynthesis. Closes #12250 * Update test to count consolidate_blocks instead of collect_2q_blocks * Fix lint * Fix solovay kitaev test * Add release note * Restore 2q block collection for synthesis translation plugin * Add rust native substitute method * Fix final test failures * Remove release note and test change The test failure fixed by a test change was incorrect and masked a logic bug that was fixed in a subsequent commit. This commit reverts that change to the test and removes the release note attempting to document a fix for a bug that only existed during development of this PR. * Fix comment leftover from rust-analyzer * Remove unused code * Simplify control flow handling * Remove unnecessary clone from substitute_node * Preallocate block op names in replace_block_with_py_op * Remove more unused imports * Optimize linalg in block collection This commit reworks the logic to reduce the number of Kronecker products and 2q matrix multiplications we do as part of computing the unitary of the block. It now computes the 1q components individually with 1q matrix multiplications and only calls kron() and a 2q matmul when a 2q gate is encountered. This reduces the number of more expensive operations we need to perform and replaces them with a much faster 1q matmul. * Use static one qubit identity matrix * Remove unnecessary lifetime annotations * Add missing docstring to new rust method * Apply suggestions from code review Co-authored-by: Kevin Hartman <[email protected]> * Fix lint * Add comment for MAX_2Q_DEPTH constant * Reuse block_qargs for each block Co-authored-by: Henry Zou <[email protected]> --------- Co-authored-by: Kevin Hartman <[email protected]> Co-authored-by: Henry Zou <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The core linalg for this pass are already in Rust, but once we have #11721 and #12205 we should move the entire contents of the pass to Rust.
The text was updated successfully, but these errors were encountered: