Skip to content

Remove thread local thrust::sort (thrust::sort with the execution policy thrust::seq) from copy_v_transform_reduce_key_aggregated_out_nbr#1627

Merged
rapids-bot[bot] merged 3 commits intorapidsai:branch-21.06from
seunghwak:bug_thrust_sort_seq
Jun 1, 2021
Merged

Conversation

@seunghwak
Copy link
Contributor

thrust::sort(thrust::seq, ....) does not work with arbitrary large input data size and this call can fail if the array size to locally sort is large. This code replaces many thread local sort with one thrust::sort call from host.

@seunghwak seunghwak requested a review from a team as a code owner May 26, 2021 01:37
@seunghwak
Copy link
Contributor Author

@ChuckHastings This fixes the error in thrust::sort(thrust::seq, ...) calls.

If I replace

karate.mtx with hollywood.mtx in
https://github.com/rapidsai/cugraph/blob/branch-21.06/cpp/tests/community/mg_louvain_test.cpp#L234

and run ./tests/MG_LOUVAIN_TEST

I got the following error.

(cuda112-cugraph) seunghwak@seunghwak-linux:~/RAPIDS/development/cugraph/cpp/build$ ./tests/MG_LOUVAIN_TEST 
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from simple_test/Louvain_MG_Testfixture
[ RUN      ] simple_test/Louvain_MG_Testfixture.CheckInt32Int32Float/0
/home/seunghwak/RAPIDS/development/cugraph/cpp/tests/community/mg_louvain_test.cpp:160: Failure
Value of: cugraph::test::compare_renumbered_vectors( handle, d_clustering_v, d_dendrogram_gathered_v)
  Actual: false
Expected: true
Google Test trace:
/home/seunghwak/RAPIDS/development/cugraph/cpp/tests/community/mg_louvain_test.cpp:210: compare modularity input: /datasets/test/datasets/hollywood.mtx
/home/seunghwak/RAPIDS/development/cugraph/cpp/tests/community/mg_louvain_test.cpp:160: Failure
Value of: cugraph::test::compare_renumbered_vectors( handle, d_clustering_v, d_dendrogram_gathered_v)
  Actual: false
Expected: true
Google Test trace:
/home/seunghwak/RAPIDS/development/cugraph/cpp/tests/community/mg_louvain_test.cpp:210: compare modularity input: /datasets/test/datasets/hollywood.mtx
/home/seunghwak/RAPIDS/development/cugraph/cpp/tests/community/mg_louvain_test.cpp:38: Failure
Expected equality of these values:
  mg_modularity
    Which is: 0.57750976
  sg_modularity
    Which is: 0.57050145
Google Test trace:
/home/seunghwak/RAPIDS/development/cugraph/cpp/tests/community/mg_louvain_test.cpp:210: compare modularity input: /datasets/test/datasets/hollywood.mtx
[  FAILED  ] simple_test/Louvain_MG_Testfixture.CheckInt32Int32Float/0, where GetParam() = 56-byte object <60-FD C9-96 59-55 00-00 25-00 00-00 00-00 00-00 25-00 00-00 00-00 00-00 02-00 00-00 6A-20 00-00 01-20 00-00 00-00 00-00 64-00 00-00 00-00 00-00 00-00 00-00 00-00 F0-3F> (56353 ms)
[----------] 1 test from simple_test/Louvain_MG_Testfixture (56353 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (56353 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] simple_test/Louvain_MG_Testfixture.CheckInt32Int32Float/0, where GetParam() = 56-byte object <60-FD C9-96 59-55 00-00 25-00 00-00 00-00 00-00 25-00 00-00 00-00 00-00 02-00 00-00 6A-20 00-00 01-20 00-00 00-00 00-00 64-00 00-00 00-00 00-00 00-00 00-00 00-00 F0-3F>

 1 FAILED TEST
(cuda112-cugraph) seunghwak@seunghwak-linux:~/RAPIDS/development/cugraph/cpp/build$ 

@seunghwak seunghwak added 3 - Ready for Review DO NOT MERGE Hold off on merging; see PR for details bug Something isn't working non-breaking Non-breaking change labels May 26, 2021
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.

Tested this with some other changes (will be in a separate PR) and got Louvain working on larger graphs.

@seunghwak seunghwak removed the DO NOT MERGE Hold off on merging; see PR for details label May 28, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2021

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.06    #1627   +/-   ##
===============================================
  Coverage                ?   59.93%           
===============================================
  Files                   ?       79           
  Lines                   ?     3484           
  Branches                ?        0           
===============================================
  Hits                    ?     2088           
  Misses                  ?     1396           
  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 595d5b2...ca37691. Read the comment docs.

@BradReesWork BradReesWork added this to the 21.06 milestone Jun 1, 2021
@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c51216c into rapidsai:branch-21.06 Jun 1, 2021
@seunghwak seunghwak deleted the bug_thrust_sort_seq branch June 24, 2021 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants