-
Notifications
You must be signed in to change notification settings - Fork 197
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
[REVIEW] Remaining sparse semiring distances #261
[REVIEW] Remaining sparse semiring distances #261
Conversation
…semiring_primitives_optim_final
…semiring_primitives_optim_final
…semiring_primitives_optim_final
…into semiring_primitive_additional_distances
This branch includes several new features and optimizations: 1. Introduces a hash table strategy to sparsify the vector in the coo spmv shared memory 2. Adds a batching strategy for rows with nnz too large to fit into shared memory 3. Removes the need for the cusparse csrgemm 4. Uses raft handle in distances_config_t rather than accepting each resource explicitly 5. Removes the naive CSR semiring code This PR is also required to merge #261, which introduces the remaining distances Authors: - Divye Gala (https://github.com/divyegala) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Corey J. Nolet (https://github.com/cjnolet) URL: #269
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had one quick comment/question about using RMM directly, the code for the distances looks good!
raft::mr::device::buffer<value_t> R_sq_norms(alloc, stream, n); | ||
|
||
// sum for mean | ||
raft::mr::device::buffer<value_t> Q_norms(alloc, stream, m); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it wouldn't be too many changes, could you use rmm uvectors in general instead? (just adding a single comment to avoid repeated ones)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't think that would be too much to do for this PR.
rerun tests |
@dantegd, I replaced the usage of the raft device buffer w/ the rmm device uvector everywhere. This is ready to be merged. |
@gpucibot merge |
This PR is intended to be merged after #207 (hash table strategy) has been merged.
This PR introduces the following distances:
Most of the changes here are from #207 and will be reviewed in that PR. The only files that need to be reviewed for this PR are
sparse/distance/l2_distance.cuh
,sparse/distance/bin_distance.cuh
,sparse/distance/lp_distances.cuh
, and their corresponding gtests:test/sparse/distance.cuh