Skip to content

feat: expose cuVS CAGRA num_random_samplings search param#3

Open
devin-ai-integration[bot] wants to merge 4 commits into
2.6from
cagra-nrs
Open

feat: expose cuVS CAGRA num_random_samplings search param#3
devin-ai-integration[bot] wants to merge 4 commits into
2.6from
cagra-nrs

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Plumbs the cuVS cagra::search_params field num_random_samplings through Knowhere's GPU_CUVS CAGRA index so it can be set at search time from Milvus.

cuVS CAGRA seeds its beam search with num_random_samplings random dataset nodes (cuVS default 1). At large per-segment N, 1 gives 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 of 1.

The change is purely additive and mirrors the existing search-param plumbing (itopk_size, search_width, team_size, …) across the three layers:

# src/index/gpu_cuvs/gpu_cuvs_cagra_config.h  (user-facing config)
+ CFG_INT num_random_samplings;
+ KNOWHERE_CONFIG_DECLARE_FIELD(num_random_samplings)
+     .description("number of initial random seed node sampling iterations")
+     .set_default(1)
+     .set_range(1, std::numeric_limits<CFG_INT::value_type>::max())
+     .for_search();
+ result.num_random_samplings = cfg.num_random_samplings;   // in to_cuvs_knowhere_config()

# src/common/cuvs/integration/cuvs_knowhere_config.hpp  (cuVS-agnostic config)
+ std::optional<int> num_random_samplings = std::nullopt;
+ config.num_random_samplings = config.num_random_samplings.value_or(1);  // in validate_cuvs_knowhere_config()

# src/common/cuvs/integration/cuvs_knowhere_index.cuh  (map into cuvs cagra::search_params)
+ result.num_random_samplings = *(config.num_random_samplings);  // in config_to_search_params<cagra>()

num_random_samplings maps 1:1 to cuVS cagra::search_params::num_random_samplings (uint32_t, default 1, valid >= 1).

Backward compatibility & risk

Backward compatible: default 1 reproduces 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

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>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

premal and others added 3 commits July 8, 2026 01:55
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant