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 Dijkstra shortest path functions #162

Merged
merged 16 commits into from
Nov 9, 2020

Conversation

mtreinish
Copy link
Member

This commit adds 2 new functions, digraph_dijkstra_shortest_paths() and
graph_dijkstra_shortest_path(), which is a function to get the shortest
path from a node in a graph. It leverages the same dijkstra's algorithm
module which has been modified to get a path in addition to the path
length.

Depends on #161
Fixes #151

This commit adds a new method to the PyDiGraph class, to_undirected(),
which will generate an undirected PyGraph object from the PyDiGraph
object.

Fixes Qiskit#153
This commit adds 2 new functions, digraph_dijkstra_shortest_paths() and
graph_dijkstra_shortest_path(), which is a function to get the shortest
path from a node in a graph. It leverages the same dijkstra's algorithm
module which has been modified to get a path in addition to the path
length.

Depends on Qiskit#161
Fixes Qiskit#151
@mtreinish
Copy link
Member Author

Labeling this as on hold because it's based on top of #161. So blocking until #161 merges

mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Oct 5, 2020
This commit migrates Terra's CouplingMap class to us retworkx internally
instead of networkx providing >10x speed improvement for CouplingMap
operations.

Requires:

Qiskit/rustworkx#157
Qiskit/rustworkx#156
Qiskit/rustworkx#144
Qiskit/rustworkx#143
Qiskit/rustworkx#147
Qiskit/rustworkx#158
Qiskit/rustworkx#162
Qiskit/rustworkx#161

all be applied to the retworkx version installed.
@mtreinish mtreinish added this to the 0.6.0 milestone Oct 6, 2020
@coveralls
Copy link

coveralls commented Oct 7, 2020

Pull Request Test Coverage Report for Build 354557157

  • 91 of 91 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 94.129%

Totals Coverage Status
Change from base Build 350177310: 0.2%
Covered Lines: 2549
Relevant Lines: 2708

💛 - Coveralls

@mtreinish mtreinish removed the on hold label Nov 2, 2020
@mtreinish
Copy link
Member Author

Now that #161 has merged this is unblocked, so removing the on hold label.

This commit fixes an issue with duplicate weight_callable functions that
happened because one was added in this PR's branch and another was
added in a different PR. The functions were mostly identical so this
just consolidates the 2.
@lcapelluto lcapelluto self-assigned this Nov 6, 2020
src/dijkstra.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
tests/test_dijkstra.py Outdated Show resolved Hide resolved
tests/test_dijkstra.py Show resolved Hide resolved
tests/test_dijkstra.py Outdated Show resolved Hide resolved
@mtreinish mtreinish merged commit 0ef76fe into Qiskit:master Nov 9, 2020
@mtreinish mtreinish deleted the add-shortest-path branch November 9, 2020 20:45
mergify bot added a commit to Qiskit/qiskit that referenced this pull request Dec 4, 2020
* WIP: Use retworkx for CouplingMap

This commit migrates Terra's CouplingMap class to us retworkx internally
instead of networkx providing >10x speed improvement for CouplingMap
operations.

Requires:

Qiskit/rustworkx#157
Qiskit/rustworkx#156
Qiskit/rustworkx#144
Qiskit/rustworkx#143
Qiskit/rustworkx#147
Qiskit/rustworkx#158
Qiskit/rustworkx#162
Qiskit/rustworkx#161

all be applied to the retworkx version installed.

* DNM: Install retworkx from source

* DNM: Add retworkx custom branch to travis

* Fix draw() copy paste error

* Fix typo

* DNK: Add source retworkx to docs build

* Fix lint

* Use graph_distance_matrix instead of floyd warshall

* DNM also add retworkx from source for image test job

* Use Gnm random function from retworkx in token swapper tests

* Remove nx import from token swapper tests

* Fix lint

* Remove install from git from CI config since retworkx 0.6.0 is released

* Fix api call

* Use retworkx generators where possible for constructors

* Remvoe source install from travis config

* Bump version in setup.py too

* Use extend_from_edge_list for from_full

* Update qiskit/transpiler/coupling.py

* Use edge_list() return and has_edge() in sabre

In Qiskit/rustworkx#204 the return type of the edge_list() method will be
returned as a custom sequence type the defer the type conversion from rust
to python. So casting to a list no longer will be a no-op after that point
so this commit removes the cast. At the same time in the sabre swap pass
one of the bottlenecks at large qubit counts is traversing that edge
list looking for edges, this updates that to use the has_edge() method
which should be faster than a full list traversal every iteration.

* Fix issue in layout_transformation pass

In #5281 the layout transformation pass was updated to handle the case
where the coupling map was not defined. In those cases for the purposes
of the layout transformation it treats the coupling map as being fully
connected. So it creates a new full coupling map to use for the token
swapper. However, it neglects that the token swapper expects an
undirected graph and was passing in a directed graph. This didn't matter
too much for the networkx based coupling map object because networkx can
handle directed or undirected in the same function. But, for retworkx
directed graphs and undirected graphs are different types an can't be
used interchangeably. This commit fixes this issue in that pass.

* Update qiskit/transpiler/passes/routing/algorithms/token_swapper.py

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* Fix Lint

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.

Add shortest path function
3 participants