Skip to content

Conversation

@divyegala
Copy link
Member

@divyegala divyegala commented Nov 13, 2024

This PR adds an option to build the full HNSW hierarchy on the CPU when converting a CAGRA index to an hnswlib index. This lets us enable an extend() API.

For hnswlib:

  1. Update to v0.7.0
  2. Remove dependency as symbols are compiled within DSO

@divyegala divyegala added feature request New feature or request non-breaking Introduces a non-breaking change labels Nov 13, 2024
@divyegala divyegala self-assigned this Nov 13, 2024
@divyegala divyegala changed the title update hnswlib to 0.7.0 HNSW CPU Hierarchy Nov 13, 2024
@jakirkham
Copy link
Member

Does this need to be added to the Conda recipes as well?

@divyegala
Copy link
Member Author

@jakirkham no actually we don't want to use the hnswlib conda package because their python distribution messes up with our python version requirements. I will be removing it from our conda dev environments as well.

We'll rely on CMake instead to get us hnswlib. There's no double compilation issue anyway since it's a header-only library.

@jakirkham
Copy link
Member

jakirkham commented Nov 14, 2024

Oh ok. Had not looked into the nuances of this package or our usage of it

Indeed if we don't use the Python package, we should drop it

There doesn't appear to be a header-only C++ package currently. So just fetching it from source seems like the thing to do

Are the headers just used in the build to produce a library? Or are we shipping their headers too? If the latter, we may want to look at siloing it somehow to avoid clobbering or affecting other installs a user may have

@divyegala
Copy link
Member Author

@jakirkham we don't need to ship the headers, they are self-contained within the libcuvs shared object.

@divyegala divyegala added breaking Introduces a breaking change and removed non-breaking Introduces a non-breaking change labels Nov 21, 2024
@divyegala divyegala marked this pull request as ready for review November 23, 2024 00:38
@divyegala divyegala requested review from a team as code owners November 23, 2024 00:38
@divyegala divyegala requested review from a team as code owners November 23, 2024 00:38
@divyegala divyegala requested a review from jameslamb November 23, 2024 00:38
Comment on lines +25 to +30
list(APPEND CUVS_CXX_FLAGS -Wall -Werror -Wno-unknown-pragmas -Wno-error=deprecated-declarations
-Wno-reorder
)
list(APPEND CUVS_CUDA_FLAGS
-Xcompiler=-Wall,-Werror,-Wno-error=deprecated-declarations,-Wno-reorder
)
Copy link
Member Author

Choose a reason for hiding this comment

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

no-reorder was added because of hnswlib v0.7.0. It is weird because the order warning that the compiler complained about in hnswlib was not actually out of order.

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Approving build/packaging changes. (I also skimmed the C++ and Python and nothing seemed noteworthy.)

@divyegala
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 441d2f1 into rapidsai:branch-24.12 Dec 2, 2024
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Introduces a breaking change CMake cpp feature request New feature or request Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants