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

add unit test for fix in PR 3840 #3845

Closed
mengdilin opened this issue Sep 9, 2024 · 0 comments
Closed

add unit test for fix in PR 3840 #3845

mengdilin opened this issue Sep 9, 2024 · 0 comments
Assignees

Comments

@mengdilin
Copy link
Contributor

TSIA: #3840 (comment)

@mengdilin mengdilin self-assigned this Sep 9, 2024
mengdilin added a commit to mengdilin/faiss that referenced this issue Sep 11, 2024
Summary:
facebookresearch#3845

Add unit tests for helper search utilities for HNSW. These utility functions live inside an anonymous namespace and each has a reference version gated behind a const bool, I refactored them so the reference version is a flag for the function which defaults to false.

If we are concerned about the performance overhead of the extra if branching (whether to use reference version or not) inside these utility functions, I'm happy to lift out the reference versions to their own functions inside the unit test

Differential Revision: D62510014
mengdilin added a commit to mengdilin/faiss that referenced this issue Sep 12, 2024
Summary:
Pull Request resolved: facebookresearch#3849

facebookresearch#3845

Add unit tests for helper search utilities for HNSW. These utility functions live inside an anonymous namespace and each has a reference version gated behind a const bool, I refactored them so the reference version is a flag for the function which defaults to false.

If we are concerned about the performance overhead of the extra if branching (whether to use reference version or not) inside these utility functions, I'm happy to lift out the reference versions to their own functions inside the unit test

Differential Revision: D62510014
mengdilin added a commit to mengdilin/faiss that referenced this issue Sep 12, 2024
Summary:
Pull Request resolved: facebookresearch#3849

facebookresearch#3845

Add unit tests for helper search utilities for HNSW. These utility functions live inside an anonymous namespace and each has a reference version gated behind a const bool, I refactored them so the reference version is a flag for the function which defaults to false.

If we are concerned about the performance overhead of the extra if branching (whether to use reference version or not) inside these utility functions, I'm happy to lift out the reference versions to their own functions inside the unit test

Reviewed By: junjieqi

Differential Revision: D62510014
mengdilin added a commit to mengdilin/faiss that referenced this issue Sep 12, 2024
Summary:
Pull Request resolved: facebookresearch#3849

facebookresearch#3845

Add unit tests for helper search utilities for HNSW. These utility functions live inside an anonymous namespace and each has a reference version gated behind a const bool, I refactored them so the reference version is a flag for the function which defaults to false.

If we are concerned about the performance overhead of the extra if branching (whether to use reference version or not) inside these utility functions, I'm happy to lift out the reference versions to their own functions inside the unit test

Reviewed By: junjieqi

Differential Revision: D62510014
facebook-github-bot pushed a commit that referenced this issue Sep 12, 2024
Summary:
Pull Request resolved: #3849

#3845

Add unit tests for helper search utilities for HNSW. These utility functions live inside an anonymous namespace and each has a reference version gated behind a const bool, I refactored them so the reference version is a flag for the function which defaults to false.

If we are concerned about the performance overhead of the extra if branching (whether to use reference version or not) inside these utility functions, I'm happy to lift out the reference versions to their own functions inside the unit test

Reviewed By: junjieqi

Differential Revision: D62510014

fbshipit-source-id: b92ed4db69d125c7830da93946f1c2374fb87b08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant