Prepare cuml for removal of deprecated raft apis by aamijar · Pull Request #7561 · rapidsai/cuml

aamijar · 2025-12-03T01:10:39Z

Resolves #7554, Depends on rapidsai/cuvs#1610 (CI won't pass until this is merged)

What does this PR do?

Removes lingering unused raft headers that will be deprecated such as #include <raft/spatial/knn/knn.cuh>, #include <raft/distance/distance.cuh>, etc.
~~Updates to raft::memory_type_from_pointer instead of the deprecated raft::spatial::knn::detail::utils::pointer_residency.~~
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.
Uses build_dendrogram_host from cuvs instead of raft.

copy-pr-bot · 2025-12-03T01:10:42Z

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.

viclafargue

Removes metric_processor from knn.hpp and knn.cu which is not needed since cuvs handles it internally.

Does that mean that the pre/post processing code in there was not interfering with what cuVS is doing and can safely be removed?

If so, shouldn't this be removed too?

lowener · 2025-12-04T17:34:25Z

cpp/tests/sg/hdbscan_test.cu

 #include <thrust/execution_policy.h>
 #include <thrust/transform.h>

+#include <cuvs/cluster/agglomerative.hpp>  // build_dendrogram_host


This is already included just below

Addressed in 5899a33

lowener · 2025-12-04T17:37:36Z

cpp/src/knn/knn.cu


-  index->metric_processor->revert(query_array);
-
  // perform post-processing to show the real distances


How about that post-processing, is it also needed ?

viclafargue · 2025-12-11T15:50:01Z

cpp/include/cuml/manifold/common.hpp

+    auto indices_mem_type = raft::memory_type_from_pointer(knn_graph.knn_indices);
+    auto dists_mem_type   = raft::memory_type_from_pointer(knn_graph.knn_dists);
+
+    return !raft::is_device_accessible(indices_mem_type) ||
+           !raft::is_device_accessible(dists_mem_type);


We are tracking an issue in CI that makes the CUDA context invalid. It looks like it may be an illegal memory accesses that happens when UMAP is given a pre-computed KNN on host memory while the HMM feature enabled (making host pointers device accessible). We disabled pre-computed KNN on host from now, but ideally we would want to enable it while disabling it in the specific case of HMM enabled (we need to investigate the HMM case separately).

Yes, this one has a merge conflict anyway, since this bit of code was reverted. So will remove the changes here

aamijar · 2025-12-14T06:58:33Z

cpp/cmake/thirdparty/get_raft.cmake

-      FORK             rapidsai
-      PINNED_TAG       ${rapids-cmake-checkout-tag}
+      FORK             aamijar
+      PINNED_TAG       raft-deprecated-apis


Revert later

aamijar · 2025-12-14T07:44:36Z

I've updated the PR so that pre and post processing code should functionally be the same. The goal is to remove the dependency on processing.cuh from raft which was implicitly included in #include <raft/spatial/knn/ann.cuh>. So I've taken the bits of code from processing.cuh as put it in knn.cu directly.

viclafargue

Thanks @aamijar! Just two questions on the KNN implementation.

viclafargue · 2025-12-15T10:41:36Z

cpp/src/knn/knn.cu

-  // ANN index
-  if (metric == ML::distance::DistanceType::CosineExpanded ||
-      metric == ML::distance::DistanceType::CorrelationExpanded) {
-    metric = index->metric = ML::distance::DistanceType::InnerProduct;


Do you know why we used to set index->metric?

index->metric is still used as usual at the beginning of the function where it is set to
index->metric = metric then forwarded to cuvs. The reason we set it to inner product in the if statement before is because of the special processing that was required since cuvs ivf-flat and ivf-pq didn't support cosine or correlation. So to do the equivalent computation we used inner product + pre/post processing.

But, if I understood correctly CorrelationExpanded is not supported. We implement pre/post processing here, but should pass InnerProduct metric to cuVS, right? It looks like in this case metric = InnerProduct, but index->metric = CorrelationExpanded. Why don't we do metric = index->metric = InnerProduct?

It's set here on line 250: https://github.com/rapidsai/cuml/pull/7561/changes#diff-8f2dc4c3ec25a07a5620ec55250f415e47d139c5bf3f752e792accec9bbe696eR250

metric variable is what is actually passed to cuvs. index->metric is just recording locally what the original metric was.

Yes, metric is what is sent to cuVS anyway so it should work, but index->metric is kept to CorrelationExpanded unlike before, but I guess this is just an implementation detail and is made on purpose.

Yes, that's what I understood

viclafargue · 2025-12-15T10:46:21Z

cpp/src/knn/knn.cu

+  auto stream      = raft::resource::get_cuda_stream(handle);
+
+  // For correlation: preprocess (center + normalize), use InnerProduct, then revert
+  if (metric == ML::distance::DistanceType::CorrelationExpanded) {


The CorrelationExpanded case seems to be handled correctly. Most metrics would use the DefaultMetricProcessor that does not do anything. But, unless I am missing something it looks like the CorrelationExpanded is not handled atm.

Did you mean CosineExpanded is not handled? CosineExpanded does have support in cuvs ivf-flat and ivf-pq now so we don't need to do special processing and override the metric to be inner product.

https://github.com/rapidsai/cuvs/blob/main/cpp/src/neighbors/ivf_pq/ivf_pq_build.cuh#L1220
https://github.com/rapidsai/cuvs/blob/main/cpp/src/neighbors/ivf_flat/ivf_flat_interleaved_scan.cuh#L1180

Yes, I meant CosineExpanded. Makes sense. What about Lp/L2 metrics postprocessing, is it not handled in cuVS? It looks like we are sending the metricArg to cuVS.

Yes L2 is handled in cuvs, but Lp is not. I've updated the code to only do processing on Lp.

viclafargue

LGTM!

aamijar · 2025-12-15T19:17:31Z

/merge

…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

raft-deprecated-apis

b2d1c07

github-actions bot added CMake CUDA/C++ labels Dec 3, 2025

github-actions bot assigned aamijar Dec 3, 2025

aamijar added 2 commits December 3, 2025 09:58

clean up

7c9199d

clean up

df7d906

aamijar added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Dec 3, 2025

Merge branch 'main' into raft-deprecated-apis

8883369

aamijar marked this pull request as ready for review December 3, 2025 10:11

aamijar requested review from a team as code owners December 3, 2025 10:11

aamijar requested review from robertmaynard and viclafargue December 3, 2025 10:11

restore spectral.cu

1d94692

github-actions bot removed the CMake label Dec 3, 2025

aamijar removed request for a team and robertmaynard December 3, 2025 10:16

aamijar mentioned this pull request Dec 3, 2025

Remove neighbors/, cluster/, distance/, spatial/, sparse/neighbors/ apis rapidsai/raft#2885

Merged

viclafargue reviewed Dec 4, 2025

View reviewed changes

lowener reviewed Dec 4, 2025

View reviewed changes

aamijar and others added 4 commits December 8, 2025 14:33

Merge branch 'main' into raft-deprecated-apis

cc11ecc

mdspan update and enable hdbscan test

f13d176

revert test and fix name

0b1204e

remove unused

5899a33

viclafargue reviewed Dec 11, 2025

View reviewed changes

aamijar and others added 3 commits December 12, 2025 23:03

Merge branch 'main' into raft-deprecated-apis

72776f0

preprocessor

0014575

clean up

ef1f3b7

aamijar added 3 commits December 14, 2025 06:47

clean up

bc4e91e

clean up

988ad8d

pin raft

fc3da25

github-actions bot added the CMake label Dec 14, 2025

aamijar commented Dec 14, 2025

View reviewed changes

viclafargue requested changes Dec 15, 2025

View reviewed changes

aamijar added 2 commits December 15, 2025 18:03

remove post-processing l2

28aef64

unpin raft

320cd83

github-actions bot removed the CMake label Dec 15, 2025

viclafargue approved these changes Dec 15, 2025

View reviewed changes

rapids-bot bot merged commit cdc006c into rapidsai:main Dec 15, 2025
106 checks passed


		index->metric_processor->revert(query_array);

		// perform post-processing to show the real distances

Conversation

aamijar commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot bot commented Dec 3, 2025

Uh oh!

viclafargue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aamijar commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

viclafargue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aamijar Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viclafargue Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aamijar Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viclafargue Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viclafargue left a comment

Choose a reason for hiding this comment

Uh oh!

aamijar commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aamijar commented Dec 3, 2025 •

edited

Loading

aamijar commented Dec 14, 2025 •

edited

Loading

aamijar Dec 15, 2025 •

edited

Loading

viclafargue Dec 15, 2025 •

edited

Loading

aamijar Dec 15, 2025 •

edited

Loading

viclafargue Dec 15, 2025 •

edited

Loading