Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Hamming distance to cuvs CAGRA #1056

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lowener
Copy link
Contributor

@lowener lowener commented Feb 7, 2025

No description provided.

Signed-off-by: Mickael Ide <mide@nvidia.com>
@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lowener
To complete the pull request process, please assign liliu-z after the PR has been reviewed.
You can assign the PR to them by writing /assign @liliu-z in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

mergify bot commented Feb 7, 2025

@lowener 🔍 Important: PR Classification Needed!

For efficient project management and a seamless review process, it's essential to classify your PR correctly. Here's how:

  1. If you're fixing a bug, label it as kind/bug.
  2. For small tweaks (less than 20 lines without altering any functionality), please use kind/improvement.
  3. Significant changes that don't modify existing functionalities should be tagged as kind/enhancement.
  4. Adjusting APIs or changing functionality? Go with kind/feature.

For any PR outside the kind/improvement category, ensure you link to the associated issue using the format: “issue: #”.

Thanks for your efforts and contribution to the community!.

Signed-off-by: Mickael Ide <mide@nvidia.com>
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.05%. Comparing base (3c46f4c) to head (d791eaf).
Report is 309 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           main    #1056       +/-   ##
=========================================
+ Coverage      0   72.05%   +72.05%     
=========================================
  Files         0       86       +86     
  Lines         0     8196     +8196     
=========================================
+ Hits          0     5906     +5906     
- Misses        0     2290     +2290     

see 86 files with indirect coverage changes

@mergify mergify bot added the ci-passed label Feb 10, 2025
@@ -63,6 +63,7 @@ static std::set<std::pair<std::string, VecType>> legal_knowhere_index = {
{IndexEnum::INDEX_GPU_IVFFLAT, VecType::VECTOR_FLOAT},
{IndexEnum::INDEX_GPU_IVFPQ, VecType::VECTOR_FLOAT},
{IndexEnum::INDEX_GPU_CAGRA, VecType::VECTOR_FLOAT},
{IndexEnum::INDEX_GPU_CAGRA, VecType::VECTOR_INT8},
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the reason for using VECTOR_INT8, not VECTOR_BINARY? This may prevent the further use case when we're using cagra with int8

@@ -18,5 +18,6 @@
#include "common/cuvs/proto/cuvs_index_kind.hpp"

namespace cuvs_knowhere {
template struct cuvs_knowhere_index<cuvs_proto::cuvs_index_kind::cagra>;
template struct cuvs_knowhere_index<cuvs_proto::cuvs_index_kind::cagra, float>;
template struct cuvs_knowhere_index<cuvs_proto::cuvs_index_kind::cagra, std::uint8_t>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants