Add SolovayKitaev as part of the default unitary synthesis plugin #15285

alexanderivrii · 2025-11-01T09:41:32Z

Summary

This PR addresses the second step from Matthew's 4-step plan #14952 (review) to correctly set the default unitary synthesis plugin for Clifford+T compilation.

Add solovay kitaev as part of the default unitary synthesis plugin that we use when it's a 1q gate and the target is clifford+t (this is all in the rust code for the pass).

It's built on top of #15265 (merged).

Details and comments

In this PR, in the unitary synthesis pass in rust, for 1-qubit unitaries we check if target/basis_gates correspond to a Clifford+T set, in which case we use the Clifford+T synthesis algorithm (and not the one based on euler basis sets). The current version integrates the SolovayKitaev algorithm but hopefully in the future we will be able to replace it by the Ross-Selinger algorithm.

Using SolovayKitaev leads to an additional complication: the first step of the algorithm is to compute the basic approximations and we definitely don't want to do this for every 1-qubit unitary in the circuit. So I have defined a data structure UnitarySynthesisData that's shared between different unitaries, and that's lazily computed (if SolovayKitaev is not going to be used, the basic approximations would not be computed). I think @jakelishman does something like this and much more in his work-in-progress of rewriting the unitary synthesis pass. I tried to keep the changes to the minimum.

There is one caveat though. If someone will call DefaultUnitarySynthesis plugin multiple times from the Python side, the basis approximations will still be computed on every call. However, in all of our main flows, we directly call the unitary synthesis pass in rust and not invoke the default plugin.

Exposing entrypoint Removing duplicated Python code Using CowArray

Extendid the SK algorithm to handle other opertations (and not just standard gates) Adding UnitarySynthesisData to avoid generating basic approximation on every synthesis call.

qiskit-bot · 2025-11-01T09:41:36Z

One or more of the following people are relevant to this code:

@Qiskit/terra-core

coveralls · 2025-11-01T10:27:46Z

Pull Request Test Coverage Report for Build 19812159716

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

95 of 112 (84.82%) changed or added relevant lines in 2 files are covered.
735 unchanged lines in 7 files lost coverage.
Overall coverage increased (+0.03%) to 88.389%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/transpiler/src/passes/unitary_synthesis.rs	83	87	95.4%
crates/synthesis/src/discrete_basis/solovay_kitaev.rs	12	25	48.0%

Files with Coverage Reduction	New Missed Lines	%
crates/transpiler/src/passes/unitary_synthesis.rs	1	93.41%
crates/qasm2/src/lex.rs	2	92.03%
crates/circuit/src/parameter_table.rs	6	91.71%
crates/circuit/src/circuit_instruction.rs	40	84.18%
crates/circuit/src/circuit_data.rs	59	89.55%
crates/circuit/src/operations.rs	157	86.76%
crates/circuit/src/dag_circuit.rs	470	84.64%

Totals
Change from base Build 19740043964:	0.03%
Covered Lines:	95740
Relevant Lines:	108317

💛 - Coveralls

aaryav-3 · 2025-11-12T19:54:42Z

crates/transpiler/src/passes/unitary_synthesis.rs

+            if is_clifford_t_basis_set(target, basis_gates, PhysicalQubit::new(qubit.0)) {
+                let solovay_kitaev = unitary_synthesis_data.get_solovay_kitaev();
+                let matrix_nalgebra = Matrix2::from_fn(|i, j| matrix[[i, j]]);
+                let circuit = solovay_kitaev.synthesize_matrix(&matrix_nalgebra, 5);


in the case of 1q synthesis, here the approximation depth of synthesize matrix is hardcoded to be 5. Is there a way to not hardcode this? and perhaps consider input argument for approximation_degree to dictate the approximation depth? although it is true that that parameter dictates fidelity in all other decomposition cases, I just thought it may make sense to give some user control over the 1q synthesis with the approximation parameter, in a differing context here. Please let me know what you think about this?

I completely agree, ideally we should translate the user-specified approximation_degree to recursion_degree used by synthesize_matrix, and note that this also depends on the basic_approximations used by the algorithm (notably on basis_gates and depth used to construct these basic approximations). I don't remember the exact guarantees of the approach, but it might be even the case that for a given approximation_degree we must choose both depth (for constructing basic approximations) and recursion_degree (for running the algorithm). In fact, we have discussed your very exact suggestion multiple times in the past (cc @Cryoris and @mtreinish) and it has been on our to-do list since forever 😅.

Another important point is that Solovay-Kitaev has a poor convergence rate, and (if I remember the experiments correctly) requires ~1000 of T-gates for the error of ~ $10^{-4}$. Thus we are strongly hoping that in the near future we will be able to replace by default the Solovay-Kitaev algorithm by the much better Ross-Selinger algorithm, which requires ~50 T-gates for the error of ~ $10^{-8}$.

For now, the default values (i.e. 5) are exactly the same as would be set by default from the python version of the pass.

I think it's fine to hardcode a default, as long as the documentation (1) clearly states these values, and (2) explains how custom values can be set via the plugin interface. Could you add these informations to the UnitarySynthesis docs?

As per my previous comment, I have added some documentation for the default unitary synthesis plugin here: 88f9697. Please take a look.

aaryav-3

Hi, thank you so much for your contribution, your PR looks great and is a clean implementation. Moving the synthesis logic to Rust and integrating Solovay-Kitaev for Clifford+T circuits is a significant step forward for performance and consistency. The refactoring is clean, and the introduction of UnitarySynthesisData to cache SK approximations is a thoughtful touch. I had just one small clarification in the code.

Further another question I had, was whether the property_set of pass manager can be leveraged to cache UnitarySynthesisData for multiple python facing calls of the :class:DefaultUnitarySynthesis plugin, as an exceptional case? albeit this is a transformation pass, we have done such transmutation in the case of :class:BasePadding which also was a transformation pass. Or is this something we strictly avoid otherwise?
Let me know what you think.

alexanderivrii · 2025-11-13T07:43:09Z

Further another question I had, was whether the property_set of pass manager can be leveraged to cache UnitarySynthesisData for multiple python facing calls of the :class:DefaultUnitarySynthesis plugin, as an exceptional case?

I think this suggestion would work. Though, since this is a bit of an edge-case, I would prefer thinking of the best possible longer-term solution and maybe postponing it to another PR. For instance, this wold require exposing UnitarySynthesisData to python, correct? And we may not want to expose all of the data, possibly motivating a further split between "internal" and "external" data. I know that @jakelishman has been working on fully refactoring the unitary synthesis pass, so he might have some thoughts here.

And again this complication would disappear if/when we can switch to the Ross-Selinger algorithm.

Cryoris · 2025-11-25T17:56:44Z

crates/transpiler/src/passes/unitary_synthesis.rs

 ) -> PyResult<DAGCircuit> {
    let out_dag = dag.copy_empty_like(VarsMode::Alike)?;
    let mut out_dag = out_dag.into_builder();
+    let mut unitary_synthesis_data = UnitarySynthesisData::new();


Each transpile run here will re-construct the same set of basic approximations in the default Clifford+T path. Do we have precedence of locally storing a cache? Because SK does have a safe serialization format that we could use instead.

Hmm, good point, since here we are hardcoding the basis gates (T/Tdg/H) and depth (12) for constructing SK approximations, we can indeed store them in a file, and have the default plugin to load approximations from the file rather than recompute them. Do I understand the suggestion correctly? What would we a good location for this file?

After some benchmarks I think we can leave caching into a file for a follow-up, in case it becomes necessary. In the most common workflow, where users previously just did something like

transpile(circuit, basis_gates=["t", "h", "cx"])

This PR is actually faster than main tested on a 10-qubit QFT circuit:

(main) Took 12.168 +- 0.352s (this PR) Took 10.449 +- 0.341s

We can point out in the docs that for repeated transpilation it would be useful to manually write the translation stage and set the SolovayKitaevPlugin with a predefined basic approximation.

Hmm, apparently we did not have any documentation for the default unitary synthesis plugin: first, it's empty, and, second, I have accidentally broken a link to the right page quite some time ago. Please take a look at the changes in 88f9697.

Cryoris · 2025-11-25T17:58:07Z

crates/transpiler/src/passes/unitary_synthesis.rs

+            if is_clifford_t_basis_set(target, basis_gates, PhysicalQubit::new(qubit.0)) {
+                let solovay_kitaev = unitary_synthesis_data.get_solovay_kitaev();
+                let matrix_nalgebra = Matrix2::from_fn(|i, j| matrix[[i, j]]);
+                let circuit = solovay_kitaev.synthesize_matrix(&matrix_nalgebra, 5);


I think it's fine to hardcode a default, as long as the documentation (1) clearly states these values, and (2) explains how custom values can be set via the plugin interface. Could you add these informations to the UnitarySynthesis docs?

test/python/transpiler/test_solovay_kitaev.py

qiskit/transpiler/preset_passmanagers/common.py

crates/transpiler/src/passes/unitary_synthesis.rs

ShellyGarion

I have some minor comments on the documentation. could be handled in a follow-up PR.

ShellyGarion · 2025-11-30T09:07:58Z

qiskit/transpiler/passes/synthesis/default_unitary_synth_plugin.py

+The default unitary synthesis plugin defines the default algorithm used by
+:class:`.UnitarySynthesis` transpiler pass to synthesize or approximate unitary
+gates in the circuit. Its behavior depends on the number of qubits in the
+target unitary and whether the target basis is Clifford+T.


I wonder if we should use the terminology of Clifford+T, or perhaps noisy vs. fault-tolerant, or pre-fault-tolerance vs. fault-tolerance?

I slightly prefer Clifford+T since other options (noisy, fault-tolerant, etc.) might mean different things to different people. But I will defer to @Cryoris for the best name to use.

ShellyGarion · 2025-11-30T09:10:11Z

qiskit/transpiler/passes/synthesis/default_unitary_synth_plugin.py

+
+* 1-qubit gates: Euler angle decompositions (see :class:`.OneQubitEulerDecomposer`).
+* 2-qubit gates: KAK/Cartan decompositions (see :class:`.TwoQubitBasisDecomposer`,
+  :class:`.TwoQubitControlledUDecomposer`, :class:`.XXDecomposer`).


should we seperate here between parametrized and non-paramterized gates? and discrete vs. countionuous sets?

Do we do this by default? Any particular suggestions?

qiskit/transpiler/passes/synthesis/default_unitary_synth_plugin.py

ShellyGarion · 2025-11-30T09:16:30Z

test/python/transpiler/test_unitary_synthesis.py

+
+        out = UnitarySynthesis(basis_gates=["cx", "h", "t", "tdg"]).run(dag)
+        self.assertTrue(
+            set(out.count_ops()).issubset(set(get_clifford_gate_names()).union({"t", "tdg"}))


should we test that when providing clifford circuit the compilation only include clifford gates?

I think such tests should be (and I believe already are) a part of SolovayKitaev testing. Here we only want to make sure that the right plugin gets used.

Also, if we later include other Clifford+T synthesis plugins and extend the tests, I don't think we should require that clifford-only circuits get compiled in only-clifford gates (if I remember correctly, there are examples where using T-gates reduces the number of 2-qubit operations).

qiskit/transpiler/passes/synthesis/plugin.py

Cryoris

Some comments on the wording, otherwise LGTM

qiskit/transpiler/passes/synthesis/default_unitary_synth_plugin.py

Cryoris · 2025-11-30T20:48:31Z

qiskit/transpiler/passes/synthesis/aqc_plugin.py

 # that they have been altered from the originals.
 """
-==============================================================================
-AQC Synthesis Plugin (in :mod:`qiskit.transpiler.passes.synthesis.aqc_plugin`)


Why were these removed? Was it just for consistency with other headers? 🙂

Yes. Also (very subjectively) this looks nicer than what we have now.

alexanderivrii added 8 commits October 28, 2025 15:34

Moving unitary synthesis code to a separate function

3fc1587

Exposing entrypoint Removing duplicated Python code Using CowArray

Merge branch 'main' into default-unitary-plugin/1

189d1ce

Exposing SolovayKitaev aglorithm to unitary synthesis

ef127c6

Extendid the SK algorithm to handle other opertations (and not just standard gates) Adding UnitarySynthesisData to avoid generating basic approximation on every synthesis call.

using default instead of sk in clifford+t transpilation pipeline

d4c43fc

fix for is_clifford_t in rust

e836419

fix to the global phase; adding a few tests

835666a

release note

7edb9d7

clippy

8e96c16

alexanderivrii requested a review from a team as a code owner November 1, 2025 09:41

alexanderivrii added the fault tolerance related to fault tolerance compilation label Nov 1, 2025

alexanderivrii added this to Qiskit 2.3 Nov 1, 2025

github-project-automation bot moved this to Ready in Qiskit 2.3 Nov 1, 2025

alexanderivrii added this to the 2.3.0 milestone Nov 1, 2025

alexanderivrii mentioned this pull request Nov 2, 2025

Add the argument fallback_on_default to UnitarySynthesis #15287

Open

aaryav-3 reviewed Nov 12, 2025

View reviewed changes

Cryoris reviewed Nov 25, 2025

View reviewed changes

alexanderivrii added 3 commits November 26, 2025 16:48

merge with main + resolve conflicts

6fad7a1

renaming data -> cache

4aec33a

improving comments

3c3cd1e

Cryoris moved this from Ready to In review in Qiskit 2.3 Nov 28, 2025

alexanderivrii added 3 commits November 29, 2025 14:03

merge with main (+ resolve conflicts)

2169bd3

docs and comments improvement

88f9697

reorganize tests

e5c0534

alexanderivrii requested a review from Cryoris November 30, 2025 08:25

typo

3a193b4

ShellyGarion reviewed Nov 30, 2025

View reviewed changes

Cryoris reviewed Nov 30, 2025

View reviewed changes

docs following review comments

bd6c55e

alexanderivrii requested a review from Cryoris December 1, 2025 05:21

Cryoris approved these changes Dec 1, 2025

View reviewed changes

Cryoris added this pull request to the merge queue Dec 1, 2025

Merged via the queue into Qiskit:main with commit d818c36 Dec 1, 2025
23 checks passed

github-project-automation bot moved this from In review to Done in Qiskit 2.3 Dec 1, 2025

alexanderivrii deleted the default-unitary-plugin/2 branch December 1, 2025 11:30

Add SolovayKitaev as part of the default unitary synthesis plugin #15285

Add SolovayKitaev as part of the default unitary synthesis plugin #15285

Uh oh!

Conversation

alexanderivrii commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Uh oh!

qiskit-bot commented Nov 1, 2025

Uh oh!

coveralls commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 19812159716

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

aaryav-3 Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaryav-3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexanderivrii commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShellyGarion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Cryoris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

alexanderivrii commented Nov 1, 2025 •

edited

Loading

coveralls commented Nov 1, 2025 •

edited

Loading

aaryav-3 Nov 12, 2025 •

edited

Loading

aaryav-3 left a comment •

edited

Loading