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

Make VF2Layout and VF2PostLayout work with control flow #8804

Merged
merged 3 commits into from
Sep 29, 2022

Conversation

jakelishman
Copy link
Member

Summary

The subgraph isomorphism problem we are solving here can fairly easily be extended to account for the available types of control flow in Qiskit right now. We just treat each control flow operation as "transparent" in the pass, i.e. as if each of its blocks were simply inserted verbatim (with the requisite wire mapping) sequentially.

We use the same weighting heuristics for control-flow loop bodies in the scoring as elsewhere; for-loops count for the number of iterations, while if-else and while loops count (all of) their bodies once.

The testing in this particular commit is slightly light, because some of the general strategies involve a transpile call, which isn't fully ready yet with control flow.

Details and comments

Getting VF2 working with control flow was very easy - this sort of "block transparency" lends itself very well to recursive closures - but filling out the tests, especially of VF2PostLayout was more challenging. I'm somewhat open to more things we might want to do, given that right now, we can't yet rely on being able to route a circuit with control flow, which stymies efforts to use transpile. Hopefully #8418 will make this available, however.

Adding this simplifies the work needed to get the control-flow modifications to the preset pass managers working.

The subgraph isomorphism problem we are solving here can fairly easily
be extended to account for the available types of control flow in Qiskit
right now.  We just treat each control flow operation as "transparent"
in the pass, i.e. as if each of its blocks were simply inserted verbatim
(with the requisite wire mapping) sequentially.

We use the same weighting heuristics for control-flow loop bodies in the
scoring as elsewhere; for-loops count for the number of iterations,
while if-else and while loops count (all of) their bodies once.

The testing in this particular commit is slightly light, because some of
the general strategies involve a `transpile` call, which isn't fully
ready yet with control flow.
@jakelishman jakelishman requested a review from a team as a code owner September 29, 2022 00:03
@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 29, 2022

Pull Request Test Coverage Report for Build 3154181627

  • 42 of 42 (100.0%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.002%) to 84.567%

Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 91.49%
Totals Coverage Status
Change from base Build 3154177975: 0.002%
Covered Lines: 60846
Relevant Lines: 71950

💛 - Coveralls

Copy link
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.

I think this LGTM, it is fairly straightforward change to make the interaction graph built with recursing into the control flow blocks. I think we can expand the testing after the fact when we have a full pipeline working. The other thing I think we'll want tests for on VF2PostLayout is the case where strict_direction=True since that is one place where VF2PostLayout and VF2Layout differ in behavior.

@jakelishman jakelishman added the Changelog: New Feature Include in the "Added" section of the changelog label Sep 29, 2022
@mergify mergify bot merged commit a01f738 into Qiskit:main Sep 29, 2022
@jakelishman jakelishman deleted the control-flow-vf2 branch September 29, 2022 22:12
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.

4 participants