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.

Sign up for GitHub

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

Jump to bottom

Remove the avx-related code in partitioning.cpp since it is slower #3095

Closed
chasingegg opened this issue Oct 17, 2023 · 1 comment
Closed

Remove the avx-related code in partitioning.cpp since it is slower #3095

chasingegg opened this issue Oct 17, 2023 · 1 comment

Comments

@chasingegg
Copy link
Contributor

In https://github.com/facebookresearch/faiss/blob/main/faiss/utils/partitioning.cpp#L753, this should be a typo? without AVX2 => with AVX2, but why we still need this avx2 version which is slower than the scalar implementation.
@chasingegg
Copy link
Contributor Author

After test, avx2 code will improve the performance, I suggest removing the confusing comments.

@chasingegg chasingegg mentioned this issue Oct 20, 2023
Closed
alexanderguzhva added a commit to alexanderguzhva/knowhere that referenced this issue Nov 27, 2023
@alexanderguzhva
Merge commit 6b761503ba073688e3ebeaa60545633dad83d1ba from Faiss mast…
8664cc9 
…er branch

Commit: 6b761503ba073688e3ebeaa60545633dad83d1ba
Parents: 6c89c8bd4e239141f9a5fad224347b80114ea585
Author: chasingegg <[email protected]>
Committer: Facebook GitHub Bot <[email protected]>
Date: Thu Nov 02 2023 13:03:52 GMT-0400 (Eastern Daylight Time)

Remove confusing comments in partitioning.cpp (#3104)

Summary:
Fix facebookresearch/faiss#3095

Pull Request resolved: facebookresearch/faiss#3104

Reviewed By: mlomeli1

Differential Revision: D50595865

Pulled By: mdouze

fbshipit-source-id: f9107bda114a77d6e1f0da32c3451b7182d32e60
Signed-off-by: Alexandr Guzhva <[email protected]>
alexanderguzhva added a commit to alexanderguzhva/knowhere that referenced this issue Nov 27, 2023
@alexanderguzhva
Merge commit 6b761503ba073688e3ebeaa60545633dad83d1ba from Faiss mast…
bb71c78 
…er branch

Commit: 6b761503ba073688e3ebeaa60545633dad83d1ba
Parents: 6c89c8bd4e239141f9a5fad224347b80114ea585
Author: chasingegg <[email protected]>
Committer: Facebook GitHub Bot <[email protected]>
Date: Thu Nov 02 2023 13:03:52 GMT-0400 (Eastern Daylight Time)

Remove confusing comments in partitioning.cpp (#3104)

Summary:
Fix facebookresearch/faiss#3095

Pull Request resolved: facebookresearch/faiss#3104

Reviewed By: mlomeli1

Differential Revision: D50595865

Pulled By: mdouze

fbshipit-source-id: f9107bda114a77d6e1f0da32c3451b7182d32e60
Signed-off-by: Alexandr Guzhva <[email protected]>
abhinavdangeti pushed a commit to blevesearch/faiss that referenced this issue Jul 12, 2024
@chasingegg @abhinavdangeti
Remove confusing comments in partitioning.cpp (facebookresearch#3104)
f437a79 
Summary:
Fix facebookresearch#3095

Pull Request resolved: facebookresearch#3104

Reviewed By: mlomeli1

Differential Revision: D50595865

Pulled By: mdouze

fbshipit-source-id: f9107bda114a77d6e1f0da32c3451b7182d32e60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove confusing comments in partitioning.cpp chasingegg/faiss
1 participant
@chasingegg