Bug fixes in Solovay Kitaev Decomposition (backport #14217) #14294
Conversation
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
* Fix _compute_rotation_axis method The method used to return [0, 0, 0] for rotation axis when theta is very close to 0 (and yet not within 1e010). This commit fixes this by noting that the rotation axis of an SO(3) matrix is simply the eigenvector of this matrix corresponding to eigenvalue 1. * Fixes related to SU(2) vs. SO(3) The implemented algorithm has a global phase uncertainty of +-1, due to approximating not the original SU(2) matrix but its projection onto SO(3). This fixes the global phase of the computed approximation. In addition, the product_su2 matrix in GateSequence was not correctly computed in many cases, leading to incorrect values when using this attribute. Since this is not needed for the actual algorithm (and spends unnecessary effort for computing it), this also completely removes this attribute. Co-authored-by: Almudena Carrera Vazquez <almudenacarreravazquez@hotmail.com> * Fix the tests. One of the tests was simply wrong because the reference circuit has the wrong global phase. The tests that check the gates in the final decomposition should not assume that SGate is included and SdgGate is not. * reno * fix to the global phase * bug fix (forgetting that gate_matrix_su2 is not SU(2) was a sequence) * adding test * Restoring the previous fast code for _compute_rotation_axis based on Rodrigues formula, while additionally handling the case of 180-degree rotation * removing old code * review comments * switching back to using _to_dag and _to_circuit I actually no longer think that these methods would ever return a circuit with unitary gates, so it does not really matter * Addressing review comments * pass over release notes combining the original and the suggested wordings * Update releasenotes/notes/fix-sk-decomposition-23da3ee4b6a10d62.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> --------- Co-authored-by: Almudena Carrera Vazquez <almudenacarreravazquez@hotmail.com> Co-authored-by: Julien Gacon <gaconju@gmail.com> (cherry picked from commit 6892608) # Conflicts: # test/python/transpiler/test_solovay_kitaev.py
|
Cherry-pick of 6892608 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
ElePT
approved these changes
May 8, 2025
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 PR fixes a number of bugs we have recently discovered in the Python code for the Solovay-Kitaev decomposition, together with @Cryoris and @anedumla.
Fixes include:
SU(2)matrix, but its projection ontoSO(3)Longer-term, we will deprecate the Python code for Solovay-Kitaev decomposition altogether, and switch to the new improved pass written in Rust that also fixes these problems (see #14203), however for backward compatibility we need to temporarily support the Python code as well, and thus it makes sense to fix the problems on the Python side too.
Details and comments
Other changes:
to_circuitandto_dagremain for backwards compatibility but are no longer used by the main pass).GateSequenceclass used to store both the SO(3) matrix asproductand the SU(2) matrix asproduct_su2, however the parameterproduct_su2was not updated correctly in various places (and it was not even clear what to do withinGateSequence.from_matrix). Asproduct_su2was almost not used anywhere in the algorithm (with the exception of one place where its usage did not seem to be correct anyway), it was simply removed to avoid confusion and unnecessary computations.This is an automatic backport of pull request #14217 done by [Mergify](https://mergify.com).