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.

Sign up for GitHub

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

Jump to bottom

nx-cugraph: add more shortest path algorithms #4199

Merged
merged 13 commits into from Mar 13, 2024
Merged

nx-cugraph: add more shortest path algorithms #4199

merged 13 commits into from Mar 13, 2024

Conversation

eriknw
Copy link
Contributor

@eriknw eriknw commented Feb 28, 2024

This begins by adding more unweighted shortest path algorithms. Next we'll do weighted via sssp, then generic.

Note that there are some performance improvements that can be made:

  • add bidirectional search between source and target
    • for bidirectional_shortest_path and has_path
  • alternatively, perform bfs from source until target is reached
  • run all_pairs* in batched groups

@eriknw eriknw requested a review from a team as a code owner February 28, 2024 14:57
@eriknw eriknw added the improvement Improvement / enhancement to an existing function label Mar 11, 2024
@BradReesWork BradReesWork added the non-breaking Non-breaking change label Mar 12, 2024
@BradReesWork BradReesWork added this to the 24.04 milestone Mar 12, 2024
rlratzel
rlratzel approved these changes Mar 12, 2024
View reviewed changes
Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

LGTM, I did have a few comments and questions which need not hold up approval.

python/nx-cugraph/README.md Show resolved Hide resolved
python/nx-cugraph/_nx_cugraph/__init__.py Outdated Show resolved Hide resolved
python/nx-cugraph/nx_cugraph/algorithms/centrality/eigenvector.py Show resolved Hide resolved
acostadon
acostadon approved these changes Mar 13, 2024
View reviewed changes
Copy link
Contributor

@acostadon acostadon left a comment

Choose a reason for hiding this comment

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

Looks great. I have a couple comments but nothing worth slowing it down.

python/nx-cugraph/README.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to explain the basic way cuGraph runs these algos. How the "conversions" are or are not done.

python/nx-cugraph/scripts/update_readme.py
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes permissions to be executable

@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 6b28aef into rapidsai:branch-24.04 Mar 13, 2024
137 checks passed
@rlratzel rlratzel mentioned this pull request Apr 4, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rlratzel rlratzel rlratzel approved these changes

@BradReesWork BradReesWork BradReesWork approved these changes

@acostadon acostadon acostadon approved these changes

Assignees
No one assigned
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Milestone
24.04
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@eriknw @BradReesWork @rlratzel @acostadon @nv-rliu