Avoid panics in Quantum Shannon Decomposition Rust code by ShellyGarion · Pull Request #15635 · Qiskit/qiskit

Shelly Garion (ShellyGarion) · 2026-02-01T15:33:42Z

Summary

Since iterative Quantum Shannon Decomposition may provide imprecise matrices, and the rust code may panic when calcualting SVD or Schur decompositions, we use the Isometry decomposition in such cases.

Details and comments

qiskit-bot · 2026-02-01T15:33:46Z

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

@Cryoris
@Qiskit/terra-core
@ajavadia

Coveralls (coveralls) · 2026-02-01T16:33:43Z

Pull Request Test Coverage Report for Build 21586424465

Details

28 of 41 (68.29%) changed or added relevant lines in 2 files are covered.
6 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.03%) to 87.972%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/library/generalized_gates/unitary.py	10	13	76.92%
crates/synthesis/src/qsd.rs	18	28	64.29%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.82%
crates/qasm2/src/lex.rs	5	92.03%

Totals
Change from base Build 21575821626:	0.03%
Covered Lines:	100031
Relevant Lines:	113708

💛 - Coveralls

Gadi Aleksandrowicz (gadial)

Thanks Shelly, LGTM. Added one minor comment.

Julien Gacon (Cryoris)

I'm not convinced this is the right way to approach this, we should properly propagate errors and not just wrap the panic into a Python error -- I'm just putting request changes so we don't eagerly merge it, I can leave a more detailed comment later

… into ctrl_unitary

Shelly Garion (ShellyGarion) · 2026-02-02T10:27:48Z

I'm not convinced this is the right way to approach this, we should properly propagate errors and not just wrap the panic into a Python error -- I'm just putting request changes so we don't eagerly merge it, I can leave a more detailed comment later

The errors were better handled by Alexander Ivrii (@alexanderivrii) in 782cfdb

Matthew Treinish (mtreinish) · 2026-02-23T13:11:54Z

+    fn from(error: QSDError) -> Self {
+        match error {
+            QSDError::TwoQubitDecompositionFailed => {
+                PyRuntimeError::new_err("two-qubit decomposition failed")


Is there not a qiskit error we should be using for these. While a Python runtime error does fit, it's kind of generic for something specific to Qiskit.

Changed in e36fe93 to return QiskitError instead.

Matthew Treinish (mtreinish) · 2026-02-23T13:14:48Z

        1.0,
        "U",
        None,
-    )?;


Is there error information from the 2q decomposer we're discarding this way? Granted the only way to keep this error information and also maintain Rust native errors is to update the 2q decomposer code. If you don't want to do this for this PR (which would be a bit much for backport) you should be able to keep the PyResult on this function and use ? or .into() on the qsd_inner returning QSDError and it should work fine.

Since we are planning to backport this, I have left the 2q decomposer code unchanged, for now. In e36fe93 we are now wrapping different QSDError types around Python/CircuitData errors, so error information should get propagated.

Matthew Treinish (mtreinish) · 2026-02-23T13:15:58Z

+
+impl From<CircuitDataError> for QSDError {
+    fn from(_error: CircuitDataError) -> Self {
+        QSDError::CircuitConstructionFailed


Doesn't this discard the inner error message? Why not either wrap the CircuitDataError or reuse the error message?

Improved in e36fe93.

Matthew Treinish (mtreinish) · 2026-02-23T13:17:11Z

    one_qubit_decomposer: &EulerBasisSet,
    depth: usize,
-) -> Result<CircuitData, CircuitDataError> {
+) -> Result<CircuitData, QSDError> {


Does this function ever return an error other than a CircuitDataError? If it doesn't why do we need to change the error type here to something broader that is never used?

The qsd_inner function may return either a circuit data error or schur decomposition error.

Matthew Treinish (mtreinish) · 2026-02-23T13:18:48Z

    angles: &mut [f64],
    vw_type_all: bool,
-) -> Result<CircuitData, CircuitDataError> {
+) -> Result<CircuitData, QSDError> {


Same question here why are you changing the error type here if the current one is more specific?

Reverted to return CircuitDataError, see fc592c8.

Matthew Treinish (mtreinish) · 2026-02-23T13:19:03Z

-    new: CircuitData,
-    qubit_map: &[Qubit],
-) -> Result<(), CircuitDataError> {
+fn append(circ: &mut CircuitData, new: CircuitData, qubit_map: &[Qubit]) -> Result<(), QSDError> {


An here too

Also reverted here too.

Matthew Treinish (mtreinish) · 2026-02-23T13:21:16Z

+                # pylint: disable=cyclic-import
+                from qiskit.quantum_info.operators import Operator
+
+                if not matrix_equal(Operator(cmat_def).to_matrix(), cmat, atol=1e-7):
+                    self.definition = Isometry(cmat, 0, 0).definition


Does this need to be under the try block? Runtime error is fairly generic and it could be masking a real error

e36fe93 changes the error to be QiskitError. This still requires a try-catch block, but we check for qiskit error instead.

Alternatively, we could change the rust qs_decomposition function to return None when unsuccessful to synthesize a unitary for whichever reason, would this be a cleaner approach?

My concern is how large the try block is here. The intent here is to only catch on errors coming from qs_decomposition right? If Operator(), matrix_equal or Isometry raise we don't want those to be handled like qs_decomposition() and should just raise. Right now the try block is covering too much code and could mask real bugs. This also applies to the same code in the method above.

but since qs_decomposition isn't numerically stable, our solution is only to fall back to Isometry, right?

the Operator() calculation check was added in a previous PR: #13688, to prevent some tests from failing due to inaccurate numerical outcome.

addressed in dcc5327

It's no longer a generic wrap that warrants a "Request changes" blocker

Matthew Treinish (mtreinish)

Thanks for the updates. I think this looks fine now. I just pushed up a small fix to the release note to make it a bit more clear what's being fixed.

* catch rust panic errors in qs_decomposition * add try/catch in qs_decompsition, and fall to Isometry if needed * add test, based on issue 15610 * formatting * add release notes * add Isometry to the test * Improving error handling for QSD synthesis * Fix misprint * enhance failed matrix test * Changing RuntimeError to QiskitError * comment * not discarding info for CircuitDataError * tightening try-except blocks * further fix to avoid infinite recursion * Update release note --------- Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> (cherry picked from commit 02fef9b) # Conflicts: # crates/synthesis/src/qsd.rs # qiskit/circuit/library/generalized_gates/unitary.py

* catch rust panic errors in qs_decomposition * add try/catch in qs_decompsition, and fall to Isometry if needed * add test, based on issue 15610 * formatting * add release notes * add Isometry to the test * Improving error handling for QSD synthesis * Fix misprint * enhance failed matrix test * Changing RuntimeError to QiskitError * comment * not discarding info for CircuitDataError * tightening try-except blocks * further fix to avoid infinite recursion * Update release note --------- Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> (cherry picked from commit 02fef9b)

) (#15802) * Avoid panics in Quantum Shannon Decomposition Rust code (#15635) * catch rust panic errors in qs_decomposition * add try/catch in qs_decompsition, and fall to Isometry if needed * add test, based on issue 15610 * formatting * add release notes * add Isometry to the test * Improving error handling for QSD synthesis * Fix misprint * enhance failed matrix test * Changing RuntimeError to QiskitError * comment * not discarding info for CircuitDataError * tightening try-except blocks * further fix to avoid infinite recursion * Update release note --------- Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> (cherry picked from commit 02fef9b) * Reinstate stronger pylint suppressions --------- Co-authored-by: Shelly Garion <46566946+ShellyGarion@users.noreply.github.com> Co-authored-by: Alexander Ivrii <alexi@il.ibm.com> Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

Shelly Garion (ShellyGarion) added 3 commits February 1, 2026 17:23

catch rust panic errors in qs_decomposition

25169af

add try/catch in qs_decompsition, and fall to Isometry if needed

87e10a8

add test, based on issue 15610

79c709e

Shelly Garion (ShellyGarion) added this to the 2.4.0 milestone Feb 1, 2026

Shelly Garion (ShellyGarion) requested a review from Gadi Aleksandrowicz (gadial) February 1, 2026 15:33

Shelly Garion (ShellyGarion) added the stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge. label Feb 1, 2026

Shelly Garion (ShellyGarion) requested a review from a team as a code owner February 1, 2026 15:33

Shelly Garion (ShellyGarion) added Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. mod: synthesis Rust This PR or issue is related to Rust code in the repository labels Feb 1, 2026

github-project-automation Bot added this to Qiskit 2.4 Feb 1, 2026

github-project-automation Bot moved this to Ready in Qiskit 2.4 Feb 1, 2026

formatting

6edaae1

Shelly Garion (ShellyGarion) mentioned this pull request Feb 1, 2026

qiskit.circuit.Gate.control() broken in Qiskit 2.3.0 #15610

Closed

add release notes

f7b3d45

Gadi Aleksandrowicz (gadial) previously approved these changes Feb 2, 2026

View reviewed changes

Comment thread test/python/circuit/test_controlled_gate.py Resolved

Julien Gacon (Cryoris) previously requested changes Feb 2, 2026

View reviewed changes

Alexander Ivrii (alexanderivrii) and others added 3 commits February 2, 2026 11:05

Merge branch 'main' into ctrl_unitary

757d63b

add Isometry to the test

481f766

Merge branch 'main' into ctrl_unitary

b1c05ef

Shelly Garion (ShellyGarion) dismissed Gadi Aleksandrowicz (gadial)’s stale review via b1c05ef February 2, 2026 09:34

Alexander Ivrii (alexanderivrii) added 2 commits February 2, 2026 12:16

Improving error handling for QSD synthesis

782cfdb

Merge commit 'refs/pull/15635/head' of https://github.com/Qiskit/qiskit…

c68db55

… into ctrl_unitary

Shelly Garion (ShellyGarion) commented Feb 2, 2026

View reviewed changes

Comment thread crates/synthesis/src/qsd.rs Resolved

Fix misprint

2c35d02

Shelly Garion (ShellyGarion) requested review from Julien Gacon (Cryoris) and Gadi Aleksandrowicz (gadial) February 2, 2026 11:35

Shelly Garion (ShellyGarion) modified the milestones: 2.4.0, 2.3.1 Feb 3, 2026

enhance failed matrix test

94fda7b

Matthew Treinish (mtreinish) reviewed Feb 23, 2026

View reviewed changes

Alexander Ivrii (alexanderivrii) added 3 commits March 5, 2026 14:50

properly wrapping around other error types

92521e6

Changing RuntimeError to QiskitError

e36fe93

comment

fc592c8

Alexander Ivrii (alexanderivrii) requested a review from Matthew Treinish (mtreinish) March 5, 2026 14:30

Alexander Ivrii (alexanderivrii) and others added 4 commits March 6, 2026 07:41

not discarding info for CircuitDataError

f92b3c9

tightening try-except blocks

dcc5327

further fix to avoid infinite recursion

23bc439

Update release note

a45b045

Matthew Treinish (mtreinish) approved these changes Mar 12, 2026

View reviewed changes

Matthew Treinish (mtreinish) changed the title ~~handle panic errors in Quantum Shannon Decomposition rust code~~ Avoid panics in Quantum Shannon Decomposition Rust code Mar 12, 2026

Matthew Treinish (mtreinish) enabled auto-merge March 12, 2026 18:58

Matthew Treinish (mtreinish) disabled auto-merge March 12, 2026 18:59

Merge branch 'main' into ctrl_unitary

d743f92

Matthew Treinish (mtreinish) enabled auto-merge March 12, 2026 19:49

Matthew Treinish (mtreinish) added this pull request to the merge queue Mar 12, 2026

Merged via the queue into Qiskit:main with commit 02fef9b Mar 12, 2026
25 checks passed

github-project-automation Bot moved this from Ready to Done in Qiskit 2.4 Mar 12, 2026

mergify Bot mentioned this pull request Mar 12, 2026

Avoid panics in Quantum Shannon Decomposition Rust code (backport #15635) #15802

Merged

Iyán (iyanmv) mentioned this pull request Mar 13, 2026

test_mps_options and test_pauli_noise_with_shot_branching crash and 4 tests from TestControlFlow fail in latest version Qiskit/qiskit-aer#2354

Closed

Shelly Garion (ShellyGarion) mentioned this pull request Mar 24, 2026

Schur decomposition fails during transpilation of a circuit built from a unitary matrix. #15870

Closed

Henry Zou (henryzou50) mentioned this pull request Jun 1, 2026

Code Bug with Synthesizing Permutation Matrix Qiskit/documentation#4928

Closed

1 of 6 tasks

Conversation

Shelly Garion (ShellyGarion) commented Feb 1, 2026

Summary

Details and comments

Uh oh!

qiskit-bot commented Feb 1, 2026

Uh oh!

Coveralls (coveralls) commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 21586424465

Details

💛 - Coveralls

Uh oh!

Gadi Aleksandrowicz (gadial) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Julien Gacon (Cryoris) 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!

Uh oh!

Shelly Garion (ShellyGarion) commented Feb 2, 2026

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!

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!

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!

Shelly Garion (ShellyGarion) Mar 12, 2026 • 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!

Matthew Treinish (mtreinish) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Coveralls (coveralls) commented Feb 1, 2026 •

edited

Loading

Julien Gacon (Cryoris) left a comment •

edited

Loading

Shelly Garion (ShellyGarion) Mar 12, 2026 •

edited

Loading