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

Fix the difference in 2D partitioning of GPUs in python and C++ #1950

Merged
merged 2 commits into from
Nov 30, 2021

Conversation

seunghwak
Copy link
Contributor

@seunghwak seunghwak commented Nov 17, 2021

If sqrt(# GPUs) is not an integer, prows > pcols in python and prows < pcols in C++.

The C++'s choice leads to better performance (especially for BFS & SSSP and on the systems with low inter-GPU communication bandwidth) and this reduces the difference between python and C++ testing/benchmarking.

@seunghwak seunghwak requested a review from a team as a code owner November 17, 2021 19:43
@seunghwak seunghwak self-assigned this Nov 17, 2021
@seunghwak seunghwak added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 17, 2021
@seunghwak seunghwak added this to the 22.02 milestone Nov 17, 2021
@rlratzel
Copy link
Contributor

rerun tests

Rerunning to get latest updates that should fix build problem.

@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2021

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-22.02    #1950   +/-   ##
===============================================
  Coverage                ?   70.28%           
===============================================
  Files                   ?      143           
  Lines                   ?     8922           
  Branches                ?        0           
===============================================
  Hits                    ?     6271           
  Misses                  ?     2651           
  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 a5c11bc...b29c977. Read the comment docs.

pcols = pcols - 1
return int(ngpus/pcols), pcols
prows = int(math.sqrt(ngpus))
while ngpus % prows != 0:
Copy link
Member

@BradReesWork BradReesWork Nov 29, 2021

Choose a reason for hiding this comment

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

this will be a very fast loop, but you should have just been able to subtract the delta of the modulo result from prows rather than looping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we do this? I am not sure I am fully understanding your intention, but here prows is varying.

Say ngpus = 14. Then, int(sqrt(14)) will be 3. 14 % 3 will be 2. prows (3) - 2 will be 1. But we want prows to be 2.

This code is based on the discussions from https://stackoverflow.com/questions/16266931/input-an-integer-find-the-two-closest-integers-which-when-multiplied-equal-th

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 550685c into rapidsai:branch-22.02 Nov 30, 2021
@seunghwak seunghwak deleted the enh_python_2d_partition branch December 2, 2021 19:09
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