Oxidize random clifford #12695

alexanderivrii · 2024-06-30T12:36:51Z

Summary

This PR implements the function random_clifford in Rust. The Rust function is called random_clifford_tableau.

This is a close port of the python function, with the exception that (1) optimizations for low-rank matrices were not yet included, (2) some of the numpy optimizations were replaced by the simpler approach presented in the appendix of the original paper.

UPDATE:

By far the main bottleneck of the process is the (already existing) Rust function binary_matmul_inner for multiplying binary matrices. With the older implementation of this function, the Rust code happens to be slower than the Python code, likely because numpy does some clever optimizations.

However, parallelizing binary_matmul_inner makes things better; finally, the Rust code is about 3x faster than Python code.

Here are the detailed results for generating 50 random Cliffords for different numbers of qubits:

PYTHON CODE: num_qubits = 5: time elapsed: 0.04
RUST CODE: num_qubits = 5: time elapsed: 0.00
PAR PYTHON CODE: num_qubits = 5: time elapsed: 0.03
=============================================================
PYTHON CODE: num_qubits = 10: time elapsed: 0.06
RUST CODE: num_qubits = 10: time elapsed: 0.00
PAR RUST CODE: num_qubits = 10: time elapsed: 0.00
=============================================================
PYTHON CODE: num_qubits = 50: time elapsed: 0.30
RUST CODE: num_qubits = 50: time elapsed: 0.11
PAR RUST CODE: num_qubits = 50: time elapsed: 0.05
=============================================================
PYTHON CODE: num_qubits = 100: time elapsed: 1.23
RUST CODE: num_qubits = 100: time elapsed: 0.88
PAR RUST CODE: num_qubits = 100: time elapsed: 0.27
=============================================================
PYTHON CODE: num_qubits = 150: time elapsed: 3.02
RUST CODE: num_qubits = 150: time elapsed: 2.86
PAR RUST CODE: num_qubits = 150: time elapsed: 0.74
=============================================================
PYTHON CODE: num_qubits = 200: time elapsed: 6.46
RUST CODE: num_qubits = 200: time elapsed: 6.92
PAR RUST CODE: num_qubits = 200: time elapsed: 1.78
=============================================================
PYTHON CODE: num_qubits = 250: time elapsed: 12.03
RUST CODE: num_qubits = 250: time elapsed: 14.30
PAR RUST CODE: num_qubits = 250: time elapsed: 3.54
=============================================================
PYTHON CODE: num_qubits = 300: time elapsed: 20.58
RUST CODE: num_qubits = 300: time elapsed: 26.63
PAR RUST CODE: num_qubits = 300: time elapsed: 6.37
=============================================================
PYTHON CODE: num_qubits = 350: time elapsed: 34.84
RUST CODE: num_qubits = 350: time elapsed: 43.99
PAR RUST CODE: num_qubits = 350: time elapsed: 10.45
=============================================================
PYTHON CODE: num_qubits = 400: time elapsed: 58.67
RUST CODE: num_qubits = 400: time elapsed: 67.97
PAR RUST CODE: num_qubits = 400: time elapsed: 15.98
=============================================================
PYTHON CODE: num_qubits = 450: time elapsed: 73.17
RUST CODE: num_qubits = 450: time elapsed: 109.03
PAR RUST CODE: num_qubits = 450: time elapsed: 26.85

…rices by significantly cheaper in-place prepend operations

coveralls · 2024-07-03T10:30:12Z

Pull Request Test Coverage Report for Build 9775885225

Details

1 of 137 (0.73%) changed or added relevant lines in 3 files are covered.
15 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.2%) to 89.648%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/accelerate/src/synthesis/clifford/mod.rs	1	10	10.0%
crates/accelerate/src/synthesis/linear/utils.rs	0	9	0.0%
crates/accelerate/src/synthesis/clifford/random_clifford.rs	0	118	0.0%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	94.02%
crates/qasm2/src/lex.rs	2	92.37%
crates/qasm2/src/parse.rs	12	96.23%

