Prepare cuvs for removal of deprecated raft apis#1610
Prepare cuvs for removal of deprecated raft apis#1610rapids-bot[bot] merged 20 commits intorapidsai:mainfrom
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. |
jinsolp
left a comment
There was a problem hiding this comment.
Thanks @aamijar , leaving a few comments.
Also, do you think you can fix the related docs as well? I think this is the only place that has outdated docs including raft files, but might be a good idea to check on your end as well : )
cuvs/cpp/src/neighbors/ball_cover.cuh
Lines 29 to 32 in ba67db1
| @@ -405,7 +405,6 @@ if(NOT BUILD_CPU_ONLY) | |||
| src/distance/distance.cu | |||
There was a problem hiding this comment.
shouldn't we add agglomerative.cu in the cmakelists?
|
Also, can we try pinning the raft branch to make sure it's working well on that branch (to ensure we haven't missed anything) |
64c26fb to
cde8d9b
Compare
|
Hi @jinsolp, I think I have addressed your comments |
There was a problem hiding this comment.
Reminder to revert before merging.
jinsolp
left a comment
There was a problem hiding this comment.
Approving with minor suggestions on the docs!
Co-authored-by: Jinsol Park <jinsolp@nvidia.com>
Co-authored-by: Jinsol Park <jinsolp@nvidia.com>
| void build_dendrogram_host(raft::resources const& handle, | ||
| const int64_t* rows, | ||
| const int64_t* cols, | ||
| const float* data, | ||
| size_t nnz, | ||
| int64_t* children, | ||
| float* out_delta, | ||
| int64_t* out_size); | ||
|
|
There was a problem hiding this comment.
cuVS only allows for mdspan based public APIs.
There was a problem hiding this comment.
Can you also drop the _host suffix? That's just an implementation detail.
|
/merge |
Resolves #7554, Depends on rapidsai/cuvs#1610 (CI won't pass until this is merged) What does this PR do? 1. Removes lingering **unused** raft headers that will be deprecated such as `#include <raft/spatial/knn/knn.cuh>`, `#include <raft/distance/distance.cuh>`, etc. 2. ~~Updates to raft::memory_type_from_pointer instead of the deprecated raft::spatial::knn::detail::utils::pointer_residency.~~ 3. Removes `metric_processor` from `knn.hpp` and `knn.cu`. The only special metric processing needed is for correlation distance which we can handle in `knn.cu` instead of using the class from `processing.cuh` from raft. The cosine distance is supported in ivf_flat and ivf_pq in cuvs so we do **not** need to use the innerproduct metric and special processing that was there before. 4. Uses `build_dendrogram_host` from cuvs instead of raft. Authors: - Anupam (https://github.com/aamijar) Approvers: - Victor Lafargue (https://github.com/viclafargue) URL: #7561
…ghbors/` apis (#2885) Supersedes #2813 and #2878 Resolves #2737 and Resolves #2872 Marking as **breaking** to get more eyes on this and mitigate risk. This PR should not break downstream libraries as long as we merge the updates to them first: rapidsai/cuvs#1610, rapidsai/cuml#7561. I've found a usage of breaking api in FAISS here: https://github.com/facebookresearch/faiss/blob/1721ebff6de6ed5a8481302123479be9d85059a2/faiss/gpu/GpuDistance.cu#L46. https://github.com/facebookresearch/faiss/blob/5b19fca3f057b837ac898af52a8eb801c4744892/faiss/gpu/impl/CuvsFlatIndex.cu#L34 What does this PR do? 1. Removes `cluster/`, `distance/`, `neighbors/` (except `detail/faiss_select/`), `sparse/neighbors/`, `spatial/` 2. Removes unused includes that will be deprecated such as `#include <raft/distance/distance.cuh>`, `#include <raft/spatial/knn/knn.cuh>`, etc. 3. Removes legacy lanczos solver (`linalg/lanczos`, `sparse/linalg/lanczos` old functions in `sparse/solver/lanczos`) and removes legacy spectral apis (`spectral/ ` except modularity_maximization and partition which are metrics used by cugraph) 4. Removes corresponding gtests, raft_runtime, and bench files. Authors: - Anupam (https://github.com/aamijar) Approvers: - Bradley Dice (https://github.com/bdice) - Corey J. Nolet (https://github.com/cjnolet) URL: #2885
Resolves rapidsai#7554, Depends on rapidsai/cuvs#1610 (CI won't pass until this is merged) What does this PR do? 1. Removes lingering **unused** raft headers that will be deprecated such as `#include <raft/spatial/knn/knn.cuh>`, `#include <raft/distance/distance.cuh>`, etc. 2. ~~Updates to raft::memory_type_from_pointer instead of the deprecated raft::spatial::knn::detail::utils::pointer_residency.~~ 3. Removes `metric_processor` from `knn.hpp` and `knn.cu`. The only special metric processing needed is for correlation distance which we can handle in `knn.cu` instead of using the class from `processing.cuh` from raft. The cosine distance is supported in ivf_flat and ivf_pq in cuvs so we do **not** need to use the innerproduct metric and special processing that was there before. 4. Uses `build_dendrogram_host` from cuvs instead of raft. Authors: - Anupam (https://github.com/aamijar) Approvers: - Victor Lafargue (https://github.com/viclafargue) URL: rapidsai#7561
Resolves #1601, Resolves #1491
What does this PR do?
#include <raft/neighbors/refine.cuh>,#include <raft/distance/distance_types.hpp>, etc.build_dendrogram_hostwhich is used by cuml. cuml currently uses the raft version. The raft version was moved to cuvs but it is not in the public interface.