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.
Add a HNSW collector that exits early when nearest neighbor queue saturates #14094
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 a HNSW collector that exits early when nearest neighbor queue saturates #14094
Changes from 22 commits
3b30c07
0b24e79
70b6144
93fb470
c5aa473
7fc49c5
aed6fd5
b7eb24f
d143bbb
51df9ee
e55f967
e3f8db3
ec1e686
a71e936
09b0229
74132f1
8d00ae8
370f513
fed77c9
88d22df
e86ebdc
e69730f
5b001ee
20a481f
1dbaa1a
c6dbf7e
55fdea2
460efd9
3d2e46b
0f3f047
eef4f97
acf5866
620e985
ca0f05d
f116141
45b2031
66bd51d
0b47585
bb57ca1
8f846b8
695a4eb
c899f29
36b9931
a84032e
5b12866
c54a596
73d5069
60f3384
3a85f8a
5231f86
54e77a3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think this kind of collector is OK. But it makes most sense to me to be a delegate collector. An abstract collector to
KnnCollector.Delegate
.Then, I also think that the
OrdinalTranslatingKnnCollector
should inherit directly fromHnswKnnCollector
always assuming that the passed in collector is aHnswKnnCollector
.Note, the default behavior for
HnswKnnCollector#nextCandidate
can simply be nothing, allowing for overriding.This might require a new
HnswGraphSearcher#search
interface to keep the old collector actions, but it can be simple to add a new one that accepts aHnswKnnCollector
and delegate to it withnew HnswKnnCollector(KnnCollector delegate)
.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 adjusted my refactoring for the seeded queries similarly. It seems nicer IMO: #14170
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.
thanks Ben. I'll incorporate your suggestions once #14170 is in.
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.
made
HnswKnnCollector
aKnnCollector.Decorator
in c6dbf7eThere 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.
@tteofili what do you think of making this more general? I think having a "nextCandidate" or "nextBlockOfVectors" is generally useful, and might be applicable to all types of kNN indices.
For example:
Do you think we can make this API general?
Maybe not, I am not sure.
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 really like this idea Ben, I'll see if I can make up something reasonable for that ;)