Skip to content

ANN_BENCH: common AnnBase::index_type - #2315

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-24.06from
achirkin:enh-ann-bench-index-type
May 15, 2024
Merged

ANN_BENCH: common AnnBase::index_type#2315
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-24.06from
achirkin:enh-ann-bench-index-type

Conversation

@achirkin

@achirkin achirkin commented May 14, 2024

Copy link
Copy Markdown
Contributor

Replace the size_t type in the AnnBase::search for the output neighbor indices with a common AnnBase::index_type.
This PR stops short of changing the behavior of the benchmarks, since it keeps using index_type = size_t.

The introduction of the new type has couple benefits:

  • Makes the usage of the index_type more clear in the code, distinguishing it from the extents type, which is usually size_t as well.
  • Makes it possible to quickly change the alias to uint32_t during development and experiments. This is needed to avoid calling extra linalg::map on the produced results when the algorithm output is not compatible with size_t.

As a small extra change, I've factored out common IVF-PQ - CAGRA-Q refinement code into a separate refine_helper function.

@achirkin achirkin added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 14, 2024
@achirkin
achirkin requested a review from a team as a code owner May 14, 2024 13:26
@github-actions github-actions Bot added the cpp label May 14, 2024

@tfeher tfeher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Artem for the PR, it is nice to have an easy way to change index type in the benchmarks. ! It is great to have the helper function for refinement. LGTM.

@achirkin

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit eb1333d into NVIDIA:branch-24.06 May 15, 2024
loulankxh pushed a commit to loulankxh/raft that referenced this pull request Oct 14, 2025
Replace the `size_t` type in the `AnnBase::search` for the output neighbor indices with a common `AnnBase::index_type`.
This PR stops short of changing the behavior of the benchmarks, since it keeps `using index_type = size_t`.

The introduction of the new type has couple benefits:
  - Makes the usage of the `index_type` more clear in the code, distinguishing it from the extents type, which is usually `size_t` as well.
  - Makes it possible to quickly change the alias to `uint32_t` during development and experiments. This is needed to avoid calling extra `linalg::map` on the produced results when the algorithm output is not compatible with `size_t`.


As a small extra change, I've factored out common IVF-PQ - CAGRA-Q refinement code into a separate `refine_helper` function.

Authors:
  - Artem M. Chirkin (https://github.com/achirkin)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)

URL: NVIDIA#2315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants