Iterative CAGRA-Q by irina-resh-nvda · Pull Request #1810 · NVIDIA/cuvs

irina-resh-nvda · 2026-02-16T15:08:36Z

Iterative cagra graph construction using CAGRA-Q search.

This PR improves the iterative CAGRA build method by enabling PQ compression: the dataset is compressed before the iterative search starts, and CAGRA-Q is used to iteratively update the KNN graph.

copy-pr-bot · 2026-02-16T15:08:40Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

… search - Configurable growth-phase in-build search params (itopk_size, search_width, max_iterations) and internal/smem dtype; itopk auto-forced on the final full-size iteration. - Decouple compression params used during iterative construction from the target index compression. - Add shuffle_dataset option; fix out-of-bounds access from the in-place raft gather by switching to an out-of-place gather.

…around) The shuffle_dataset path used an out-of-place gather into a temporary buffer to work around an illegal memory access in raft's in-place gather overload when n_rows * row_len exceeded 2^31 (32-bit index overflow). That bug is now fixed upstream in raft (NVIDIA/raft#3059, closes #3055), which the cuvs raft pin now includes. Revert to the in-place gather to drop the extra full-size temporary allocation and copy.

aamijar · 2026-07-22T23:23:55Z

Hi @irina-resh-nvda, thanks for the contribution! I've added some commits to clean up some small things in this PR:

Merge main into this branch and fix merge conflicts 278a4f3
Run pre-commit run --all-files to fix style checker cc65291
There were some unrelated .clangd and .gitignore changes I saw so I reverted that 25e6d7d
There was an unrelated warning change in cuvs_bench so I reverted that 6bca275
The tests/CMakeLists.txt has a duplicate file added so I removed that here 9d44b23
There were some unrelated changes from auto to explicit return type which I reverted 73c9bbc
Two commented out code lines were removed 23cf815

aamijar · 2026-07-22T23:28:27Z

+  //   {std::optional<bool>{std::nullopt}},
+  //   {cuvs::neighbors::MergeStrategy::MERGE_STRATEGY_PHYSICAL,
+  //    cuvs::neighbors::MergeStrategy::MERGE_STRATEGY_LOGICAL});
+  // inputs.insert(inputs.end(), inputs2.begin(), inputs2.end());


Why has this test been commented out?

this is a leftover from my testing, i'll fix it

aamijar · 2026-07-22T23:28:57Z

+    {16},   // k
+    {32},   // degree
+    {       // graph_build_algo::IVF_PQ,
+     // graph_build_algo::NN_DESCENT,


Same question: what is the reason for the changes here?

sorry, testing artefact, will fix

aamijar · 2026-07-22T23:29:17Z

    {true},
    {false},
-    {0.995},
+    {0.01},


Why has the recall changed to 1%?

aamijar · 2026-07-22T23:29:31Z

+  //   {std::optional<bool>{std::nullopt}},
+  //   {cuvs::neighbors::MergeStrategy::MERGE_STRATEGY_PHYSICAL,
+  //    cuvs::neighbors::MergeStrategy::MERGE_STRATEGY_LOGICAL});
+  // inputs.insert(inputs.end(), inputs2.begin(), inputs2.end());


Same question: why was this commented out?

aamijar · 2026-07-22T23:30:38Z

-    // Create a dataset with 1000 points
-    constexpr int64_t n_dataset = 1000;
+    // Create a dataset with 10000 points
+    constexpr int64_t n_dataset = 10000;


What is the reason this test file has been changed to use n_dataset = 10,000?

aamijar · 2026-07-22T23:32:44Z

+                                  dev_ptr,
+                                  n_bytes,
+                                  cudaMemcpyDeviceToHost,
+                                  writeback_stream_));


We should have a raft function that does this already.

// use memcpy instead of raft::copy to avoid strange behavior with HMM/ATS memory

If there is a problem here, we should try to root fix that in raft.

i think this change is indeed some kind of diff artefact

aamijar · 2026-07-22T23:33:40Z

+                                  host_view_.data_handle() + src_row_offset * host_view_.extent(1),
+                                  n_bytes,
+                                  cudaMemcpyHostToDevice,
+                                  prefetch_stream_));


Same as before: let's try to use the raft copy function.

aamijar · 2026-07-22T23:34:54Z

+  cudaPointerAttributes attr;
+  RAFT_CUDA_TRY(cudaPointerGetAttributes(&attr, ptr));
+  return attr.hostPointer != nullptr;
+}


We should already have utilities somewhere to do check if a pointer is host or device accessible, either within in raft or cuvs.
I can try to find those functions for you.

…build-time random seeds

tfeher · 2026-07-23T14:36:48Z

+ * @tparam IdxT The type of the index
+ */
+template <typename T, typename IdxT>
+class batched_device_view_from_host {


Can't we use batch_load_iterator for the same purpose?

It looks like a merge/diff view artifact. I think @mfoerste4 has added this new class earlier in his refactoring and I asked exactly the same question. There was seemingly a good reason for it, but I forgot it :) @mfoerste4 could you please remind?

Bumping this, if we don't need this utility we can remove a bunch of code from this PR from this file utils.hpp and delete the test_batched_device_view_from_host.cu test file.

Yes, this seems to be a merge issue. The class and test do not exist on main anymore.

irina-resh-nvda · 2026-07-24T13:41:03Z

the new main merge introduced some crashes, im investigating

achirkin

Hi, a few suggestions here, mainly about the use of raft resources and helper utilities in cuvs.

achirkin · 2026-07-27T12:10:21Z

+{
+  auto stream = raft::resource::get_cuda_stream(res);
+
+  auto dev_knn_graph = raft::make_device_matrix<IdxT, int64_t>(res, curr_query_size, curr_topk);


How big the curr_query_size here - is it bounded?
Consider using raft::resource::get_workspace_resource_ref(res) or raft::resource::get_large_workspace_resource_ref(res) depending on the answer to correctly track this as a temporary allocation.

It's bounded by the dataset size, so i will use get_large_workspace_resource_ref as you suggested

achirkin · 2026-07-27T12:16:41Z

+  dev_output_graph =
+    raft::make_device_matrix<IdxT, int64_t>(res, curr_query_size, next_graph_degree);


The same question about being bounded and using the workspace resource here.
Also, do you allocate it multiple times (i.e. by calling it multiple times)?

if no: maybe use the return semantics here for clarity (instead of passing the output variable by reference)?

if yes: it would make sense to first clear the variable, and then assign a new one - to avoid two large arrays being allocated at the same time (increasing the memory footprint of the whole thing). Perhaps, doing this outside of the search_and_optimize function and changing the function to return the new graph would make sense in this case too.

same N-limited boundedness. Will change to return, and release the old graph before allocating the new one to reduce peak memory use

achirkin · 2026-07-27T12:41:55Z

+ * @tparam IdxT The type of the index
+ */
+template <typename T, typename IdxT>
+class batched_device_view_from_host {


It looks like a merge/diff view artifact. I think @mfoerste4 has added this new class earlier in his refactoring and I asked exactly the same question. There was seemingly a good reason for it, but I forgot it :) @mfoerste4 could you please remind?

… instead

…emporaries in iterative build (~2.7x faster build)

aamijar · 2026-08-10T17:02:41Z

It looks like a lot of files were edited to change the copyright header? I think we should revert those.
Update I fixed it in these commits:
355d240
8cd4191

This commit 6ad9234 reverted minor diff in a test file.

We should look through the merge conflicts due to Datasets API and resolve them.
Fyi a few cagra-q fixes are being worked on here #2413

This reverts commit d9c6bfd.

copy-pr-bot · 2026-08-10T22:00:05Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-project-automation Bot added this to Unstructured Data Processing Feb 16, 2026

aamijar assigned irina-resh-nvda Feb 24, 2026

aamijar added non-breaking Introduces a non-breaking change feature request New feature or request labels Feb 24, 2026

cjnolet moved this to In Progress in Unstructured Data Processing Feb 27, 2026

josephine-wolf-oberholtzer added this to Unstructured Data Processing Jul 1, 2026

josephine-wolf-oberholtzer moved this to In Progress in Unstructured Data Processing Jul 1, 2026

irina-resh-nvda added 7 commits July 14, 2026 00:53

chore(cagra): drop local dev artifacts (.clangd, .gitignore entries)

23aed4e

feat(cagra): add batched_device_view_from_host utility and unit test

a4ebee6

fix(cagra): bound random seed selection to graph size during build

90fcf80

feat(bench): expose iterative CAGRA-Q build/search params in cuvs_bench

90d1219

test(cagra): VPQ/iterative build test updates

a938958

irina-resh-nvda force-pushed the iterative_cagra_q branch from 8fc10ac to 4f4068a Compare July 15, 2026 08:46

Merge branch 'main' into iterative_cagra_q

c804088

irina-resh-nvda marked this pull request as ready for review July 15, 2026 08:48

irina-resh-nvda requested review from a team as code owners July 15, 2026 08:48

aamijar and others added 2 commits July 22, 2026 14:57

Merge branch 'main' into iterative_cagra_q

278a4f3

fix style

cc65291

aamijar changed the title ~~Iterative cagra q~~ Iterative graph build using CAGRA-Q search Jul 22, 2026

aamijar changed the title ~~Iterative graph build using CAGRA-Q search~~ Iterative CAGRA-Q Jul 22, 2026

aamijar added 5 commits July 22, 2026 22:19

restore clangd and gitignore changes

25e6d7d

revert cuvs_bench warning

6bca275

remove whitespace

6ed4ab5

remove duplicate file in cmakelists.txt

9d44b23

revert to auto for type deduction

73c9bbc

remove commented out code

23cf815

aamijar requested changes Jul 22, 2026

View reviewed changes

aamijar mentioned this pull request Jul 22, 2026

Accept VPQ dataset in Iterative graph build #2355

Open

fix(cagra): pass graph_size to persistent single-CTA kernel to bound …

c2f9b6a

…build-time random seeds

tfeher reviewed Jul 23, 2026

View reviewed changes

achirkin requested changes Jul 27, 2026

View reviewed changes

irina-resh-nvda added 5 commits August 6, 2026 00:28

Brought back the tests

cad2e8f

Fixed the test

0e98b34

Removed dataset shuffle

8a378cb

Removed unused pointer residency helper; use memory_type_from_pointer…

aa2227b

… instead

Pre-commit changes

d9c6bfd

irina-resh-nvda requested review from a team as code owners August 10, 2026 13:01

irina-resh-nvda requested a review from gforsyth August 10, 2026 13:01

perf(cagra): per-batch VPQ query reconstruction + device-pool graph t…

ae89f9f

…emporaries in iterative build (~2.7x faster build)

Revert "Pre-commit changes"

355d240

This reverts commit d9c6bfd.

aamijar added 2 commits August 10, 2026 22:04

revert another spdx change

8cd4191

revert test to minimize diff

6ad9234

		dev_output_graph =
		raft::make_device_matrix<IdxT, int64_t>(res, curr_query_size, next_graph_degree);

Uh oh!

Conversation

irina-resh-nvda commented Feb 16, 2026 • edited by aamijar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot Bot commented Feb 16, 2026

Uh oh!

aamijar commented Jul 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irina-resh-nvda commented Jul 24, 2026

Uh oh!

achirkin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aamijar commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot Bot commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

irina-resh-nvda commented Feb 16, 2026 •

edited by aamijar

Loading

aamijar commented Aug 10, 2026 •

edited

Loading