Switch TwoQubitWeylDecomposition to use nalgebra internally#15960
Open
mtreinish wants to merge 3 commits intoQiskit:mainfrom
Open
Switch TwoQubitWeylDecomposition to use nalgebra internally#15960mtreinish wants to merge 3 commits intoQiskit:mainfrom
mtreinish wants to merge 3 commits intoQiskit:mainfrom
Conversation
This commit moves to using Matrix2 as the array type used internally for the TwoQubitBasisDecomposer. Matrix2 is a fixed size stack allocated matrix type that has several performance advantages especially for matmul because the compiler can reason about a fixed number of operations and better optimize the implementation. Similarly we avoid a lot of heap allocations. This will improve the runtime performance of the two qubit basis decomposer. This is part of the ongoing effort to move to using nalgebra's fixed size matrix types Matrix4 and Matrix2 inside all of the two qubit decomposer paths. We will still use faer for the involved linear algebra such as eigenvalue decomposition where it is faster and more numerically stable. This doesn't get us all the way to this goal, it's just another step on the journey. There are still places in the module that are using ndarray as the array types, this is mostly because they're used with either the Weyl decomposition or the one qubit euler decomposition. In particular there are a couple of duplicate methods either prefixed or postfixed with nalgebra to either return or convert to/from an nalgebra object which are temporary while we're in the middle of the transition. The goal is to remove these as we migrate the rest of the two qubit decomposers to be using nalgebra for the storage type.
This is the next step towards using nalgebra as the matrix container type in the two qubit decomposers. This commit updates the weyl_decomposition module to leverage nalgebra to contain all the 1 and 2 qubit unitary matrices used during the decomposition. This has the same advantages outlined in the previous patches in this series, mainly that the nalgebra types used are fixed size and stack allocated which provides advantages for these small matrices. This commit also undoes some of the temporary work in the previous commit that was adding a dual nalgebra path for some utility functions in the two qubit basis decomposer. Now that both the weyl decomposition and the two qubit basis decomposer are based on storage types in nalgebra a lot of these aren't needed anymore and can be removed.
Collaborator
|
One or more of the following people are relevant to this code:
|
Member
Author
|
I dug up an old benchmarking script I used > 2 yrs ago from when I first wrote the 2q decomposers in Rust (and increased the number of unitaries by 10x) to get some targeted benchmarks of this PR. import statistics
import time
from qiskit.synthesis.two_qubit import TwoQubitBasisDecomposer
#rom qiskit.synthesis.unitary.qsd import qs_decomposition
from qiskit.circuit.library import CXGate
from qiskit.quantum_info import random_unitary
decomp = TwoQubitBasisDecomposer(CXGate(), euler_basis="ZSXX")
unitaries = [random_unitary(4).data for _ in range(100000)]
runtimes = []
for mat in unitaries:
start = time.perf_counter()
decomp(mat)
stop = time.perf_counter()
runtimes.append(stop - start)
print(f"Mean runtime per unitary: {statistics.geometric_mean(runtimes)} sec.")
print(f"Total runtime for 100k unitary matrices: {sum(runtimes)} sec.")Running this on this PR I got: while main got: I do feel like there is even more performance tuning we can do now that we're avoiding the dynamic memory overhead. But I didn't want to dive down that particular rabbit hole now/again since I could probably spend the rest of the release cycle trying to shave nanoseconds off this. |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
This is the next step towards using nalgebra as the matrix container
type in the two qubit decomposers. This commit updates the
weyl_decomposition module to leverage nalgebra to contain all the 1 and
2 qubit unitary matrices used during the decomposition.
This has the same advantages outlined in the previous patches in this
series, mainly that the nalgebra types used are fixed size and stack
allocated which provides advantages for these small matrices.
This commit also undoes some of the temporary work in the previous
commit that was adding a dual nalgebra path for some utility functions
in the two qubit basis decomposer. Now that both the weyl decomposition
and the two qubit basis decomposer are based on storage types in
nalgebra a lot of these aren't needed anymore and can be removed.
Details and comments
This is based on top of #15928 and will need to be rebased after that PR merges. In the meantime you can view the contents of just this PR by looking at the HEAD commit: b3525ba