Skip to content

Conversation

@alonre24
Copy link
Collaborator

In brute force - instead of asserting that we hadn't computed the scores when the number of returned results by the batch iterator is 0, we should assert that the number of returned results by the iterator is 0 if the scores hadn't been computed yet (otherwise, we crash after asking for 0 results).
In HNSW - we should insert the entry point node to the candidates heap only when both the returned results count is 0 and the top_candidates_extra queue is empty. Otherwise, we might get that the entry point will be returned twice (if we ask for 0 results, and moved the entry point to the top_candidates_extra, and then reinsert the entry point to the candidates).

@alonre24 alonre24 requested review from DvirDukhan and GuyAv46 March 13, 2022 15:21
@codecov
Copy link

codecov bot commented Mar 13, 2022

Codecov Report

Merging #147 (1356e4e) into main (8cbf10a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #147   +/-   ##
=======================================
  Coverage   92.60%   92.61%           
=======================================
  Files          39       39           
  Lines        1962     1963    +1     
=======================================
+ Hits         1817     1818    +1     
  Misses        145      145           
Impacted Files Coverage Δ
...ecSim/algorithms/brute_force/bf_batch_iterator.cpp 100.00% <100.00%> (ø)
src/VecSim/algorithms/hnsw/hnsw_batch_iterator.cpp 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cbf10a...1356e4e. Read the comment docs.

Copy link
Collaborator

@GuyAv46 GuyAv46 left a comment

Choose a reason for hiding this comment

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

THANK YOU

@alonre24 alonre24 merged commit 497d228 into main Mar 14, 2022
@alonre24 alonre24 deleted the batch_iterator_fix_for_zero_results branch March 14, 2022 07:00
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.

3 participants