Fix benchmark regressions from #1846 - #2384
Conversation
achirkin
added
bug
|
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. |
|
/ok to test |
|
/ok to test |
|
@achirkin its too late for anything to be merged into 26.08. You'll need to retarget to main. |
…enchmark-provided dataset to avoid ambiguity w.r.t. data residence
achirkin
changed the title
|
Sure. I'll just finish it up today and let you decide whether to re-target and whether to optionally backport the fixes later if necessary. |
tfeher
left a comment
tfeher
left a comment
There was a problem hiding this comment.
Thanks Artem, most of it looks good, but I have some questions.
| host_index_ = std::make_shared<host_index_type>(cuvs::neighbors::cagra::build( | ||
| handle_, host_params, cuvs::neighbors::make_host_standard_dataset_view(dataset_view_host))); | ||
| index_ = | ||
| std::make_shared<index_type>(detail::to_graph_only_index<T, IdxT>(handle_, *host_index_)); |
There was a problem hiding this comment.
What is the differnece betweenhost_index_and index_? Isn't host_index_ already a graph_only index? (due to add_dataset_on_build=false?
There was a problem hiding this comment.
host_index_ (host_standard_index) is introduced as a workaround in this PR alongside index_ (device_padded_index, searchable) to support various input dataset types with minimum number of copies.
| staging = raft::make_device_matrix<T, int64_t>(handle_, rows, dim_); | ||
| raft::copy(staging.data_handle(), | ||
| dataset, | ||
| static_cast<size_t>(rows) * dim_, |
There was a problem hiding this comment.
If this is called on the whole dataset, then we might run OOM. We should have a vpq quantization method that takes host input data.
3 participants
Fix a few issues introduced to the benchmark executables limiting the functionality:
The changeset is limited to C++ benchmarks, thus doesn't affect the library or any dependencies.