Skip to content

Remove fp16 kernels that have no public entry point - #268

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA: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 into
NVIDIA:branch-24.08from
tfeher:nofp16

Conversation

@tfeher

@tfeher tfeher commented Jul 31, 2024

Copy link
Copy Markdown
Contributor

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

@cjnolet cjnolet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@tfeher

tfeher commented Aug 1, 2024

Copy link
Copy Markdown
Contributor Author

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

tfeher commented Aug 1, 2024

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 2ebbeca into NVIDIA: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: NVIDIA#268
landrumb pushed a commit to landrumb/cuvs that referenced this pull request Jul 8, 2026
`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: NVIDIA#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.

4 participants