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

remove faiss from cuml #5293

Merged
merged 22 commits into from
Mar 28, 2023
Merged

remove faiss from cuml #5293

merged 22 commits into from
Mar 28, 2023

Conversation

benfred
Copy link
Member

@benfred benfred commented Mar 22, 2023

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
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

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

// or __host__ __device__ lambda`
auto post_process = ReachabilityPostProcess<value_t>{handle, core_dists, alpha};

raft::neighbors::detail::tiled_brute_force_knn(handle,
Copy link
Member

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.

Copy link
Member

@cjnolet cjnolet left a comment

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.

@@ -47,7 +47,6 @@ HELP="$0 [<target> ...] [<flag> ...]
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
Member

Choose a reason for hiding this comment

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

Yay!!!

cpp/src/hdbscan/detail/reachability.cuh Show resolved Hide resolved
Copy link
Member

@cjnolet cjnolet left a comment

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 rapidsai/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 (rapidsai/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
Copy link
Member

cjnolet commented Mar 28, 2023

/merge

@rapids-bot rapids-bot bot merged commit 9f1f8ef into rapidsai:branch-23.04 Mar 28, 2023
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
Development

Successfully merging this pull request may close these issues.

3 participants