Skip to content

Improve MG PageRank scalability#2038

Merged
rapids-bot[bot] merged 85 commits intorapidsai:branch-22.04from
seunghwak:enh_mg_pagerank2
Mar 14, 2022
Merged

Improve MG PageRank scalability#2038
rapids-bot[bot] merged 85 commits intorapidsai:branch-22.04from
seunghwak:enh_mg_pagerank2

Conversation

@seunghwak
Copy link
Contributor

@seunghwak seunghwak commented Jan 26, 2022

Improve MG PageRank performance & scalability in multi-node many GPU systems

…utation (currently with the temporary mechanism to support stream priorities, eventually, rmm should be updated to support this)
@seunghwak seunghwak added 2 - In Progress improvement Improvement / enhancement to an existing function DO NOT MERGE Hold off on merging; see PR for details non-breaking Non-breaking change labels Jan 26, 2022
@seunghwak seunghwak added this to the 22.04 milestone Jan 26, 2022
@seunghwak seunghwak self-assigned this Jan 26, 2022
@seunghwak seunghwak requested a review from a team as a code owner January 26, 2022 23:40
@seunghwak seunghwak changed the title [WIP][skip-ci] Improve MG PageRank scalability Improve MG PageRank scalability Mar 9, 2022
@seunghwak seunghwak added 3 - Ready for Review and removed 2 - In Progress DO NOT MERGE Hold off on merging; see PR for details labels Mar 9, 2022
@seunghwak
Copy link
Contributor Author

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2022

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-22.04    #2038   +/-   ##
===============================================
  Coverage                ?   73.63%           
===============================================
  Files                   ?      154           
  Lines                   ?    10327           
  Branches                ?        0           
===============================================
  Hits                    ?     7604           
  Misses                  ?     2723           
  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 93dba00...a4f6528. Read the comment docs.

@BradReesWork BradReesWork requested a review from aschaffer March 11, 2022 17:17
major_tmp_buffers.reserve(num_concurrent_loops);
for (size_t i = 0; i < num_concurrent_loops; ++i) {
size_t max_size{0};
for (size_t j = i; j < graph_view.get_number_of_local_adj_matrix_partitions();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not do a max_element() here (lines 581-584)? It's more readable and possibly faster. I understand you have a stride, but you can pass a sequence and calculate the stride inside a lambda comparer, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I pass a sequence?

Can we do this without using thrust/boost (i.e. without using counting_iterator/transform_iterator).

We can create an additional vector storing a sequence, but then, I am not sure the code will be more readable.

Could you show me the code?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use counting_iterator/transform_iterator on std::vector<> with thrust::host policy and, say, thrust::reduce() or thrust::maximum_element() but the counter arithmetic can be messy. You're right the resulting code would probably NOT be any more readable.

major_tmp_buffers.reserve(num_concurrent_loops);
for (size_t i = 0; i < num_concurrent_loops; ++i) {
size_t max_size{0};
for (size_t j = i; j < graph_view.get_number_of_local_adj_matrix_partitions();
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use counting_iterator/transform_iterator on std::vector<> with thrust::host policy and, say, thrust::reduce() or thrust::maximum_element() but the counter arithmetic can be messy. You're right the resulting code would probably NOT be any more readable.

@ChuckHastings
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 5fe65f6 into rapidsai:branch-22.04 Mar 14, 2022
@seunghwak seunghwak deleted the enh_mg_pagerank2 branch August 11, 2022 23:24
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.

5 participants