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

swap_cancellation pass #5902

Closed
wants to merge 4 commits into from
Closed

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Feb 24, 2021

As part of preserving unitary during transpilation, some swap gates are inserted with #5280.

These swaps might be adjacent to swap gates introduced by the routing passes. Pairs of swaps can be cancelled as pairs of cnots.

This pass is a copy of the cx_cancellation pass, but for swap gates.

@1ucian0 1ucian0 requested a review from a team as a code owner February 24, 2021 17:12
Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

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

Is it possible to instead generalize the CXCancellation pass to handle other self-inverse gates? Ideally, this could be stored as a property of the Gate classes in the standard library, but even for the short term, the pass could hold a hard-coded list of self-inverse gates. What do you think?

@1ucian0 1ucian0 marked this pull request as draft May 17, 2021 12:11
Returns:
DAGCircuit: Transformed DAG.
"""
swap_runs = dag.collect_runs(["swap"])
Copy link
Member

Choose a reason for hiding this comment

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

Does this actually work as expected? The collect_runs method was mostly for 1q gates I don't think we have any tests of using it with >1q gates. Looking at the rustworkx code for the function (https://github.com/Qiskit/rustworkx/blob/main/src/dag_algo/mod.rs#L460-L506 ) it should work I think, but having a testing with partially overlapping swaps (eg, swap(0,1) -> swap(1, 2) -> swap (2, 0)) to make sure the behavior is sound

@jakelishman
Copy link
Member

I believe this PR is superseded by the extra work in InverseCancellation that eventually resulted in #11210. Feel free to re-open if there's more to discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants