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

Lift trivial transpiler passes to handle control flow #8752

Merged
merged 2 commits into from
Sep 15, 2022

Conversation

jakelishman
Copy link
Member

Summary

This converts all transpiler passes whose structure allows their run methods to be made trivially recursive, either by eagerly modifying all control-flow operations in a depth-first pass before running the method on the top level of the circuit, or by inserting the recursion during an interior loop.

There are other passes that are relatively simple to handle as well (UnitarySynthesis is one), but their current structure would require just a shade more work than the trivial cases in this commit.

Details and comments

Close #8445 (by obsoleting it). We can rework #8565 to do essentially this as well - we probably just want to take a bit more care than that PR currently does not to redo all the setup work in the pass.

This converts all transpiler passes whose structure allows their `run`
methods to be made trivially recursive, either by eagerly modifying all
control-flow operations in a depth-first pass before running the method
on the top level of the circuit, or by inserting the recursion during an
interior loop.

There are other passes that are relatively simple to handle as well
(`UnitarySynthesis` is one), but their current structure would require
just a shade more work than the trivial cases in this commit.
@jakelishman jakelishman requested a review from a team as a code owner September 14, 2022 14:47
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Sep 14, 2022

Pull Request Test Coverage Report for Build 3063338228

  • 45 of 45 (100.0%) changed or added relevant lines in 12 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 84.462%

Totals Coverage Status
Change from base Build 3063335322: 0.008%
Covered Lines: 59534
Relevant Lines: 70486

💛 - Coveralls

Copy link
Contributor

@ewinston ewinston left a comment

Choose a reason for hiding this comment

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

Overall this LGTM. Just a minor comment about tests.

@jakelishman jakelishman added automerge Changelog: None Do not include in changelog labels Sep 15, 2022
@jakelishman
Copy link
Member Author

Ok sure, I'll tag this for automerge and we can revisit later if needed. I've not added a changelog because I imagine we'll do that in a unified manner for all the control-flow transpiler work.

@mergify mergify bot merged commit 64a9eb1 into Qiskit:main Sep 15, 2022
@jakelishman jakelishman deleted the trivial-transpiler-recursion branch September 15, 2022 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants