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 all_pairs functions #278

Merged
merged 22 commits into from
May 26, 2021
Merged

Conversation

mtreinish
Copy link
Member

This commit adds 2 new api functions all_pairs_dijkstra_shortest_paths()
and all_pairs_dijkstra_path_lengths(). These two functions are similar
to the existing dijkstra functions but they differ in that it will
return dictionaries over all nodes in the graph instead of just for a
single user provided node. Additionally these functions are multihreaded
and the level of parallelism can be controlled by the appropriate
environment variables.

Fixes #271

This commit adds 2 new api functions all_pairs_dijkstra_shortest_paths()
and all_pairs_dijkstra_path_lengths(). These two functions are similar
to the existing dijkstra functions but they differ in that it will
return dictionaries over all nodes in the graph instead of just for a
single user provided node. Additionally these functions are multihreaded
and the level of parallelism can be controlled by the appropriate
environment variables.

Fixes Qiskit#271
@coveralls
Copy link

coveralls commented Mar 18, 2021

Pull Request Test Coverage Report for Build 865105436

  • 316 of 352 (89.77%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.3%) to 95.502%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/iterators.rs 316 352 89.77%
Totals Coverage Status
Change from base Build 864718571: -1.3%
Covered Lines: 5669
Relevant Lines: 5936

💛 - Coveralls

src/lib.rs Outdated Show resolved Hide resolved
This commit expands the test coverage to cover gaps that existed in the
earlier testing. This increased test coverage found some bugs when empty
graphs or graphs without edges were passed into the new functions, those
bugs have been fixed.
@mtreinish
Copy link
Member Author

I ran some benchmarks last night on this:

import time

import retworkx as rx
import networkx as nx

rx_graph = rx.directed_gnp_random_graph(5000, .9)
nx_graph = nx.DiGraph(list(rx_graph.edge_list()))

start = time.time()
rx.digraph_all_pairs_dijkstra_shortest_paths(rx_graph, lambda _: 1)
stop = time.time()
print("retworkx: %s" % str(stop - start))

nx_start = time.time()
# Need to cast as a dict because networkx returns an iterator
dict(nx.all_pairs_dijkstra_path(nx_graph))
nx_stop = time.time()
print("networkx: %s" % str(nx_stop - nx_start))
Library Time (sec)
retworkx 93.04283165931702
networkx 14554.81346654892

src/lib.rs Outdated Show resolved Hide resolved
@mtreinish mtreinish added this to the 0.9.0 milestone May 18, 2021
Copy link
Contributor

@nahumsa nahumsa left a comment

Choose a reason for hiding this comment

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

Great implementation overall. Using AllPairsPathLenghtMapping and AllPairsPathMapping makes it a clean implementation. Just had a few questions regarding the docs.

retworkx/__init__.py Outdated Show resolved Hide resolved
retworkx/__init__.py Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
retworkx/__init__.py Outdated Show resolved Hide resolved
Co-authored-by: Nahum Rosa Cruz Sa <49600259+nahumsa@users.noreply.github.com>
@mtreinish mtreinish merged commit 187d819 into Qiskit:main May 26, 2021
@mtreinish mtreinish deleted the all_pairs-dijkstra branch May 26, 2021 22:01
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 functions for all_pairs_dijkstra_path and all_pairs_dijkstra_path_length
3 participants