Skip to content

Conversation

@alexanderivrii
Copy link
Member

Summary

In the first installment of the Clifford+T transpilation pipeline, we have hardcoded the usage of the Solovay-Kitaev unitary synthesis plugin for decomposing single-qubit unitary gates (and note that currently Solovay-Kitaev is still the only Clifford+T decomposition available inside Qiskit itself). However, this prevents users from specifying custom unitary synthesis plugins. This PR fixes this oversight by taking the argument unitary_synthesis_method into account.

Details and comments

Not sure if this change warrants release notes.

@alexanderivrii alexanderivrii added this to the 2.3.0 milestone Aug 26, 2025
@alexanderivrii alexanderivrii requested a review from a team as a code owner August 26, 2025 15:14
@alexanderivrii alexanderivrii added the fault tolerance related to fault tolerance compilation label Aug 26, 2025
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's important that users provide a unitary synthesis method that is compatible with the basis gates we hardcode here. Is there currently a reasonable error if I pick an existing method which does not support H T Tdg? Otherwise we should document this clearly somewhere, e.g. in the docstring.

@coveralls
Copy link

coveralls commented Aug 26, 2025

Pull Request Test Coverage Report for Build 18710890172

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.

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.01%) to 88.221%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 1 92.8%
crates/circuit/src/parameter/symbol_expr.rs 2 73.23%
Totals Coverage Status
Change from base Build 18701196961: 0.01%
Covered Lines: 93474
Relevant Lines: 105954

💛 - Coveralls

@Cryoris
Copy link
Contributor

Cryoris commented Aug 28, 2025

Do you think there's a way of testing this? Probably not since we can't register mock plugins during the tests, right?

But maybe we could test that transpilation fails if you give a unitary_synthesis_method that is not compatible with the discrete basis.

@alexanderivrii
Copy link
Member Author

alexanderivrii commented Oct 22, 2025

It's important that users provide a unitary synthesis method that is compatible with the basis gates we hardcode here. Is there currently a reasonable error if I pick an existing method which does not support H T Tdg? Otherwise we should document this clearly somewhere, e.g. in the docstring.

I have clarified this in f68a31d.

One way I can think of producing an error message as above is to write a transpiler pass FailWithErrorMessage that always fails, and add ConditionalController([FailWithErrorMessage], condition=_not_clifford_t_gates_in_dag) to the list of passes being run. Do we want this / is there is an even simpler implementation that does not involve adding the "always failing" transpiler pass?

@mtreinish mtreinish self-assigned this Oct 23, 2025
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I said offline that the pass should fallback to the default my remarks were actually incorrect, sorry about that. In looking at the unitary synthesis code again I was only half remembering, and it only applies for the min/max qubit arguments are set for a plugin and the plugin doesn't support the gate qubit count being synthesized. It doesn't apply this logic generally to other options.

To get this to work correctly I think the best way is to add a new option to the UnitarySynthesis pass that will have the pass fall back to the default plugin if the synthesis doesn't work on the provided target or returns None (and the original gate isn't target supported). Since this is a behavior change for the pass it needs to be an new option that is opt-in, but we can use it in the translation stage. I think this PR really needs to be 4 separate PRs (including this one) to implement this correctly:

  1. Update the default synthesis plugin to only use the rust code. I didn't realize it was still in Python and we just had a second copy of it lying around. The .run() method should just call the rust code (we might need to expose a new python entrypoint to it) and the python class is only needed to provide the plugin interface.
  2. 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).
  3. Add the new option to UnitarySynthesis to fallback based on whether the synthesis conforms to the target or not.
  4. Update this PR based on the 3 above steps to leverage the new mode for UnitarySynthesis in the translation stage.

Comment on lines +504 to +505
if unitary_synthesis_method == "default":
unitary_synthesis_method = "sk"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this done here and not in the default synthesis method? For this to work correctly I would expect the default unitary synthesis plugin to understand it needs to use solovay kitaev when in the presence of a clifford+t basis set.
Doing the plugin selection out of band in the translation stage plugin like this will result in an infinite loop (until we reach the pass manager iteration limit) if there is an synthesis method selected for clifford+t basis provided to the preset pass manager. Since the translation stage is used in the optimization loop to correct for the cases when there is a non-target aware optimization or synthesis.

Additionally, baking this into the default plugin will operate solely in the rust domain and be much faster to execute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fault tolerance related to fault tolerance compilation

Projects

Status: Ready
Status: To do

Development

Successfully merging this pull request may close these issues.

5 participants