feat: expose cuVS CAGRA num_random_samplings search param#3
Open
devin-ai-integration[bot] wants to merge 4 commits into
Open
feat: expose cuVS CAGRA num_random_samplings search param#3devin-ai-integration[bot] wants to merge 4 commits into
devin-ai-integration[bot] wants to merge 4 commits into
Conversation
Plumb the cuVS cagra::search_params num_random_samplings field through the GPU_CUVS CAGRA index so it is settable at search time. Defaults to 1 (the cuVS default), so behavior is unchanged unless explicitly set. Higher values (e.g. 8) improve recall on large per-segment datasets at the cost of some search latency, with no reindex required. Signed-off-by: premal <premal@6sense.com> Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…zation COSINE was mapped to InnerProduct with a manual row_normalize in float→data_type, which truncates unit-length fractions to 0 for int8. Use cuVS-native CosineExpanded which computes per-row L2 norms in float internally — correct for all dtypes. Remove the now-redundant pre-normalization blocks in train() and search(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The pre-built libcuvs IVF_PQ knn-graph builder rejects CosineExpanded (cagra_build.cuh: only L2Expanded or InnerProduct supported). NN_DESCENT supports CosineExpanded, so route IVF_PQ+CosineExpanded builds to NN_DESCENT. Combined with the CosineExpanded metric routing (002bf79), this fixes int8 COSINE recall without a libcuvs rebuild. Signed-off-by: premal <premal@6sense.com> Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The pre-built libcuvs has no compiled CAGRA kernels for CosineExpanded+int8 (build: IVF_PQ rejects it; search: no dataset descriptor). Revert COSINE to InnerProduct and normalize int8 correctly: cast int8->float, L2-normalize per row in float, rescale x127 and round back to int8, so every stored vector has ~equal L2 norm and InnerProduct ranking == cosine. fp32/fp16 keep in-type row_normalize. Applies to both train() and search(). Signed-off-by: premal <premal@6sense.com> Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration Bot
added a commit
that referenced
this pull request
Jul 12, 2026
…oad accounting Codex P1 review fixes for the GPU HNSW path: #1 Quantized-cosine inverse-norm semantics. The CPU cosine index records inverse L2 norms from the ORIGINAL input vectors (HasInverseL2Norms:: get_inverse_l2_norms()). The GPU upload previously recomputed norms from decoded/quantized codes (and normalized decoded fp32 vectors in place), which diverges from the CPU scores/traversal for lossy SQ8/int8/fp16/bf16. Now upload the CPU index's stored inverse norms and apply them in the kernel; only flat-fp32 cosine (decoded==original) keeps normalizing in place. Vendored faiss copy kept byte-identical with 6si/faiss. #2 Reload/search use-after-free race. gpu_index_ is now an atomic<shared_ptr<GpuIndexHNSW>>; Search() takes a shared-ownership snapshot for the whole search so a concurrent Deserialize() reload that resets/rebuilds the member cannot free an in-use index. Atomic readiness only guarantees visibility, not lifetime. #3 Phase-separated load-resource accounting. Resource gains maxMemoryCost (transient peak during load; defaults to 0 so other indexes keep their existing heuristic). GpuHnswIndexNode reports memoryCost=0 (CPU copy freed after upload) and maxMemoryCost covering the download buffer + deserialized CPU index + decode/graph staging. Tests (tests/ut/test_gpu_search.cc, [gpu_hnsw_p1]): native low-precision device paths (fp16/bf16/sq8 via non-mock fp32 node), SQ8/fp16/bf16 cosine CPU-vs-GPU parity asserting per-query top-1 id + per-result score on same-direction/ different-magnitude vectors, search-vs-concurrent-reload lifetime stress, >4 concurrent searches with varying nq/k/ef vs own ground truth, and load-resource estimate assertions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: premal <premal@6sense.com> Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Plumbs the cuVS
cagra::search_paramsfieldnum_random_samplingsthrough Knowhere'sGPU_CUVSCAGRA index so it can be set at search time from Milvus.cuVS CAGRA seeds its beam search with
num_random_samplingsrandom dataset nodes (cuVS default1). At large per-segment N,1gives poor recall; raising it (e.g.8) recovers recall for ~30% latency overhead, with no reindex (it is a search-time param). Knowhere previously did not declare this field, so it stayed pinned at the cuVS default of1.The change is purely additive and mirrors the existing search-param plumbing (
itopk_size,search_width,team_size, …) across the three layers:num_random_samplingsmaps 1:1 to cuVScagra::search_params::num_random_samplings(uint32_t, default1, valid>= 1).Backward compatibility & risk
Backward compatible: default
1reproduces the current cuVS default exactly, so behavior is unchanged unless a user explicitly sets the param. Search-time only (.for_search()), so no reindex and no serialization/ABI impact. Total: 3 files, +10 / -0 LOC.Link to Devin session: https://6sense.devinenterprise.com/sessions/ca163e56725d4f04978fd68f0162db5b