opensearch-project / k-NN

🆕 Find the k-nearest neighbors (k-NN) for your vector data
https://opensearch.org/docs/latest/search-plugins/knn/index/
Apache License 2.0
156 stars 123 forks source link

Switch MINGW32 to MINGW64 #2090

Closed heemin32 closed 2 months ago

heemin32 commented 2 months ago

Description

In the CI, we uses MINGW64. With MINGW32, the hamming distance calculation is not correct which result in bad recall for binary vector in window platform.

Related Issues

N/A

Check List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

heemin32 commented 2 months ago

Test succeeded in https://github.com/opensearch-project/k-NN/pull/2080

naveentatikonda commented 2 months ago

@heemin32 did you check with @peterzhuamazon about this change ?

peterzhuamazon commented 2 months ago

@heemin32 did you check with @peterzhuamazon about this change ?

Hi, we already have mingw64 installed for our windows runner. What is the outcome if we do the switch from 32 to 64 in your code?

Thanks.

heemin32 commented 2 months ago

@heemin32 did you check with @peterzhuamazon about this change ?

Hi, we already have mingw64 installed for our windows runner. What is the outcome if we do the switch from 32 to 64 in your code?

Thanks.

With MINGW32, the hamming distance calculation is not correct which result in bad recall for binary vector in window platform.

peterzhuamazon commented 2 months ago

@heemin32 did you check with @peterzhuamazon about this change ?

Hi, we already have mingw64 installed for our windows runner. What is the outcome if we do the switch from 32 to 64 in your code? Thanks.

With MINGW32, the hamming distance calculation is not correct which result in bad recall for binary vector in window platform.

Thanks, I have no issues with this now. 👍