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

Fix tracking of routing permutation in Sabre with disjoint backends #13833

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

jakelishman
Copy link
Member

Summary

If the backing coupling graph is disjoint, and unused components of the coupling graph would not be considered when constructing the complete routing permutation. In practice, Sabre aborts immediately after layout without attempting to route, if it needed to split the DAG across more than one disjoint component, because it can't guarantee correctness of the final routing in the presence of component-spanning barriers or classical communication, so the only way for a component to be forgotten is if the backend is disjoint, but the DAG fits into a single component.

Details and comments

Fix #13732, though there's a whole cornucopia of bugs split between very old versions of Qiskit, this commit's parent, and the example in the issue itself that lead from that issue seeing a regression in Qiskit 1.3 to this one.

In actuality, this bug has been around since the disjoint-coupling handling was first added in #9802 (qiskit-terra 0.24), and the test case in this PR would fail that version too (the fact that TranspileLayout.initial_index_layout wasn't added til later notwithstanding - the first assertion fails).

If the backing coupling graph is disjoint, and unused components of the
coupling graph would not be considered when constructing the complete
routing permutation.  In practice, Sabre aborts immediately after layout
without attempting to route, if it needed to split the DAG across more
than one disjoint component, because it can't guarantee correctness of
the final routing in the presence of component-spanning barriers or
classical communication, so the only way for a component to be forgotten
is if the backend is disjoint, but the DAG fits into a single component.
@jakelishman jakelishman added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels Feb 12, 2025
@jakelishman jakelishman added this to the 1.3.3 milestone Feb 12, 2025
@jakelishman jakelishman requested a review from a team as a code owner February 12, 2025 22:29
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM!! Thanks @jakelishman for diving right into the rabbit hole!

qc.cx(2, 0)

disjoint = CouplingMap([(0, 1), (1, 2), (3, 4), (4, 5)])
layout_routing_pass = SabreLayout(disjoint, seed=2025_02_12, swap_trials=1, layout_trials=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Very precise seed

@ElePT ElePT added this pull request to the merge queue Feb 13, 2025
github-merge-queue bot pushed a commit that referenced this pull request Feb 13, 2025
…13833)

If the backing coupling graph is disjoint, and unused components of the
coupling graph would not be considered when constructing the complete
routing permutation.  In practice, Sabre aborts immediately after layout
without attempting to route, if it needed to split the DAG across more
than one disjoint component, because it can't guarantee correctness of
the final routing in the presence of component-spanning barriers or
classical communication, so the only way for a component to be forgotten
is if the backend is disjoint, but the DAG fits into a single component.
@ElePT ElePT removed this pull request from the merge queue due to a manual request Feb 13, 2025
@ElePT
Copy link
Contributor

ElePT commented Feb 13, 2025

Hmm I removed the PR from the queue because I saw the failed neko test in the merge CI run, but I don't think it's related to this PR. I think it's related to changes in qiskit-aer.

@ElePT ElePT added this pull request to the merge queue Feb 13, 2025
Merged via the queue into Qiskit:main with commit b933179 Feb 13, 2025
17 checks passed
mergify bot pushed a commit that referenced this pull request Feb 13, 2025
…13833)

If the backing coupling graph is disjoint, and unused components of the
coupling graph would not be considered when constructing the complete
routing permutation.  In practice, Sabre aborts immediately after layout
without attempting to route, if it needed to split the DAG across more
than one disjoint component, because it can't guarantee correctness of
the final routing in the presence of component-spanning barriers or
classical communication, so the only way for a component to be forgotten
is if the backend is disjoint, but the DAG fits into a single component.

(cherry picked from commit b933179)
@jakelishman jakelishman deleted the sabre-disjoint-routing branch February 13, 2025 10:09
github-merge-queue bot pushed a commit that referenced this pull request Feb 13, 2025
…13833) (#13835)

If the backing coupling graph is disjoint, and unused components of the
coupling graph would not be considered when constructing the complete
routing permutation.  In practice, Sabre aborts immediately after layout
without attempting to route, if it needed to split the DAG across more
than one disjoint component, because it can't guarantee correctness of
the final routing in the presence of component-spanning barriers or
classical communication, so the only way for a component to be forgotten
is if the backend is disjoint, but the DAG fits into a single component.

(cherry picked from commit b933179)

Co-authored-by: Jake Lishman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: transpiler Issues and PRs related to Transpiler stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when running final_index_layout(filter_ancillas=False)
3 participants