-
Notifications
You must be signed in to change notification settings - Fork 12
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
Priority queue iterator - [MOD-5529] #406
Conversation
@@ -1655,140 +1655,6 @@ TYPED_TEST(HNSWTest, testInitialSizeEstimation_No_InitialCapacity) { | |||
VecSimIndex_Free(index); | |||
} | |||
|
|||
TYPED_TEST(HNSWTest, testIncomingEdgesSize) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is no longer needed as we now have no guarantee on the behavior of choosing between candidates with equal distance (aka choosing at random).
e14aafd
to
5753227
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #406 +/- ##
==========================================
- Coverage 95.66% 95.61% -0.06%
==========================================
Files 66 66
Lines 4180 4174 -6
==========================================
- Hits 3999 3991 -8
- Misses 181 183 +2
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool!
Removing this confusing -distance was very necessary :)
much cleaner
Had some comment see below
* changed implementation of max heap to have random order iterator * changed heuristic use to use heap iterator * fix sort * bug fix (flip flag) * reserving and using ranges * removing a no-longer relevant test * making heap iterator const * remove the usage of ranges (TODO: return in the future) * review fixes
* changed implementation of max heap to have random order iterator * changed heuristic use to use heap iterator * fix sort * bug fix (flip flag) * reserving and using ranges * removing a no-longer relevant test * making heap iterator const * remove the usage of ranges (TODO: return in the future) * review fixes
Describe the changes in the pull request
This PR exposes a random order iterator for the
max_priority_queue
, and utilizes it in the neighbor choosing heuristic in HNSW. This improvement reduces heap usage upon insertion and deletion.As a side effect, we no longer prefer the candidate with the larger internal id on equal distance candidates, and we choose "randomly" (we sort the candidates by score alone).
Main objects this PR modified
begin()
andend()
ofmax_priority_queue
Mark if applicable