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.

Sign up for GitHub

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

Jump to bottom

[FEA] Port brute force knn from FAISS to RAFT primitives #798

Closed
cjnolet opened this issue Sep 2, 2022 · 1 comment · Fixed by #1202
Closed

[FEA] Port brute force knn from FAISS to RAFT primitives #798

cjnolet opened this issue Sep 2, 2022 · 1 comment · Fixed by #1202
Assignees
@benfred
Labels
FAISS feature request New feature or request

Comments

@cjnolet
Copy link
Member

cjnolet commented Sep 2, 2022

As we are working to integrate RAFT into FAISS, we need to remove all dependence on FAISS to avoid a circular dependency problem. Since the code is pretty close to optimal for moderate values of k, we might be better off just copying the k-selection code into RAFT for the time being (providing proper attribution to FAISS, another with the appropriate licenses, of course).

Essentially, we need to port the bfknn to use RAFT primitives. Successful implementation of this issue will include all of the distances which are currently supported by bfknn and replacement of bfknn from FAISS in our brute_force_knn with our own version. Ideally, we would also provide more customization around the batch sizes in order to make our API a little more flexible.
@cjnolet cjnolet added the feature request New feature or request label Sep 2, 2022
@cjnolet cjnolet mentioned this issue Sep 2, 2022
Open
@github-actions
Copy link

github-actions bot commented Oct 2, 2022

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@cjnolet cjnolet assigned benfred and unassigned lowener Nov 8, 2022
@bdice bdice mentioned this issue Jan 5, 2023
Merged
@cjnolet cjnolet added the FAISS label Jan 30, 2023
@cjnolet cjnolet mentioned this issue Mar 12, 2023
Merged
rapids-bot bot pushed a commit that referenced this issue Mar 17, 2023
@benfred
Replace faiss bfKnn (#1202)
1a3c028 
Replace faiss bfKnn with code that leverages our pairwise_distance api and select_k api - by tiling over the inputs.

This lets us remove faiss as a dependency

Closes #798 
Closes #1159

Authors:
  - Ben Frederickson (https://github.com/benfred)
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Ray Douglass (https://github.com/raydouglass)

URL: #1202
@github-project-automation github-project-automation bot moved this from In Progress to Done in VS/ML/DM Primitives Release Board Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@benfred benfred

Labels
FAISS feature request New feature or request
Projects
VS/ML/DM Primitives Release Board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace faiss bfKnn benfred/raft
3 participants
@benfred @cjnolet @lowener