Optimise `SparsePauliOp.from_operator` #11557

jakelishman · 2024-01-13T02:42:08Z

Summary

This rewrites the from_operator handling (again!) from the initial Rust implementation of the recursive matrix-addition form into an iterative approach that re-uses the same scratch memory all the way down. This is significantly faster, and allocates far less often, although in practice the peak heap memory usage will be not dissimilar.

The algorithm is rewritten to be a manual stack-based iteration, rather than a functional recursion. The size of a single stack entry in the iteration is one usize, which is drastically smaller than whatever per-function-call stack will have been used before.

Details and comments

This is the rewrite alluded to in #11133. Using the same timing script, the new graph looks like:

where the absolute timing of the the 10q operator is ~40ms compared to ~227ms, so it's a 5-6x speedup at that scale (for a fully dense operator). Decompositions of up to 4q operators are now entirely lost in the noise of the construction time of SparsePauliOp (which tbh probably says more about the overheads of the quantum_info module than anything else...).

Still no parallelism here, but honestly, I'm not even sure it's worth putting in the time to do that other than casual interest.

qiskit-bot · 2024-01-13T02:42:14Z

One or more of the the following people are requested to review this:

@Eric-Arellano
@Qiskit/terra-core
@kevinhartman
@mtreinish

coveralls · 2024-01-13T03:29:43Z

Pull Request Test Coverage Report for Build 11597689008

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

316 of 338 (93.49%) changed or added relevant lines in 1 file are covered.
69 unchanged lines in 8 files lost coverage.
Overall coverage increased (+0.05%) to 88.721%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/accelerate/src/sparse_pauli_op.rs	316	338	93.49%

Files with Coverage Reduction	New Missed Lines	%
qiskit/circuit/library/iqp.py	1	96.15%
crates/accelerate/src/two_qubit_decompose.rs	1	92.09%
crates/accelerate/src/unitary_synthesis.rs	1	92.2%
crates/qasm2/src/lex.rs	4	92.73%
qiskit/compiler/assembler.py	5	96.32%
qiskit/compiler/transpiler.py	7	92.93%
qiskit/transpiler/preset_passmanagers/generate_preset_pass_manager.py	14	92.0%
qiskit/providers/basic_provider/basic_simulator.py	36	87.76%

Totals
Change from base Build 11591703186:	0.05%
Covered Lines:	75712
Relevant Lines:	85337

💛 - Coveralls

mtreinish

There's also a merge conflict now that #11388 merged

releasenotes/notes/fast-sparse-pauli-operator-b41cacf11e8c4e0e.yaml

jakelishman · 2024-05-01T21:33:21Z

Now rebased over main. Apparently I touched a couple of unrelated comments at some point that I mistakenly thought were part of this PR not a previous one, but oh well.

crates/accelerate/src/sparse_pauli_op.rs

ShellyGarion · 2024-09-25T12:36:01Z

Before looking into this very smart, efficient and complicated code, I looked into the tests, and found only one very simple test for the SparsePauliOp.from_operator method (which tests only operators which are 2-qubit Paulis with coefficient=1):

qiskit/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py

Line 148 in dcd41e9

def test_from_operator(self):

Since this PR (as well the one before it #11133) replaced a simple code of about ~10 lines in Python by a quite complicated and much longer code in Rust, I would suggest to enhance the test code by some tests of the following form:

labels = ["XI", "YZ", "YY", "ZZ"]  # it's better to provide a longer list of labels, preferably 3-4 qubits  
coeffs = [-3, 4.4j, 0.2 - 0.1j, 66.12] # provide a random list of coefficients, include also complex numbers
target = np.zeros((4, 4), dtype=complex)
for coeff, label in zip(coeffs, labels):
    target += coeff * pauli_mat(label)
op = SparsePauliOp.from_operator(Operator(target))
# assert that op and target are the same (up to a permutation)

There are also several methods that tests the to_matrix and to_operator methods - it's also possible to add a from_operator method to these tests (although all of them use the same parameters given above):

qiskit/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py

Line 239 in dcd41e9

def test_to_matrix(self):

qiskit/test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py

Line 303 in dcd41e9

def test_to_operator(self):

ShellyGarion

This is a very optimized version of the original code (~10 lines in Python) as well as the code in the paper of HBG (<100 lines in Python).
The code is of high quality, and so my main concern is that these optimizations should be better explained and documented, and some further tests should be added (see my previous comment).

crates/accelerate/src/sparse_pauli_op.rs

ShellyGarion · 2024-10-13T13:36:49Z

crates/accelerate/src/sparse_pauli_op.rs

+            out_list.push(PauliLocation::begin(num_qubits));
+        }
+        _ => {
+            unsafe { scratch.set_len(side * side) };


Is there a way to test that there is no memory leakage?

I can write a Rust-space test, since that'll cause all this code to be run under Miri, which checks this kind of thing. In the mean time I made this scratch.set_len(scratch.capacity()) so it's clear that it's not setting it wider than is allocated.

crates/accelerate/src/sparse_pauli_op.rs

ShellyGarion · 2024-10-13T13:43:22Z

crates/accelerate/src/sparse_pauli_op.rs

+    // This set of `extend` calls is effectively an 8-fold unrolling of the "natural" loop through
+    // each bit, where the initial `if` statements are handling the remainder (the up-to 7
+    // least-significant bits).  In practice, it's probably unlikely that people are decomposing
+    // 16q+ operators, since that's a pretty huge matrix already.


Is it possible that an operator will have many qubits but with a few Pauli terms (sparse)?

Yeah, very much so. Everything here should handle those cases fine, though.

crates/accelerate/src/sparse_pauli_op.rs

This rewrites the `from_operator` handling (again!) from the initial Rust implementation of the recursive matrix-addition form into an iterative approach that re-uses the same scratch memory all the way down. This is significantly faster, and allocates far less often, although in practice the peak heap memory usage will be not dissimilar. The algorithm is rewritten to be a manual stack-based iteration, rather than a functional recursion. The size of a single stack entry in the iteration is one `usize`, which is drastically smaller than whatever per-function-call stack will have been used before.

jakelishman · 2024-10-29T18:41:21Z

I've updated all the documentation - I'll add some more tests tomorrow.

jakelishman · 2024-10-30T16:40:15Z

Ok, I've pushed up a handful more tests in 80a3e87, which should substantially increase the mathematical coverage, and refactored a small amount to make it possible to write a couple of simple Rust-space tests so all the unsafe code gets run under Miri as well.

ShellyGarion

LGTM, thanks!

jakelishman added performance Changelog: New Feature Include in the "Added" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) Rust This PR or issue is related to Rust code in the repository labels Jan 13, 2024

