Skip to content

remove faiss from cuml - #5293

Merged
rapids-bot[bot] merged 22 commits into
NVIDIA:branch-23.04from
benfred:remove_faiss
Mar 28, 2023
Merged

remove faiss from cuml#5293
rapids-bot[bot] merged 22 commits into
NVIDIA:branch-23.04from
benfred:remove_faiss

Conversation

@benfred

@benfred benfred commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Remove faiss from cuml, by changing the mutual reachability code to use the new
tiled_brute_force_knn code in RAFT - instead of using faiss calls

@benfred
benfred requested review from a team as code owners March 22, 2023 18:59
@github-actions github-actions Bot added ci CMake conda conda issue CUDA/C++ Cython / Python Cython or Python issue labels Mar 22, 2023

@ajschmidt8 ajschmidt8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

Comment thread cpp/src/hdbscan/detail/reachability.cuh Outdated
// or __host__ __device__ lambda`
auto post_process = ReachabilityPostProcess<value_t>{handle, core_dists, alpha};

raft::neighbors::detail::tiled_brute_force_knn(handle,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can even set an identity_op for the default epilogue setting here just so we instantiate that by default.

@cjnolet cjnolet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great! Very minor things as usual.

Comment thread build.sh
and profiling enabled (WARNING: Impacts performance)
--ccache - Use ccache to cache previous compilations
--nocloneraft - CMake will clone RAFT even if it is in the environment, use this flag to disable that behavior
--static-faiss - Force CMake to use the FAISS static libs, cloning and building them if necessary

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yay!!!

Comment thread cpp/src/hdbscan/detail/reachability.cuh

@cjnolet cjnolet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This side looks great!

@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 28, 2023
rapids-bot Bot pushed a commit to NVIDIA/raft that referenced this pull request Mar 28, 2023
Add the ability for a user to specify an epilogue function to run after the distance in the brute_force::knn call.

This lets us remove faiss from cuml, by updating the hdbscan reachability code (NVIDIA/cuml#5293)

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

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

URL: #1371
@cjnolet

cjnolet commented Mar 28, 2023

Copy link
Copy Markdown
Collaborator

/merge

@rapids-bot
rapids-bot Bot merged commit 9f1f8ef into NVIDIA:branch-23.04 Mar 28, 2023
loulankxh pushed a commit to loulankxh/raft that referenced this pull request Oct 14, 2025
Add the ability for a user to specify an epilogue function to run after the distance in the brute_force::knn call.

This lets us remove faiss from cuml, by updating the hdbscan reachability code (NVIDIA/cuml#5293)

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

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

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

Labels

ci CMake conda conda issue CUDA/C++ Cython / Python Cython or Python issue 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.

4 participants