Skip to content

CAGRA search: int64_t indices in the API#769

Merged
rapids-bot[bot] merged 17 commits intorapidsai:branch-25.04from
achirkin:fea-cagra-int64-search-api
Apr 1, 2025
Merged

CAGRA search: int64_t indices in the API#769
rapids-bot[bot] merged 17 commits intorapidsai:branch-25.04from
achirkin:fea-cagra-int64-search-api

Conversation

@achirkin
Copy link
Contributor

@achirkin achirkin commented Mar 17, 2025

Add int64_t as the type of returned neighbors in cagra::search while keeping the index type unchanged.

Down the implementation, the PR introduces the OutputIndexT template parameter and handles it differently for each implementation:

  • multi-kernel implementation already does the copy after search in a separate kernel, we change the index type during this copy.
  • multi-cta kernel runs as before, followed by top-k; conversion of the index type happens in a separate map kernel only if a simple type cast wouldn't suffice (i.e. data sizes do not match).
  • single-cta kernel must always run as a single kernel to maintain 'persistent' mode performance. I erase the index type in the kernel signature and pass the sizeof(OutputIndexT) as a tag in the pointer. Based on this tag, a proper output-writing function is dispatched in the epilogue of the single-cta kernel. By using this technique I avoid instantiating multiple kernels at the negligible runtime cost.

Note: we cannot simply instantiate cagra with index type int64_t, because this will make the graph twice larger and will reduce the search throughput.

To test the new search overloads, I turn AnnCagraTest::testCagra into a template and add just two instances to the *.cu files to avoid bloating the number of tests.

The new search overloads are used in the ann-bench wrappers, which slightly improves the reported QPS in some cases due to reducing the index conversion overhead.

@achirkin achirkin added feature request New feature or request non-breaking Introduces a non-breaking change labels Mar 17, 2025
@achirkin achirkin self-assigned this Mar 17, 2025
@github-actions github-actions bot added the cpp label Mar 17, 2025
@achirkin achirkin marked this pull request as ready for review March 21, 2025 09:24
@achirkin achirkin requested a review from a team as a code owner March 21, 2025 09:24
@rapidsai rapidsai deleted a comment from copy-pr-bot bot Mar 21, 2025
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Artem, it looks good, just a few documentation issues. It is great that you could implement this without additional kernel instantiations.

@achirkin achirkin requested a review from tfeher March 31, 2025 08:28
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Artem for the update LGTM!

@tfeher
Copy link
Contributor

tfeher commented Apr 1, 2025

/merge

@rapids-bot rapids-bot bot merged commit 61a714b into rapidsai:branch-25.04 Apr 1, 2025
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cpp feature request New feature or request non-breaking Introduces a non-breaking change

Development

Successfully merging this pull request may close these issues.

2 participants