Skip to content

Use rust-native error for CommutativeOptimization - #16197

Open
raynelfss wants to merge 3 commits into
Qiskit:mainfrom
raynelfss:use-commutation-opt-error
Open

Use rust-native error for CommutativeOptimization#16197
raynelfss wants to merge 3 commits into
Qiskit:mainfrom
raynelfss:use-commutation-opt-error

Conversation

@raynelfss

@raynelfss raynelfss commented May 16, 2026

Copy link
Copy Markdown
Contributor

The following commits modify CommutativeOptimization to use rust-native errors throughout its pipeline.
We implement the CommutativeOptimizationError enumeration which derives from DAGCircuitError, RemoveIdentityEquivError, and CommutationError as well as adding two new variants for python specific errors dealing with parameters and pauli evolution gates.

Pre-requisites

AI/LLM disclosure

  • I didn't use LLM tooling, or only used it privately.
  • I used the following tool to help write this PR description:
  • I used the following tool to generate or modify code:

@raynelfss raynelfss added this to the 2.5.0 milestone May 16, 2026
@raynelfss
raynelfss requested a review from a team as a code owner May 16, 2026 15:43
@raynelfss raynelfss added the on hold Can not fix yet label May 16, 2026
@raynelfss raynelfss added the Changelog: None Do not include in the GitHub Release changelog. label May 16, 2026
@raynelfss
raynelfss requested a review from jakelishman May 16, 2026 15:43
@raynelfss raynelfss added Rust This PR or issue is related to Rust code in the repository mod: transpiler Issues and PRs related to Transpiler labels May 16, 2026
@qiskit-bot

Copy link
Copy Markdown
Collaborator

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

  • @Qiskit/terra-core

@coveralls

coveralls commented May 16, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 26459707076

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.002%) to 87.518%

Details

  • Coverage increased (+0.002%) from the base build.
  • Patch coverage: 19 uncovered changes across 3 files (32 of 51 lines covered, 62.75%).
  • 4 coverage regressions across 2 files.

Uncovered Changes

File Changed Covered %
crates/transpiler/src/passes/commutative_optimization.rs 34 23 67.65%
crates/transpiler/src/passes/remove_identity_equiv.rs 12 5 41.67%
crates/transpiler/src/commutation_checker.rs 1 0 0.0%
Total (4 files) 51 32 62.75%

Coverage Regressions

4 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
crates/qasm2/src/lex.rs 3 92.54%
crates/transpiler/src/commutation_checker.rs 1 88.66%

Coverage Stats

Coverage Status
Relevant Lines: 124014
Covered Lines: 108535
Line Coverage: 87.52%
Coverage Strength: 956333.88 hits per line

💛 - Coveralls

raynelfss added 3 commits May 26, 2026 11:48
The following commits modify `RemoveIdentityEquiv` to use rust-native errors throughout its pipeline.
In this PR we introduce the error enumeration `RemoveIdentityEquivEerror` which only has two variants, one for `DAGCircuitError` and the other is for a specific case involving `PauliEvolution` gates that only exist in Python currently.
These changes mostly depend on Qiskit#16134.
The following commits modify `CommutationAnalysis` to use rust-native errors throughout its pipeline.
This is done mostly by implementing `DAGCircuitInnerError` based on Qiskit#16134 as a possible error in `CommutationError`.
The following commits modify `CommutativeOptimization` to use rust-native errors throughout its pipeline.
We implement the `CommutativeOptimizationError` enumeration which derives from `DAGCircuitError`, `RemoveIdentityEquivError`, and `CommutationError` as well as adding two new variants for python specific errors dealing with parameters and pauli evolution gates.
@raynelfss
raynelfss force-pushed the use-commutation-opt-error branch from a729c5e to 2f72743 Compare May 26, 2026 16:00
Comment on lines +58 to +59
#[error(transparent)]
DAGCircuit(#[from] DAGError),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need this new variant?

#[error(transparent)]
RemoveIdentityEq(#[from] RemoveIdentityEquivError),
#[error(transparent)]
CommutationAnalysis(#[from] CommutationError),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the naming here can be confusing: CommutativeOptimization does not use the CommutationAnalysis pass or functionality there, but just calls the commutation checker. Could we change the variant name here to clarify this separation? E.g. just call this CommutationError or CommutationCheck or something?

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

Labels

Changelog: None Do not include in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler on hold Can not fix yet Rust This PR or issue is related to Rust code in the repository

Projects

Status: Blocked

Development

Successfully merging this pull request may close these issues.

5 participants