Closed
Conversation
Contributor
|
Could you explain what this is? |
Contributor
Author
When popping the first element of the heap, it could be invalid. So in this case |
Contributor
|
I believe this can happen only if a -1 id gets pushed on the heap, which does never happen, correct? |
Contributor
Author
It'll be set to -1 when popping the min element. https://github.com/facebookresearch/faiss/blob/main/faiss/impl/HNSW.cpp#L877 |
Contributor
|
@alexanderguzhva has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
Confirmed for both the bug and the recall rate improvement. |
Contributor
|
@alexanderguzhva merged this pull request in 159641a. |
Contributor
|
Thanks a lot for this fix! it increases the accuracy of HNSW by a lot! |
BZO95
added a commit
to BZO95/faiss
that referenced
this pull request
Apr 10, 2025
Summary: Pull Request resolved: facebookresearch/faiss#2771 Test Plan: A DEFINITE BUG. Fixing it improves the recall rate. Reproduce with the following code: ``` MinimaxHeap heap(1); heap.push(1, 1.0); float v1 = 0; heap.pop(&v1); heap.push(1, 1.0); assert(heap.nvalid == 1); ``` Baseline ``` [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (064c246e0)]$ taskset -c 72-95 ./bench_hnsw.par 16 hnsw load data Testing HNSW Flat add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 4 Adding 1 elements at level 4 Adding 26 elements at level 3 Adding 951 elements at level 2 Adding 30276 elements at level 1 Adding 968746 elements at level 0 Done in 14893.718 ms search efSearch 16 bounded queue True 0.007 ms per query, R@1 0.8823, missing rate 0.0000 efSearch 16 bounded queue False 0.008 ms per query, R@1 0.9120, missing rate 0.0000 efSearch 32 bounded queue True 0.012 ms per query, R@1 0.9565, missing rate 0.0000 efSearch 32 bounded queue False 0.011 ms per query, R@1 0.9641, missing rate 0.0000 efSearch 64 bounded queue True 0.018 ms per query, R@1 0.9889, missing rate 0.0000 efSearch 64 bounded queue False 0.019 ms per query, R@1 0.9896, missing rate 0.0000 efSearch 128 bounded queue True 0.036 ms per query, R@1 0.9970, missing rate 0.0000 efSearch 128 bounded queue False 0.037 ms per query, R@1 0.9970, missing rate 0.0000 efSearch 256 bounded queue True 0.062 ms per query, R@1 0.9991, missing rate 0.0000 efSearch 256 bounded queue False 0.067 ms per query, R@1 0.9991, missing rate 0.0000 [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (fc6e9b938|remote/fbsource/stable...)]$ taskset -c 72-95 ./bench_hnsw.par 4 hnsw_sq load data Testing HNSW with a scalar quantizer training add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 5 Adding 1 elements at level 5 Adding 15 elements at level 4 Adding 194 elements at level 3 Adding 3693 elements at level 2 Adding 58500 elements at level 1 Adding 937597 elements at level 0 Done in 8900.962 ms search efSearch 16 0.003 ms per query, R@1 0.7365, missing rate 0.0000 efSearch 32 0.006 ms per query, R@1 0.8712, missing rate 0.0000 efSearch 64 0.011 ms per query, R@1 0.9415, missing rate 0.0000 efSearch 128 0.018 ms per query, R@1 0.9778, missing rate 0.0000 efSearch 256 0.036 ms per query, R@1 0.9917, missing rate 0.0000 ``` Candidate: ``` [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (064c246e0)]$ taskset -c 72-95 ./bench_hnsw.par 16 hnsw load data I0408 09:45:20.949554 3024612 ContainerResourceMonitor.cpp:68] devserver cgroup2Reader creation is successful Testing HNSW Flat add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 4 Adding 1 elements at level 4 Adding 26 elements at level 3 Adding 951 elements at level 2 Adding 30276 elements at level 1 Adding 968746 elements at level 0 Done in 14243.637 ms search efSearch 16 bounded queue True 0.006 ms per query, R@1 0.9122, missing rate 0.0000 efSearch 16 bounded queue False 0.006 ms per query, R@1 0.9122, missing rate 0.0000 efSearch 32 bounded queue True 0.011 ms per query, R@1 0.9643, missing rate 0.0000 efSearch 32 bounded queue False 0.011 ms per query, R@1 0.9644, missing rate 0.0000 efSearch 64 bounded queue True 0.018 ms per query, R@1 0.9880, missing rate 0.0000 efSearch 64 bounded queue False 0.020 ms per query, R@1 0.9880, missing rate 0.0000 efSearch 128 bounded queue True 0.036 ms per query, R@1 0.9969, missing rate 0.0000 efSearch 128 bounded queue False 0.035 ms per query, R@1 0.9969, missing rate 0.0000 efSearch 256 bounded queue True 0.064 ms per query, R@1 0.9994, missing rate 0.0000 efSearch 256 bounded queue False 0.062 ms per query, R@1 0.9994, missing rate 0.0000 [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (6de3a2d76)]$ taskset -c 72-95 ./bench_hnsw.par 4 hnsw_sq load data Testing HNSW with a scalar quantizer training add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 5 Adding 1 elements at level 5 Adding 15 elements at level 4 Adding 194 elements at level 3 Adding 3693 elements at level 2 Adding 58500 elements at level 1 Adding 937597 elements at level 0 Done in 8451.601 ms search efSearch 16 0.004 ms per query, R@1 0.8025, missing rate 0.0000 efSearch 32 0.006 ms per query, R@1 0.8925, missing rate 0.0000 efSearch 64 0.011 ms per query, R@1 0.9480, missing rate 0.0000 efSearch 128 0.019 ms per query, R@1 0.9793, missing rate 0.0000 efSearch 256 0.035 ms per query, R@1 0.9919, missing rate 0.0000 ``` Reviewed By: mdouze Differential Revision: D44815702 Pulled By: alexanderguzhva fbshipit-source-id: ca7c7e83a6560316af543bde125ac703bf2e1dac
samanthawaters8882michaeldonovan
added a commit
to samanthawaters8882michaeldonovan/faiss
that referenced
this pull request
Oct 12, 2025
Summary: Pull Request resolved: facebookresearch/faiss#2771 Test Plan: A DEFINITE BUG. Fixing it improves the recall rate. Reproduce with the following code: ``` MinimaxHeap heap(1); heap.push(1, 1.0); float v1 = 0; heap.pop(&v1); heap.push(1, 1.0); assert(heap.nvalid == 1); ``` Baseline ``` [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (064c246e0)]$ taskset -c 72-95 ./bench_hnsw.par 16 hnsw load data Testing HNSW Flat add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 4 Adding 1 elements at level 4 Adding 26 elements at level 3 Adding 951 elements at level 2 Adding 30276 elements at level 1 Adding 968746 elements at level 0 Done in 14893.718 ms search efSearch 16 bounded queue True 0.007 ms per query, R@1 0.8823, missing rate 0.0000 efSearch 16 bounded queue False 0.008 ms per query, R@1 0.9120, missing rate 0.0000 efSearch 32 bounded queue True 0.012 ms per query, R@1 0.9565, missing rate 0.0000 efSearch 32 bounded queue False 0.011 ms per query, R@1 0.9641, missing rate 0.0000 efSearch 64 bounded queue True 0.018 ms per query, R@1 0.9889, missing rate 0.0000 efSearch 64 bounded queue False 0.019 ms per query, R@1 0.9896, missing rate 0.0000 efSearch 128 bounded queue True 0.036 ms per query, R@1 0.9970, missing rate 0.0000 efSearch 128 bounded queue False 0.037 ms per query, R@1 0.9970, missing rate 0.0000 efSearch 256 bounded queue True 0.062 ms per query, R@1 0.9991, missing rate 0.0000 efSearch 256 bounded queue False 0.067 ms per query, R@1 0.9991, missing rate 0.0000 [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (fc6e9b938|remote/fbsource/stable...)]$ taskset -c 72-95 ./bench_hnsw.par 4 hnsw_sq load data Testing HNSW with a scalar quantizer training add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 5 Adding 1 elements at level 5 Adding 15 elements at level 4 Adding 194 elements at level 3 Adding 3693 elements at level 2 Adding 58500 elements at level 1 Adding 937597 elements at level 0 Done in 8900.962 ms search efSearch 16 0.003 ms per query, R@1 0.7365, missing rate 0.0000 efSearch 32 0.006 ms per query, R@1 0.8712, missing rate 0.0000 efSearch 64 0.011 ms per query, R@1 0.9415, missing rate 0.0000 efSearch 128 0.018 ms per query, R@1 0.9778, missing rate 0.0000 efSearch 256 0.036 ms per query, R@1 0.9917, missing rate 0.0000 ``` Candidate: ``` [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (064c246e0)]$ taskset -c 72-95 ./bench_hnsw.par 16 hnsw load data I0408 09:45:20.949554 3024612 ContainerResourceMonitor.cpp:68] devserver cgroup2Reader creation is successful Testing HNSW Flat add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 4 Adding 1 elements at level 4 Adding 26 elements at level 3 Adding 951 elements at level 2 Adding 30276 elements at level 1 Adding 968746 elements at level 0 Done in 14243.637 ms search efSearch 16 bounded queue True 0.006 ms per query, R@1 0.9122, missing rate 0.0000 efSearch 16 bounded queue False 0.006 ms per query, R@1 0.9122, missing rate 0.0000 efSearch 32 bounded queue True 0.011 ms per query, R@1 0.9643, missing rate 0.0000 efSearch 32 bounded queue False 0.011 ms per query, R@1 0.9644, missing rate 0.0000 efSearch 64 bounded queue True 0.018 ms per query, R@1 0.9880, missing rate 0.0000 efSearch 64 bounded queue False 0.020 ms per query, R@1 0.9880, missing rate 0.0000 efSearch 128 bounded queue True 0.036 ms per query, R@1 0.9969, missing rate 0.0000 efSearch 128 bounded queue False 0.035 ms per query, R@1 0.9969, missing rate 0.0000 efSearch 256 bounded queue True 0.064 ms per query, R@1 0.9994, missing rate 0.0000 efSearch 256 bounded queue False 0.062 ms per query, R@1 0.9994, missing rate 0.0000 [aguzhva@devgpu005.ftw6 ~/fbsource/buck-out/v2/gen/fbcode/faiss/benchs (6de3a2d76)]$ taskset -c 72-95 ./bench_hnsw.par 4 hnsw_sq load data Testing HNSW with a scalar quantizer training add hnsw_add_vertices: adding 1000000 elements on top of 0 (preset_levels=0) max_level = 5 Adding 1 elements at level 5 Adding 15 elements at level 4 Adding 194 elements at level 3 Adding 3693 elements at level 2 Adding 58500 elements at level 1 Adding 937597 elements at level 0 Done in 8451.601 ms search efSearch 16 0.004 ms per query, R@1 0.8025, missing rate 0.0000 efSearch 32 0.006 ms per query, R@1 0.8925, missing rate 0.0000 efSearch 64 0.011 ms per query, R@1 0.9480, missing rate 0.0000 efSearch 128 0.019 ms per query, R@1 0.9793, missing rate 0.0000 efSearch 256 0.035 ms per query, R@1 0.9919, missing rate 0.0000 ``` Reviewed By: mdouze Differential Revision: D44815702 Pulled By: alexanderguzhva fbshipit-source-id: ca7c7e83a6560316af543bde125ac703bf2e1dac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.