Skip to content

Do not construct layout passes with no coupling map #14353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 18, 2025

Conversation

jakelishman
Copy link
Member

Summary

The builtin plugins fairly frequently construct instances of the layout passes with coupling_map=None (or similar), even when this is against the documentation of the passes. A couple of passes, but not all, have defensive code in their run methods to raise a better error in this case, but even so, there is an assumption that passes will not be run with coupling_map=None.

The preset pass managers do follow this assumption; conditional code prevents those passes from ever being called. However, we know that there's no coupling map ahead of time, so we can simply not even construct the layout passes, let alone add them to the PassManager.

Details and comments

Input normalisation typically wants to be done in pass initialisation, not deferred to runtime (though expensive properties can still be computed only on first run and cached). The current strategy of silently ignoring invalid inputs until runtime makes it harder to do this; it's clearer to have the pass-manager construction logic never produce objects in invalid states in the first place.

The builtin plugins fairly frequently construct instances of the layout
passes with `coupling_map=None` (or similar), even when this is against
the documentation of the passes.  A couple of passes, but not all, have
defensive code in their `run` methods to raise a better error in this
case, but even so, there is an assumption that passes will not be _run_
with `coupling_map=None`.

The preset pass managers do follow this assumption; conditional code
prevents those passes from ever being called.  However, we know that
there's no coupling map ahead of time, so we can simply not even
construct the layout passes, let alone add them to the `PassManager`.
@jakelishman jakelishman added this to the 2.1.0 milestone May 13, 2025
@jakelishman jakelishman requested a review from a team as a code owner May 13, 2025 08:31
@jakelishman jakelishman added Changelog: API Change Include in the "Changed" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels May 13, 2025
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14991951790

Details

  • 11 of 13 (84.62%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.003%) to 88.716%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/preset_passmanagers/builtin_plugins.py 11 13 84.62%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 5 91.73%
Totals Coverage Status
Change from base Build 14991351960: 0.003%
Covered Lines: 76084
Relevant Lines: 85761

💛 - Coveralls

Copy link
Member

@alexanderivrii alexanderivrii left a comment

Choose a reason for hiding this comment

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

Thanks @jakelishman, this looks great. I am not convinced that this is technically even an API change as there is no effect on using the modified plugins, and this does not force the users to upgrade their custom layout pass manager stage plugins.

@alexanderivrii alexanderivrii added this pull request to the merge queue May 18, 2025
Merged via the queue into Qiskit:main with commit a18e151 May 18, 2025
25 checks passed
@jakelishman jakelishman deleted the no-layout-presets branch May 18, 2025 09:13
rahaman-quantum pushed a commit to rahaman-quantum/qiskit that referenced this pull request Jun 20, 2025
The builtin plugins fairly frequently construct instances of the layout
passes with `coupling_map=None` (or similar), even when this is against
the documentation of the passes.  A couple of passes, but not all, have
defensive code in their `run` methods to raise a better error in this
case, but even so, there is an assumption that passes will not be _run_
with `coupling_map=None`.

The preset pass managers do follow this assumption; conditional code
prevents those passes from ever being called.  However, we know that
there's no coupling map ahead of time, so we can simply not even
construct the layout passes, let alone add them to the `PassManager`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog mod: transpiler Issues and PRs related to Transpiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants