Skip to content

Conversation

@alexanderivrii
Copy link
Member

@alexanderivrii alexanderivrii commented Nov 24, 2025

Summary

While reviewing/experimenting with Jake's recent improvements to VF2 (especially the on-the-fly scoring mechanism and the revamp of VF2Layout and VF2PostLayout passes), I've realized that the (very expensive) extra minimization stage of SabrePreLayout is now completely obsolete -- as this minimization is now fully handled by the VF2Layout itself (both in a much better and a much more performant way). This PR removes the obsolete logic, and updates the pass description (including the description of the arguments max_trials_vf2 and max_calls_vf2 to match the relevant argument of VF2Layout).

Details and comments

The SabrePreLayout pass only helps for a very niche set of examples (such as mapping ring-connected circuits into heavy-hex coupling maps) and hence is not included in the default pipeline. Possibly we could include it for optimization_level=3.

If the pass were to be integrated into the transpiler pipeline, it would be run after the VF2Layout pass. To avoid repeating the computation of checking whether there is a perfect map from the circuit's interaction graph into the device connectivity graph, I have added a new argument min_distance; setting it to 2 would skip the unnecessary check.

I have also added a new ASV benchmark for transpiling the efficient_su2(num_qubits=89) circuit into the heavy-hex graph. We already have a similar efficient_su2(num_qubits=100) benchmark, for which VF2 can find a perfect mapping. For 89 the perfect mapping does not exist. Note that Benchpress has both 89 and 100 benchmarks.

@alexanderivrii alexanderivrii requested a review from a team as a code owner November 24, 2025 08:56
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Nov 24, 2025

Pull Request Test Coverage Report for Build 19963223115

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

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • 28 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.02%) to 88.337%

Files with Coverage Reduction New Missed Lines %
crates/circuit/src/parameter/parameter_expression.rs 1 82.3%
crates/qasm2/src/expr.rs 1 93.82%
crates/qasm2/src/lex.rs 8 91.77%
crates/qasm2/src/parse.rs 18 96.15%
Totals Coverage Status
Change from base Build 19941879215: -0.02%
Covered Lines: 96049
Relevant Lines: 108730

💛 - Coveralls

Copy link
Member

Choose a reason for hiding this comment

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

Can you split the new benchmark out into a separate pr? That will oet let us start tracking perform independently sooner but also track improvements or regressions caused by functional PRs to qiskit.

Copy link
Member Author

Choose a reason for hiding this comment

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

This makes sense, removed the ASV change in 5fba876.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added the benchmark in #15419.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants