Make heuristic for TwoQubitPeepholeOptimization configurable by mtreinish · Pull Request #16290 · Qiskit/qiskit

mtreinish · 2026-05-27T18:08:33Z

This commit adds an option to the TwoQubitPeepholeOptimization transpiler pass added in #13419 to allow configuring the heuristic priority when instantiating the pass. When the pass is making a decision on which potential synthesis outcome out of multiple is the best or when to replace a block with the best synthesis outcome there are three metrics we look at, the estimated fidelity, the number of 2q gates, and the total number of gates. The order of this comparison is inherently flexible and prioritizes different aspects of the synthesis. This exposes a new option to the pass to enable users to specify which aspect they want the pass to prioritize.

Previously the pass was hard coded to prioritize 2q gate count, but as was discussed in the PR review and the commit history of the PR branch this isn't necessarily the ideal choice. Intuiatively assuming relatively accurate error rates in the target the estimated fidelity should be the first priority. This was changed to prioritize two qubit gate count during development as a debugging step and left in place while we worked on issues with the TwoQubitControlledUDecomposer's 1q component handling (see #16123). Now that the TwoQubitControlledUDecomposer issue has been resolved we can explore switching the default, this new argument will make it much easier to do side by side comparisons of the different options. The intent is to enable updating the usage in the preset pass managers (added in #16136) without having to modify the pass's rust code.

This does not include a release not as the the new pass has not been included in a release yet so there is no addition to a public API in this PR.

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:

This commit adds an option to the TwoQubitPeepholeOptimization transpiler pass added in Qiskit#13419 to allow configuring the heuristic priority when instantiating the pass. When the pass is making a decision on which potential synthesis outcome out of multiple is the best or when to replace a block with the best synthesis outcome there are three metrics we look at, the estimated fidelity, the number of 2q gates, and the total number of gates. The order of this comparison is inherently flexible and prioritizes different aspects of the synthesis. This exposes a new option to the pass to enable users to specify which aspect they want the pass to prioritize. Previously the pass was hard coded to prioritize 2q gate count, but as was discussed in the PR review and the commit history of the PR branch this isn't necessarily the ideal choice. Intuiatively assuming relatively accurate error rates in the target the estimated fidelity should be the first priority. This was changed to prioritize two qubit gate count during development as a debugging step and left in place while we worked on issues with the TwoQubitControlledUDecomposer's 1q component handling (see Qiskit#16123). Now that the TwoQubitControlledUDecomposer issue has been resolved we can explore switching the default, this new argument will make it much easier to do side by side comparisons of the different options. The intent is to enable updating the usage in the preset pass managers (added in Qiskit#16136) without having to modify the pass's rust code. This does not include a release not as the the new pass has not been included in a release yet so there is no addition to a public API in this PR.

qiskit-bot · 2026-05-27T18:08:38Z

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

@Qiskit/terra-core

coveralls · 2026-05-27T18:42:55Z

Coverage Report for CI Build 26544164435

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.01%) to 87.62%

Details

Coverage increased (+0.01%) from the base build.
Patch coverage: 12 uncovered changes across 1 file (104 of 116 lines covered, 89.66%).
2 coverage regressions across 2 files.

Uncovered Changes

File	Changed	Covered	%
crates/transpiler/src/passes/two_qubit_peephole.rs	105	93	88.57%
Total (2 files)	116	104	89.66%

Coverage Regressions

2 previously-covered lines in 2 files lost coverage.

File	Lines Losing Coverage	Coverage
crates/circuit/src/parameter/parameter_expression.rs	1	91.04%
crates/qasm2/src/lex.rs	1	92.8%

Coverage Stats


Relevant Lines:	124095
Covered Lines:	108732
Line Coverage:	87.62%
Coverage Strength:	990794.43 hits per line

💛 - Coveralls

The dervied PartialOrd trait for an enum will use a lexicographic ordering based on the order of the variants defined in an enum as the first match. The fields are secondary to that. This is not semantically correct for how the score enums are defined because there is no ordering between mismatched variants. In practice this was not likely to be an issue, because it would be an internally incorrect logic and as the code is written it's impossible for the enums to be compare differing variants. But to ensure the PartialOrd implementation is correct even if the edge case is never used, this commit replaces the derived impl of the PartialOrd trait with a hand implemented one that uses the correct logic.

ShellyGarion

Thanks for improving the peephole optimization heuristic.
I would be happy to see a test that distinguishes between various heuristics.

ShellyGarion · 2026-06-02T09:56:07Z

+pub enum HeuristicPriority {
+    EstimatedFidelity,
+    TwoQGate,
+    TotalGate,


I wonder if we should call these:

TwoQGateCount, TotalGateCount,

since in the future we might be interested in Depth? but we can also keep these names as is, just add a comment.

ShellyGarion · 2026-06-02T09:57:22Z

+    TotalGate,
+}
+
+/// Scored used as the heuristic of the unitary synthesis output


Suggested change

/// Scored used as the heuristic of the unitary synthesis output

/// Score used as the heuristic of the unitary synthesis output

ShellyGarion · 2026-06-02T09:58:31Z

+
+/// Scored used as the heuristic of the unitary synthesis output
+///
+/// This differs from from [`ComparisonScore`] since the unitary synthesis


Suggested change

/// This differs from from [`ComparisonScore`] since the unitary synthesis

/// This differs from [`ComparisonScore`] since the unitary synthesis

ShellyGarion · 2026-06-02T09:58:44Z

+/// Scored used as the heuristic of the unitary synthesis output
+///
+/// This differs from from [`ComparisonScore`] since the unitary synthesis
+/// scoring is trying to maximize so we use negative counts and i64. The


Suggested change

/// scoring is trying to maximize so we use negative counts and i64. The

/// scoring aims to maximize so we use negative counts and i64. The

ShellyGarion · 2026-06-02T09:58:59Z

+///
+/// This differs from from [`ComparisonScore`] since the unitary synthesis
+/// scoring is trying to maximize so we use negative counts and i64. The
+/// comparison score is minimizing the gate counts so it uses usize which is


Suggested change

/// comparison score is minimizing the gate counts so it uses usize which is

/// comparison score minimizes the gate counts so it uses usize which is

ShellyGarion · 2026-06-02T09:59:58Z

+/// the natural type of the counts.
+#[derive(Debug, Clone, Copy, PartialEq)]
+enum BestSynthesisHeuristicScore {
+    GatePriority(i64, f64, i64),


Again, I wonder if GateCountPriority is a better name, or we can keep this name, and comment that it's count (and not depth)?

ShellyGarion · 2026-06-02T10:11:28Z

+                if two potential synthesis outcomes for a given block:
+
+                    * Output A: 2 two qubit gates, an estimated fidelity of 0.98, and 8 total gates versus
+                    * Output B: 3 two qubit gates, an estimated fidelity of 0.99, and 6 total gates.


I like this example! can we add something like this example to the tests?
Namely, that changing the heuristic gives a different circuit?

mtreinish added this to the 2.5.0 milestone May 27, 2026

mtreinish requested a review from a team as a code owner May 27, 2026 18:08

mtreinish requested a review from jakelishman May 27, 2026 18:08

mtreinish added Changelog: None Do not include in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler labels May 27, 2026

github-project-automation Bot added this to Qiskit 2.5 May 27, 2026

github-project-automation Bot moved this to Ready in Qiskit 2.5 May 27, 2026

ShellyGarion reviewed Jun 2, 2026

View reviewed changes

ShellyGarion self-assigned this Jun 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make heuristic for TwoQubitPeepholeOptimization configurable#16290

Make heuristic for TwoQubitPeepholeOptimization configurable#16290
mtreinish wants to merge 2 commits into
Qiskit:mainfrom
mtreinish:make-peephole-heuristic-configurable

mtreinish commented May 27, 2026

Uh oh!

qiskit-bot commented May 27, 2026

Uh oh!

coveralls commented May 27, 2026 •

edited

Loading

Uh oh!

ShellyGarion left a comment

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

ShellyGarion Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	/// Scored used as the heuristic of the unitary synthesis output
	/// Score used as the heuristic of the unitary synthesis output

	/// This differs from from [`ComparisonScore`] since the unitary synthesis
	/// This differs from [`ComparisonScore`] since the unitary synthesis

	/// scoring is trying to maximize so we use negative counts and i64. The
	/// scoring aims to maximize so we use negative counts and i64. The

	/// comparison score is minimizing the gate counts so it uses usize which is
	/// comparison score minimizes the gate counts so it uses usize which is

Conversation

mtreinish commented May 27, 2026

AI/LLM disclosure

Uh oh!

qiskit-bot commented May 27, 2026

Uh oh!

coveralls commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 26544164435

Coverage increased (+0.01%) to 87.62%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

ShellyGarion left a comment

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ShellyGarion Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented May 27, 2026 •

edited

Loading