jakelishman added this to the 1.0.0 milestone Jan 13, 2024

jakelishman requested a review from a team as a code owner January 13, 2024 02:42

jakelishman force-pushed the faster-pauli-decomposition branch from e13384d to 3d477f6 Compare January 13, 2024 02:42

jakelishman requested a review from nonhermitian as a code owner January 13, 2024 03:09

jakelishman force-pushed the faster-pauli-decomposition branch from f0a95f1 to 2bc120c Compare January 13, 2024 15:27

jakelishman removed the request for review from nonhermitian January 13, 2024 15:30

mtreinish modified the milestones: 1.0.0, 1.1.0 Jan 23, 2024

mtreinish self-assigned this Mar 22, 2024

jakelishman force-pushed the faster-pauli-decomposition branch from 22f7962 to 589137b Compare April 7, 2024 15:30

mtreinish reviewed May 1, 2024

View reviewed changes

releasenotes/notes/fast-sparse-pauli-operator-b41cacf11e8c4e0e.yaml Outdated Show resolved Hide resolved

jakelishman force-pushed the faster-pauli-decomposition branch from 589137b to 06121b9 Compare May 1, 2024 21:30

mtreinish assigned jlapeyre and unassigned mtreinish May 1, 2024

jakelishman commented May 1, 2024

View reviewed changes

crates/accelerate/src/sparse_pauli_op.rs Outdated Show resolved Hide resolved

jakelishman commented May 1, 2024

View reviewed changes

crates/accelerate/src/sparse_pauli_op.rs Outdated Show resolved Hide resolved

mtreinish modified the milestones: 1.1.0, 1.2.0 May 2, 2024

mtreinish modified the milestones: 1.2.0, 1.3.0 Jul 31, 2024

jakelishman force-pushed the faster-pauli-decomposition branch from 58caacb to 9ad36a3 Compare September 20, 2024 10:14

ShellyGarion self-assigned this Sep 23, 2024

ShellyGarion reviewed Oct 13, 2024

View reviewed changes

jakelishman added 2 commits October 29, 2024 18:38

Improve documentation

e2a97e4

jakelishman force-pushed the faster-pauli-decomposition branch from 9ad36a3 to e2a97e4 Compare October 29, 2024 18:38

jakelishman added 2 commits October 30, 2024 11:31

Merge remote-tracking branch 'ibm/main' into faster-pauli-decomposition

c07e17e

Increase test coverage

80a3e87

ShellyGarion approved these changes Oct 31, 2024

View reviewed changes

jakelishman added this pull request to the merge queue Oct 31, 2024

Merged via the queue into Qiskit:main with commit 4e6fd36 Oct 31, 2024
17 checks passed

jakelishman deleted the faster-pauli-decomposition branch October 31, 2024 14:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimise `SparsePauliOp.from_operator` #11557

Optimise `SparsePauliOp.from_operator` #11557

jakelishman commented Jan 13, 2024 •

edited

Loading

qiskit-bot commented Jan 13, 2024

coveralls commented Jan 13, 2024 •

edited

Loading

mtreinish left a comment

jakelishman commented May 1, 2024

ShellyGarion commented Sep 25, 2024 •

edited

Loading

ShellyGarion left a comment

ShellyGarion Oct 13, 2024

jakelishman Oct 29, 2024

ShellyGarion Oct 13, 2024

jakelishman Oct 29, 2024

jakelishman commented Oct 29, 2024

jakelishman commented Oct 30, 2024

ShellyGarion left a comment

Optimise SparsePauliOp.from_operator #11557

Optimise SparsePauliOp.from_operator #11557

Conversation

jakelishman commented Jan 13, 2024 • edited Loading

Summary

Details and comments

qiskit-bot commented Jan 13, 2024

coveralls commented Jan 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11597689008

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

mtreinish left a comment

Choose a reason for hiding this comment

jakelishman commented May 1, 2024

ShellyGarion commented Sep 25, 2024 • edited Loading

ShellyGarion left a comment

Choose a reason for hiding this comment

ShellyGarion Oct 13, 2024

Choose a reason for hiding this comment

jakelishman Oct 29, 2024

Choose a reason for hiding this comment

ShellyGarion Oct 13, 2024

Choose a reason for hiding this comment

jakelishman Oct 29, 2024

Choose a reason for hiding this comment

jakelishman commented Oct 29, 2024

jakelishman commented Oct 30, 2024

ShellyGarion left a comment

Choose a reason for hiding this comment

Optimise `SparsePauliOp.from_operator` #11557

Optimise `SparsePauliOp.from_operator` #11557

jakelishman commented Jan 13, 2024 •

edited

Loading

coveralls commented Jan 13, 2024 •

edited

Loading

ShellyGarion commented Sep 25, 2024 •

edited

Loading