Spectral Embedding #871
Spectral Embedding #871
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
aamijar
added
feature request
cpp/include/cuvs/preprocessing/spectral/spectral_embedding_types.hpp
Outdated
Show resolved
Hide resolved
| raft::copy(knn_coo.cols(), knn_cols.data_handle(), nnz, stream); | ||
| raft::copy(knn_coo.vals(), d_distances.data_handle(), nnz, stream); | ||
|
|
||
| raft::sparse::COO<float> coo_no_zeros(stream); // Don't pre-allocate dimensions |
There was a problem hiding this comment.
It's okay to use this API in internal code, but we need to make sure we're using the new raft sparse coo_matrix_view in any public APIs. raft::sparse::COO is deprecated and we'll eventually have to change this call. Please create an issue in RAFT for updating this call to use the new raft sparse API types and reference that issue here for completeness. That way we can do a simple grep to find this and fix it.
There was a problem hiding this comment.
I'm currently working on using the new coo_matrix types and adding support for the relevant functions in raft. Tracking here: rapidsai/raft#2659 rapidsai/raft#2656
Although I'm running into some cuda invalid memory accesses and I can't seem to debug why its happening. It was passing with my gtests, but when I connected it with cuML it didn't pass the pytests. I've narrowed it down to fail in the gtests when I call transform twice in a row.
There was a problem hiding this comment.
Okay, I think I found the issue which is that I needed to initialize some vectors with zeros.
| const int one = sym_coo.nnz; | ||
| raft::copy(row_ind.data_handle() + row_ind.size() - 1, &one, 1, stream); | ||
|
|
||
| auto csr_structure = raft::make_device_compressed_structure_view<int, int, int>( |
There was a problem hiding this comment.
You are ure using this here- why not also use the coo version above? That'll save us a lot of refactoring time in the future.
There was a problem hiding this comment.
The new csr matrix types are supported in laplacian and lanczos functions. The new coo matrix types aren't supported in the functions where I need them, so that's why I am using the legacy ones. However, I'm trying to migrate to use the new types but currently stuck debugging #871 (comment)
There was a problem hiding this comment.
Now that you've been able to wrap this through cuML, can you provide a sense of the speedup for different datasets? It would be super helpful to know what we are working with here.
| uint64_t seed; | ||
| }; | ||
|
|
||
| // template <typename IndexTypeT, typename ValueTypeT> |
There was a problem hiding this comment.
enable support for <uint32_t/uint64_t, float/double>
No description provided.