Preparing sparse primitives for movement to RAFT #3157
Conversation
…_knn Conflicts: cpp/src_prims/sparse/cusparse_wrappers.h
…l swap them out shortly)
…ce algorithm to make pieces more reusable
Conflicts: CHANGELOG.md
…version seems super expensive, but maybe it's necessary.
|
@divyegala This is ready for re-review when you get a moment. No rush! I started scraping through the prims to make them all use |
cjnolet
added
4 - Waiting on Reviewer
|
@cjnolet dense prims in RAFT are following |
|
@divyegala Thanks! Case-by-case sounds fine to me, so long as the arguments aren't creating redundancy that will lead to unexpected side-effects (e.g. by taking both a handle and a separate stream). I'll submit a PR to clean those up at some point. |
|
@teju85, @divyegala In that case, we should continue the discussion about this publicly (and after we return from the holidays, please). I'd like to avoid the cases where the streams are used inconsistently. |
cjnolet
added
5 - Ready to Merge
|
At primitives level, we should certainly have everyone of it accept a separate |
JohnZed
added
6 - Okay to Auto-Merge
and removed
5 - Ready to Merge
|
rerun tests |
This Pull Request adds initial support for multi-node multi-GPU DBSCAN, and fixes the bugs identified in #3094. It works by copying the dataset on all the workers and giving ownership of a subset of points to each one. The workers compute a partial clustering with the knowledge of the relationships between their points and the rest of the dataset, and the partial clusterings are merged to form the final labeling. This merging algorithm is also used to accumulate the results in case a batch-wise approach is used on a worker to limit the memory consumption. The multi-GPU implementation gives great speedups for large datasets, while for small datasets the performance is dominated by the Dask launch overhead, as shown in the figure below:  Notes: - I have renamed variables in the DBSCAN implementation to match our style conventions (snake case). Sorry for the noise that it adds to this PR. - I refactored some CSR tests to accept multiple test cases instead of hardcoded ones, in order to add corner cases to weak CC. PR #3157 by @cjnolet changed the location of these tests, so I moved those that I had already refactored accordingly. At the moment only the tests that were in `cpp/test/prims/csr.cu` previously have been refactored. I was thinking that the others can be refactored later, I'd like @cjnolet's opinion on this refactoring. - Regarding testing, the MNMG tests are mostly a copy of the single-GPU ones, though I removed a few tests with very small datasets to avoid problems with MNMG (it doesn't really support the edge case where a worker owns 0 sample, as I think it's a fair assumption that MNMG DBSCAN isn't used with such a tiny dataset). - Also regarding tests, I changed the comparison function to account for the fact that border points are ambiguous. It assumes that the labeling of core points is minimal in both our implementation and the reference, so if this assumption changes we will need to update the tests accordingly. If you want to access a pseudo-code description and proof of the new algorithm, feel free to contact me. Tagging people to whom this PR is relevant: @teju85 @tfeher @MatthiasKohl @canonizer Authors: - Louis Sugy (@Nyrio) Approvers: - Tamas Bela Feher (@tfeher) - Corey J. Nolet (@cjnolet) URL: #3382
This PR contains the initial steps to move many of the sparse prims API over to raft, including:
MLCommon::Sparsenamespaces toraft::sparsedevice_bufferanddeviceAllocator.Closes [FEA] Move sparse prims to RAFT #3106