Skip to content

Delegate BasePass.__call__ to PassManager.run#13820

Merged
mtreinish merged 4 commits intoQiskit:mainfrom
jakelishman:direct-pass-call
Mar 6, 2025
Merged

Delegate BasePass.__call__ to PassManager.run#13820
mtreinish merged 4 commits intoQiskit:mainfrom
jakelishman:direct-pass-call

Conversation

@jakelishman
Copy link
Copy Markdown
Member

@jakelishman jakelishman commented Feb 10, 2025

This ensures that calling a pass directly will follow the same execution logic as a regular pass-manager construction. This particularly matters for passes that have requires. It also ensures that the conversion from a circuit and back to a DAG will follow the same rules, which is expected since it is a shorthand notation, but liable to get out of sync as they are complex.

Summary

Details and comments

Fix #13444.

Requires #13821 to effect the warning on the future behaviour change.

This ensures that calling a pass directly will follow the same execution
logic as a regular pass-manager construction.  This particularly matters
for passes that have `requires`.  It also ensures that the conversion
from a circuit and back to a DAG will follow the same rules, which is
expected since it is a shorthand notation, but liable to get out of sync
as they are complex.
@jakelishman jakelishman added Changelog: Added Add an "Added" entry in the GitHub Release changelog. Changelog: Changed Add a "Changed" entry in the GitHub Release changelog. Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler labels Feb 10, 2025
@jakelishman jakelishman added this to the 2.0.0 milestone Feb 10, 2025
@jakelishman jakelishman requested review from a team and nonhermitian as code owners February 10, 2025 18:24
@qiskit-bot
Copy link
Copy Markdown
Collaborator

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

# is equivalent to the original circuit
perm = pass_.property_set["virtual_permutation_layout"].to_permutation(qc.qubits)
res.append(PermutationGate(perm), [0, 1, 2, 3, 4])
res.append(PermutationGate(res.layout.routing_permutation()), [0, 1, 2, 3, 4])
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

PassManager.run explicitly clears out virtual_permutation_layout from a property_set after execution, since it updates the layout fields. This new version of the PermutationGate is more properly the behaviour we want to test, though.

@jakelishman
Copy link
Copy Markdown
Member Author

Oh whoops, I forgot I hadn't fixed that test error locally.

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 13, 2025

Pull Request Test Coverage Report for Build 13687355258

Details

  • 11 of 11 (100.0%) changed or added relevant lines in 3 files are covered.
  • 20 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.006%) to 87.004%

Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/unitary_synthesis.rs 1 94.49%
crates/qasm2/src/lex.rs 1 92.23%
crates/qasm2/src/parse.rs 18 96.68%
Totals Coverage Status
Change from base Build 13683975981: -0.006%
Covered Lines: 76067
Relevant Lines: 87429

💛 - Coveralls

@1ucian0 1ucian0 self-assigned this Feb 27, 2025
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

This LGTM, it seems pretty straightforward and it'll be good to unify the logic between __call__ and PassManager.run()

try:
return meth(*meth_args, **meth_kwargs)
except TranspilerError:
# If it's already a `TranspilerError` subclass, don't erase the extra information.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I remember discussing this offline, but TranspilerError being a subclass of PassManagerError seems so weird. But it's hard to change this now so this makes sense.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I assume it was intended as a backwards-compatibility shim for a move to PassManagerError during the genericisation of PassManager, but I don't really know.

@mtreinish mtreinish enabled auto-merge March 5, 2025 23:49
@mtreinish mtreinish added this pull request to the merge queue Mar 6, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 6, 2025
@mtreinish mtreinish added this pull request to the merge queue Mar 6, 2025
Merged via the queue into Qiskit:main with commit 1b43c89 Mar 6, 2025
@jakelishman jakelishman deleted the direct-pass-call branch March 6, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Added Add an "Added" entry in the GitHub Release changelog. Changelog: Changed Add a "Changed" entry in the GitHub Release changelog. Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Running ALAPScheduleAnalysis directly on a circuit fails

5 participants