Skip to content

Prepare cuml for removal of deprecated raft apis #7561

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.

Sign up for GitHub

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

Merged
rapids-bot merged 17 commits into from
Dec 15, 2025
Merged

Prepare cuml for removal of deprecated raft apis #7561

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b2d1c07
raft-deprecated-apis
aamijar Dec 3, 2025
7c9199d
clean up
aamijar Dec 3, 2025
df7d906
clean up
aamijar Dec 3, 2025
8883369
Merge branch 'main' into raft-deprecated-apis
aamijar Dec 3, 2025
1d94692
restore spectral.cu
aamijar Dec 3, 2025
cc11ecc
Merge branch 'main' into raft-deprecated-apis
aamijar Dec 8, 2025
f13d176
mdspan update and enable hdbscan test
aamijar Dec 11, 2025
0b1204e
revert test and fix name
aamijar Dec 11, 2025
5899a33
remove unused
aamijar Dec 11, 2025
72776f0
Merge branch 'main' into raft-deprecated-apis
aamijar Dec 13, 2025
0014575
preprocessor
aamijar Dec 14, 2025
ef1f3b7
clean up
aamijar Dec 14, 2025
bc4e91e
clean up
aamijar Dec 14, 2025
988ad8d
clean up
aamijar Dec 14, 2025
fc3da25
pin raft
aamijar Dec 14, 2025
28aef64
remove post-processing l2
aamijar Dec 15, 2025
320cd83
unpin raft
aamijar Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions cpp/include/cuml/manifold/common.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

#pragma once

#include <raft/spatial/knn/detail/ann_utils.cuh>
#include <raft/core/handle.hpp>
#include <raft/core/memory_type.hpp>

#include <stdint.h>

Expand Down Expand Up @@ -110,10 +111,11 @@ struct manifold_precomputed_knn_inputs_t : public manifold_inputs_t<value_t> {
{
// Return true if data is on CPU (need to allocate device memory)
// Return false if data is already on device (no allocation needed)
auto pointer_residency = raft::spatial::knn::detail::utils::check_pointer_residency(
knn_graph.knn_indices, knn_graph.knn_dists);
return pointer_residency == raft::spatial::knn::detail::utils::pointer_residency::host_only ||
pointer_residency == raft::spatial::knn::detail::utils::pointer_residency::mixed;
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);
Copy link
Copy Markdown
Contributor

@viclafargue viclafargue Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Member Author

@aamijar aamijar Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
};

Expand Down
3 changes: 0 additions & 3 deletions cpp/include/cuml/neighbors/knn.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

#include <cuml/common/distance_type.hpp>

#include <raft/spatial/knn/detail/processing.hpp> // MetricProcessor

#include <cstdint>
#include <memory>
#include <vector>
Expand Down Expand Up @@ -89,7 +87,6 @@ struct knnIndex {
ML::distance::DistanceType metric;
float metricArg;
int nprobe;
std::unique_ptr<raft::spatial::knn::MetricProcessor<float>> metric_processor;
int device;

std::unique_ptr<knnIndexImpl> pimpl;
Expand Down
24 changes: 0 additions & 24 deletions cpp/src/knn/knn.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#include <raft/core/device_resources.hpp>
#include <raft/core/handle.hpp>
#include <raft/label/classlabels.cuh>
#include <raft/spatial/knn/ann.cuh>
#include <raft/spatial/knn/knn.cuh>
#include <raft/util/cuda_utils.cuh>

#include <rmm/device_uvector.hpp>
Expand Down Expand Up @@ -216,20 +214,6 @@ void approx_knn_build_index(raft::handle_t& handle,
auto ivf_ft_pams = dynamic_cast<IVFFlatParam*>(params);
auto ivf_pq_pams = dynamic_cast<IVFPQParam*>(params);

index->metric_processor = raft::spatial::knn::create_processor<false, float>(
static_cast<raft::distance::DistanceType>(metric),
n,
D,
0,
raft::resource::get_cuda_stream(handle));
// For cosine/correlation distance, the metric processor translates distance
// to inner product via pre/post processing - pass the translated metric to
// ANN index
if (metric == ML::distance::DistanceType::CosineExpanded ||
metric == ML::distance::DistanceType::CorrelationExpanded) {
metric = index->metric = ML::distance::DistanceType::InnerProduct;
Copy link
Copy Markdown
Contributor

@viclafargue viclafargue Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member Author

@aamijar aamijar Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@viclafargue viclafargue Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

@aamijar aamijar Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member Author

@aamijar aamijar Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@viclafargue viclafargue Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

@aamijar aamijar Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I understood

}
index->metric_processor->preprocess(index_array);
auto index_view = raft::make_device_matrix_view<const float, int64_t>(index_array, n, D);

if (ivf_ft_pams) {
Expand All @@ -256,8 +240,6 @@ void approx_knn_build_index(raft::handle_t& handle,
} else {
RAFT_FAIL("Unrecognized index type.");
}

index->metric_processor->revert(index_array);
}

void approx_knn_search(raft::handle_t& handle,
Expand All @@ -268,9 +250,6 @@ void approx_knn_search(raft::handle_t& handle,
float* query_array,
int n)
{
index->metric_processor->preprocess(query_array);
index->metric_processor->set_num_queries(k);

auto indices_view = raft::make_device_matrix_view<int64_t, int64_t>(indices, n, k);
auto distances_view = raft::make_device_matrix_view<float, int64_t>(distances, n, k);

Expand All @@ -294,8 +273,6 @@ void approx_knn_search(raft::handle_t& handle,
RAFT_FAIL("The model is not trained");
}

index->metric_processor->revert(query_array);

// perform post-processing to show the real distances
Copy link
Copy Markdown
Contributor

@lowener lowener Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (index->metric == ML::distance::DistanceType::L2SqrtExpanded ||
index->metric == ML::distance::DistanceType::L2SqrtUnexpanded ||
Expand All @@ -311,7 +288,6 @@ void approx_knn_search(raft::handle_t& handle,
raft::pow_const_op<float>(p),
raft::resource::get_cuda_stream(handle));
}
index->metric_processor->postprocess(distances);
}

void knn_classify(raft::handle_t& handle,
Expand Down
1 change: 0 additions & 1 deletion cpp/src/knn/knn_opg_common.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include <cumlprims/opg/matrix/part_descriptor.hpp>
#include <raft/core/comms.hpp>
#include <raft/core/handle.hpp>
#include <raft/spatial/knn/knn.cuh>
#include <raft/util/cuda_utils.cuh>
#include <raft/util/cudart_utils.hpp>

Expand Down
1 change: 0 additions & 1 deletion cpp/src/metrics/pairwise_distance.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <cuml/metrics/metrics.hpp>

#include <raft/core/handle.hpp>
#include <raft/distance/distance.cuh>

#include <cuvs/distance/distance.hpp>

Expand Down
1 change: 0 additions & 1 deletion cpp/src/svm/linear.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include <raft/core/handle.hpp>
#include <raft/core/nvtx.hpp>
#include <raft/distance/kernels.cuh>
#include <raft/label/classlabels.cuh>
#include <raft/linalg/gemm.cuh>
#include <raft/linalg/gemv.cuh>
Expand Down
2 changes: 0 additions & 2 deletions cpp/src/svm/smosolver.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
#include "ws_util.cuh"

#include <raft/core/handle.hpp>
#include <raft/distance/distance_types.hpp>
#include <raft/distance/kernels.cuh>
#include <raft/linalg/detail/cublas_wrappers.hpp>
#include <raft/linalg/gemv.cuh>
#include <raft/linalg/unary_op.cuh>
Expand Down
1 change: 0 additions & 1 deletion cpp/src_prims/selection/knn.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <cuml/neighbors/knn.hpp>

#include <raft/core/handle.hpp>
#include <raft/distance/distance.cuh>
#include <raft/label/classlabels.cuh>
#include <raft/util/cuda_utils.cuh>
#include <raft/util/cudart_utils.hpp>
Expand Down
3 changes: 1 addition & 2 deletions cpp/tests/prims/knn_classify.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2019-2024, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/

#include "test_utils.h"

#include <raft/label/classlabels.cuh>
#include <raft/random/make_blobs.cuh>
#include <raft/spatial/knn/knn.cuh>
#include <raft/util/cuda_utils.cuh>
#include <raft/util/cudart_utils.hpp>

Expand Down
1 change: 0 additions & 1 deletion cpp/tests/prims/knn_regression.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <raft/label/classlabels.cuh>
#include <raft/linalg/reduce.cuh>
#include <raft/random/rng.cuh>
#include <raft/spatial/knn/knn.cuh>
#include <raft/util/cuda_utils.cuh>
#include <raft/util/cudart_utils.hpp>

Expand Down
1 change: 0 additions & 1 deletion cpp/tests/sg/dbscan_test.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <cuml/metrics/metrics.hpp>

#include <raft/core/handle.hpp>
#include <raft/distance/distance.cuh>
#include <raft/linalg/transpose.cuh>
#include <raft/util/cuda_utils.cuh>
#include <raft/util/cudart_utils.hpp>
Expand Down
18 changes: 9 additions & 9 deletions cpp/tests/sg/hdbscan_test.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <cuml/cluster/hdbscan.hpp>
#include <cuml/common/distance_type.hpp>

#include <raft/cluster/detail/agglomerative.cuh> // build_dendrogram_host
#include <raft/core/handle.hpp>
#include <raft/linalg/transpose.cuh>
#include <raft/sparse/coo.hpp>
Expand All @@ -23,6 +22,7 @@
#include <thrust/execution_policy.h>
#include <thrust/transform.h>

#include <cuvs/cluster/agglomerative.hpp> // build_dendrogram_host
Copy link
Copy Markdown
Contributor

@lowener lowener Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already included just below

Copy link
Copy Markdown
Member Author

@aamijar aamijar Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 5899a33

#include <cuvs/cluster/agglomerative.hpp>
#include <cuvs/distance/distance.hpp>
#include <cuvs/neighbors/all_neighbors.hpp>
Expand Down Expand Up @@ -165,14 +165,14 @@ class ClusterCondensingTest : public ::testing::TestWithParam<ClusterCondensingI
/**
* Build dendrogram of MST
*/
raft::cluster::detail::build_dendrogram_host(handle,
mst_src.data(),
mst_dst.data(),
mst_data.data(),
params.n_row - 1,
out_children.data(),
out_delta.data(),
out_size.data());
cuvs::cluster::agglomerative::helpers::build_dendrogram_host(handle,
mst_src.data(),
mst_dst.data(),
mst_data.data(),
params.n_row - 1,
out_children.data(),
out_delta.data(),
out_size.data());

/**
* Condense Hierarchy
Expand Down
2 changes: 0 additions & 2 deletions cpp/tests/sg/umap_parametrizable_test.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@

#include <raft/core/handle.hpp>
#include <raft/core/host_coo_matrix.hpp>
#include <raft/distance/distance.cuh>
#include <raft/linalg/reduce_rows_by_key.cuh>
#include <raft/spatial/knn/knn.cuh>
#include <raft/util/cuda_utils.cuh>
#include <raft/util/cudart_utils.hpp>

Expand Down
Loading