-
Notifications
You must be signed in to change notification settings - Fork 51
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
Filter HNSW during BFS in a single pass #1940
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1940 +/- ##
==========================================
- Coverage 83.97% 83.93% -0.04%
==========================================
Files 329 329
Lines 18708 18708
==========================================
- Hits 15710 15703 -7
- Misses 2998 3005 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6a54e6f
to
911a9cd
Compare
// We just need to perform BFS, the replacement is the closest node to the actual | ||
// best solution. This algorithm takes a lazy approach to computing the similarity of | ||
// candidates. | ||
let mut visited_nodes = HashSet::new(); | ||
let mut candidates = VecDeque::from([x]); | ||
let mut visited = HashSet::new(); |
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.
We can go ~30% faster by replacing this with a bitfield or similar. A HashSet
is too slow in the case where we need to scan many nodes.
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.
I just did it for funsies 😝
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.
you can also use FxHashSet which is not cryptographically secure, so its way faster.
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.
Just one thing to discuss!
…b into single_vectors_bfs_search
…b into single_vectors_bfs_search
…b into single_vectors_bfs_search
let mut neighbours = vec![(entry_point.node, 0.)]; | ||
while crnt_layer != 0 { | ||
for crnt_layer in (0..=entry_point.layer).rev() { |
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.
we were skipping layer 0
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.
hah, I thought it was on purpose. Should have asked!
Description
Describe the proposed changes made in this PR.
How was this PR tested?
Describe how you tested this PR.