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

[ENH] Added Random Walks COO convertor and profiling #1531

Merged
merged 36 commits into from
Apr 28, 2021

Conversation

aschaffer
Copy link
Collaborator

This PR is used to track enhancements to Random Walks functionality:

  1. Paths2COO convertor: converts coalesced vertex/weight paths to COO format + offsets (including C++ API for Cython);
  2. RW profiling;
  3. Moving functionality / tests out of experimental sub-dirs;

aschaffer and others added 25 commits November 30, 2020 15:00
Update forked branch-0.18 from release
Update branch-0.19 from release
update forked from release branch-0.19
Merge from release branch-0.19
Merge latest rapidsai:branch-0.19 into aschaffer:branch-0.19
@aschaffer aschaffer requested review from a team as code owners April 13, 2021 23:24
cpp/include/algorithms.hpp Outdated Show resolved Hide resolved
cpp/include/utilities/cython.hpp Show resolved Hide resolved
cpp/src/experimental/random_walks.cuh Show resolved Hide resolved
cpp/src/utilities/high_res_timer.hpp Outdated Show resolved Hide resolved
cpp/src/utilities/high_res_timer.hpp Outdated Show resolved Hide resolved
cpp/tests/sampling/random_walks_profiling.cu Outdated Show resolved Hide resolved
cpp/tests/sampling/rw_low_level_test.cu Outdated Show resolved Hide resolved
@rlratzel rlratzel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 14, 2021
@BradReesWork BradReesWork added this to the 0.20 milestone Apr 15, 2021
@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-0.20@f6cecf5). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             branch-0.20    NVIDIA/thrust#1531   +/-   ##
==============================================
  Coverage               ?   60.26%           
==============================================
  Files                  ?       73           
  Lines                  ?     3214           
  Branches               ?        0           
==============================================
  Hits                   ?     1937           
  Misses                 ?     1277           
  Partials               ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6cecf5...ce7ba58. Read the comment docs.

Copy link
Collaborator

@ChuckHastings ChuckHastings left a comment

Choose a reason for hiding this comment

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

Once comment... we can address it later.

@@ -782,3 +782,121 @@ TEST_F(RandomWalksPrimsTest, SimpleGraphRandomWalk)

ASSERT_TRUE(test_all_paths);
}

TEST(RandomWalksSpecialCase, SingleRandomWalk)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason this can't be rw_low_level_test.cpp? We've been trying to migrate to .cpp files where possible since they compile faster.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. check_col_indices() calls thrust.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth considering migrating that to a utility file at some point.

Note we should probably use thrust::count_if in that function due to this
https://github.com/NVIDIA/thrust/issues/1016

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8b1004e into rapidsai:branch-0.20 Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants