Skip to content
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

Add new transpiler exception class for too many qubits #11241

Merged
merged 4 commits into from Nov 14, 2023

Conversation

mtreinish
Copy link
Member

Summary

This commit adds a new exception class for when the transpiler is given a circuit too many qubits for a given backend. Previously the generic TranspilerError was raised for this, but it made it difficult for downstream users to catch as it wasn't easy to differentiate this error condition from other TranspilerError exceptions. There isn't any backwards compatibility issues with this because the new CircuitToWideForTarget class is a subclass of TranspilerError so any of the previous catches for TranspilerError will still catch this.

Details and comments

This commit adds a new exception class for when the transpiler is
given a circuit too many qubits for a given backend. Previously the
generic TranspilerError was raised for this, but it made it difficult
for downstream users to catch as it wasn't easy to differentiate this
error condition from other TranspilerError exceptions. There isn't any
backwards compatibility issues with this because the new
CircuitToWideForTarget class is a subclass of TranspilerError so any of
the previous catches for TranspilerError will still catch this.
@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Nov 14, 2023
@mtreinish mtreinish added this to the 1.0.0 milestone Nov 14, 2023
@mtreinish mtreinish requested a review from a team as a code owner November 14, 2023 18:03
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

This seems like a generally sensible option. Would ApplyLayout and SetLayout potentially raise this as well?

qiskit/transpiler/exceptions.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Nov 14, 2023

Pull Request Test Coverage Report for Build 6869462528

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 5 of 5 (100.0%) changed or added relevant lines in 3 files are covered.
  • 19 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 85.911%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
crates/qasm2/src/lex.rs 6 91.67%
crates/qasm2/src/parse.rs 12 97.13%
Totals Coverage Status
Change from base Build 6865596785: -0.02%
Covered Lines: 65926
Relevant Lines: 76738

💛 - Coveralls

@mtreinish
Copy link
Member Author

mtreinish commented Nov 14, 2023

This seems like a generally sensible option. Would ApplyLayout and SetLayout potentially raise this as well?

It turns out the answer is no because neither SetLayout or ApplyLayout take in a target. They just blindly set the layout in the property set or take the layout from the property set and transform the circuit based on that. I think in general we'll want to refactor how this check is handled because besides not catching the generate_preset_pass_manager() case the current logic also precludes something like qiskit-qubit-reuse shrinking the circuit prior to layout. I'm thinking the easiest way is to add a new pass that runs in the pre_layout stage which checks if the circuit is too big for the target and raises with this exception. But that's outside the scope of this PR I think.

@jakelishman
Copy link
Member

Yeah that sounds all good to me - if there's nothing else that should raise it, I'm happy this PR is complete from the feature side. Do you want to enforce the new type of the exception in the transpile call with a test?

@mtreinish
Copy link
Member Author

Do you want to enforce the new type of the exception in the transpile call with a test?

I adjusted the existing test to catch the more specific error in: 15af4e2

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@jakelishman jakelishman modified the milestones: 1.0.0, 1.0.0pre1 Nov 14, 2023
@jakelishman jakelishman added this pull request to the merge queue Nov 14, 2023
Merged via the queue into Qiskit:main with commit d68076b Nov 14, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants