(Translated by https://www.hiragana.jp/)
Remove fp16 kernels that have no public entry point by tfeher · Pull Request #268 · rapidsai/cuvs · GitHub
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 fp16 kernels that have no public entry point #268

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Jul 31, 2024

libcuvs.so contains fp16 kernels that are not accessible (missing headers and missing public entry points). This PR removes the unused kernel.

@tfeher tfeher requested a review from a team as a code owner July 31, 2024 17:41
@tfeher tfeher added bug Something isn't working non-breaking Introduces a non-breaking change DO NOT MERGE and removed cpp CMake labels Jul 31, 2024
cpp/CMakeLists.txt Outdated 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.

LGTM!

@tfeher
Copy link
Contributor Author

tfeher commented Aug 1, 2024

Moving comments from PR description here:

Supporting FP16 is beneficial for decreasing memory footprint of large dataset, therefore it is planned to revert this PR in the future and add the missing interfaces.

#264 adds the missing pieces to run vector search with FP16 data, but it comes with ~ 50 MB increase in binary size:

Filename build time binary size
CMakeFiles/cuvs.dir/src/neighbors/cagra_search_half.cu.o 15:03 min 33.5 MB
CMakeFiles/cuvs.dir/src/neighbors/cagra_build_half.cu.o 6:50 min 11.5 MB
CMakeFiles/cuvs.dir/src/neighbors/ivf_pq/detail/ivf_pq_build_extend_half_int64_t.cu.o 4:41 min 9.0 MB
CMakeFiles/cuvs.dir/src/neighbors/cagra_serialize_half.cu.o 60.411 s 1.3 MB
CMakeFiles/cuvs.dir/src/neighbors/ivf_pq/detail/ivf_pq_search_half_int64_t.cu.o 93.104 s 1.7 MB

It is somewhat surprising that we have so large binary size increase, all these components shall be just thin wrappers around functions compiled in separate object files.

@tfeher
Copy link
Contributor Author

tfeher commented Aug 1, 2024

/merge

@rapids-bot rapids-bot bot merged commit 2ebbeca into rapidsai:branch-24.08 Aug 1, 2024
54 checks passed
divyegala pushed a commit to divyegala/cuvs that referenced this pull request Aug 7, 2024
`libcuvs.so` contains fp16 kernels that are not accessible (missing headers and missing public entry points). This PR removes the unused kernel.

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

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

URL: rapidsai#268
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CMake cpp non-breaking Introduces a non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

3 participants