[REVIEW] Remaining sparse semiring distances #261
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…semiring_primitives_optim_final
cjnolet
commented
Jun 15, 2021
…semiring_primitives_optim_final
…semiring_primitives_optim_final
…into semiring_primitive_additional_distances
rapids-bot bot
pushed a commit
that referenced
this pull request
Jun 23, 2021
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
dantegd
requested changes
Jun 24, 2021
| 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.
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.
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. |
dantegd
approved these changes
Jul 12, 2021
|
@gpucibot merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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