Skip to content

Commit ea4c1d6

Browse files
committed
remove build_result completely and remove build() overloads that return build_result
1 parent 3ea5f56 commit ea4c1d6

17 files changed

Lines changed: 96 additions & 139 deletions

File tree

c/src/neighbors/cagra.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -200,21 +200,21 @@ void _build(cuvsResources_t res,
200200
// Device `cagra::build` requires a row stride compatible with 16-byte alignment; bare DLPack
201201
// buffers (e.g. small dim) are often tightly packed and must be copied via `make_padded_dataset`.
202202
if (cuvs::neighbors::device_matrix_row_width_matches_cagra_required(mds)) {
203-
auto view = cuvs::neighbors::make_padded_dataset_view(*res_ptr, mds);
204-
auto build_res = cuvs::neighbors::cagra::build(*res_ptr, index_params, view);
205-
auto* raw = new cuvs::neighbors::cagra::index<T, uint32_t>(std::move(build_res.idx));
203+
auto view = cuvs::neighbors::make_padded_dataset_view(*res_ptr, mds);
204+
auto index = cuvs::neighbors::cagra::build(*res_ptr, index_params, view);
205+
auto* raw = new cuvs::neighbors::cagra::index<T, uint32_t>(std::move(index));
206206
assign_standalone_index<T>(output_index, output_index->dtype, raw);
207207
} else {
208-
auto padded = cuvs::neighbors::make_padded_dataset(*res_ptr, mds);
209-
auto build_res = cuvs::neighbors::cagra::build(*res_ptr, index_params, padded->as_dataset_view());
208+
auto padded = cuvs::neighbors::make_padded_dataset(*res_ptr, mds);
209+
auto index = cuvs::neighbors::cagra::build(*res_ptr, index_params, padded->as_dataset_view());
210210
if (index_params.compression.has_value()) {
211-
auto* raw = new cuvs::neighbors::cagra::index<T, uint32_t>(std::move(build_res.idx));
211+
auto* raw = new cuvs::neighbors::cagra::index<T, uint32_t>(std::move(index));
212212
assign_standalone_index<T>(output_index, output_index->dtype, raw);
213213
} else {
214214
auto* holder = new cuvs_cagra_c_api_lifetime_holder<T>{
215215
cuvs::neighbors::wrap_any_owning(std::move(padded)),
216216
raft::device_matrix<T, int64_t>(*res_ptr),
217-
std::move(build_res.idx)};
217+
std::move(index)};
218218
assign_lifetime_holder<T>(output_index, output_index->dtype, holder);
219219
}
220220
}
@@ -232,16 +232,16 @@ void _build(cuvsResources_t res,
232232
nullptr, std::move(storage), std::move(result.idx)};
233233
assign_lifetime_holder<T>(output_index, output_index->dtype, holder);
234234
} else {
235-
auto padded = cuvs::neighbors::make_padded_dataset(*res_ptr, mds);
236-
auto build_res = cuvs::neighbors::cagra::build(*res_ptr, index_params, padded->as_dataset_view());
235+
auto padded = cuvs::neighbors::make_padded_dataset(*res_ptr, mds);
236+
auto index = cuvs::neighbors::cagra::build(*res_ptr, index_params, padded->as_dataset_view());
237237
if (index_params.compression.has_value()) {
238-
auto* raw = new cuvs::neighbors::cagra::index<T, uint32_t>(std::move(build_res.idx));
238+
auto* raw = new cuvs::neighbors::cagra::index<T, uint32_t>(std::move(index));
239239
assign_standalone_index<T>(output_index, output_index->dtype, raw);
240240
} else {
241241
auto* holder = new cuvs_cagra_c_api_lifetime_holder<T>{
242242
cuvs::neighbors::wrap_any_owning(std::move(padded)),
243243
raft::device_matrix<T, int64_t>(*res_ptr),
244-
std::move(build_res.idx)};
244+
std::move(index)};
245245
assign_lifetime_holder<T>(output_index, output_index->dtype, holder);
246246
}
247247
}

cpp/bench/ann/src/cuvs/cuvs_cagra_wrapper.h

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -227,21 +227,21 @@ void cuvs_cagra<T, IdxT>::build(const T* dataset, size_t nrow)
227227
cudaPointerAttributes ptr_attrs{};
228228
RAFT_CUDA_TRY(cudaPointerGetAttributes(&ptr_attrs, mds.data_handle()));
229229
const bool device_src = (reinterpret_cast<T const*>(ptr_attrs.devicePointer) != nullptr);
230-
// `build_result` is move-only; use a non-const `br` per branch so
231-
// `std::move(br.idx)` moves (a const `br` would try to copy the deleted
230+
// `cagra::index` is move-only; use a non-const `index` per branch so
231+
// `std::move(index)` moves (a const `index` would try to copy the deleted
232232
// cagra::index copy ctor).
233233
if (device_src && src_stride == required_stride) {
234234
auto const pdv = cuvs::neighbors::make_padded_dataset_view(handle_, mds);
235235
*input_dataset_v_ = raft::make_device_matrix_view<const T, int64_t, raft::row_major>(
236236
mds.data_handle(), static_cast<int64_t>(nrow), static_cast<int64_t>(dim_));
237-
auto br = cuvs::neighbors::cagra::build<T, IdxT>(handle_, params, pdv);
238-
index_ = std::make_shared<cuvs::neighbors::cagra::index<T, IdxT>>(std::move(br.idx));
237+
auto index = cuvs::neighbors::cagra::build<T, IdxT>(handle_, params, pdv);
238+
index_ = std::make_shared<cuvs::neighbors::cagra::index<T, IdxT>>(std::move(index));
239239
} else {
240240
auto padded = cuvs::neighbors::make_padded_dataset(handle_, mds);
241-
auto br =
241+
auto index =
242242
cuvs::neighbors::cagra::build<T, IdxT>(handle_, params, padded->as_dataset_view());
243243
*dataset_ = std::move(padded->data_);
244-
index_ = std::make_shared<cuvs::neighbors::cagra::index<T, IdxT>>(std::move(br.idx));
244+
index_ = std::make_shared<cuvs::neighbors::cagra::index<T, IdxT>>(std::move(index));
245245
}
246246
}
247247
} else {
@@ -297,16 +297,14 @@ void cuvs_cagra<T, IdxT>::build(const T* dataset, size_t nrow)
297297
RAFT_CUDA_TRY(cudaPointerGetAttributes(&sub_attrs, mds_sub.data_handle()));
298298
const bool sub_device = (reinterpret_cast<T const*>(sub_attrs.devicePointer) != nullptr);
299299
if (sub_device && src_sub == req_sub) {
300-
sub_index = std::move(
301-
cuvs::neighbors::cagra::build<T, IdxT>(
302-
handle_, params, cuvs::neighbors::make_padded_dataset_view(handle_, mds_sub))
303-
.idx);
300+
sub_index = cuvs::neighbors::cagra::build<T, IdxT>(
301+
handle_, params, cuvs::neighbors::make_padded_dataset_view(handle_, mds_sub));
304302
} else {
305303
auto padded_sub = cuvs::neighbors::make_padded_dataset(handle_, mds_sub);
306-
auto out = cuvs::neighbors::cagra::build<T, IdxT>(
304+
auto index = cuvs::neighbors::cagra::build<T, IdxT>(
307305
handle_, params, padded_sub->as_dataset_view());
308306
sub_dataset_buffers_->push_back(std::move(padded_sub->data_));
309-
sub_index = std::move(out.idx);
307+
sub_index = std::move(index);
310308
}
311309
} else {
312310
auto mds_sub = sub_dev;
@@ -318,16 +316,14 @@ void cuvs_cagra<T, IdxT>::build(const T* dataset, size_t nrow)
318316
RAFT_CUDA_TRY(cudaPointerGetAttributes(&sub_attrs, mds_sub.data_handle()));
319317
const bool sub_device = (reinterpret_cast<T const*>(sub_attrs.devicePointer) != nullptr);
320318
if (sub_device && src_sub == req_sub) {
321-
sub_index = std::move(
322-
cuvs::neighbors::cagra::build<T, IdxT>(
323-
handle_, params, cuvs::neighbors::make_padded_dataset_view(handle_, mds_sub))
324-
.idx);
319+
sub_index = cuvs::neighbors::cagra::build<T, IdxT>(
320+
handle_, params, cuvs::neighbors::make_padded_dataset_view(handle_, mds_sub));
325321
} else {
326322
auto padded_sub = cuvs::neighbors::make_padded_dataset(handle_, mds_sub);
327-
auto out = cuvs::neighbors::cagra::build<T, IdxT>(
323+
auto index = cuvs::neighbors::cagra::build<T, IdxT>(
328324
handle_, params, padded_sub->as_dataset_view());
329325
sub_dataset_buffers_->push_back(std::move(padded_sub->data_));
330-
sub_index = std::move(out.idx);
326+
sub_index = std::move(index);
331327
}
332328
}
333329
}

cpp/include/cuvs/neighbors/cagra.hpp

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,6 @@ static_assert(std::is_aggregate_v<search_params>);
384384
template <typename T, typename IdxT>
385385
struct index;
386386
template <typename T, typename IdxT>
387-
struct build_result;
388-
template <typename T, typename IdxT>
389387
struct ace_build_result;
390388

391389
template <typename T, typename IdxT>
@@ -958,22 +956,6 @@ struct index : cuvs::neighbors::index {
958956
* @}
959957
*/
960958

961-
/**
962-
* Result of `cagra::build` for APIs that return extra state alongside the index. Host-matrix builds
963-
* that attach a padded device copy on the index store it in `index::index_owning_dataset_storage_`
964-
* when needed. When deprecated `index_params::compression` is set, VPQ is trained inside `build`
965-
* and stored on the index. Otherwise, for explicit VPQ, train with
966-
* `cuvs::preprocessing::quantize::pq::make_vpq_dataset` and attach via `index::update_dataset` with
967-
* `vpq.as_dataset_view()` while keeping the `vpq_dataset` alive.
968-
*/
969-
template <typename T, typename IdxT>
970-
struct build_result {
971-
cuvs::neighbors::cagra::index<T, IdxT> idx;
972-
973-
/** Implicit conversion to index (moves `idx` out). */
974-
operator cuvs::neighbors::cagra::index<T, IdxT>() && { return std::move(idx); }
975-
};
976-
977959
/**
978960
* Result of merging CAGRA indices. The index holds a view over \p dataset; caller must keep
979961
* \p dataset alive for the lifetime of \p idx. If \p index_params passed to \p cagra::merge had
@@ -1036,7 +1018,7 @@ struct ace_build_result {
10361018
*
10371019
* @return the constructed cagra index
10381020
*
1039-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1021+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
10401022
* `make_padded_dataset_view` / `make_padded_dataset` for the view. Matrix overloads do
10411023
* not support VPQ compression.
10421024
*/
@@ -1082,7 +1064,7 @@ auto build(raft::resources const& res,
10821064
*
10831065
* @return the constructed cagra index
10841066
*
1085-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1067+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
10861068
* `make_padded_dataset` for host uploads. For ACE returning `ace_build_result`, use
10871069
* `build_ace`. Matrix overloads do not support VPQ compression.
10881070
*/
@@ -1128,7 +1110,7 @@ auto build(raft::resources const& res,
11281110
*
11291111
* @return the constructed cagra index
11301112
*
1131-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1113+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
11321114
* `make_padded_dataset_view` / `make_padded_dataset` for the view. Matrix overloads do
11331115
* not support VPQ compression.
11341116
*/
@@ -1173,7 +1155,7 @@ auto build(raft::resources const& res,
11731155
*
11741156
* @return the constructed cagra index
11751157
*
1176-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1158+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
11771159
* `make_padded_dataset` for host uploads. For ACE returning `ace_build_result`, use
11781160
* `build_ace`. Matrix overloads do not support VPQ compression.
11791161
*/
@@ -1220,7 +1202,7 @@ auto build(raft::resources const& res,
12201202
*
12211203
* @return the constructed cagra index
12221204
*
1223-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1205+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
12241206
* `make_padded_dataset_view` / `make_padded_dataset` for the view. Matrix overloads do
12251207
* not support VPQ compression.
12261208
*/
@@ -1268,7 +1250,7 @@ auto build(raft::resources const& res,
12681250
*
12691251
* @return the constructed cagra index
12701252
*
1271-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1253+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
12721254
* `make_padded_dataset` for host uploads. For ACE returning `ace_build_result`, use
12731255
* `build_ace`. Matrix overloads do not support VPQ compression.
12741256
*/
@@ -1316,7 +1298,7 @@ auto build(raft::resources const& res,
13161298
*
13171299
* @return the constructed cagra index
13181300
*
1319-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1301+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
13201302
* `make_padded_dataset_view` / `make_padded_dataset` for the view. Matrix overloads do
13211303
* not support VPQ compression.
13221304
*/
@@ -1364,7 +1346,7 @@ auto build(raft::resources const& res,
13641346
*
13651347
* @return the constructed cagra index
13661348
*
1367-
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `build_result`, using
1349+
* @deprecated Prefer `cagra::build(res, params, dataset_view)` returning `cagra::index`, using
13681350
* `make_padded_dataset` for host uploads. For ACE returning `ace_build_result`, use
13691351
* `build_ace`. Matrix overloads do not support VPQ compression.
13701352
*/
@@ -1381,7 +1363,7 @@ auto build(raft::resources const& res,
13811363
*
13821364
* Requires `graph_build_params` to be `ace_params`. For a single `cagra::index` return with
13831365
* internal lifetime management, use `cagra::build(res, params, host_view)` (backward
1384-
* compatible). For the generic padded-`dataset_view` path that returns `build_result`, use
1366+
* compatible). For the generic padded-`dataset_view` path, use
13851367
* `cagra::build(res, params, make_padded_dataset* / view)`.
13861368
*/
13871369
auto build_ace(raft::resources const& res,
@@ -1425,7 +1407,7 @@ template <typename T, typename IdxT = uint32_t>
14251407
auto build(raft::resources const& res,
14261408
const cuvs::neighbors::cagra::index_params& params,
14271409
cuvs::neighbors::any_dataset_view<T, int64_t> const& dataset)
1428-
-> cuvs::neighbors::cagra::build_result<T, IdxT>;
1410+
-> cuvs::neighbors::cagra::index<T, IdxT>;
14291411

14301412
/**
14311413
* @brief Same as `build<T, IdxT>(res, params, dataset_view)` but deduces \p T from
@@ -1439,7 +1421,7 @@ template <typename T, typename IdxT = uint32_t>
14391421
auto build(raft::resources const& res,
14401422
const cuvs::neighbors::cagra::index_params& params,
14411423
cuvs::neighbors::device_padded_dataset_view<T, int64_t> const& dataset)
1442-
-> cuvs::neighbors::cagra::build_result<T, IdxT>
1424+
-> cuvs::neighbors::cagra::index<T, IdxT>
14431425
{
14441426
return cuvs::neighbors::cagra::build<T, IdxT>(
14451427
res, params, cuvs::neighbors::any_dataset_view<T, int64_t>(dataset));

cpp/src/neighbors/cagra.cuh

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,7 @@ index<T, IdxT> build(
325325
"device data, use cagra::build with raft::device_matrix_view or a device dataset_view.");
326326
auto hview = raft::make_host_matrix_view<const T, int64_t, row_major>(
327327
dataset.data_handle(), dataset.extent(0), dataset.extent(1));
328-
auto bres = detail::build_from_host_matrix<T, IdxT>(res, params, hview);
329-
return std::move(bres.idx);
328+
return detail::build_from_host_matrix<T, IdxT>(res, params, hview);
330329
}
331330

332331
/**
@@ -337,9 +336,9 @@ index<T, IdxT> build(
337336
* stores the original view when `attach_dataset_on_build` is true.
338337
*/
339338
template <typename T, typename IdxT>
340-
build_result<T, IdxT> build(raft::resources const& res,
341-
const index_params& params,
342-
cuvs::neighbors::any_dataset_view<T, int64_t> const& dataset)
339+
index<T, IdxT> build(raft::resources const& res,
340+
const index_params& params,
341+
cuvs::neighbors::any_dataset_view<T, int64_t> const& dataset)
343342
{
344343
return cuvs::neighbors::cagra::detail::build_from_device_matrix<T, IdxT>(res, params, dataset);
345344
}

cpp/src/neighbors/cagra_build_inst.cu.in

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,8 @@ auto build(raft::resources const& handle,
3232
raft::device_matrix_view<const data_t, int64_t, raft::row_major> dataset)
3333
-> cuvs::neighbors::cagra::index<data_t, index_t>
3434
{
35-
// `build` on `any_dataset_view` returns `build_result`; convert to `index` via implicit
36-
// conversion.
3735
auto padded = cuvs::neighbors::make_padded_dataset_view(handle, dataset);
38-
cuvs::neighbors::cagra::index<data_t, index_t> out =
39-
cuvs::neighbors::cagra::build(handle, params, padded);
40-
return out;
36+
return cuvs::neighbors::cagra::build(handle, params, padded);
4137
}
4238

4339
// TODO(removal): Deprecated build(host_matrix_view)->index (delete with cagra.hpp declarations).
@@ -55,9 +51,8 @@ auto build(raft::resources const& handle,
5551
return ::cuvs::neighbors::cagra::finalize_index_from_ace(
5652
::cuvs::neighbors::cagra::detail::build_ace<data_t, index_t>(handle, params, dataset));
5753
}
58-
auto bres = ::cuvs::neighbors::cagra::detail::build_from_host_matrix<data_t, index_t>(
54+
return ::cuvs::neighbors::cagra::detail::build_from_host_matrix<data_t, index_t>(
5955
handle, params, dataset);
60-
return std::move(bres.idx);
6156
}
6257

6358
auto build_ace(raft::resources const& handle,
@@ -68,7 +63,8 @@ auto build_ace(raft::resources const& handle,
6863
RAFT_EXPECTS(
6964
std::holds_alternative<graph_build_params::ace_params>(params.graph_build_params),
7065
"cagra::build_ace requires graph_build_params to be ace_params. For cagra::index, use "
71-
"cagra::build(res, params, host_view). For build_result, use cagra::build(res, params, view).");
66+
"cagra::build(res, params, host_view). For non-ACE views, use cagra::build(res, params, "
67+
"dataset_view).");
7268
RAFT_EXPECTS(raft::get_device_for_address(dataset.data_handle()) == -1,
7369
"ACE: Dataset must be on host for ACE build");
7470
return ::cuvs::neighbors::cagra::detail::build_ace<data_t, index_t>(handle, params, dataset);
@@ -77,7 +73,7 @@ auto build_ace(raft::resources const& handle,
7773
// Definition lives in cagra.cuh; callers that only include cagra.hpp need this symbol in libcuvs.
7874
// The device_matrix_view overload above may inline the any_dataset_view template, so emit it
7975
// explicitly.
80-
template build_result<data_t, index_t> build<data_t, index_t>(
76+
template cuvs::neighbors::cagra::index<data_t, index_t> build<data_t, index_t>(
8177
raft::resources const& res,
8278
const index_params& params,
8379
cuvs::neighbors::any_dataset_view<data_t, int64_t> const& dataset);

0 commit comments

Comments
 (0)