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] Rename libraft-distance to libraft #824

Closed
4 tasks done
cjnolet opened this issue Sep 13, 2022 · 1 comment · Fixed by #1333
Closed
4 tasks done

[FEA] Rename libraft-distance to libraft #824

cjnolet opened this issue Sep 13, 2022 · 1 comment · Fixed by #1333
Labels
feature request New feature or request inactive-30d

Comments

@cjnolet
Copy link
Member

cjnolet commented Sep 13, 2022 •
edited
Loading

we had originally created two pre-compiled shared libs and had planned to create potentially more of them over time. However, we have started putting more APIs in libraft-distance than just distance things and we really only need one additional package as a result of the FAISS dependency for a lot of the nearest neighbors stuff.

We should rename libraft_distance.so to just libraft.so to be more consistent with other rapids projects. Once we are able to remove the FAISS dependency, we can also merge the source files from libraft_nn.so into libraft.so and just have a single package for runtime APIs and precompiled template specializations.

  • Rename libraft distance everywhere (cmake, code, etc..) to libraft
  • Create a new component for compiled that will specify to enable the compiled library. Also need to change RAFT_COMPILE_DIST_LIBRARY -> RAFT_COMPILE_LIB
  • Rename src/distance to src/
  • Update all documentation to refer just to the compiled library as libraft and not libraft-distance.
@cjnolet cjnolet added the feature request New feature or request label Sep 13, 2022
@github-actions
Copy link

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 mentioned this issue Mar 12, 2023
Merged
5 tasks
rapids-bot bot pushed a commit that referenced this issue Mar 20, 2023
@cjnolet
Consolidate pre-compiled specializations into single libraft binary (…
97f8ad7 
…#1333)

Now that we no longer need to rely on the FAISS dependency, this PR:
1. Consolidates  the `libraft-distance` and `libraft-nn` targets into a single `libraft` artifact
2. Introduces a new `raft::compiled` target to link against the new `libraft.so` binary. Removes `raft::distance`and `raft::nn` targets.
3. Consolidates `RAFT_DISTANCE_COMPILED` and `RAFT_NN_COMPILED` pre-processor vars into a single `RAFT_COMPILED` (to match similar pattern implementated by `spdlog`)
4. Consolidates `specializations.cuh` headers
5. Updates all docs, scripts, and build infra

This change has been a long time coming and is intended to be a 23.04 feature. This is further going to require updates to several projects downstream. Here's a checklist to track that progress:
- [x] offline announcement
- [x] cuml rapidsai/cuml#5272
- [x] cugraph rapidsai/cugraph#3348
- [x] cuopt rapidsai/cuopt#1023
- [x] cugraph-ops rapidsai/cugraph-ops#429


This PR depended on #1340 (removing FAISS from the build) and on #1202 (replacing the FAISS bfknn w/ our own), both of which have been merged. 

Closes #824

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

Approvers:
  - Sevag H (https://github.com/sevagh)
  - Divye Gala (https://github.com/divyegala)
  - Ben Frederickson (https://github.com/benfred)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1333
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request inactive-30d
Projects
VS/ML/DM Primitives Release Board
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Consolidate pre-compiled specializations into single libraft binary cjnolet/raft
1 participant
@cjnolet