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

Add full path transpile() support for disjoint backends #9840

Merged
merged 14 commits into from
Apr 17, 2023

Conversation

mtreinish
Copy link
Member

Summary

This commit finalizes the last piece of the full path transpile()
support at all optimization levels. The primary piece to accomplish this
was adding DenseLayout support for targeting a disjoint CouplingMap.
With this piece added then all the default transpiler paths in the
preset pass managers are able to support running with a disjoint
CouplingMap. The biggest exception is the TrivialLayout pass which can
result in an invalid layout being selected (where 2q connectivity
crosses connected components). To handle this edge case a check is added
to each routing stage to ensure the selected layout is valid for the
coupling map and it is routable. This enables all the default paths at
all 4 optimization levels for transpile() to be usable with disjoint
connectivity. For optimization_level=0/TrivialLayout if the trivial
layout is invalid the routing pass will raise an error saying as much.

It's worth pointing out that NoiseAdaptiveLayout still doesn't
support disjoint connectivity. However, since it's performance is poor
and it's not used by default in any preset passmanager we can add this
at a later date, but all other combinations of layout and routing
methods should work (given the caveats with TrivialLayout) above.

Details and comments

This PR is based on top of #9802 and #9710 and is tagged as on hold until they both merge.
It will be be rebased after both of those PRs merge. To see the contents of just this PR you can
view:

mtreinish/qiskit-core@layout_and_route_disjoint_coupling_maps...mtreinish:qiskit-core:full-transpile-disjoint-support

@mtreinish mtreinish added on hold Can not fix yet Changelog: New Feature Include in the "Added" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels Mar 22, 2023
@mtreinish mtreinish added this to the 0.24.0 milestone Mar 22, 2023
@mtreinish mtreinish requested a review from a team as a code owner March 22, 2023 21:08
@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 Mar 22, 2023

Pull Request Test Coverage Report for Build 4703893491

  • 49 of 51 (96.08%) changed or added relevant lines in 7 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 85.834%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/layout/dense_layout.py 29 30 96.67%
qiskit/transpiler/passes/routing/bip_mapping.py 1 2 50.0%
Files with Coverage Reduction New Missed Lines %
qiskit/transpiler/coupling.py 1 93.96%
crates/qasm2/src/lex.rs 5 91.9%
Totals Coverage Status
Change from base Build 4703027952: 0.02%
Covered Lines: 70539
Relevant Lines: 82181

💛 - Coveralls

@mtreinish mtreinish force-pushed the full-transpile-disjoint-support branch from 78bdfb0 to c3fdaca Compare March 23, 2023 21:32
@kdk kdk mentioned this pull request Apr 3, 2023
6 tasks
@mtreinish mtreinish linked an issue Apr 3, 2023 that may be closed by this pull request
@mtreinish mtreinish force-pushed the full-transpile-disjoint-support branch from c3fdaca to f4f3595 Compare April 6, 2023 12:46
@kdk kdk self-assigned this Apr 6, 2023
@mtreinish mtreinish force-pushed the full-transpile-disjoint-support branch from da52e9b to 8df2130 Compare April 6, 2023 19:14
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Apr 6, 2023
Building off Qiskit#9840 which adds full path support in all the preset pass
managers for targetting backends with a disconnected coupling graph,
this commit adds support for ignoring qubits that do not support any
operations. When a Target is generated from Qiskit#9911 with `filter_faulty`
set to `True` this will potentially result in qubits being present in
the `Target` without any supported operations. In these cases the
layout passes in the transpiler might inadvertently use these qubits
only to fail in the basis translator because there are no instructions
available. This commit adds filtering of connected components from the
list of output connected components if the `Target` does have any
supported instructions on a qubit.

This works by building a copy of the coupling map's internal graph
that removes the nodes which do not have any supported operations.
Then when we compute the connected components of this graph it will
exclude any components of isolated qubits without any operations
supported. A similar change is made to the coupling graph we pass to
rustworkx.vf2_mapping() inside the vf2 layout family of passes.
@mtreinish mtreinish removed the on hold Can not fix yet label Apr 6, 2023
This commit finalizes the last piece of the full path transpile()
support at all optimization levels. The primary piece to accomplish this
was adding DenseLayout support for targeting a disjoint CouplingMap.
With this piece added then all the default transpiler paths in the
preset pass managers are able to support running with a disjoint
CouplingMap. The biggest exception is the TrivialLayout pass which can
result in an invalid layout being selected (where 2q connectivity
crosses connected components). To handle this edge case a check is added
to each routing stage to ensure the selected layout is valid for the
coupling map and it is routable. This enables all the default paths at
all 4 optimization levels for transpile() to be usable with disjoint
connectivity. For optimization_level=0/TrivialLayout if the trivial
layout is invalid the routing pass will raise an error saying as much.

It's worth pointing out that NoiseAdaptiveLayout still doesn't
support disjoint connectivity. However, since it's performance is poor
and it's not used by default in any preset passmanager we can add this
at a later date, but all other combinations of layout and routing
methods should work (given the caveats with TrivialLayout) above.
Now that Qiskit#9802 supports shared classical bits correctly this commit
re-enables the tests disabled in the previous commit.
@mtreinish mtreinish force-pushed the full-transpile-disjoint-support branch from 8df2130 to 0041f3f Compare April 13, 2023 21:22
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Apr 13, 2023
Building off Qiskit#9840 which adds full path support in all the preset pass
managers for targetting backends with a disconnected coupling graph,
this commit adds support for ignoring qubits that do not support any
operations. When a Target is generated from Qiskit#9911 with `filter_faulty`
set to `True` this will potentially result in qubits being present in
the `Target` without any supported operations. In these cases the
layout passes in the transpiler might inadvertently use these qubits
only to fail in the basis translator because there are no instructions
available. This commit adds filtering of connected components from the
list of output connected components if the `Target` does have any
supported instructions on a qubit.

This works by building a copy of the coupling map's internal graph
that removes the nodes which do not have any supported operations.
Then when we compute the connected components of this graph it will
exclude any components of isolated qubits without any operations
supported. A similar change is made to the coupling graph we pass to
rustworkx.vf2_mapping() inside the vf2 layout family of passes.
Copy link
Contributor

@kevinhartman kevinhartman left a comment

Choose a reason for hiding this comment

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

Just a few small comments.

qiskit/transpiler/passes/routing/bip_mapping.py Outdated Show resolved Hide resolved
test/python/compiler/test_transpiler.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kevinhartman kevinhartman left a comment

Choose a reason for hiding this comment

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

LGTM.

@kevinhartman kevinhartman added this pull request to the merge queue Apr 17, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 17, 2023
@mtreinish mtreinish added this pull request to the merge queue Apr 17, 2023
Merged via the queue into Qiskit:main with commit 5dbac1e Apr 17, 2023
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Apr 17, 2023
Building off Qiskit#9840 which adds full path support in all the preset pass
managers for targetting backends with a disconnected coupling graph,
this commit adds support for ignoring qubits that do not support any
operations. When a Target is generated from Qiskit#9911 with `filter_faulty`
set to `True` this will potentially result in qubits being present in
the `Target` without any supported operations. In these cases the
layout passes in the transpiler might inadvertently use these qubits
only to fail in the basis translator because there are no instructions
available. This commit adds filtering of connected components from the
list of output connected components if the `Target` does have any
supported instructions on a qubit.

This works by building a copy of the coupling map's internal graph
that removes the nodes which do not have any supported operations.
Then when we compute the connected components of this graph it will
exclude any components of isolated qubits without any operations
supported. A similar change is made to the coupling graph we pass to
rustworkx.vf2_mapping() inside the vf2 layout family of passes.
@mtreinish mtreinish deleted the full-transpile-disjoint-support branch April 18, 2023 20:39
kevinhartman added a commit that referenced this pull request Apr 18, 2023
* Filter/ignore qubits in Target without any operations

Building off #9840 which adds full path support in all the preset pass
managers for targetting backends with a disconnected coupling graph,
this commit adds support for ignoring qubits that do not support any
operations. When a Target is generated from #9911 with `filter_faulty`
set to `True` this will potentially result in qubits being present in
the `Target` without any supported operations. In these cases the
layout passes in the transpiler might inadvertently use these qubits
only to fail in the basis translator because there are no instructions
available. This commit adds filtering of connected components from the
list of output connected components if the `Target` does have any
supported instructions on a qubit.

This works by building a copy of the coupling map's internal graph
that removes the nodes which do not have any supported operations.
Then when we compute the connected components of this graph it will
exclude any components of isolated qubits without any operations
supported. A similar change is made to the coupling graph we pass to
rustworkx.vf2_mapping() inside the vf2 layout family of passes.

* Expand testing

* Make filtered qubit coupling map a Target.build_coupling_map option

This commit reworks the logic to construct a filtered coupling map as an
optional argument on `Target.build_coupling_map()`. This makes the
filtering a function of the Target object itself, which is where the
context/data about which qubits support operations or not lives. The
previous versions of this PR had a weird mix of responsibilities where
the target would generate a coupling map and then we'd pass the target
to that coupling map to do an additional round of filtering on it.

* Apply suggestions from code review

Co-authored-by: John Lapeyre <[email protected]>

* Fix incorrect set construction

* Expand docstring on build_coupling_map argument

* Rework logic in vf2 passes for filtering

* Update argument name in disjoint_utils.py

* Inline second argument for require_layout_isolated_to_component

Co-authored-by: Kevin Hartman <[email protected]>

* Update qiskit/transpiler/passes/layout/vf2_post_layout.py

Co-authored-by: Kevin Hartman <[email protected]>

* Apply suggestions from code review

Co-authored-by: Kevin Hartman <[email protected]>

* Remove unnecessary len()

* Inline second arg for dense_layout too

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Kevin Hartman <[email protected]>
ikkoham pushed a commit to ikkoham/qiskit-terra that referenced this pull request Apr 19, 2023
* Add full path transpile() support for disjoint backends

This commit finalizes the last piece of the full path transpile()
support at all optimization levels. The primary piece to accomplish this
was adding DenseLayout support for targeting a disjoint CouplingMap.
With this piece added then all the default transpiler paths in the
preset pass managers are able to support running with a disjoint
CouplingMap. The biggest exception is the TrivialLayout pass which can
result in an invalid layout being selected (where 2q connectivity
crosses connected components). To handle this edge case a check is added
to each routing stage to ensure the selected layout is valid for the
coupling map and it is routable. This enables all the default paths at
all 4 optimization levels for transpile() to be usable with disjoint
connectivity. For optimization_level=0/TrivialLayout if the trivial
layout is invalid the routing pass will raise an error saying as much.

It's worth pointing out that NoiseAdaptiveLayout still doesn't
support disjoint connectivity. However, since it's performance is poor
and it's not used by default in any preset passmanager we can add this
at a later date, but all other combinations of layout and routing
methods should work (given the caveats with TrivialLayout) above.

* Fix test determinism

* Add future facing test for shared classical bits between components

* Enable shared control flow test

Now that Qiskit#9802 supports shared classical bits correctly this commit
re-enables the tests disabled in the previous commit.

* Remove unused condition for faulty qubits backnedv1 path

* Remove opt level 1 variant of test_chained_data_dependency

* Use enumerate() in check_layout_isolated_to_component()

Co-authored-by: Kevin Hartman <[email protected]>

* Expand level 0 test coverage

* Update test/python/compiler/test_transpiler.py

Co-authored-by: Kevin Hartman <[email protected]>

* s/check_layout_isolated_to_component/require_layout_isolated_to_component/g

---------

Co-authored-by: Kevin Hartman <[email protected]>
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
* Add full path transpile() support for disjoint backends

This commit finalizes the last piece of the full path transpile()
support at all optimization levels. The primary piece to accomplish this
was adding DenseLayout support for targeting a disjoint CouplingMap.
With this piece added then all the default transpiler paths in the
preset pass managers are able to support running with a disjoint
CouplingMap. The biggest exception is the TrivialLayout pass which can
result in an invalid layout being selected (where 2q connectivity
crosses connected components). To handle this edge case a check is added
to each routing stage to ensure the selected layout is valid for the
coupling map and it is routable. This enables all the default paths at
all 4 optimization levels for transpile() to be usable with disjoint
connectivity. For optimization_level=0/TrivialLayout if the trivial
layout is invalid the routing pass will raise an error saying as much.

It's worth pointing out that NoiseAdaptiveLayout still doesn't
support disjoint connectivity. However, since it's performance is poor
and it's not used by default in any preset passmanager we can add this
at a later date, but all other combinations of layout and routing
methods should work (given the caveats with TrivialLayout) above.

* Fix test determinism

* Add future facing test for shared classical bits between components

* Enable shared control flow test

Now that Qiskit#9802 supports shared classical bits correctly this commit
re-enables the tests disabled in the previous commit.

* Remove unused condition for faulty qubits backnedv1 path

* Remove opt level 1 variant of test_chained_data_dependency

* Use enumerate() in check_layout_isolated_to_component()

Co-authored-by: Kevin Hartman <[email protected]>

* Expand level 0 test coverage

* Update test/python/compiler/test_transpiler.py

Co-authored-by: Kevin Hartman <[email protected]>

* s/check_layout_isolated_to_component/require_layout_isolated_to_component/g

---------

Co-authored-by: Kevin Hartman <[email protected]>
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
* Filter/ignore qubits in Target without any operations

Building off Qiskit#9840 which adds full path support in all the preset pass
managers for targetting backends with a disconnected coupling graph,
this commit adds support for ignoring qubits that do not support any
operations. When a Target is generated from Qiskit#9911 with `filter_faulty`
set to `True` this will potentially result in qubits being present in
the `Target` without any supported operations. In these cases the
layout passes in the transpiler might inadvertently use these qubits
only to fail in the basis translator because there are no instructions
available. This commit adds filtering of connected components from the
list of output connected components if the `Target` does have any
supported instructions on a qubit.

This works by building a copy of the coupling map's internal graph
that removes the nodes which do not have any supported operations.
Then when we compute the connected components of this graph it will
exclude any components of isolated qubits without any operations
supported. A similar change is made to the coupling graph we pass to
rustworkx.vf2_mapping() inside the vf2 layout family of passes.

* Expand testing

* Make filtered qubit coupling map a Target.build_coupling_map option

This commit reworks the logic to construct a filtered coupling map as an
optional argument on `Target.build_coupling_map()`. This makes the
filtering a function of the Target object itself, which is where the
context/data about which qubits support operations or not lives. The
previous versions of this PR had a weird mix of responsibilities where
the target would generate a coupling map and then we'd pass the target
to that coupling map to do an additional round of filtering on it.

* Apply suggestions from code review

Co-authored-by: John Lapeyre <[email protected]>

* Fix incorrect set construction

* Expand docstring on build_coupling_map argument

* Rework logic in vf2 passes for filtering

* Update argument name in disjoint_utils.py

* Inline second argument for require_layout_isolated_to_component

Co-authored-by: Kevin Hartman <[email protected]>

* Update qiskit/transpiler/passes/layout/vf2_post_layout.py

Co-authored-by: Kevin Hartman <[email protected]>

* Apply suggestions from code review

Co-authored-by: Kevin Hartman <[email protected]>

* Remove unnecessary len()

* Inline second arg for dense_layout too

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Kevin Hartman <[email protected]>
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 mod: transpiler Issues and PRs related to Transpiler priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot target certain sub-graphs on a faulty backend
5 participants