Totals
Change from base Build 9775174998:	-0.2%
Covered Lines:	65052
Relevant Lines:	72564

💛 - Coveralls

ShellyGarion

LGTM. the code looks ok but is there a way to check the randomness or at least that assuming the same random choice, we get the same output as in the python code?

ShellyGarion · 2024-07-25T09:49:23Z

How much this code improves when you port it from python to rust?

coveralls · 2024-08-22T06:55:14Z

Pull Request Test Coverage Report for Build 10579201069

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

158 of 159 (99.37%) changed or added relevant lines in 4 files are covered.
651 unchanged lines in 33 files lost coverage.
Overall coverage decreased (-0.4%) to 89.21%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/accelerate/src/synthesis/clifford/random_clifford.rs	116	117	99.15%

Files with Coverage Reduction	New Missed Lines	%
qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py	1	96.88%
qiskit/providers/backend_compat.py	1	89.19%
crates/qasm2/src/expr.rs	1	94.02%
qiskit/transpiler/passes/basis/unroll_3q_or_more.py	1	94.44%
qiskit/transpiler/passes/optimization/split_2q_unitaries.py	2	93.75%
qiskit/circuit/instruction.py	2	95.27%
qiskit/transpiler/passes/basis/unroll_custom_definitions.py	2	93.33%
crates/circuit/src/imports.rs	3	77.78%
crates/circuit/src/bit_data.rs	4	94.21%
qiskit/synthesis/two_qubit/two_qubit_decompose.py	4	93.51%

Totals
Change from base Build 10493633069:	-0.4%
Covered Lines:	71696
Relevant Lines:	80368

💛 - Coveralls

…s with >= 10 qubits

Cryoris

Nice that it's faster in the parallel version 👍🏻 👍🏻 Just curious: did you run a profiling to see which parts of the Rust code are the slowest? 🙂

crates/accelerate/src/synthesis/clifford/mod.rs

Cryoris · 2024-08-26T08:57:39Z

crates/accelerate/src/synthesis/linear/utils.rs

+
+use crate::getenv_use_multiple_threads;
+
+const PARALLEL_THRESHOLD: usize = 10;


Would it be helpful to add a comment on this number? I assume it's a heuristic from when the parallel execution is actually faster? 🙂

This code for parallel execution mimics the code in pauli_exp_val.rs, though there the threshold is chosen to be 19. Indeed, I have chosen 10 based on some local experiments.

Could you add a comment explaining this, so we know it in the future?

Sure, added an explicit comment in 7f443e0

crates/accelerate/src/synthesis/clifford/random_clifford.rs

Cryoris · 2024-08-26T09:02:03Z

crates/accelerate/src/synthesis/clifford/random_clifford.rs

+        perm[i] = inds[k];
+        inds.remove(k);


Can this ever run into the problem that it's trying to access an entry that has been removed? Maybe it's obvious that it cannot but I don't quite see it directly 🤔

Indeed, this was not clear to me either, but this is both a direct port of the now removed python function, and the pseudocode in the referenced paper.

crates/accelerate/src/synthesis/clifford/random_clifford.rs

Cryoris · 2024-08-26T09:09:21Z

crates/accelerate/src/synthesis/clifford/random_clifford.rs

+
+    // Compute stabilizer table
+    let zero = Array2::from_elem((num_qubits, num_qubits), false);
+    let prod1 = binary_matmul_inner(gamma1.view(), delta1.view()).unwrap();


If you're looking for more improvements, we could have a custom multiplication that takes into account that the upper triangular part of delta is always false (or by adding a flag to the binary_matmul_inner) 🤔

Hmm, indeed we probably could exploit this somehow. If you think it's worth it, we can think about this and treat this in a potential follow-up.

It was just a suggestion since you mentioned that the matrix multiplications are slow, but I'm fine not adding this at this point, as I'm not sure random_clifford is a bottleneck -- so up to you 🙂

I like the suggestion, but would rather not go down this rabbit hole for this PR. As they say, "premature optimization is the root of all evil" 🙂.

Cryoris · 2024-08-26T09:13:35Z

crates/accelerate/src/synthesis/clifford/random_clifford.rs

+    let mut gamma2: Array2<bool> = Array2::from_elem((num_qubits, num_qubits), false);
+    for i in 0..num_qubits {
+        gamma2[[i, i]] = rng.gen();
+    }
+    fill_tril(gamma2.view_mut(), &mut rng, true);


Another idea for improvement would be to have fill_tril do the random diagonal, and avoid constructing the matrix just to rewrite almost all entries again 🙂 E.g.

fn get_tril(rng: ... , symmetric: bool, random_diag: bool) -> Array2<bool>

where random_diag=false sets the diagonal to true (for the delta matrices) and random_diag=true creates the random diagonal for the gamma matrices

Technically speaking only half of the entries (when symmetric is False). :) I will use the same excuse that I tried to keep the code as close as possible to the original. In any case I don't think this makes much runtime difference (compared to the matrix multiplication between table1 and table).

crates/accelerate/src/synthesis/linear/utils.rs

alexanderivrii · 2024-08-26T15:42:15Z

Thanks for the review! To answer your question, I was manually instrumenting my code to print the times to execute various blocks until I saw that the single most expensive line is the binary multiplication of table1 by table.

ShellyGarion

LGTM, thanks @alexanderivrii

ShellyGarion · 2024-08-27T09:19:54Z

crates/accelerate/src/synthesis/clifford/random_clifford.rs

+}
+
+/// Add symmetric random boolean value to off diagonal entries.
+fn fill_tril(mut mat: ArrayViewMut2<bool>, rng: &mut Pcg64Mcg, symmetric: bool) {


perhaps this function should be in linear utils? it's useful to generate a random symmetric boolean matrix, which can represent a CZ circuit. In this case, it should be called something like "random_boolean matrix".

I don't think we should expose the two very special "random linear" functions required for this PR: one that generates random symmetric binary matrices with 0s on the diagonal, and another that generates upper-triangular random symmetric binary matrices with 1s on the diagonal.

ok, but I still think that the name "fill_tril" could be changed to something more meaningful (it also doesn't appear in the paper)

But... this is the name from our Python code :). Ok, will try to think of better names for this and other functions.

qiskit/quantum_info/operators/symplectic/random.py

ShellyGarion

LGTM. Thanks @alexanderivrii .
If @Cryoris has no further comments, then we can merge it.

Cryoris

LGTM thanks for all the work! If we ever need to revisit I think we have some ideas in this PR to even optimize the code a bit more 🙂

alexanderivrii added 30 commits June 12, 2024 17:03

starting to experiment

51a7e3b

porting code

62f4f70

messy code porting

8b2d5d4

printing statements to enable debugging

6456f1a

merge conflicts

ed97fb7

fixes

e7d86f9

fixing phase

3a26b75

removing some of the printing statements

07a9382

fixing inaccuracy for cost computation

52c26c2

Moving some of the functionality to SymplecticMatrix class

7122fed

reducing the number of warnings

f21bf7e

formatting

2715497

replacing expensive adjoint and compose operations for symplectic mat…

419e9bc

…rices by significantly cheaper in-place prepend operations

Merge branch 'main' into oxidize-cliffords

3879cad

resolving merge conflicts

bd0d1f1

cleanup

fdd3e9d

code cleanup

57c6a4f

cleanup

c0f5d5d

cleanup

e3072a0

cleanup

6d3f497

cleanup

8394de5

cleanup

1901df1

cleanup

073a68c

cleanup

ed9c743

using fast lookup

e42f44b

cleanup

0aaba7f

clippy

3880a39

including params in gate_seq to avoid mapping

3a6ef0b

removing unnecessary inner function

b324f22

cleanup

1cccf27

fixing merge conflicts

eaa659e

ShellyGarion reviewed Jul 8, 2024

View reviewed changes

ShellyGarion added the mod: quantum info Related to the Quantum Info module (States & Operators) label Jul 8, 2024

1ucian0 modified the milestones: 1.2.0, 1.3.0beta Jul 18, 2024

Merge branch 'main' into oxidize-random-clifford

86f96b9

parallelized computation for binary matrix multiplication for matrice…

38bed6b

…s with >= 10 qubits

alexanderivrii changed the title ~~[WIP] Oxidize random clifford~~ Oxidize random clifford Aug 22, 2024

updating python code and adding release notes

21f4229

alexanderivrii requested a review from ShellyGarion August 22, 2024 11:03

lint, test, reno

148f9ba

Cryoris reviewed Aug 26, 2024

View reviewed changes

alexanderivrii added 3 commits August 26, 2024 14:53

removing obsolete python code

a020dc1

removing todo (after experimenting with alternative implementation)

0a310d7

removing another obsolete todo

29acd8e

alexanderivrii requested a review from Cryoris August 26, 2024 15:42

unused import

c67ecaa

ShellyGarion reviewed Aug 27, 2024

View reviewed changes

adding comment

7f443e0

ShellyGarion reviewed Aug 29, 2024

View reviewed changes

Cryoris approved these changes Sep 3, 2024

View reviewed changes

Cryoris added this pull request to the merge queue Sep 3, 2024

Merged via the queue into Qiskit:main with commit f882fd5 Sep 3, 2024
15 checks passed

alexanderivrii deleted the oxidize-random-clifford branch September 3, 2024 15:13

ShellyGarion added the Changelog: New Feature Include in the "Added" section of the changelog label Nov 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Oxidize random clifford #12695

Oxidize random clifford #12695

alexanderivrii commented Jun 30, 2024 •

edited

Loading

coveralls commented Jul 3, 2024 •

edited

Loading

ShellyGarion left a comment

ShellyGarion commented Jul 25, 2024

coveralls commented Aug 22, 2024 •

edited

Loading

Cryoris left a comment

Cryoris Aug 26, 2024

alexanderivrii Aug 26, 2024

Cryoris Aug 27, 2024

alexanderivrii Aug 27, 2024

Cryoris Aug 26, 2024

alexanderivrii Aug 26, 2024

Cryoris Aug 26, 2024

alexanderivrii Aug 26, 2024

Cryoris Aug 27, 2024

alexanderivrii Aug 27, 2024

Cryoris Aug 26, 2024

alexanderivrii Aug 26, 2024

alexanderivrii commented Aug 26, 2024

ShellyGarion left a comment

ShellyGarion Aug 27, 2024

alexanderivrii Aug 27, 2024

ShellyGarion Aug 28, 2024

alexanderivrii Aug 28, 2024

ShellyGarion left a comment

Cryoris left a comment


		use crate::getenv_use_multiple_threads;

		const PARALLEL_THRESHOLD: usize = 10;

Oxidize random clifford #12695

Oxidize random clifford #12695

Conversation

alexanderivrii commented Jun 30, 2024 • edited Loading

Summary

coveralls commented Jul 3, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9775885225

Details

💛 - Coveralls

ShellyGarion left a comment

Choose a reason for hiding this comment

ShellyGarion commented Jul 25, 2024

coveralls commented Aug 22, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10579201069

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Cryoris left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderivrii commented Aug 26, 2024

ShellyGarion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShellyGarion left a comment

Choose a reason for hiding this comment

Cryoris left a comment

Choose a reason for hiding this comment

alexanderivrii commented Jun 30, 2024 •

edited

Loading

coveralls commented Jul 3, 2024 •

edited

Loading

coveralls commented Aug 22, 2024 •

edited

Loading