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

Support control flow in CommutationAnalysis and CommutativeCancellation #9423

Closed
Tracked by #9417 ...
jakelishman opened this issue Jan 23, 2023 · 3 comments
Closed
Tracked by #9417 ...
Assignees
Labels
mod: transpiler Issues and PRs related to Transpiler type: feature request New feature or request
Milestone

Comments

@jakelishman
Copy link
Member

jakelishman commented Jan 23, 2023

What should we add?

Part of #9417.

These two passes likely come as a pair, but this issue can be split in two if the discovery phase finds that that is preferable.

Details

The commutation analysis and cancellation is the main addition to the optimisation loop at transpiler optimisation level 2. This currently doesn't support control-flow, mostly because the CommutationAnalysis pass doesn't have a way of passing the information about nested blocks through to CommutativeCancellation in the property set. To solve this, we need to:

  • come up with a way to pass the information between the two transpiler passes
  • make CommutationAnalysis recurse through control-flow blocks, generating this analysis
  • have CommutativeCancellation be able to interpret the new information, and enter control-flow blocks to effect it.

This shouldn't be too difficult, beyond finding a data format; commutation relations can't span control-flow blocks, so a fairly simple recursive structure and visitor should be fine. When designing the data format, we may want to consider related work on DAGCircuit.collect_runs, since the collection and exchange of information is largely the same. See #9425.

It is possible that a control-flow block could commute blockwise with another, or with non-controlled operations. For the purposes of this initial support, we do not need to support recognising these cases, but it will be a goal in the future. Please note on closure of this issue whether this was achieved or not.

@jakelishman jakelishman added the type: feature request New feature or request label Jan 23, 2023
@jakelishman jakelishman added this to the 0.24.0 milestone Jan 23, 2023
@jakelishman jakelishman added the mod: transpiler Issues and PRs related to Transpiler label Jan 23, 2023
@jakelishman jakelishman changed the title Support control-flow in CommutationAnalysis and CommutativeCancellation Support control flow in CommutationAnalysis and CommutativeCancellation Jan 23, 2023
@jakelishman jakelishman modified the milestones: 0.24.0, 0.25.0 Apr 3, 2023
@mtreinish
Copy link
Member

The first step for this is being implemented by #9143

@jlapeyre
Copy link
Contributor

jlapeyre commented Jun 27, 2023

It looks like modifying CommutationAnalysis might not be useful at the moment. Even if we modified it to support control flow ops in some sense, we have no use for it. See #9143 (comment)

In fact, I think @ewinston 's approach might also work well for #9426 as well.

EDIT: It does work. See

@mtreinish
Copy link
Member

This was implemented by: #9143 it superseded the need for the commutation analysis pass to be updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: transpiler Issues and PRs related to Transpiler type: feature request New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants