Forward merge fix#2140
Merged
Merged
Conversation
This PR converts the current Sphinx docs to Fern in preparation for the move to docs.nvidia.com. Instead of manually composing the API reference docs, this PR also generates API reference docs for all supported languages directly from the code (as is standard in Fern). There's a lot of files in this PR, and most of the markdown files are either copied over to the Fern directory format from the old Sphinx docs, or they've been auto-generated using the new API reference docs generation scripts (`generate_api_reference.py` in the changes). When reviewing this PR, it's probably better to start with the non-markdown files, then build the docs and run them locally. The docs can be built in the usual way with `./build.sh docs`. You can run them locally using the following command: ``` fern/build_docs.sh dev --port 3000 --backend-port 3001 ``` Authors: - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Bradley Dice (https://github.com/bdice) - Divye Gala (https://github.com/divyegala) - Robert Maynard (https://github.com/robertmaynard) URL: rapidsai#2067
Closes rapidsai#1989. Adds multi-GPU support to KMeans fit for host-resident data, with two modes: - **OpenMP (cuVS SNMG)**: A single process drives all local GPUs via OMP threads and raw NCCL. Activated automatically when the handle is a `device_resources_snmg`. - **RAFT comms (Ray / Dask / MPI)**: Each rank is a separate process that calls fit with its own data shard and an initialized RAFT communicator. Coordination uses the RAFT comms. Both modes share the same core Lloyd's loop, batched streaming of host data, NCCL/comms allreduce of centroid sums and counts, and synchronized convergence. Supports sample weights, n_init best-of-N restarts, KMeansPlusPlus initialization, and float/double. Falls back to single-GPU when neither multi-GPU resources nor comms are present. Authors: - Victor Lafargue (https://github.com/viclafargue) - Tarang Jain (https://github.com/tarang-jain) Approvers: - Tarang Jain (https://github.com/tarang-jain) - Micka (https://github.com/lowener) - Dante Gama Dessavre (https://github.com/dantegd) URL: rapidsai#2017
Closes rapidsai#1901 Previous Code - We almost always allocate device side fp16 arrays. This was for... - allowing wmma usage - allowing data modification for `CosineExpanded` preprocessing Current PR Changes - This PR makes nn-descent reuse the user's device-side data buffer when input data is already on device, instead of always allocating and copying into a separate staging buffer. This roughly halves the peak device memory footprint for the common UMAP/HDBSCAN call path where the dataset is already on GPU. - Remove preprocessing for `CosineExpanded` metric (because we don't want to allocate additional device side data arrays) and do the computation inside the `calculate_metric` function. ### Peak memory usage Changes - food data (5M x 384) = 7.25GiB - sports data (13M x 284) = 18.55GiB - notice how for FP32->FP16 Device (meaning data is already on device), previous code allocates a new fp16 array, resulting in more gpu mem usage. This PR ensures that we convert to fp16 on-the-fly (resulting in the overhead in time) instead of allocating new fp16 memory for that. <img width="6570" height="3597" alt="performance_metrics" src="https://github.com/user-attachments/assets/9ea9efae-d8be-4990-a513-7c2cfcf0d718" /> ### Performance Changes - Conversion Overhead: On-the-fly conversion introduces negligible overhead. - Cosine Metric: Now reads l2 norms inside the `calculate_metric` function, aligning with access pattern used by the L2 distance metric. Adds minimal overhead (e.g. previously 18.2937s VS 18.7598s for 5Mx384 data) Authors: - Jinsol Park (https://github.com/jinsolp) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Divye Gala (https://github.com/divyegala) URL: rapidsai#1928
Closes rapidsai#1873 Authors: - Divye Gala (https://github.com/divyegala) Approvers: - Dante Gama Dessavre (https://github.com/dantegd) - Corey J. Nolet (https://github.com/cjnolet) URL: rapidsai#2030
Closes rapidsai#1291. ### Overview IVF-SQ combines an inverted file (IVF) partitioning scheme with 8-bit scalar quantization (SQ8) of residuals. Each float32 dimension is compressed to a single uint8 code, giving a 4x memory reduction over IVF-Flat while retaining high recall. The index implements various metrics (L2, inner-product, and cosine distance), data type (float, half) and also filtering. ### Build - **K-Means clustering** : A training subset (controlled by `kmeans_trainset_fraction`) is sampled from the dataset. Balanced K-Means is run on it to produce `n_lists` centroids that partition the vector space. - **SQ parameter training** : The training vectors are assigned to their nearest centroid and residuals are computed. A custom CUDA kernel computes the per-dimension min/max of these residuals (fast CUB reduction in shared memory). The observed range is expanded by a 5% margin on each side to reduce clipping on unseen data. The delta for each dimension is computed as `sq_delta[d] = (range + 2*margin) / 255`. These two per-dimension parameters (`sq_vmin` the lower end of the range, `sq_delta` the scale or quantization step) are stored in the index and are all that is needed to encode/decode any vector. - **Data insertion** : If `add_data_on_build` is true (the default), the full dataset is inserted via the extend path described below. ### Extend Extend adds new vectors to an existing index without retraining centroids or SQ parameters and in a batched fashion: - **Cluster assignment** : New vectors are assigned to their nearest centroid via K-Means predict. ~(adaptative centers : when enabled, centroids are incrementally updated as new data arrives, and center norms are recomputed)~. - **List resizing** : Per-list sizes are histogrammed and the IVF lists are grown to accommodate the new vectors, respecting the interleaved group alignment (kIndexGroupSize = 32). - **Residual computation + SQ encoding** : A first kernel computes residuals. A second kernel quantize each residual dimension to `uint8` and write the code into the interleaved list layout. ### Search Search proceeds in three stages: - **Coarse search (GEMM-based)** : All query-to-centroid distances are computed in a single batched GEMM (`queries x centers^T`), with metric-specific pre/post-processing. The top `n_probes` nearest clusters per query are selected via `select_k`. - **Fine scan (ivf_sq_scan_kernel)** : This is the performance-critical kernel, templated on `<BlockDim, Capacity, Metric, IdxT, SampleFilter>`. It operates in two grid modes controlled by the compile-time Capacity parameter: - **Fused top-k path** (Capacity > 0): grid is `(grid_dim_x, n_queries)`, each block loops over its share of probes and maintains a block_sort (warpsort) priority queue, writing only the local top-k to global memory. grid_dim_x is chosen via an occupancy query (`cudaOccupancyMaxActiveBlocksPerMultiprocessor`) to saturate the GPU. Instantiated for Capacity ∈ {32, 64, 128, 256}, covering k ≤ 256. - **Materialized path** (Capacity == 0): grid is `(n_probes, n_queries)`, one block per (query, probe) pair writes every distance to a flat buffer. Used as a fallback when k > 256 or the fused kernel has zero occupancy. Three dim-length shared-memory arrays are filled in two phases: Phase 1 (invariant across probes) loads sq_delta and query terms; Phase 2 (per-probe) folds in the centroid so the inner loop reduces to s_query_term − code × s_sq_scale (L2) or s_query_term × (s_aux + code × s_sq_scale) (IP/Cosine). Each warp processes one interleaved group of 32 vectors; each lane loads a uint4, giving a fully coalesced 512-byte read per dimension block. Distance accumulation is fused with decoding — no separate decompress pass. - **Final top-k selection** : In the fused path, a second select_k merges the per-block top-k results across grid_dim_x blocks (skipped when grid_dim_x == 1). In the materialized path, a single select_k over the full buffer extracts the global top-k. Distances then receive metric-specific fixups (e.g. sqrt for L2Sqrt) and list-local positions are mapped back to original dataset IDs. ### Benchmarks on B200 <div align="center"> Build benchmark (in seconds) : <img width="437" height="71" alt="ivf_sq_build_bench" src="https://github.com/user-attachments/assets/27a364bc-1aaf-410e-ac86-40ae288d1c1a" /> <div/> <br> <br> <div align="center"> <img width="640" height="380" alt="wiki_all_10M-bench" src="https://github.com/user-attachments/assets/b11ab729-8440-478d-bbaa-e1c99c68bd12" /> <div/> <br> <div align="center"> <img width="640" height="380" alt="deep-100M-bench" src="https://github.com/user-attachments/assets/eca818a3-642c-4176-a650-816c6aa5a4ee" /> <div/> Authors: - Victor Lafargue (https://github.com/viclafargue) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Gil Forsyth (https://github.com/gforsyth) - Kyle Edwards (https://github.com/KyleFromNVIDIA) - Divye Gala (https://github.com/divyegala) - Micka (https://github.com/lowener) - Tamas Bela Feher (https://github.com/tfeher) URL: rapidsai#1865
This PR fixes the usage of the workspace resource in the cagra pipeline to increase stability. * cagra::optimize - usage of workspace resource for large data where the large workspace resource should be used instead * ivf-pq - fix of inner/outer batchsize estimate that were unaware of each other and potentially oversubscribing available memory * kmeans_balanced - limit workspace usage by available memory instead of hard-coded value CC @tfeher Authors: - Malte Förster (https://github.com/mfoerste4) Approvers: - Tamas Bela Feher (https://github.com/tfeher) URL: rapidsai#2135
Adding several new pages to the API guides for core cuvs types- 1. Memory management (with RMM) 2. Multi-dimensional arrays 3. Multi-GPU 4. Resources Authors: - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Divye Gala (https://github.com/divyegala) URL: rapidsai#2117
Depends on rapidsai#1865. Do not merge. Authors: - Victor Lafargue (https://github.com/viclafargue) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Tamas Bela Feher (https://github.com/tfeher) - Micka (https://github.com/lowener) URL: rapidsai#1910
cjnolet
added
improvement
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds IVF‑SQ ANN index (C++ API, JIT‑LTO kernels, build/extend/search/serialize), C API wrapper and tests, benchmarks integration, multi‑GPU batched KMeans with init helpers and tests, NN‑Descent/CAGRA/IVF‑PQ internals tweaks, and migrates docs from Sphinx to Fern with CI and conda updates. ChangesIVF-SQ index feature (C++/C APIs, JIT‑LTO, benchmarks, tests) Public contracts and build wiring JIT‑LTO IVF‑SQ fragments and kernels Build, extend, search, and serialize implementations C API header and wrapper Tests and benchmarks integration KMeans multi-GPU (MG) and tests API docs clarifications Core device logic and heuristics MG batched fit and init utilities cluster_cost overload and MG dispatch Tests and shared blob utilities Neighbors maintenance (NN‑Descent, CAGRA, IVF‑PQ) CAGRA allocator and IVF‑PQ workspace heuristics NN‑Descent dtype/SMEM refactor and runtime dispatch Docs migration to Fern and CI/env updates CI/workflows and build scripts Conda envs and dependencies Replace Sphinx docs with Fern equivalents Remove old Sphinx sources and helpers Estimated code review effort Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/check-c-abi.yaml (1)
86-86:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix malformed GitHub Actions expression in
env(MERGE_BASE_SHA).In
.github/workflows/check-c-abi.yamlline 86,MERGE_BASE_SHAis set with${[ steps.merge-base.outputs.merge_base_sha }}, which is not valid GitHub Actions expression syntax; it should be${{ steps.merge-base.outputs.merge_base_sha }}.Suggested fix
- MERGE_BASE_SHA: ${[ steps.merge-base.outputs.merge_base_sha }} + MERGE_BASE_SHA: ${{ steps.merge-base.outputs.merge_base_sha }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/check-c-abi.yaml at line 86, The env entry for MERGE_BASE_SHA uses a malformed GitHub Actions expression `${[ steps.merge-base.outputs.merge_base_sha }}`; update the value to use the correct expression syntax by replacing it with `${{ steps.merge-base.outputs.merge_base_sha }}` so MERGE_BASE_SHA references steps.merge-base.outputs.merge_base_sha correctly and the braces are balanced.
🟡 Minor comments (15)
fern/pages/api_basics.md-45-47 (1)
45-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix malformed C section heading.
Cfollowed by^looks like an accidental conversion artifact and won’t render as a proper section heading. Use a standard markdown heading (### C) for consistency with the other language sections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/api_basics.md` around lines 45 - 47, Replace the stray "C" line followed by the caret "^" (the conversion artifact shown as `C` and `^`) with a proper Markdown heading consistent with other language sections—e.g., change it to "### C" so the section renders correctly and matches the other language headings.fern/pages/api_basics.md-14-14 (1)
14-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse descriptive link text instead of “here”.
Both links use generic anchor text, which hurts readability and fails markdownlint MD059. Rename the link text to describe the target (for example, “RMM C++ example” / “RMM Python MemoryResource objects”).
Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/api_basics.md` at line 14, Replace the generic link text "here" with descriptive anchor text: for the C++ example sentence ("configuring RMM to use a pool allocator in C++ (derived from the RMM example here)") change the link target text to something like "RMM C++ example", and for the other occurrence at line ~28 replace "here" with "RMM Python MemoryResource objects" (or similarly descriptive wording); update both anchor texts while keeping the same URLs so the links are accessible and markdownlint MD059 is satisfied..github/workflows/check-c-abi.yaml-131-131 (1)
131-131:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse an absolute URL in PR comments instead of a relative path.
../fern/pages/c_api/index.mdin an issue comment is not a stable navigable link for contributors. Prefer a full docs URL or a GitHub blob URL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/check-c-abi.yaml at line 131, Replace the relative link "../fern/pages/c_api/index.md" used in the PR comment text in the check-c-abi workflow with an absolute, stable URL (either the published docs URL or a GitHub blob URL) so contributors can navigate reliably; locate the string "../fern/pages/c_api/index.md" in the workflow step that constructs the comment and substitute it with the full docs URL (e.g., https://your-docs-domain/... or https://github.com/<org>/<repo>/blob/<branch>/fern/pages/c_api/index.md).fern/pages/c_api/c-api-distance-pairwise-distance.md-25-25 (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete or remove the empty “Usage example” section.
Line 25 introduces “Usage example:” but no example is provided. Either add a minimal snippet or drop the heading to avoid dead content.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/c_api/c-api-distance-pairwise-distance.md` at line 25, The markdown contains an empty "Usage example:" heading in c-api-distance-pairwise-distance.md; either remove that heading or add a minimal example showing how to call the C API for pairwise distance (e.g., a short snippet demonstrating initialization, calling the pairwise distance function, and printing results). Update the section titled "Usage example:" to contain that minimal snippet or delete the heading to avoid dead content.fern/pages/c_api/c-api-neighbors-brute-force.md-73-73 (1)
73-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse equality notation in dtype constraint docs.
The constraints use
kDLDataType.bits = ...; this reads as assignment. Please change these to equality checks (==) to avoid ambiguity in API requirements.As per coding guidelines, for
docs/**/*we should prioritize “Accuracy” and “Clarity.”Also applies to: 102-102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/c_api/c-api-neighbors-brute-force.md` at line 73, The documentation uses assignment syntax for type constraints (e.g., "kDLDataType.bits = 32") which is ambiguous; update all occurrences to use equality notation "==" for the DLDataType checks (change "kDLDataType.bits = 32" and "kDLDataType.bits = 16" to "kDLDataType.bits == 32" and "kDLDataType.bits == 16", and ensure any other instances like "kDLDataType.code = ..." are "kDLDataType.code == ...") so the constraints for DLDataType.code and DLDataType.bits are clear and consistent.fern/docs.yml-29-30 (1)
29-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorrect the C-guidelines redirect destination.
Line 29 redirects a C-guidelines URL to a C++-guidelines page. This looks unintended and can misroute users looking for C-specific guidance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/docs.yml` around lines 29 - 30, The redirect mapping incorrectly points the C-guidelines source ("/cuvs/developer-guide/guidelines/c-guidelines") to the C++ page; update the destination for that source to the proper C-guidelines URL (e.g., "/cuvs/developer-guide/coding-guidelines/c-guidelines") by replacing the destination value in the mapping so the source and destination correspond to C rather than C++.fern/pages/api_guide.md-47-49 (1)
47-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix likely broken “Other APIs” link slugs.
Line 47, Line 48, and Line 49 use
/other-ap-is/..., which appears to be a typo and likely breaks navigation. Please update these to the correct slug used by the docs site routing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/api_guide.md` around lines 47 - 49, The three markdown links for "Dynamic Batching", "K-selection", and "Pairwise Distances" use the typo slug `/other-ap-is/...`; update each link to the correct slug `/other-apis/...` so the links read `/user-guide/api-guides/other-apis/dynamic-batching`, `/user-guide/api-guides/other-apis/k-selection`, and `/user-guide/api-guides/other-apis/pairwise-distances`.fern/pages/build.md-28-30 (1)
28-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign CMake minimum version with repository dependencies.
Line 28 documents CMake 3.26.4+, but the dependency spec requires CMake 4.0+. This mismatch can cause failed local builds when users follow the docs literally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/build.md` around lines 28 - 30, Update the documented CMake minimum requirement string "CMake 3.26.4 or newer." in the build docs to match the repository dependency spec by changing it to "CMake 4.0 or newer" (or "CMake 4.0.0 or newer"); search for the exact phrase "CMake 3.26.4 or newer." in fern/pages/build.md and replace it, and scan other docs for the same phrase to keep all documentation consistent with the repo dependency spec.fern/pages/c_api/c-api-cluster-kmeans.md-35-35 (1)
35-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove TODO placeholders from published API docs.
These TODO notes are user-facing and make the API lifecycle messaging ambiguous in generated docs. Replace them with finalized deprecation/removal wording.
As per coding guidelines, documentation changes should prioritize clarity and consistency.
Also applies to: 78-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/c_api/c-api-cluster-kmeans.md` at line 35, Remove the user-facing "TODO" CalVer placeholder from the kmeans parameter doc and replace it with a clear, final deprecation/removal statement: locate the text referencing inertia_check and cuvsKMeansParams_v2 (and the duplicate mention at the later block) and change "TODO: CalVer for the replacement: 26.08" to a finalized phrase such as "Removed in cuvsKMeansParams_v2 (removed in CalVer 26.08)" or "Deprecated: inertia_check removed in cuvsKMeansParams_v2 (CalVer 26.08)" so the API lifecycle messaging is explicit and consistent in the c-api-cluster-kmeans.md content.cpp/src/neighbors/ivf_sq/ivf_sq_search.cuh-746-753 (1)
746-753:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
k > 0before entering search.The current checks allow zero-column outputs, but downstream logic assumes a positive
kfor capacity selection and top-k operations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/src/neighbors/ivf_sq/ivf_sq_search.cuh` around lines 746 - 753, Add a precondition check that k is positive before any search logic: validate the parameter k (used for output columns / top-k operations) with a RAFT_EXPECTS(k > 0, "k must be > 0") before the existing RAFT_EXPECTS on queries/neighbors/distances and before any capacity selection or top-k call sites (e.g., where neighbors.extent(1) and distances.extent(1) are used to size buffers or invoke top-k routines). This ensures zero-column outputs are rejected early and prevents downstream assumptions about positive k in the functions that perform capacity selection and top-k operations.fern/pages/c_api/c-api-core-c-api.md-286-286 (1)
286-286:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorrect parameter direction.
The
resparameter is an output pointer that receives the created resource handle, so its direction should be "out" not "in".📝 Proposed fix
-| `res` | in | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle | +| `res` | out | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle |As per coding guidelines, documentation must accurately describe parameters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/c_api/c-api-core-c-api.md` at line 286, The documentation incorrectly marks the parameter `res` as "in"; update the parameter direction to "out" for `res` (`cuvsResources_t*`) in the parameter table so it correctly reflects that `res` is an output pointer receiving the created resource handle (ensure the row showing `res | in | [cuvsResources_t*]` is changed to `res | out | [cuvsResources_t*]`).fern/pages/api_interoperability.md-7-7 (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix typo and improve clarity.
The text contains a typo: "out C API" should be "our C API". Additionally, consider replacing "At the moment" with "Currently" for conciseness, and add a comma before "however" for proper punctuation.
📝 Proposed fix
-At the moment, `DLManagedTensor` from DLPack v0.8 is compatible with out C API however we will soon upgrade to `DLManagedTensorVersioned` from DLPack v1.0 as it will help us maintain ABI and API compatibility. +Currently, `DLManagedTensor` from DLPack v0.8 is compatible with our C API, however we will soon upgrade to `DLManagedTensorVersioned` from DLPack v1.0 as it will help us maintain ABI and API compatibility.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/api_interoperability.md` at line 7, Fix the typo and punctuation in the documentation sentence: change "out C API" to "our C API", replace "At the moment" with "Currently" for conciseness, and add a comma before "however" so the sentence reads clearly when referencing DLManagedTensor (DLPack v0.8) and DLManagedTensorVersioned (DLPack v1.0) to maintain ABI/API compatibility.fern/pages/c_api/c-api-core-c-api.md-149-149 (1)
149-149:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorrect parameter direction.
The
resparameter is an output pointer that receives the created resource handle, so its direction should be "out" not "in".📝 Proposed fix
-| `res` | in | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle | +| `res` | out | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle |As per coding guidelines, documentation must accurately describe parameters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/c_api/c-api-core-c-api.md` at line 149, The table entry for the `res` parameter incorrectly marks it as an input; change the parameter direction from "in" to "out" for `res` (the output pointer that receives the created handle) in the documentation row referencing `cuvsResources_t` so it reads "| `res` | out | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle |".fern/pages/c_api/c-api-core-c-api.md-266-266 (1)
266-266:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorrect parameter direction.
The
resparameter is an output pointer that receives the created resource handle, so its direction should be "out" not "in".📝 Proposed fix
-| `res` | in | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle | +| `res` | out | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle |As per coding guidelines, documentation must accurately describe parameters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fern/pages/c_api/c-api-core-c-api.md` at line 266, The parameter documentation for the `res` parameter is incorrect: update the table row that documents `res` (`cuvsResources_t*`) to mark its direction as "out" instead of "in" so it correctly reflects that `res` is an output pointer receiving the created resource handle; locate the table entry containing the text "`res` | in | [`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) | cuvsResources_t opaque C handle" and change "in" to "out".cpp/src/neighbors/ivf_sq/ivf_sq_build.cuh-166-180 (1)
166-180:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing CUDA error check after kernel launches.
The
launch_fused_column_minmaxfunction launches kernels but doesn't check for launch errors. Per coding guidelines, all CUDA calls must have explicit error checking.Proposed fix
template <int BlockSize, typename T> inline void launch_fused_column_minmax( const T* data, float* col_min, float* col_max, int64_t n_rows, uint32_t dim, cudaStream_t stream) { if (dim % 4 == 0) { fused_column_minmax_kernel<BlockSize, 4, T> <<<dim / 4, BlockSize, 0, stream>>>(data, col_min, col_max, n_rows, dim); } else if (dim % 2 == 0) { fused_column_minmax_kernel<BlockSize, 2, T> <<<dim / 2, BlockSize, 0, stream>>>(data, col_min, col_max, n_rows, dim); } else { fused_column_minmax_kernel<BlockSize, 1, T> <<<dim, BlockSize, 0, stream>>>(data, col_min, col_max, n_rows, dim); } + RAFT_CUDA_TRY(cudaPeekAtLastError()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/src/neighbors/ivf_sq/ivf_sq_build.cuh` around lines 166 - 180, The kernel launches in launch_fused_column_minmax (calling fused_column_minmax_kernel) lack CUDA error checks; after each <<<>>> launch capture and check the launch/error status (e.g., call cudaGetLastError() or cudaPeekAtLastError() and, if using streams, consider cudaStreamSynchronize or cudaGetLastError immediately) and handle failures by returning/throwing or logging a clear message that includes the kernel name (fused_column_minmax_kernel) and the chosen pack width (1/2/4) and BlockSize so callers can diagnose which instantiation failed.
🧹 Nitpick comments (3)
cpp/include/cuvs/neighbors/ivf_sq.hpp (1)
178-227: 💤 Low valueConsider adding brief Doxygen comments to public accessor methods.
Per coding guidelines, public API functions should have Doxygen documentation. The class-level documentation is excellent, but individual accessor methods like
metric(),size(),dim(),list_sizes(), etc. lack brief descriptions. While the class documentation explains the overall concept well, brief@briefcomments on accessors would improve API discoverability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/include/cuvs/neighbors/ivf_sq.hpp` around lines 178 - 227, Add brief Doxygen `@brief` comments for each public accessor to improve discoverability: annotate methods such as metric(), size(), dim(), n_lists(), conservative_memory_allocation(), list_sizes() (both overloads), centers() (both overloads), center_norms() (both overloads) and allocate_center_norms(), sq_vmin()/sq_vmin() const, sq_delta()/sq_delta() const, accum_sorted_sizes() (both overloads), data_ptrs()/data_ptrs() const, inds_ptrs()/inds_ptrs() const, and lists() (both overloads) with one-line `@brief` descriptions placed immediately above their declarations in ivf_sq.hpp; keep the existing signatures (const/noexcept/[[nodiscard]]), use the project’s Doxygen style (single-line `@brief`) and keep descriptions concise (what value is returned and any important semantic note).ci/build_docs.sh (1)
65-65: 💤 Low valueArray expansion may fail when
FERN_DOCS_ARGSis empty.With
set -u(nounset),"${FERN_DOCS_ARGS[@]}"on an empty array can cause an "unbound variable" error in older Bash versions (pre-4.4). Consider using${FERN_DOCS_ARGS[@]+"${FERN_DOCS_ARGS[@]}"}for compatibility, or ensure CI runners use Bash 4.4+.Proposed fix for broader compatibility
-fern/build_docs.sh "${FERN_DOCS_MODE}" "${FERN_DOCS_ARGS[@]}" +fern/build_docs.sh "${FERN_DOCS_MODE}" ${FERN_DOCS_ARGS[@]+"${FERN_DOCS_ARGS[@]}"}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/build_docs.sh` at line 65, The call to fern/build_docs.sh uses "${FERN_DOCS_ARGS[@]}" which can trigger an unbound-variable error under set -u when FERN_DOCS_ARGS is unset or empty on older Bash versions; update the invocation to safely expand the array by guarding the expansion (e.g. use the ${FERN_DOCS_ARGS[@]+"${FERN_DOCS_ARGS[@]}"} pattern) or ensure FERN_DOCS_ARGS is defaulted to an empty array before use (initialize FERN_DOCS_ARGS=() earlier in build_docs.sh) so the call to fern/build_docs.sh "${FERN_DOCS_MODE}" "${FERN_DOCS_ARGS[@]}" becomes safe.cpp/tests/neighbors/ann_ivf_sq.cuh (1)
242-247: 💤 Low valueConsider adding a static_assert or extending type support for robustness.
The
if constexprblock only generates random data forfloatandhalftypes. If someone instantiates the test with a differentDataT(e.g.,double), thedatabaseandsearch_queriesuvectors would remain uninitialized, leading to undefined behavior in tests.Since test instantiations are currently controlled, this isn't breaking anything now, but a compile-time guard would prevent future misuse.
💡 Suggested improvement
void SetUp() override { database.resize(ps.num_db_vecs * ps.dim, stream_); search_queries.resize(ps.num_queries * ps.dim, stream_); + static_assert(std::is_same_v<DataT, float> || std::is_same_v<DataT, half>, + "AnnIVFSQTest only supports float or half DataT"); + raft::random::RngState r(1234ULL); - if constexpr (std::is_same_v<DataT, float> || std::is_same_v<DataT, half>) { - raft::random::uniform( - handle_, r, database.data(), ps.num_db_vecs * ps.dim, DataT(0.1), DataT(2.0)); - raft::random::uniform( - handle_, r, search_queries.data(), ps.num_queries * ps.dim, DataT(0.1), DataT(2.0)); - } + raft::random::uniform( + handle_, r, database.data(), ps.num_db_vecs * ps.dim, DataT(0.1), DataT(2.0)); + raft::random::uniform( + handle_, r, search_queries.data(), ps.num_queries * ps.dim, DataT(0.1), DataT(2.0)); raft::resource::sync_stream(handle_); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/tests/neighbors/ann_ivf_sq.cuh` around lines 242 - 247, The test only fills database and search_queries when DataT is float or half, so add a compile-time guard to prevent silent uninitialized data: insert a static_assert using DataT (e.g., static_assert(std::is_same_v<DataT,float> || std::is_same_v<DataT,half>, "ann_ivf_sq test only supports float/half")) near the top of the test or before the if constexpr; alternatively expand the if constexpr to include other supported types (e.g., double) and call raft::random::uniform for those types to ensure database and search_queries uvectors are initialized. Ensure the static_assert or the extended branches reference DataT, database, search_queries, and raft::random::uniform so the intent is clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@c/include/cuvs/neighbors/ivf_sq.h`:
- Around line 147-155: Add complete Doxygen blocks for each public exported
getter in this header (e.g., cuvsIvfSqIndexGetNLists, cuvsIvfSqIndexGetDim,
cuvsIvfSqIndexGetSize) by documenting all parameters with `@param` (describe the
input index and the output pointer and its ownership/validity) and the return
value with `@return` (list possible cuvsError_t results and what they mean);
ensure the style and wording match other exported APIs in the file (including
getters/search/extend) for consistency and apply the same complete
`@param/`@return coverage to the other public functions in the range noted (lines
269-340).
In `@c/src/neighbors/ivf_sq.cpp`:
- Around line 189-215: In cuvsIvfSqBuild (and the other C API entry functions
noted) add explicit null-pointer validation for all incoming external pointers
(params, dataset_tensor, dataset_tensor->dl_tensor,
dataset_tensor->dl_tensor.data if used, and index) before dereferencing them; if
any are null return an appropriate cuvsError_t via the existing error path
(e.g., fail early through translate_exceptions or return a cuvs error) instead
of proceeding—place these checks at the top of cuvsIvfSqBuild before calling
_reset_index or _build, and apply the same pattern to the other functions
covering ranges 217-255, 269-313, and 315-369, referencing the same symbols
(_reset_index, _build, index->addr, dataset_tensor, params, index) when making
the fixes.
- Around line 323-335: The code currently sets index->dtype when uninitialized
but then blindly branches on incoming vectors.dtype in cuvsIvfSqExtend (calling
_extend<float> or _extend<half>), allowing mismatched inputs to be inserted into
an existing index; change the logic so that if index->dtype is already
initialized (index->dtype.code != 0 || index->dtype.bits != 0) you first verify
vectors.dtype.code and vectors.dtype.bits exactly match index->dtype.code and
index->dtype.bits and RAFT_FAIL with a clear message if they differ; only when
index->dtype is uninitialized keep the current behavior of assigning
index->dtype = vectors.dtype and proceeding to call the appropriate _extend
template based on vectors.dtype.
In `@c/tests/neighbors/run_ivf_sq_c.c`:
- Around line 32-89: The code ignores cuvsError_t returns from
cuvsIvfSqIndexCreate, cuvsIvfSqIndexParamsCreate, cuvsIvfSqBuild,
cuvsIvfSqSearch and cuvsStreamSync; update run_ivf_sq to check each call's
return value, on error jump to a single cleanup: block that only destroys
handles that were successfully created (index via cuvsIvfSqIndexDestroy,
build_params via cuvsIvfSqIndexParamsDestroy, search_params via
cuvsIvfSqSearchParamsDestroy) and propagate/return the error code; use boolean
flags or null-checks to track which of index/build_params/search_params were
initialized before destroying them.
In `@cpp/bench/ann/src/cuvs/cuvs_benchmark.cu`:
- Around line 89-90: The branch that matches algo_name == "cuvs_ivf_sq" should
also accept the alias "raft_ivf_sq" to preserve compatibility; update the
conditional that detects the IVF-SQ algorithm (where
cuvs::bench::cuvs_ivf_sq<T>::build_param is used) to check for both
"cuvs_ivf_sq" and "raft_ivf_sq" (and make the same change in the corresponding
second occurrence around the later branch) so existing configs using the raft_*
name still route to the cuvs_ivf_sq build_param and factory logic.
In `@cpp/bench/ann/src/cuvs/cuvs_ivf_sq_wrapper.h`:
- Around line 96-102: In cuvs_ivf_sq<T>::set_search_param (and similarly in the
search method), guard against uninitialized wrapper state by checking index_ is
non-null before using index_->size() or constructing filter_ with
make_cuvs_filter, and ensure filter_ is only used when valid; replace the unsafe
dynamic_cast<const search_param&>(param) with a checked cast (e.g.,
dynamic_cast<const search_param*>(...)) and handle a null result by
returning/throwing a clear error or status rather than allowing an exception;
apply the same null-checks and early error returns in the search path (the code
that assumes index_/filter_ are initialized) and document that build/load must
have been called or provide a safe no-op/error if not.
In `@cpp/src/cluster/detail/kmeans_balanced.cuh`:
- Around line 214-217: The code aligns minibatch_size down to a multiple of 64
using raft::round_down_safe which can yield 0 for inputs 1..63; ensure
minibatch_size is clamped to the valid range [1, n_rows] after alignment by
applying a max-of-1 step (e.g., after calling raft::round_down_safe and before
using minibatch_size in the loop, set minibatch_size =
std::max<IdxT>(minibatch_size, IdxT{1}) or otherwise clamp to at least 1) so the
subsequent loop that uses minibatch_size cannot get a zero step.
In `@cpp/src/cluster/detail/kmeans_mg.cuh`:
- Around line 480-490: The device-side normalization in raft::linalg::map uses
*d_wt_sum_ptr without guarding against zero, which can divide-by-zero when all
sample weights are zero; add a check after the allreduce that computes d_wt_sum
(the host-side sum/variable backing d_wt_sum_ptr) and if it is zero either
return/throw (restore the RAFT_EXPECTS behavior) or set a safe non-zero
denominator (e.g., 1) before launching raft::linalg::map; alternatively modify
the lambda used in raft::linalg::map (referencing d_wt_sum_ptr, weight,
n_samples) to branch on *d_wt_sum_ptr == 0 and handle that case without
performing a division.
In `@fern/pages/api_interoperability.md`:
- Line 23: The example incorrectly assigns the host pointer to the DLPack
tensor: replace setting dataset_tensor.dl_tensor.data = dataset with using the
device pointer dataset_dev so the DLPack GPU tensor points to device memory;
update the assignment that writes into dataset_tensor.dl_tensor.data to use
dataset_dev (and ensure any comments/variable names referencing device memory
reflect dataset_dev) so the example accurately represents a GPU device buffer.
- Line 15: The example has a dimension mismatch for the array named dataset: it
is declared as float dataset[2][1] (2 rows, 1 column) but initialized with two
values in a single row ({{0.2, 0.1}}). Fix by making the declaration match the
initializer (e.g., change dataset to float dataset[1][2]) or alter the
initializer to supply two rows (e.g., {{0.2}, {0.1}}) so that the declaration
float dataset[2][1] and the initialization agree.
In `@fern/pages/c_api/c-api-cluster-kmeans.md`:
- Around line 73-74: The `metric` parameter rows are empty; update the docs for
both params structs to explain `metric` (type `cuvsDistanceType`)—list allowed
enum values, the default value, and how each choice affects distance computation
and k-means behavior (e.g., Euclidean vs. Cosine changes centroid calculation
and nearest-centroid assignment), and note any constraints or compatibility with
input data; ensure the description appears in both `metric` rows that reference
`cuvsDistanceType`.
---
Outside diff comments:
In @.github/workflows/check-c-abi.yaml:
- Line 86: The env entry for MERGE_BASE_SHA uses a malformed GitHub Actions
expression `${[ steps.merge-base.outputs.merge_base_sha }}`; update the value to
use the correct expression syntax by replacing it with `${{
steps.merge-base.outputs.merge_base_sha }}` so MERGE_BASE_SHA references
steps.merge-base.outputs.merge_base_sha correctly and the braces are balanced.
---
Minor comments:
In @.github/workflows/check-c-abi.yaml:
- Line 131: Replace the relative link "../fern/pages/c_api/index.md" used in the
PR comment text in the check-c-abi workflow with an absolute, stable URL (either
the published docs URL or a GitHub blob URL) so contributors can navigate
reliably; locate the string "../fern/pages/c_api/index.md" in the workflow step
that constructs the comment and substitute it with the full docs URL (e.g.,
https://your-docs-domain/... or
https://github.com/<org>/<repo>/blob/<branch>/fern/pages/c_api/index.md).
In `@cpp/src/neighbors/ivf_sq/ivf_sq_build.cuh`:
- Around line 166-180: The kernel launches in launch_fused_column_minmax
(calling fused_column_minmax_kernel) lack CUDA error checks; after each <<<>>>
launch capture and check the launch/error status (e.g., call cudaGetLastError()
or cudaPeekAtLastError() and, if using streams, consider cudaStreamSynchronize
or cudaGetLastError immediately) and handle failures by returning/throwing or
logging a clear message that includes the kernel name
(fused_column_minmax_kernel) and the chosen pack width (1/2/4) and BlockSize so
callers can diagnose which instantiation failed.
In `@cpp/src/neighbors/ivf_sq/ivf_sq_search.cuh`:
- Around line 746-753: Add a precondition check that k is positive before any
search logic: validate the parameter k (used for output columns / top-k
operations) with a RAFT_EXPECTS(k > 0, "k must be > 0") before the existing
RAFT_EXPECTS on queries/neighbors/distances and before any capacity selection or
top-k call sites (e.g., where neighbors.extent(1) and distances.extent(1) are
used to size buffers or invoke top-k routines). This ensures zero-column outputs
are rejected early and prevents downstream assumptions about positive k in the
functions that perform capacity selection and top-k operations.
In `@fern/docs.yml`:
- Around line 29-30: The redirect mapping incorrectly points the C-guidelines
source ("/cuvs/developer-guide/guidelines/c-guidelines") to the C++ page; update
the destination for that source to the proper C-guidelines URL (e.g.,
"/cuvs/developer-guide/coding-guidelines/c-guidelines") by replacing the
destination value in the mapping so the source and destination correspond to C
rather than C++.
In `@fern/pages/api_basics.md`:
- Around line 45-47: Replace the stray "C" line followed by the caret "^" (the
conversion artifact shown as `C` and `^`) with a proper Markdown heading
consistent with other language sections—e.g., change it to "### C" so the
section renders correctly and matches the other language headings.
- Line 14: Replace the generic link text "here" with descriptive anchor text:
for the C++ example sentence ("configuring RMM to use a pool allocator in C++
(derived from the RMM example here)") change the link target text to something
like "RMM C++ example", and for the other occurrence at line ~28 replace "here"
with "RMM Python MemoryResource objects" (or similarly descriptive wording);
update both anchor texts while keeping the same URLs so the links are accessible
and markdownlint MD059 is satisfied.
In `@fern/pages/api_guide.md`:
- Around line 47-49: The three markdown links for "Dynamic Batching",
"K-selection", and "Pairwise Distances" use the typo slug `/other-ap-is/...`;
update each link to the correct slug `/other-apis/...` so the links read
`/user-guide/api-guides/other-apis/dynamic-batching`,
`/user-guide/api-guides/other-apis/k-selection`, and
`/user-guide/api-guides/other-apis/pairwise-distances`.
In `@fern/pages/api_interoperability.md`:
- Line 7: Fix the typo and punctuation in the documentation sentence: change
"out C API" to "our C API", replace "At the moment" with "Currently" for
conciseness, and add a comma before "however" so the sentence reads clearly when
referencing DLManagedTensor (DLPack v0.8) and DLManagedTensorVersioned (DLPack
v1.0) to maintain ABI/API compatibility.
In `@fern/pages/build.md`:
- Around line 28-30: Update the documented CMake minimum requirement string
"CMake 3.26.4 or newer." in the build docs to match the repository dependency
spec by changing it to "CMake 4.0 or newer" (or "CMake 4.0.0 or newer"); search
for the exact phrase "CMake 3.26.4 or newer." in fern/pages/build.md and replace
it, and scan other docs for the same phrase to keep all documentation consistent
with the repo dependency spec.
In `@fern/pages/c_api/c-api-cluster-kmeans.md`:
- Line 35: Remove the user-facing "TODO" CalVer placeholder from the kmeans
parameter doc and replace it with a clear, final deprecation/removal statement:
locate the text referencing inertia_check and cuvsKMeansParams_v2 (and the
duplicate mention at the later block) and change "TODO: CalVer for the
replacement: 26.08" to a finalized phrase such as "Removed in
cuvsKMeansParams_v2 (removed in CalVer 26.08)" or "Deprecated: inertia_check
removed in cuvsKMeansParams_v2 (CalVer 26.08)" so the API lifecycle messaging is
explicit and consistent in the c-api-cluster-kmeans.md content.
In `@fern/pages/c_api/c-api-core-c-api.md`:
- Line 286: The documentation incorrectly marks the parameter `res` as "in";
update the parameter direction to "out" for `res` (`cuvsResources_t*`) in the
parameter table so it correctly reflects that `res` is an output pointer
receiving the created resource handle (ensure the row showing `res | in |
[cuvsResources_t*]` is changed to `res | out | [cuvsResources_t*]`).
- Line 149: The table entry for the `res` parameter incorrectly marks it as an
input; change the parameter direction from "in" to "out" for `res` (the output
pointer that receives the created handle) in the documentation row referencing
`cuvsResources_t` so it reads "| `res` | out |
[`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) |
cuvsResources_t opaque C handle |".
- Line 266: The parameter documentation for the `res` parameter is incorrect:
update the table row that documents `res` (`cuvsResources_t*`) to mark its
direction as "out" instead of "in" so it correctly reflects that `res` is an
output pointer receiving the created resource handle; locate the table entry
containing the text "`res` | in |
[`cuvsResources_t*`](/api-reference/c-api-core-c-api#cuvsresources-t) |
cuvsResources_t opaque C handle" and change "in" to "out".
In `@fern/pages/c_api/c-api-distance-pairwise-distance.md`:
- Line 25: The markdown contains an empty "Usage example:" heading in
c-api-distance-pairwise-distance.md; either remove that heading or add a minimal
example showing how to call the C API for pairwise distance (e.g., a short
snippet demonstrating initialization, calling the pairwise distance function,
and printing results). Update the section titled "Usage example:" to contain
that minimal snippet or delete the heading to avoid dead content.
In `@fern/pages/c_api/c-api-neighbors-brute-force.md`:
- Line 73: The documentation uses assignment syntax for type constraints (e.g.,
"kDLDataType.bits = 32") which is ambiguous; update all occurrences to use
equality notation "==" for the DLDataType checks (change "kDLDataType.bits = 32"
and "kDLDataType.bits = 16" to "kDLDataType.bits == 32" and "kDLDataType.bits ==
16", and ensure any other instances like "kDLDataType.code = ..." are
"kDLDataType.code == ...") so the constraints for DLDataType.code and
DLDataType.bits are clear and consistent.
---
Nitpick comments:
In `@ci/build_docs.sh`:
- Line 65: The call to fern/build_docs.sh uses "${FERN_DOCS_ARGS[@]}" which can
trigger an unbound-variable error under set -u when FERN_DOCS_ARGS is unset or
empty on older Bash versions; update the invocation to safely expand the array
by guarding the expansion (e.g. use the
${FERN_DOCS_ARGS[@]+"${FERN_DOCS_ARGS[@]}"} pattern) or ensure FERN_DOCS_ARGS is
defaulted to an empty array before use (initialize FERN_DOCS_ARGS=() earlier in
build_docs.sh) so the call to fern/build_docs.sh "${FERN_DOCS_MODE}"
"${FERN_DOCS_ARGS[@]}" becomes safe.
In `@cpp/include/cuvs/neighbors/ivf_sq.hpp`:
- Around line 178-227: Add brief Doxygen `@brief` comments for each public
accessor to improve discoverability: annotate methods such as metric(), size(),
dim(), n_lists(), conservative_memory_allocation(), list_sizes() (both
overloads), centers() (both overloads), center_norms() (both overloads) and
allocate_center_norms(), sq_vmin()/sq_vmin() const, sq_delta()/sq_delta() const,
accum_sorted_sizes() (both overloads), data_ptrs()/data_ptrs() const,
inds_ptrs()/inds_ptrs() const, and lists() (both overloads) with one-line `@brief`
descriptions placed immediately above their declarations in ivf_sq.hpp; keep the
existing signatures (const/noexcept/[[nodiscard]]), use the project’s Doxygen
style (single-line `@brief`) and keep descriptions concise (what value is returned
and any important semantic note).
In `@cpp/tests/neighbors/ann_ivf_sq.cuh`:
- Around line 242-247: The test only fills database and search_queries when
DataT is float or half, so add a compile-time guard to prevent silent
uninitialized data: insert a static_assert using DataT (e.g.,
static_assert(std::is_same_v<DataT,float> || std::is_same_v<DataT,half>,
"ann_ivf_sq test only supports float/half")) near the top of the test or before
the if constexpr; alternatively expand the if constexpr to include other
supported types (e.g., double) and call raft::random::uniform for those types to
ensure database and search_queries uvectors are initialized. Ensure the
static_assert or the extended branches reference DataT, database,
search_queries, and raft::random::uniform so the intent is clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cb20e47b-bc6c-4aea-9000-57c47b2a8f60
⛔ Files ignored due to path filters (17)
fern/assets/images/build_benchmarks.pngis excluded by!**/*.pngfern/assets/images/clustering_methods_overview.pngis excluded by!**/*.pngfern/assets/images/diskann_disk_architecture.pngis excluded by!**/*.pngfern/assets/images/globally_partitioned_index.svgis excluded by!**/*.svgfern/assets/images/graph_index_build_search.pngis excluded by!**/*.pngfern/assets/images/hash_ivf_graph_index.pngis excluded by!**/*.pngfern/assets/images/hash_partition_build_search.pngis excluded by!**/*.pngfern/assets/images/index_recalls.pngis excluded by!**/*.pngfern/assets/images/ivf_flat_index.pngis excluded by!**/*.pngfern/assets/images/ivf_index_build_search.pngis excluded by!**/*.pngfern/assets/images/ivf_vs_graph_index.pngis excluded by!**/*.pngfern/assets/images/locally_partitioned_index.svgis excluded by!**/*.svgfern/assets/images/pareto_curve_vector_search.svgis excluded by!**/*.svgfern/assets/images/recall_buckets.pngis excluded by!**/*.pngfern/assets/images/refinement_reranking.pngis excluded by!**/*.pngfern/assets/images/tech_stack.pngis excluded by!**/*.pngfern/assets/rapids_logo.pngis excluded by!**/*.png
📒 Files selected for processing (283)
.github/CODEOWNERS.github/workflows/build.yaml.github/workflows/check-c-abi.yaml.github/workflows/pr.yamlbuild.shc/CMakeLists.txtc/include/cuvs/core/all.hc/include/cuvs/neighbors/ivf_sq.hc/src/neighbors/ivf_sq.cppc/src/neighbors/ivf_sq.hppc/tests/CMakeLists.txtc/tests/neighbors/ann_ivf_sq_c.cuc/tests/neighbors/c_api.cc/tests/neighbors/run_ivf_sq_c.cci/build_docs.shci/release/update-version.shconda/environments/all_cuda-129_arch-aarch64.yamlconda/environments/all_cuda-129_arch-x86_64.yamlconda/environments/all_cuda-132_arch-aarch64.yamlconda/environments/all_cuda-132_arch-x86_64.yamlcpp/CMakeLists.txtcpp/bench/ann/CMakeLists.txtcpp/bench/ann/src/cuvs/cuvs_ann_bench_param_parser.hcpp/bench/ann/src/cuvs/cuvs_benchmark.cucpp/bench/ann/src/cuvs/cuvs_ivf_sq.cucpp/bench/ann/src/cuvs/cuvs_ivf_sq_wrapper.hcpp/include/cuvs/cluster/kmeans.hppcpp/include/cuvs/detail/jit_lto/ivf_sq/scan_fragments.hppcpp/include/cuvs/neighbors/ivf_flat.hppcpp/include/cuvs/neighbors/ivf_sq.hppcpp/src/cluster/detail/kmeans.cuhcpp/src/cluster/detail/kmeans_balanced.cuhcpp/src/cluster/detail/kmeans_common.cuhcpp/src/cluster/detail/kmeans_mg.cuhcpp/src/cluster/detail/kmeans_mg_batched.cuhcpp/src/cluster/detail/kmeans_mg_batched_init.cuhcpp/src/cluster/kmeans.cuhcpp/src/cluster/kmeans_fit_double.cucpp/src/cluster/kmeans_fit_float.cucpp/src/cluster/kmeans_impl.cuhcpp/src/core/mnmg_comms.cuhcpp/src/neighbors/detail/cagra/graph_core.cuhcpp/src/neighbors/detail/nn_descent.cuhcpp/src/neighbors/detail/nn_descent_gnnd.hppcpp/src/neighbors/ivf_pq/ivf_pq_search.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/accumulate_distance_impl.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/accumulate_distance_kernel.cu.incpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/accumulate_distance_matrix.jsoncpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/device_functions.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/filter_kernel.cu.incpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/filter_matrix.jsoncpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/finalize_distance_impl.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/finalize_distance_kernel.cu.incpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/finalize_distance_matrix.jsoncpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/kernel_def.hppcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/scan_impl.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/scan_kernel.cu.incpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/scan_matrix.jsoncpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/scan_planner.hppcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/setup_invariant_smem_impl.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/setup_invariant_smem_kernel.cu.incpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/setup_invariant_smem_matrix.jsoncpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/setup_per_probe_smem_impl.cuhcpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/setup_per_probe_smem_kernel.cu.incpp/src/neighbors/ivf_sq/detail/jit_lto_kernels/setup_per_probe_smem_matrix.jsoncpp/src/neighbors/ivf_sq/ivf_sq_build.cuhcpp/src/neighbors/ivf_sq/ivf_sq_build_extend_float_uint8_t_int64_t.cucpp/src/neighbors/ivf_sq/ivf_sq_build_extend_half_uint8_t_int64_t.cucpp/src/neighbors/ivf_sq/ivf_sq_search.cuhcpp/src/neighbors/ivf_sq/ivf_sq_search_uint8_t_int64_t.cucpp/src/neighbors/ivf_sq/ivf_sq_serialize.cuhcpp/src/neighbors/ivf_sq/ivf_sq_serialize_uint8_t.cucpp/src/neighbors/ivf_sq_index.cppcpp/tests/CMakeLists.txtcpp/tests/cluster/kmeans.cucpp/tests/cluster/kmeans_mg_batched.cucpp/tests/cluster/kmeans_test_blobs.cuhcpp/tests/neighbors/ann_ivf_sq.cuhcpp/tests/neighbors/ann_ivf_sq/test_float_int64_t.cucpp/tests/neighbors/ann_ivf_sq/test_half_int64_t.cudependencies.yamldocs/Makefiledocs/README.mddocs/make.batdocs/source/_static/collapse_overloads.jsdocs/source/_static/references.cssdocs/source/advanced_topics.rstdocs/source/api_basics.rstdocs/source/api_docs.rstdocs/source/api_interoperability.rstdocs/source/build.rstdocs/source/c_api.rstdocs/source/c_api/cluster.rstdocs/source/c_api/cluster_kmeans_c.rstdocs/source/c_api/core_c_api.rstdocs/source/c_api/distance.rstdocs/source/c_api/neighbors.rstdocs/source/c_api/neighbors_all_neighbors_c.rstdocs/source/c_api/neighbors_bruteforce_c.rstdocs/source/c_api/neighbors_cagra_c.rstdocs/source/c_api/neighbors_hnsw_c.rstdocs/source/c_api/neighbors_ivf_flat_c.rstdocs/source/c_api/neighbors_ivf_pq_c.rstdocs/source/c_api/neighbors_mg.rstdocs/source/c_api/neighbors_vamana_c.rstdocs/source/c_api/preprocessing.rstdocs/source/choosing_and_configuring_indexes.rstdocs/source/comparing_indexes.rstdocs/source/conf.pydocs/source/cpp_api.rstdocs/source/cpp_api/cluster.rstdocs/source/cpp_api/cluster_agglomerative.rstdocs/source/cpp_api/cluster_kmeans.rstdocs/source/cpp_api/cluster_spectral.rstdocs/source/cpp_api/distance.rstdocs/source/cpp_api/neighbors.rstdocs/source/cpp_api/neighbors_all_neighbors.rstdocs/source/cpp_api/neighbors_bruteforce.rstdocs/source/cpp_api/neighbors_cagra.rstdocs/source/cpp_api/neighbors_dynamic_batching.rstdocs/source/cpp_api/neighbors_epsilon_neighborhood.rstdocs/source/cpp_api/neighbors_filter.rstdocs/source/cpp_api/neighbors_hnsw.rstdocs/source/cpp_api/neighbors_ivf_flat.rstdocs/source/cpp_api/neighbors_ivf_pq.rstdocs/source/cpp_api/neighbors_mg.rstdocs/source/cpp_api/neighbors_nn_descent.rstdocs/source/cpp_api/neighbors_refine.rstdocs/source/cpp_api/neighbors_vamana.rstdocs/source/cpp_api/preprocessing.rstdocs/source/cpp_api/preprocessing_pca.rstdocs/source/cpp_api/preprocessing_quantize.rstdocs/source/cpp_api/preprocessing_spectral_embedding.rstdocs/source/cpp_api/selection.rstdocs/source/cpp_api/stats.rstdocs/source/cuvs_bench/build.rstdocs/source/cuvs_bench/datasets.rstdocs/source/cuvs_bench/index.rstdocs/source/cuvs_bench/param_tuning.rstdocs/source/cuvs_bench/pluggable_backend.rstdocs/source/developer_guide.mddocs/source/filtering.rstdocs/source/getting_started.rstdocs/source/index.rstdocs/source/integrations.rstdocs/source/integrations/faiss.rstdocs/source/integrations/kinetica.rstdocs/source/integrations/lucene.rstdocs/source/integrations/milvus.rstdocs/source/neighbors/all_neighbors.rstdocs/source/neighbors/bruteforce.rstdocs/source/neighbors/cagra.rstdocs/source/neighbors/ivfflat.rstdocs/source/neighbors/ivfpq.rstdocs/source/neighbors/neighbors.rstdocs/source/neighbors/vamana.rstdocs/source/python_api.rstdocs/source/python_api/cluster.rstdocs/source/python_api/cluster_kmeans.rstdocs/source/python_api/distance.rstdocs/source/python_api/neighbors.rstdocs/source/python_api/neighbors_all_neighbors.rstdocs/source/python_api/neighbors_brute_force.rstdocs/source/python_api/neighbors_cagra.rstdocs/source/python_api/neighbors_hnsw.rstdocs/source/python_api/neighbors_ivf_flat.rstdocs/source/python_api/neighbors_ivf_pq.rstdocs/source/python_api/neighbors_mg_cagra.rstdocs/source/python_api/neighbors_mg_ivf_flat.rstdocs/source/python_api/neighbors_mg_ivf_pq.rstdocs/source/python_api/neighbors_multi_gpu.rstdocs/source/python_api/neighbors_nn_decent.rstdocs/source/python_api/preprocessing.rstdocs/source/rust_api/index.rstdocs/source/sphinxext/github_link.pydocs/source/tuning_guide.rstdocs/source/vector_databases_vs_vector_search.rstdocs/source/working_with_ann_indexes.rstdocs/source/working_with_ann_indexes_c.rstdocs/source/working_with_ann_indexes_cpp.rstdocs/source/working_with_ann_indexes_python.rstdocs/source/working_with_ann_indexes_rust.rstfern/README.mdfern/build_docs.shfern/developer_guide/abi_stability.mdfern/docs.ymlfern/fern.config.jsonfern/pages/advanced_topics.mdfern/pages/api_basics.mdfern/pages/api_docs.mdfern/pages/api_guide.mdfern/pages/api_interoperability.mdfern/pages/benchmarking_guide.mdfern/pages/build.mdfern/pages/c_api/c-api-cluster-kmeans.mdfern/pages/c_api/c-api-core-c-api.mdfern/pages/c_api/c-api-distance-distance.mdfern/pages/c_api/c-api-distance-pairwise-distance.mdfern/pages/c_api/c-api-neighbors-all-neighbors.mdfern/pages/c_api/c-api-neighbors-brute-force.mdfern/pages/c_api/c-api-neighbors-cagra.mdfern/pages/c_api/c-api-neighbors-common.mdfern/pages/c_api/c-api-neighbors-hnsw.mdfern/pages/c_api/c-api-neighbors-ivf-flat.mdfern/pages/c_api/c-api-neighbors-ivf-pq.mdfern/pages/c_api/c-api-neighbors-ivf-sq.mdfern/pages/c_api/c-api-neighbors-mg-cagra.mdfern/pages/c_api/c-api-neighbors-mg-common.mdfern/pages/c_api/c-api-neighbors-mg-ivf-flat.mdfern/pages/c_api/c-api-neighbors-mg-ivf-pq.mdfern/pages/c_api/c-api-neighbors-nn-descent.mdfern/pages/c_api/c-api-neighbors-refine.mdfern/pages/c_api/c-api-neighbors-tiered-index.mdfern/pages/c_api/c-api-neighbors-vamana.mdfern/pages/c_api/c-api-preprocessing-pca.mdfern/pages/c_api/c-api-preprocessing-quantize-binary.mdfern/pages/c_api/c-api-preprocessing-quantize-pq.mdfern/pages/c_api/c-api-preprocessing-quantize-scalar.mdfern/pages/c_api/index.mdfern/pages/c_guidelines.mdfern/pages/cluster/index.mdfern/pages/cluster/kmeans.mdfern/pages/cluster/single_linkage.mdfern/pages/cluster/spectral.mdfern/pages/clustering.mdfern/pages/coding_guidelines.mdfern/pages/comparing_indexes.mdfern/pages/contributing.mdfern/pages/core_types.mdfern/pages/cpp_api/cpp-api-cluster-agglomerative.mdfern/pages/cpp_api/cpp-api-cluster-kmeans.mdfern/pages/cpp_api/cpp-api-cluster-spectral.mdfern/pages/cpp_api/cpp-api-common-types-copy-serialization-and-utility-apis.mdfern/pages/cpp_api/cpp-api-common-types-dense-array-views.mdfern/pages/cpp_api/cpp-api-common-types-dense-view-factories.mdfern/pages/cpp_api/cpp-api-common-types-errors-and-logging.mdfern/pages/cpp_api/cpp-api-common-types-execution-resources.mdfern/pages/cpp_api/cpp-api-common-types-layouts-and-extents.mdfern/pages/cpp_api/cpp-api-common-types-owning-array-factories.mdfern/pages/cpp_api/cpp-api-common-types-owning-dense-arrays.mdfern/pages/cpp_api/cpp-api-common-types-sparse-array-types.mdfern/pages/cpp_api/cpp-api-common-types.mdfern/pages/cpp_api/cpp-api-distance-distance.mdfern/pages/cpp_api/cpp-api-distance-grammian.mdfern/pages/cpp_api/cpp-api-neighbors-all-neighbors.mdfern/pages/cpp_api/cpp-api-neighbors-ball-cover.mdfern/pages/cpp_api/cpp-api-neighbors-brute-force.mdfern/pages/cpp_api/cpp-api-neighbors-cagra.mdfern/pages/cpp_api/cpp-api-neighbors-common.mdfern/pages/cpp_api/cpp-api-neighbors-composite-index.mdfern/pages/cpp_api/cpp-api-neighbors-dynamic-batching.mdfern/pages/cpp_api/cpp-api-neighbors-epsilon-neighborhood.mdfern/pages/cpp_api/cpp-api-neighbors-hnsw.mdfern/pages/cpp_api/cpp-api-neighbors-ivf-flat.mdfern/pages/cpp_api/cpp-api-neighbors-ivf-pq.mdfern/pages/cpp_api/cpp-api-neighbors-ivf-sq.mdfern/pages/cpp_api/cpp-api-neighbors-nn-descent.mdfern/pages/cpp_api/cpp-api-neighbors-refine.mdfern/pages/cpp_api/cpp-api-neighbors-scann.mdfern/pages/cpp_api/cpp-api-neighbors-tiered-index.mdfern/pages/cpp_api/cpp-api-neighbors-vamana.mdfern/pages/cpp_api/cpp-api-preprocessing-pca.mdfern/pages/cpp_api/cpp-api-preprocessing-quantize-binary.mdfern/pages/cpp_api/cpp-api-preprocessing-quantize-pq.mdfern/pages/cpp_api/cpp-api-preprocessing-quantize-scalar.mdfern/pages/cpp_api/cpp-api-preprocessing-spectral-embedding.mdfern/pages/cpp_api/cpp-api-selection-select-k.mdfern/pages/cpp_api/cpp-api-stats-silhouette-score.mdfern/pages/cpp_api/cpp-api-stats-trustworthiness-score.mdfern/pages/cpp_api/cpp-api-util-cutlass-utils.mdfern/pages/cpp_api/cpp-api-util-file-io.mdfern/pages/cpp_api/index.mdfern/pages/cpp_guidelines.mdfern/pages/cuvs_bench/datasets.mdfern/pages/cuvs_bench/index.mdfern/pages/cuvs_bench/install.mdfern/pages/cuvs_bench/introduction.mdfern/pages/cuvs_bench/param_tuning.mdfern/pages/cuvs_bench/pluggable_backend.mdfern/pages/cuvs_bench/running.mdfern/pages/cuvs_bench/wiki_all_dataset.mdfern/pages/developer_guide.mdfern/pages/getting_started.md
💤 Files with no reviewable changes (101)
- docs/source/c_api/cluster.rst
- docs/source/cpp_api/distance.rst
- docs/source/working_with_ann_indexes_rust.rst
- docs/source/neighbors/cagra.rst
- docs/source/cpp_api/stats.rst
- docs/source/python_api/neighbors_cagra.rst
- docs/source/integrations/kinetica.rst
- docs/source/python_api.rst
- docs/source/_static/collapse_overloads.js
- docs/source/cpp_api/neighbors_all_neighbors.rst
- docs/source/cpp_api/neighbors_filter.rst
- docs/source/cpp_api/neighbors_refine.rst
- docs/source/cpp_api/cluster_agglomerative.rst
- docs/source/python_api/neighbors.rst
- docs/source/cuvs_bench/pluggable_backend.rst
- docs/source/rust_api/index.rst
- docs/source/c_api/neighbors.rst
- docs/source/cpp_api/neighbors.rst
- docs/source/python_api/neighbors_all_neighbors.rst
- docs/source/neighbors/all_neighbors.rst
- docs/source/cuvs_bench/param_tuning.rst
- docs/source/api_docs.rst
- docs/source/neighbors/neighbors.rst
- docs/source/neighbors/ivfpq.rst
- docs/source/cpp_api/preprocessing_spectral_embedding.rst
- docs/source/cpp_api/neighbors_ivf_flat.rst
- docs/source/cpp_api/selection.rst
- docs/source/cpp_api/neighbors_bruteforce.rst
- docs/source/cpp_api/cluster.rst
- docs/source/cpp_api/neighbors_hnsw.rst
- docs/source/getting_started.rst
- docs/source/c_api/neighbors_hnsw_c.rst
- docs/source/neighbors/bruteforce.rst
- docs/source/c_api/cluster_kmeans_c.rst
- docs/source/cpp_api/neighbors_mg.rst
- docs/source/c_api/neighbors_cagra_c.rst
- docs/source/index.rst
- docs/source/c_api/neighbors_bruteforce_c.rst
- docs/source/c_api.rst
- docs/source/python_api/neighbors_ivf_pq.rst
- docs/source/python_api/preprocessing.rst
- docs/make.bat
- docs/source/cpp_api/neighbors_vamana.rst
- docs/source/neighbors/vamana.rst
- docs/source/cpp_api/neighbors_epsilon_neighborhood.rst
- .github/workflows/build.yaml
- docs/source/c_api/neighbors_all_neighbors_c.rst
- docs/source/c_api/neighbors_vamana_c.rst
- docs/source/c_api/neighbors_ivf_pq_c.rst
- docs/source/cpp_api/preprocessing_quantize.rst
- docs/Makefile
- docs/source/working_with_ann_indexes_c.rst
- docs/source/c_api/neighbors_ivf_flat_c.rst
- docs/source/python_api/neighbors_hnsw.rst
- docs/source/cpp_api/neighbors_cagra.rst
- docs/source/api_interoperability.rst
- docs/source/c_api/neighbors_mg.rst
- docs/source/cpp_api/neighbors_ivf_pq.rst
- docs/source/filtering.rst
- docs/source/python_api/distance.rst
- docs/source/comparing_indexes.rst
- docs/source/cpp_api/cluster_kmeans.rst
- docs/source/neighbors/ivfflat.rst
- docs/source/python_api/neighbors_mg_ivf_flat.rst
- docs/source/cpp_api/cluster_spectral.rst
- docs/source/python_api/neighbors_brute_force.rst
- docs/source/integrations.rst
- docs/source/build.rst
- docs/source/integrations/faiss.rst
- docs/source/cuvs_bench/datasets.rst
- docs/source/c_api/core_c_api.rst
- docs/source/c_api/distance.rst
- docs/source/working_with_ann_indexes_cpp.rst
- docs/source/python_api/cluster.rst
- docs/source/vector_databases_vs_vector_search.rst
- docs/source/cpp_api.rst
- docs/source/conf.py
- docs/source/integrations/milvus.rst
- docs/source/advanced_topics.rst
- docs/source/python_api/neighbors_multi_gpu.rst
- docs/source/cpp_api/neighbors_dynamic_batching.rst
- docs/source/python_api/neighbors_ivf_flat.rst
- docs/source/api_basics.rst
- docs/source/sphinxext/github_link.py
- docs/source/choosing_and_configuring_indexes.rst
- docs/source/c_api/preprocessing.rst
- docs/source/working_with_ann_indexes.rst
- docs/source/_static/references.css
- docs/source/cpp_api/preprocessing.rst
- docs/source/python_api/neighbors_mg_ivf_pq.rst
- docs/source/working_with_ann_indexes_python.rst
- docs/source/tuning_guide.rst
- docs/source/python_api/neighbors_nn_decent.rst
- docs/source/python_api/neighbors_mg_cagra.rst
- docs/source/python_api/cluster_kmeans.rst
- docs/source/cpp_api/preprocessing_pca.rst
- docs/source/developer_guide.md
- docs/source/integrations/lucene.rst
- docs/source/cuvs_bench/index.rst
- docs/source/cpp_api/neighbors_nn_descent.rst
- docs/source/cuvs_bench/build.rst
Comment on lines
+147
to
+155
| /** Get the number of clusters/inverted lists */ | ||
| CUVS_EXPORT cuvsError_t cuvsIvfSqIndexGetNLists(cuvsIvfSqIndex_t index, int64_t* n_lists); | ||
|
|
||
| /** Get the dimensionality of the data */ | ||
| CUVS_EXPORT cuvsError_t cuvsIvfSqIndexGetDim(cuvsIvfSqIndex_t index, int64_t* dim); | ||
|
|
||
| /** Get the size of the index */ | ||
| CUVS_EXPORT cuvsError_t cuvsIvfSqIndexGetSize(cuvsIvfSqIndex_t index, int64_t* size); | ||
|
|
There was a problem hiding this comment.
Complete Doxygen blocks for all public exported APIs.
Several public functions in this header are missing full @param/@return-style documentation. Please make the docs complete and uniform across all exported functions (especially getters/search/extend).
As per coding guidelines, public C API headers must include complete Doxygen documentation for all public functions.
Also applies to: 269-340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@c/include/cuvs/neighbors/ivf_sq.h` around lines 147 - 155, Add complete
Doxygen blocks for each public exported getter in this header (e.g.,
cuvsIvfSqIndexGetNLists, cuvsIvfSqIndexGetDim, cuvsIvfSqIndexGetSize) by
documenting all parameters with `@param` (describe the input index and the output
pointer and its ownership/validity) and the return value with `@return` (list
possible cuvsError_t results and what they mean); ensure the style and wording
match other exported APIs in the file (including getters/search/extend) for
consistency and apply the same complete `@param/`@return coverage to the other
public functions in the range noted (lines 269-340).
Comment on lines
+189
to
+215
| extern "C" cuvsError_t cuvsIvfSqBuild(cuvsResources_t res, | ||
| cuvsIvfSqIndexParams_t params, | ||
| DLManagedTensor* dataset_tensor, | ||
| cuvsIvfSqIndex_t index) | ||
| { | ||
| return cuvs::core::translate_exceptions([=] { | ||
| auto dataset = dataset_tensor->dl_tensor; | ||
|
|
||
| if (dataset.dtype.code != kDLFloat || | ||
| (dataset.dtype.bits != 32 && dataset.dtype.bits != 16)) { | ||
| RAFT_FAIL("Unsupported dataset DLtensor dtype: %d and bits: %d", | ||
| dataset.dtype.code, | ||
| dataset.dtype.bits); | ||
| } | ||
|
|
||
| _reset_index(index); | ||
|
|
||
| index->dtype.code = dataset.dtype.code; | ||
| index->dtype.bits = dataset.dtype.bits; | ||
|
|
||
| if (dataset.dtype.code == kDLFloat && dataset.dtype.bits == 32) { | ||
| index->addr = reinterpret_cast<uintptr_t>(_build<float>(res, *params, dataset_tensor)); | ||
| } else if (dataset.dtype.code == kDLFloat && dataset.dtype.bits == 16) { | ||
| index->addr = reinterpret_cast<uintptr_t>(_build<half>(res, *params, dataset_tensor)); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add explicit null-pointer validation at C API boundaries.
These entry points dereference external pointers (params, dataset_tensor, index, output pointers) without precondition checks, which can crash on invalid user input instead of returning cuvsError_t.
Suggested pattern
extern "C" cuvsError_t cuvsIvfSqBuild(...){
return cuvs::core::translate_exceptions([=] {
+ RAFT_EXPECTS(res != nullptr, "res must not be null");
+ RAFT_EXPECTS(params != nullptr, "params must not be null");
+ RAFT_EXPECTS(dataset_tensor != nullptr, "dataset must not be null");
+ RAFT_EXPECTS(index != nullptr, "index must not be null");
auto dataset = dataset_tensor->dl_tensor;
...
});
}Also applies to: 217-255, 269-313, 315-369
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@c/src/neighbors/ivf_sq.cpp` around lines 189 - 215, In cuvsIvfSqBuild (and
the other C API entry functions noted) add explicit null-pointer validation for
all incoming external pointers (params, dataset_tensor,
dataset_tensor->dl_tensor, dataset_tensor->dl_tensor.data if used, and index)
before dereferencing them; if any are null return an appropriate cuvsError_t via
the existing error path (e.g., fail early through translate_exceptions or return
a cuvs error) instead of proceeding—place these checks at the top of
cuvsIvfSqBuild before calling _reset_index or _build, and apply the same pattern
to the other functions covering ranges 217-255, 269-313, and 315-369,
referencing the same symbols (_reset_index, _build, index->addr, dataset_tensor,
params, index) when making the fixes.
Comment on lines
+323
to
+335
| if (index->dtype.code == 0 && index->dtype.bits == 0) { | ||
| index->dtype.code = vectors.dtype.code; | ||
| index->dtype.bits = vectors.dtype.bits; | ||
| } | ||
|
|
||
| if (vectors.dtype.code == kDLFloat && vectors.dtype.bits == 32) { | ||
| _extend<float>(res, new_vectors, new_indices, *index); | ||
| } else if (vectors.dtype.code == kDLFloat && vectors.dtype.bits == 16) { | ||
| _extend<half>(res, new_vectors, new_indices, *index); | ||
| } else { | ||
| RAFT_FAIL( | ||
| "Unsupported vectors DLtensor dtype: %d and bits: %d", vectors.dtype.code, vectors.dtype.bits); | ||
| } |
There was a problem hiding this comment.
Reject extend calls whose vector dtype mismatches the existing index dtype.
cuvsIvfSqExtend only branches on incoming vector dtype; it does not enforce compatibility with an already-built/deserialized index dtype. That can route incompatible data into the existing index state.
Suggested guard
if (index->dtype.code == 0 && index->dtype.bits == 0) {
index->dtype.code = vectors.dtype.code;
index->dtype.bits = vectors.dtype.bits;
+} else {
+ RAFT_EXPECTS(index->dtype.code == vectors.dtype.code &&
+ index->dtype.bits == vectors.dtype.bits,
+ "new_vectors dtype must match index dtype");
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@c/src/neighbors/ivf_sq.cpp` around lines 323 - 335, The code currently sets
index->dtype when uninitialized but then blindly branches on incoming
vectors.dtype in cuvsIvfSqExtend (calling _extend<float> or _extend<half>),
allowing mismatched inputs to be inserted into an existing index; change the
logic so that if index->dtype is already initialized (index->dtype.code != 0 ||
index->dtype.bits != 0) you first verify vectors.dtype.code and
vectors.dtype.bits exactly match index->dtype.code and index->dtype.bits and
RAFT_FAIL with a clear message if they differ; only when index->dtype is
uninitialized keep the current behavior of assigning index->dtype =
vectors.dtype and proceeding to call the appropriate _extend template based on
vectors.dtype.
Comment on lines
+32
to
+89
| cuvsIvfSqIndex_t index; | ||
| cuvsIvfSqIndexCreate(&index); | ||
|
|
||
| cuvsIvfSqIndexParams_t build_params; | ||
| cuvsIvfSqIndexParamsCreate(&build_params); | ||
| build_params->metric = metric; | ||
| build_params->n_lists = n_lists; | ||
| cuvsIvfSqBuild(res, build_params, &dataset_tensor, index); | ||
|
|
||
| DLManagedTensor queries_tensor; | ||
| queries_tensor.dl_tensor.data = (void*)query_data; | ||
| queries_tensor.dl_tensor.device.device_type = kDLCUDA; | ||
| queries_tensor.dl_tensor.ndim = 2; | ||
| queries_tensor.dl_tensor.dtype.code = kDLFloat; | ||
| queries_tensor.dl_tensor.dtype.bits = 32; | ||
| queries_tensor.dl_tensor.dtype.lanes = 1; | ||
| int64_t queries_shape[2] = {n_queries, n_dim}; | ||
| queries_tensor.dl_tensor.shape = queries_shape; | ||
| queries_tensor.dl_tensor.strides = NULL; | ||
|
|
||
| DLManagedTensor neighbors_tensor; | ||
| neighbors_tensor.dl_tensor.data = (void*)neighbors_data; | ||
| neighbors_tensor.dl_tensor.device.device_type = kDLCUDA; | ||
| neighbors_tensor.dl_tensor.ndim = 2; | ||
| neighbors_tensor.dl_tensor.dtype.code = kDLInt; | ||
| neighbors_tensor.dl_tensor.dtype.bits = 64; | ||
| neighbors_tensor.dl_tensor.dtype.lanes = 1; | ||
| int64_t neighbors_shape[2] = {n_queries, n_neighbors}; | ||
| neighbors_tensor.dl_tensor.shape = neighbors_shape; | ||
| neighbors_tensor.dl_tensor.strides = NULL; | ||
|
|
||
| DLManagedTensor distances_tensor; | ||
| distances_tensor.dl_tensor.data = (void*)distances_data; | ||
| distances_tensor.dl_tensor.device.device_type = kDLCUDA; | ||
| distances_tensor.dl_tensor.ndim = 2; | ||
| distances_tensor.dl_tensor.dtype.code = kDLFloat; | ||
| distances_tensor.dl_tensor.dtype.bits = 32; | ||
| distances_tensor.dl_tensor.dtype.lanes = 1; | ||
| int64_t distances_shape[2] = {n_queries, n_neighbors}; | ||
| distances_tensor.dl_tensor.shape = distances_shape; | ||
| distances_tensor.dl_tensor.strides = NULL; | ||
|
|
||
| cuvsIvfSqSearchParams_t search_params; | ||
| cuvsIvfSqSearchParamsCreate(&search_params); | ||
| search_params->n_probes = n_probes; | ||
| cuvsIvfSqSearch(res, | ||
| search_params, | ||
| index, | ||
| &queries_tensor, | ||
| &neighbors_tensor, | ||
| &distances_tensor, | ||
| (cuvsFilter){.type = NO_FILTER}); | ||
| cuvsStreamSync(res); | ||
|
|
||
| cuvsIvfSqSearchParamsDestroy(search_params); | ||
| cuvsIvfSqIndexParamsDestroy(build_params); | ||
| cuvsIvfSqIndexDestroy(index); | ||
| } |
There was a problem hiding this comment.
Handle cuvs* return codes and centralize cleanup on failure in run_ivf_sq
c/tests/neighbors/run_ivf_sq_c.c (lines 32-89) ignores cuvsError_t results from cuvsIvfSqIndexCreate, cuvsIvfSqIndexParamsCreate, cuvsIvfSqBuild, cuvsIvfSqSearch, and cuvsStreamSync, then unconditionally uses/destroys index, build_params, and search_params. On a failure, this can cascade into invalid handle use and incorrect cleanup.
Check every return value and route to a single cleanup: path that destroys only handles that were successfully created (and, if desired, return/propagate the error).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@c/tests/neighbors/run_ivf_sq_c.c` around lines 32 - 89, The code ignores
cuvsError_t returns from cuvsIvfSqIndexCreate, cuvsIvfSqIndexParamsCreate,
cuvsIvfSqBuild, cuvsIvfSqSearch and cuvsStreamSync; update run_ivf_sq to check
each call's return value, on error jump to a single cleanup: block that only
destroys handles that were successfully created (index via
cuvsIvfSqIndexDestroy, build_params via cuvsIvfSqIndexParamsDestroy,
search_params via cuvsIvfSqSearchParamsDestroy) and propagate/return the error
code; use boolean flags or null-checks to track which of
index/build_params/search_params were initialized before destroying them.
Comment on lines
+89
to
+90
| if (algo_name == "cuvs_ivf_sq") { | ||
| typename cuvs::bench::cuvs_ivf_sq<T>::build_param param; |
There was a problem hiding this comment.
Preserve alias compatibility for IVF-SQ algorithm name.
This branch only recognizes "cuvs_ivf_sq", while nearby factories consistently accept both raft_* and cuvs_* names. That can break existing benchmark configs using "raft_ivf_sq".
Suggested fix
- if (algo_name == "cuvs_ivf_sq") {
+ if (algo_name == "raft_ivf_sq" || algo_name == "cuvs_ivf_sq") {
typename cuvs::bench::cuvs_ivf_sq<T>::build_param param;
parse_build_param<T>(conf, param);
a = std::make_unique<cuvs::bench::cuvs_ivf_sq<T>>(metric, dim, param);
}- if (algo_name == "cuvs_ivf_sq") {
+ if (algo_name == "raft_ivf_sq" || algo_name == "cuvs_ivf_sq") {
auto param = std::make_unique<typename cuvs::bench::cuvs_ivf_sq<T>::search_param>();
parse_search_param<T>(conf, *param);
return param;
}Also applies to: 165-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/bench/ann/src/cuvs/cuvs_benchmark.cu` around lines 89 - 90, The branch
that matches algo_name == "cuvs_ivf_sq" should also accept the alias
"raft_ivf_sq" to preserve compatibility; update the conditional that detects the
IVF-SQ algorithm (where cuvs::bench::cuvs_ivf_sq<T>::build_param is used) to
check for both "cuvs_ivf_sq" and "raft_ivf_sq" (and make the same change in the
corresponding second occurrence around the later branch) so existing configs
using the raft_* name still route to the cuvs_ivf_sq build_param and factory
logic.
Comment on lines
+214
to
+217
| IdxT minibatch_size = std::max<IdxT>(IdxT{1}, static_cast<IdxT>(available_ws_size / mem_per_row)); | ||
|
|
||
| minibatch_size = raft::round_down_safe<IdxT>(minibatch_size, IdxT{64}); | ||
| minibatch_size = std::min<IdxT>(minibatch_size, n_rows); |
There was a problem hiding this comment.
Prevent zero minibatch after 64-alignment.
Line 216 can round minibatch_size from 1..63 down to 0. That can propagate to the loop step at Line 406 and cause a non-terminating loop.
Suggested fix
- IdxT minibatch_size = std::max<IdxT>(IdxT{1}, static_cast<IdxT>(available_ws_size / mem_per_row));
-
- minibatch_size = raft::round_down_safe<IdxT>(minibatch_size, IdxT{64});
- minibatch_size = std::min<IdxT>(minibatch_size, n_rows);
+ IdxT minibatch_size = std::max<IdxT>(IdxT{1}, static_cast<IdxT>(available_ws_size / mem_per_row));
+ minibatch_size = raft::round_down_safe<IdxT>(minibatch_size, IdxT{64});
+ if (n_rows > 0 && minibatch_size == 0) { minibatch_size = IdxT{1}; }
+ minibatch_size = std::min<IdxT>(minibatch_size, n_rows);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| IdxT minibatch_size = std::max<IdxT>(IdxT{1}, static_cast<IdxT>(available_ws_size / mem_per_row)); | |
| minibatch_size = raft::round_down_safe<IdxT>(minibatch_size, IdxT{64}); | |
| minibatch_size = std::min<IdxT>(minibatch_size, n_rows); | |
| IdxT minibatch_size = std::max<IdxT>(IdxT{1}, static_cast<IdxT>(available_ws_size / mem_per_row)); | |
| minibatch_size = raft::round_down_safe<IdxT>(minibatch_size, IdxT{64}); | |
| if (n_rows > 0 && minibatch_size == 0) { minibatch_size = IdxT{1}; } | |
| minibatch_size = std::min<IdxT>(minibatch_size, n_rows); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/src/cluster/detail/kmeans_balanced.cuh` around lines 214 - 217, The code
aligns minibatch_size down to a multiple of 64 using raft::round_down_safe which
can yield 0 for inputs 1..63; ensure minibatch_size is clamped to the valid
range [1, n_rows] after alignment by applying a max-of-1 step (e.g., after
calling raft::round_down_safe and before using minibatch_size in the loop, set
minibatch_size = std::max<IdxT>(minibatch_size, IdxT{1}) or otherwise clamp to
at least 1) so the subsequent loop that uses minibatch_size cannot get a zero
step.
Comment on lines
+480
to
+490
| // Normalize weights so they sum to n_samples (per rank). Reading the sum from | ||
| // a device pointer avoids a host copy / stream sync. When the sum already | ||
| // equals n_samples this is a numerical no-op (matches single-GPU behavior). | ||
| const DataT* d_wt_sum_ptr = d_wt_sum.data_handle(); | ||
| raft::linalg::map( | ||
| handle, | ||
| weight, | ||
| [n_samples, d_wt_sum_ptr] __device__(DataT w) { | ||
| return w * static_cast<DataT>(n_samples) / *d_wt_sum_ptr; | ||
| }, | ||
| raft::make_const_mdspan(weight)); |
There was a problem hiding this comment.
Potential division by zero when all sample weights are zero.
The old implementation validated that the weight sum was positive with RAFT_EXPECTS. The new device-side normalization divides by *d_wt_sum_ptr without checking for zero. If all weights are zero, this causes undefined behavior (division by zero on GPU).
Proposed fix
Add a host-side check after the allreduce or handle zero in the lambda:
+ // Copy sum to host to validate before normalizing
+ DataT h_wt_sum = DataT(0);
+ raft::copy(handle, raft::make_host_scalar_view(&h_wt_sum), raft::make_const_mdspan(d_wt_sum.view()));
+ raft::resource::sync_stream(handle, stream);
+ RAFT_EXPECTS(h_wt_sum > DataT(0), "Sample weights must sum to a positive value");
+
// Normalize weights so they sum to n_samples (per rank).
const DataT* d_wt_sum_ptr = d_wt_sum.data_handle();
raft::linalg::map(
handle,
weight,
[n_samples, d_wt_sum_ptr] __device__(DataT w) {
return w * static_cast<DataT>(n_samples) / *d_wt_sum_ptr;
},
raft::make_const_mdspan(weight));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/src/cluster/detail/kmeans_mg.cuh` around lines 480 - 490, The device-side
normalization in raft::linalg::map uses *d_wt_sum_ptr without guarding against
zero, which can divide-by-zero when all sample weights are zero; add a check
after the allreduce that computes d_wt_sum (the host-side sum/variable backing
d_wt_sum_ptr) and if it is zero either return/throw (restore the RAFT_EXPECTS
behavior) or set a safe non-zero denominator (e.g., 1) before launching
raft::linalg::map; alternatively modify the lambda used in raft::linalg::map
(referencing d_wt_sum_ptr, weight, n_samples) to branch on *d_wt_sum_ptr == 0
and handle that case without performing a division.
| #include <dlpack/dlpack.h> | ||
|
|
||
| // Create data representation in host memory | ||
| float dataset[2][1] = {{0.2, 0.1}}; |
There was a problem hiding this comment.
Fix array dimension mismatch.
The array is declared as float dataset[2][1] (2 rows, 1 column) but initialized as {{0.2, 0.1}} which attempts to place both values in the first row. This is a dimension mismatch.
🐛 Proposed fix
Either fix the declaration to match the initialization:
-float dataset[2][1] = {{0.2, 0.1}};
+float dataset[1][2] = {{0.2, 0.1}};Or fix the initialization to match the declaration:
-float dataset[2][1] = {{0.2, 0.1}};
+float dataset[2][1] = {{0.2}, {0.1}};As per coding guidelines, documentation code examples must compile and run correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| float dataset[2][1] = {{0.2, 0.1}}; | |
| float dataset[1][2] = {{0.2, 0.1}}; |
Suggested change
| float dataset[2][1] = {{0.2, 0.1}}; | |
| float dataset[2][1] = {{0.2}, {0.1}}; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@fern/pages/api_interoperability.md` at line 15, The example has a dimension
mismatch for the array named dataset: it is declared as float dataset[2][1] (2
rows, 1 column) but initialized with two values in a single row ({{0.2, 0.1}}).
Fix by making the declaration match the initializer (e.g., change dataset to
float dataset[1][2]) or alter the initializer to supply two rows (e.g., {{0.2},
{0.1}}) so that the declaration float dataset[2][1] and the initialization
agree.
|
|
||
| // Use DLPack for representing the data as a tensor | ||
| DLManagedTensor dataset_tensor; | ||
| dataset_tensor.dl_tensor.data = dataset; |
There was a problem hiding this comment.
Use device pointer instead of host pointer.
Line 23 assigns the host pointer dataset to dataset_tensor.dl_tensor.data, but the preceding code (lines 17-19) copies data to the device pointer dataset_dev. Since this is demonstrating device memory representation with DLPack on a GPU, the tensor should reference the device pointer.
🔧 Proposed fix
-dataset_tensor.dl_tensor.data = dataset;
+dataset_tensor.dl_tensor.data = dataset_dev;As per coding guidelines, documentation code examples must be accurate and run correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| dataset_tensor.dl_tensor.data = dataset; | |
| dataset_tensor.dl_tensor.data = dataset_dev; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@fern/pages/api_interoperability.md` at line 23, The example incorrectly
assigns the host pointer to the DLPack tensor: replace setting
dataset_tensor.dl_tensor.data = dataset with using the device pointer
dataset_dev so the DLPack GPU tensor points to device memory; update the
assignment that writes into dataset_tensor.dl_tensor.data to use dataset_dev
(and ensure any comments/variable names referencing device memory reflect
dataset_dev) so the example accurately represents a GPU device buffer.
Comment on lines
+73
to
+74
| | `metric` | [`cuvsDistanceType`](/api-reference/c-api-distance-distance#cuvsdistancetype) | | | ||
|
|
There was a problem hiding this comment.
Document metric field semantics for both params structs.
Both metric rows currently have empty descriptions, leaving a key behavior-defining parameter undocumented.
As per coding guidelines, documentation changes must be complete for API parameters and behavior.
Also applies to: 114-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@fern/pages/c_api/c-api-cluster-kmeans.md` around lines 73 - 74, The `metric`
parameter rows are empty; update the docs for both params structs to explain
`metric` (type `cuvsDistanceType`)—list allowed enum values, the default value,
and how each choice affects distance computation and k-means behavior (e.g.,
Euclidean vs. Cosine changes centroid calculation and nearest-centroid
assignment), and note any constraints or compatibility with input data; ensure
the description appears in both `metric` rows that reference `cuvsDistanceType`.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
c/include/cuvs/neighbors/ivf_sq.h (1)
264-264:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix inconsistent type name in documentation.
The
@paramdescription saysivfSqIndexbut the actual parameter type iscuvsIvfSqIndex_t. Update for consistency with the function signature.- * `@param`[in] index ivfSqIndex which has been returned by `cuvsIvfSqBuild` + * `@param`[in] index cuvsIvfSqIndex_t which has been returned by `cuvsIvfSqBuild`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@c/include/cuvs/neighbors/ivf_sq.h` at line 264, The doc comment for the parameter currently uses the inconsistent type name "ivfSqIndex"; update the `@param` line to use the actual typedef name cuvsIvfSqIndex_t to match the function signature (the parameter returned by cuvsIvfSqBuild). Locate the comment near cuvsIvfSqBuild (and the declaration that mentions this parameter) and replace "ivfSqIndex" with "cuvsIvfSqIndex_t" so the documentation and code types are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@c/include/cuvs/neighbors/ivf_sq.h`:
- Line 264: The doc comment for the parameter currently uses the inconsistent
type name "ivfSqIndex"; update the `@param` line to use the actual typedef name
cuvsIvfSqIndex_t to match the function signature (the parameter returned by
cuvsIvfSqBuild). Locate the comment near cuvsIvfSqBuild (and the declaration
that mentions this parameter) and replace "ivfSqIndex" with "cuvsIvfSqIndex_t"
so the documentation and code types are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e6707cb4-2d8f-4183-8f34-36cf59a7d1ce
📒 Files selected for processing (2)
c/include/cuvs/neighbors/ivf_sq.hcpp/include/cuvs/neighbors/ivf_sq.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- cpp/include/cuvs/neighbors/ivf_sq.hpp
divyegala
approved these changes
May 29, 2026
josephine-wolf-oberholtzer
merged commit f4108b9
into
rapidsai:main
163 of 166 checks passed
github-project-automation
Bot
moved this from In Progress
to Done
in Unstructured Data Processing
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
6 participants
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.
No description provided.