`libraft` and `pylibraft` API for CAGRA build and HNSW search by divyegala · Pull Request #2022 · rapidsai/raft

divyegala · 2023-11-23T02:53:23Z

Closes #1772

…ib_pylibraft

cpp/CMakeLists.txt

cpp/include/raft/neighbors/cagra_hnswlib.hpp

cpp/include/raft/neighbors/cagra_hnswlib_types.hpp

cpp/include/raft/neighbors/detail/cagra_hnswlib.hpp

cpp/include/raft/neighbors/cagra_hnswlib_types.hpp

python/pylibraft/pylibraft/neighbors/cagra_hnswlib.pyx

cpp/include/raft/neighbors/detail/cagra_hnswlib.hpp

…ib_pylibraft

cjnolet

I think the general idea is getting there, but there's still a lot of api leakage and we have an opportunity here for a dedicated HNSW API (independent from hnswlib).

cpp/cmake/thirdparty/get_hnswlib.cmake

cpp/include/raft/neighbors/cagra_hnswlib_types.hpp

cpp/include/raft/neighbors/detail/cagra/cagra_serialize.cuh

cpp/include/raft/neighbors/detail/cagra_hnswlib.hpp

cpp/include/raft/neighbors/hnswlib_types.hpp

cpp/include/raft_runtime/neighbors/cagra.hpp

cpp/include/raft_runtime/neighbors/cagra_hnswlib.hpp

cpp/src/raft_runtime/neighbors/cagra_hnswlib.cpp

cpp/include/raft_runtime/neighbors/cagra_hnswlib.hpp

divyegala · 2023-12-15T08:53:47Z

@cjnolet I addressed your suggestions and reworked the filenames/namespaces/structure to a pure hnsw namespace. There's one thing that I'd like your opinion on and something I would consider a nice-to-have. As you requested, I moved the serialization and de-serialization functions into hnsw_serialize.cuh, however, here's the problem:

serialize accepts a CAGRA index and needs to be CUDA compatible
deserialize returns an hnsw index and does not need to be CUDA compatible

As a result of putting these two functions in the same header, they need to be compiled in the same .cu translation unit because hnswlib is not true header-only: it does not have inline functions hence all headers that contain an hnswlib included header must be compiled in the same TU.

By splitting deserialize and serialize into two different headers and thus, two TUs, we will be able to:

Compile serialize in hnswlib_serialize.cu
Compile deserialize and search in hnsw.cpp

Another benefit of this split is that users will be able to build CPU-only RAFT and still be able to load hnsw indexes and search them. If this change is acceptable to you, then I will go ahead and make it.

divyegala · 2023-12-19T13:03:59Z

@lowener I filed an issue for filtering functions as a follow-on, this PR is pretty big already

cjnolet · 2024-01-06T19:45:53Z

cpp/include/raft_runtime/neighbors/cagra.hpp

+             raft::host_matrix_view<const T, int64_t, row_major> dataset)              \
+    ->raft::neighbors::cagra::index<T, IdxT>;                                          \
+                                                                                       \
+  void build_device(raft::resources const& handle,                                     \


Why not just overload?

Not sure, this change predates this PR

…gra_hnswlib_pylibraft

copy-pr-bot · 2024-01-22T23:57:40Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

…nto cagra_hnswlib_pylibraft

…gra_hnswlib_pylibraft

cjnolet · 2024-01-26T01:32:34Z

/merge

This PR conditionally includes `hnsw` sources, to prevent build errors like those seen in cuGraph after #2022 was merged. See also: rapidsai/cugraph#4121, rapidsai/cugraph#4122 Authors: - Divye Gala (https://github.com/divyegala) Approvers: - Corey J. Nolet (https://github.com/cjnolet) - Bradley Dice (https://github.com/bdice) - Robert Maynard (https://github.com/robertmaynard)

…ai#2022) Closes rapidsai#1772 Authors: - Divye Gala (https://github.com/divyegala) - Corey J. Nolet (https://github.com/cjnolet) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Micka (https://github.com/lowener) - Corey J. Nolet (https://github.com/cjnolet) URL: rapidsai#2022

This PR conditionally includes `hnsw` sources, to prevent build errors like those seen in cuGraph after rapidsai#2022 was merged. See also: rapidsai/cugraph#4121, rapidsai/cugraph#4122 Authors: - Divye Gala (https://github.com/divyegala) Approvers: - Corey J. Nolet (https://github.com/cjnolet) - Bradley Dice (https://github.com/bdice) - Robert Maynard (https://github.com/robertmaynard)

hnswlib serialize python API

c60ae05

divyegala added feature request New feature or request non-breaking Non-breaking change labels Nov 23, 2023

divyegala self-assigned this Nov 23, 2023

github-actions bot added cpp CMake python labels Nov 23, 2023

divyegala added 5 commits November 23, 2023 03:48

fix typo, refactor cython

7f04f97

add cpp index and python load,search methods with runtime API

4b2b2c6

Merge remote-tracking branch 'upstream/branch-24.02' into cagra_hnswl…

c62c8e3

…ib_pylibraft

add static_assert guard for (u)int hnswlib serialize

92e5717

passing float tests

7f78851

divyegala marked this pull request as ready for review November 28, 2023 04:28

divyegala requested review from a team as code owners November 28, 2023 04:28

divyegala added 4 commits November 28, 2023 04:29

fix docs

29f0774

try to write in native dtype

e2e1fc3

update mypy, solve error

b8a4c50

attempt to use rapids_cpm_find for hnswlib

e02a0e3

lowener requested changes Dec 12, 2023

View reviewed changes

divyegala added 5 commits December 14, 2023 11:45

rework to not expose hnswlib headers in runtime API

54106a1

readd copyright check

ebdce10

Merge remote-tracking branch 'upstream/branch-24.02' into cagra_hnswl…

c29313a

…ib_pylibraft

print binary dir

1da21e4

address review

e5cd5f6

divyegala requested a review from lowener December 14, 2023 13:10

cjnolet requested changes Dec 15, 2023

View reviewed changes

address review, enable int8 in hnswlib

460bc98

lowener approved these changes Dec 19, 2023

View reviewed changes

cjnolet and others added 3 commits January 6, 2024 10:17

Merge branch 'branch-24.02' into cagra_hnswlib_pylibraft

114d176

Merge branch 'branch-24.02' into cagra_hnswlib_pylibraft

691d254

Fix some style

c30671b

cjnolet requested changes Jan 19, 2024

View reviewed changes

vyasr and others added 6 commits January 19, 2024 18:32

Get wheel builds to compile

44c88e7

fix docs

304ae7a

Export hnswlib dependency in raft

d10bb4f

fix doc again

1d8e804

Merge remote-tracking branch 'origin/cagra_hnswlib_pylibraft' into ca…

05bc41b

…gra_hnswlib_pylibraft

Merge branch 'branch-24.02' into cagra_hnswlib_pylibraft

306de24

Specify the hnswlib version in the export

ae543e7

vyasr force-pushed the cagra_hnswlib_pylibraft branch from c6b896f to ae543e7 Compare January 23, 2024 00:13

divyegala added 8 commits January 23, 2024 02:09

more doc fixes

ad05438

Merge branch 'cagra_hnswlib_pylibraft' of github.com:divyegala/raft i…

d8a3e06

…nto cagra_hnswlib_pylibraft

hopefully final doc fix

94d08cd

style fix

54bf32c

Merge branch 'branch-24.02' into cagra_hnswlib_pylibraft

8e8549b

merging upstream

aa16015

Merge remote-tracking branch 'origin/cagra_hnswlib_pylibraft' into ca…

e62d3bc

…gra_hnswlib_pylibraft

pre commit fixes

1897cf7

cjnolet approved these changes Jan 26, 2024

View reviewed changes

rapids-bot bot merged commit 2822532 into rapidsai:branch-24.02 Jan 26, 2024

bdice mentioned this pull request Jan 29, 2024

Conditionally include hnsw wrapper source in CMake #2135

Merged

Conversation

divyegala commented Nov 23, 2023 • edited by cjnolet Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cjnolet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

divyegala commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

divyegala commented Dec 19, 2023

Uh oh!

cjnolet Jan 6, 2024

Choose a reason for hiding this comment

Uh oh!

divyegala Jan 19, 2024

Choose a reason for hiding this comment

Uh oh!

copy-pr-bot bot commented Jan 22, 2024

Uh oh!

cjnolet commented Jan 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

divyegala commented Nov 23, 2023 •

edited by cjnolet

Loading

divyegala commented Dec 15, 2023 •

edited

Loading