Skip to content

Remove fp16 kernels that have no public entry point#268

Merged
rapids-bot[bot] merged 4 commits intorapidsai:branch-24.08from
tfeher:nofp16
Aug 1, 2024
Merged

Remove fp16 kernels that have no public entry point#268
rapids-bot[bot] merged 4 commits intorapidsai:branch-24.08from
tfeher:nofp16

Conversation

@tfeher
Copy link
Copy Markdown
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
Comment thread cpp/CMakeLists.txt Outdated
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

tfeher commented Aug 1, 2024

/merge

@rapids-bot rapids-bot Bot merged commit 2ebbeca into rapidsai:branch-24.08 Aug 1, 2024
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

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants