Skip to content
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

support int8 for hnsw #65

Merged
merged 12 commits into from
Oct 23, 2024
Merged

support int8 for hnsw #65

merged 12 commits into from
Oct 23, 2024

Conversation

inabao
Copy link
Collaborator

@inabao inabao commented Oct 15, 2024

issue: #64

  • support int8 for hnsw
  • support testing the performance of a dataset with data type int8 using test_performance

Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
@inabao inabao added the kind/feature New feature or request label Oct 15, 2024
@inabao inabao self-assigned this Oct 15, 2024
jinjiabao.jjb added 4 commits October 15, 2024 15:03
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
@wxyucs wxyucs assigned LHT129 and unassigned inabao Oct 16, 2024
@LHT129 LHT129 self-requested a review October 16, 2024 06:59
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
@inabao inabao requested a review from ShawnShawnYou as a code owner October 22, 2024 07:26
Copy link
Collaborator

@LHT129 LHT129 left a comment

Choose a reason for hiding this comment

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

LGTM

src/index/hnsw.cpp Outdated Show resolved Hide resolved
vectors = (void*)base->GetInt8Vectors();
data_size = dim_ * sizeof(int8_t);
} else {
throw std::invalid_argument("fail to support this data type");
Copy link
Collaborator

Choose a reason for hiding this comment

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

the exception message need include unknown type id

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i removed this choice because the part will be completed during the parameter checking phase

Copy link
Collaborator

Choose a reason for hiding this comment

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

For this function, it is still recommended to add a check for the parameter type_.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
jinjiabao.jjb added 4 commits October 23, 2024 11:54
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Signed-off-by: jinjiabao.jjb <jinjiabao.jjb@antgroup.com>
Copy link
Collaborator

@jiaweizone jiaweizone left a comment

Choose a reason for hiding this comment

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

LGTM

} else if (type == vsag::DataTypes::DATA_TYPE_INT8) {
fstdistfunc_ = vsag::GetINT8InnerProductDistanceFunc(dim);
data_size_ = dim * sizeof(int8_t);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here need process unsupported type

src/data_type.h Outdated
case DataTypes::DATA_TYPE_FP16:
return "float16";
}
return "unknown type";
Copy link
Collaborator

Choose a reason for hiding this comment

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

return "unknown type (" + std::to_string(type(int)) + ")";

@inabao inabao merged commit fb01104 into main Oct 23, 2024
2 checks passed
@inabao inabao deleted the support-int8-hnsw branch October 23, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants