Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update select-k heuristic #1985

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

benfred
Copy link
Member

@benfred benfred commented Nov 10, 2023

With #1878 merged, the performance of the radix select algorithms is much improved and we no longer need to incorporate the faiss block select algorithm. With #1878 merged, faiss block select goes from being the 3rd ranked selection algorithm, to the 5th.

This regenerates the heuristic function with the latest benchmark times, and removes the faiss block select in favour of kWarpImmediate and kRadix11bitsExtraPass.

With rapidsai#1878 merged, the performance
of the radix select algorithms is much improved and we no longer need
to incorporate the faiss block select algorithm. With rapidsai#1878 merged,
faiss block select goes from being the 3rd ranked selection algorithm,
to the 5th.

This regenerates the heuristic function with the latest benchmark times,
and removes the faiss block select in favour of kWarpImmediate and
kRadix11bitsExtraPass.
@benfred benfred requested a review from a team as a code owner November 10, 2023 23:59
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the cpp label Nov 10, 2023
@benfred benfred added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 11, 2023
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 @benfred for the update, the PR looks good to me!

@cjnolet
Copy link
Member

cjnolet commented Nov 14, 2023

/merge

@rapids-bot rapids-bot bot merged commit 77bc461 into rapidsai:branch-23.12 Nov 14, 2023
61 checks passed
@achirkin
Copy link
Contributor

achirkin commented Nov 21, 2023

@benfred Cool, just noticed this change! Does this close #1975 ? Also, is there anything still blocking us from removing the legacy faiss neighbors::detail::select_k ?

benfred added a commit to benfred/raft that referenced this pull request Nov 29, 2023
Remove the selection_faiss instantiations. Since rapidsai#1985, we haven't
been using the faiss select_k code and these aren't necessary anymore.
This should lead to a 70MB improvement in libraft.so binary size.

This also removes the raft::spatial::select_k code in favour of matrix::
select_k - the spatial version was marked deprecated, and didn't
switch between the best selection algorithms for the input size.
@benfred benfred mentioned this pull request Nov 29, 2023
@benfred benfred deleted the update_select_heuristic branch November 29, 2023 22:09
@benfred
Copy link
Member Author

benfred commented Nov 29, 2023

@achirkin I think we can close out #1975 with this change - and I've also removed the legacy neibhors::detail::select_k code in #2027

@benfred benfred linked an issue Nov 29, 2023 that may be closed by this pull request
rapids-bot bot pushed a commit that referenced this pull request Nov 30, 2023
Remove the selection_faiss instantiations. Since #1985, we haven't been using the faiss select_k code and these aren't necessary anymore. This should lead to a 70MB improvement in libraft.so binary size.

This also removes the raft::spatial::select_k code in favour of matrix:: select_k - the spatial version was marked deprecated, and didn't switch between the best selection algorithms for the input size.

Authors:
  - Ben Frederickson (https://github.com/benfred)

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

URL: #2027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

[ENH] Re-apply the matrix::select_k heuristics
4 participants