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

feat: add more unit test #21

Merged
merged 11 commits into from
Feb 13, 2025
Merged

feat: add more unit test #21

merged 11 commits into from
Feb 13, 2025

Conversation

kemingy
Copy link
Owner

@kemingy kemingy commented Feb 13, 2025

No description provided.

@Copilot Copilot bot review requested due to automatic review settings February 13, 2025 09:04

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

@kemingy kemingy enabled auto-merge (squash) February 13, 2025 09:06
@kemingy kemingy changed the title fix: update the binary dot product bench dim feat: add more unit test Feb 13, 2025
@kemingy kemingy requested a review from Copilot February 13, 2025 12:03

Choose a reason for hiding this comment

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

Copilot reviewed 7 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • python/tests/test_kmeans.py: Evaluated as low risk
  • Cargo.toml: Evaluated as low risk
  • src/kmeans.rs: Evaluated as low risk
Comments suppressed due to low confidence (8)

src/simd.rs:469

  • The removal of the #[target_feature(enable = "sse2,avx,avx2")] attribute from the binary_dot_product_simd function can lead to undefined behavior if the required target features are not available. Please ensure the required features are enabled.
#[target_feature(enable = "sse2,avx,avx2")]

src/simd.rs:491

  • The inline attribute was removed from the mm256_popcnt_epi64 function. This might affect performance. Consider adding #[inline(always)] to ensure the function is inlined.
unsafe fn mm256_popcnt_epi64(x: __m256i) -> __m256i {

src/rabitq.rs:163

  • The function name native_dot_produce should be corrected to native_dot_product.
let diff = neg_dot_product(&lhs, &rhs) + native_dot_produce(&lhs, &rhs);

src/rabitq.rs:468

  • [nitpick] The variable name dim is ambiguous. It should be renamed to dimension.
for dim in [1, 2, 4, 8, 10].into_iter() {

src/rabitq.rs:489

  • [nitpick] The variable name dim is ambiguous. It should be renamed to dimension.
for dim in [64, 128, 256, 320, 1024].into_iter() {

src/rabitq.rs:508

  • [nitpick] The variable name dim is ambiguous. It should be renamed to dimension.
for dim in [64, 128, 256, 320, 1024].into_iter() {

src/rabitq.rs:530

  • [nitpick] The variable name dim is ambiguous. It should be renamed to dimension.
for dim in [32, 64, 124, 128, 132].into_iter() {

src/distance.rs:163

  • The function name 'native_dot_produce' is incorrect. It should be 'native_dot_product'.
let diff = neg_dot_product(&lhs, &rhs) + native_dot_produce(&lhs, &rhs);
Signed-off-by: Keming <[email protected]>
Signed-off-by: Keming <[email protected]>
@kemingy kemingy merged commit e69aed9 into main Feb 13, 2025
6 checks passed
@kemingy kemingy deleted the binary_ip branch February 13, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant