Don't run routing in SabreLayout with split components #10000
Conversation
This commit fixes an issue in the `SabreLayout` pass when run on backends with a disjoint connectivity graph. The `SabreLayout` pass internally runs routing as part of its operation and as a performance efficiency we use that routing output by default. However, in the case of a disjoint coupling map we are splitting the circuit up into different subcircuits to map that to the connected components on the backend. Doing final routing as part of that split context is no sound because depsite the quantum operations being isolated to each component, other operations could have a dependency between the components. This was previously discovered during the development of Qiskit#9802 to be the case with classical data/bits, but since merging that it also has come up with barriers. To prevent any issues in the future this commit changes the SabreLayout pass to never use the routing output during the layout search when there is >1 connected component because we *need* to rerun routing on the full circuit after applying the layout. Fixes Qiskit#9995
|
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:
|
Pull Request Test Coverage Report for Build 4756578146
💛 - Coveralls |
| # We also skip routing here if there is more than one connected | ||
| # component we ran layout on. We can only reliably route the full dag | ||
| # in this case if there is any dependency between the components | ||
| # (typically shared classical data or barriers). |
There was a problem hiding this comment.
This is a bit unclear to me. What does this mean ?
there is more than one connected component we ran layout on
There are more than one connected components. Did we run layout on some of them?
Can we reliably route if there is a dependency or if there is no dependency?
There was a problem hiding this comment.
The way the layout problem gets split up is a function of both the circuit and the coupling map. If the coupling map is disjoint and the circuit isn't we only run layout over a single connected component from the coupling map. The only case when we're not safe to execute routing as part of this pass is when we have more than one circuit component and coupling map component so that we run the inner layout function >1 time. In those cases there is a potential data dependency across the output from the split layout runs' connected components (which is the combination of the connected components from the circuit applied to connected components on the device) in the non-quantum component of the circuit.
* Don't run routing in SabreLayout with split components This commit fixes an issue in the `SabreLayout` pass when run on backends with a disjoint connectivity graph. The `SabreLayout` pass internally runs routing as part of its operation and as a performance efficiency we use that routing output by default. However, in the case of a disjoint coupling map we are splitting the circuit up into different subcircuits to map that to the connected components on the backend. Doing final routing as part of that split context is no sound because depsite the quantum operations being isolated to each component, other operations could have a dependency between the components. This was previously discovered during the development of Qiskit#9802 to be the case with classical data/bits, but since merging that it also has come up with barriers. To prevent any issues in the future this commit changes the SabreLayout pass to never use the routing output during the layout search when there is >1 connected component because we *need* to rerun routing on the full circuit after applying the layout. Fixes Qiskit#9995 * Remove unused shared_clbits logic * Remove swap and routing assertions from disjoint sabre layout tests * Fix lint
Summary
This commit fixes an issue in the
SabreLayoutpass when run on backends with a disjoint connectivity graph. TheSabreLayoutpass internally runs routing as part of its operation and as a performance efficiency we use that routing output by default. However, in the case of a disjoint coupling map we are splitting the circuit up into different subcircuits to map that to the connected components on the backend. Doing final routing as part of that split context is no sound because depsite the quantum operations being isolated to each component, other operations could have a dependency between the components. This was previously discovered during the development of #9802 to be the case with classical data/bits, but since merging that it also has come up with barriers. To prevent any issues in the future this commit changes the SabreLayout pass to never use the routing output during the layout search when there is >1 connected component because we need to rerun routing on the full circuit after applying the layout.Details and comments
Fixes #9995