Fix filtered CAGRA search returning incorrect recall due to kHostMmap#2303
Fix filtered CAGRA search returning incorrect recall due to kHostMmap#2303rupakroynv wants to merge 1 commit into
Conversation
|
The benchmark was using The PR description suggests that the issue was incorrect results without ATS or performance degradation with ATS. Am I remembering things right that you were using a GH200 system that was ATS-enabled, but the issue you ran into was with correctness? Aside: for formatting, please run |
cjnolet
added
bug
rupakroynv
force-pushed
the
fix/cagra-filtered-search-kdevice
branch
2 times, most recently
from
0a510f5 to
8a2597c
Compare
…tset Root cause: cuvs_cagra::get_preference() returned kHostMmap for the filter bitset. GPU kernels (bitset_view::count, bitset_view::test) cannot read file-backed lazy mmap pages: - Non-ATS GPUs (H100, A100): no host memory access at all -> garbage filter - ATS GPUs (GH200): hardware reaches host memory but un-faulted file pages return zeros -> wrong filtering_rate -> incorrect recall In both cases recall collapsed to approximately the filter pass rate. Fix 1 (filter memory type): Add filter_memory_type to algo_property (ann_types.hpp) so the filter bitset memory type can be set independently of the dataset. cuvs_cagra sets filter_memory_type = kDevice so the framework cudaMemcpy's the filter to GPU before passing it to CAGRA, while dataset_memory_type stays kHostMmap to support datasets larger than GPU memory (e.g. deep-100M at 38.4 GB on a 40 GB GPU). Fix 2 (OOM prevention): set_search_param() unconditionally called copy_with_padding(), doubling device memory usage for the dataset. Skip copy_with_padding() when the data is already on device and no 16-byte alignment padding is needed. Fix 3 (copy() crash): sub_indices_ accumulated corrupted state when the kDevice dataset update path ran. Reset it via placement new in copy() for single-index CAGRA to prevent bad_array_new_length. Fix 4 (filter_bitset_file): conf.hpp only supported filtering_rate (random bitset generation). Pre-computed filter files specified via filter_bitset_file in the JSON config were silently ignored, so filter_bitset was always null and CAGRA ran unfiltered searches. Added filter_bitset_file to dataset_conf (conf.hpp), parse it from the JSON dataset block, pass it to the dataset constructor (benchmark.hpp), and load it from disk (dataset.hpp). Fix 5 (recall calculation): benchmark.hpp called non-existent gt_set() API. Restored the correct parallel gt_maps()-based recall computation from main. Fix 6 (include): composite/merge.hpp does not exist in the pre-built libcuvs.so; changed to composite/index.hpp. Tested on GH200 480GB (ATS) with deep-1M, 5% filter, inner product, k=10: Unpatched: Recall=0.0495 for all itopk (filter not applied) Patched: itopk=64->0.45, 128->0.73, 256->0.92, 512->0.94, 1024->0.9999 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rupakroynv
force-pushed
the
fix/cagra-filtered-search-kdevice
branch
from
8a2597c to
cb5fd8e
Compare
Done
Updated the PR description. Its is a correctness issue with or without ATS support.
Done |
3 participants
Description
Filtered CAGRA search returns incorrect recall when the benchmark harness passes the filter bitset using MemoryType::kHostMmap. This affects CUVS_CAGRA_ANN_BENCH when running filtered search workloads with a pre-computed filter bitset file.
Root Cause
cuvs_cagra::get_preference() returned MemoryType::kHostMmap for both the dataset and the filter bitset. kHostMmap allocates a file-backed MAP_PRIVATE mmap where pages are not pre-faulted and physical memory mappings.
Inside CAGRA's search dispatch (cagra.cuh), bitset_view::count() is called to estimate the filtering rate. That function wraps the bitset pointer in raft::make_device_vector_view and passes it to raft::popc, which launches a CUDA kernel that reads from the pointer assuming it is device memory. A host mmap pointer is not valid device memory. So, the kernel reads garbage or zeros, producing a wrong filtering_rate and causing CAGRA to return incorrect results that caused recall collapse to approximately the filter pass rate (~5%), regardless of itopk.
A secondary issue: the benchmark harness had no support for loading a pre-computed filter bitset from a file (filter_bitset_file in the JSON config). Only filtering_rate (random bitset generation) was supported. The filter_bitset_file field was silently ignored, causing filter_bitset() to return nullptr and CAGRA to run completely unfiltered searches.
Fix
Add std::optional filter_memory_type to algo_property so the filter bitset memory type can be set independently of the dataset. cuvs_cagra sets filter_memory_type = kDevice while keeping dataset_memory_type = kHostMmap. This preserves support for datasets larger than GPU memory while ensuring the filter bitset is explicitly cudaMemcpy'd to device memory before any GPU kernel accesses it. The filter_memory_type field defaults to nullopt (falls back to dataset_memory_type) so all other algorithms are unaffected.
Parse filter_bitset_file from the JSON dataset block and load it into the dataset as a blob<bitset_carrier_type>. Previously only filtering_rate (random bitset) was supported; pre-computed filter files were silently ignored.
set_search_param() unconditionally called copy_with_padding(), which allocates a second full device copy of the dataset. Skip copy_with_padding() when the data is already on device and no 16-byte alignment padding is needed.
sub_indices_ can be left in a corrupted state when the dataset update code path runs. For single-index CAGRA it is always logically empty, so reset it via placement new before invoking the copy constructor to prevent bad_array_new_length.
Notes