-
Notifications
You must be signed in to change notification settings - Fork 98
Add support for half in CAGRA+HNSW #813
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
base: branch-25.06
Are you sure you want to change the base?
Conversation
Signed-off-by: Mickael Ide <[email protected]>
Signed-off-by: Mickael Ide <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your improvement to the docs! Looks like you caught a lot of typos.
cpp/src/neighbors/detail/hnsw.hpp
Outdated
} else if (metric == cuvs::distance::DistanceType::InnerProduct) { | ||
space_ = std::make_unique<hnswlib::InnerProductSpace>(dim); | ||
space_ = | ||
std::make_unique<hnswlib::InnerProductSpace<T, typename hnsw_dist_t<T>::type>>(dim); | ||
} | ||
} else if constexpr (std::is_same_v<T, std::int8_t> or std::is_same_v<T, std::uint8_t>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With your changes to the template functions in hnswlib, I think we should be able to support int8
and uint8
for InnerProduct now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I added support for that in the PR
Signed-off-by: Mickael Ide <[email protected]>
Signed-off-by: Mickael Ide <[email protected]>
Signed-off-by: Mickael Ide <[email protected]>
Signed-off-by: Mickael Ide <[email protected]>
/ok to test 6660dff |
Signed-off-by: Mickael Ide <[email protected]>
/ok to test c6a93c2 |
# Note that inner_product tests use normalized input which we cannot | ||
# represent in int8, therefore we test only sqeuclidean metric here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this comment now?
This PR add support for half dtype for HNSW in C++, C and python, as well as some tests with it. I had to modify a bit the HNSW patch in order to enable computation on half data types.
I also added the support of inner-product distance for int8/uint8 data type.