-
Notifications
You must be signed in to change notification settings - Fork 123
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 support for Lucene int4 SQ #2253
base: main
Are you sure you want to change the base?
Add support for Lucene int4 SQ #2253
Conversation
b107895
to
3a52916
Compare
src/main/java/org/opensearch/knn/index/engine/lucene/LuceneMethodResolver.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Naveen Tatikonda <[email protected]>
3a52916
to
a897b80
Compare
@naveentatikonda can you link the benchmarks or provide a link where the benchmarks are present for Lucene SQ 4 bit |
@navneet1v Pls find the benchmarking recall results below
|
Thanks for sharing the results. |
Yeah, that's definitely a good idea, we can see better recall by trading off latency. But, I thought that we only want to support rescoring for only on_disk mode and as of today we are only supporting it for Faiss engine. Also, we might not include this (as 8x compression) as part of on_disk because we prefer to use Faiss engine over Lucene. From UX perspective, you want to add rescoring support to Lucene with SQ irrespective of on_disk ? |
This is a good point. I think rescoring and on_disk should be 2 different things, I should be able to do rescoring without mentioning on_disk mode. I feel this is getting tangled more and more as when I think about it. I think we should trigger a discussion around can rescoring be used outside of on_disk mode or it is always tied to on_disk? |
We are able to do rescoring without specifying on_disk. on_disk just sets default rescoring. Issue is we do not support re-scoring for lucene because we use Lucene's query. But, we should onboard support for it with this. |
if this is case, then I think we should start working on implementing the rescoring feature for Lucene query clause. Given the recall is quite not good for int4. |
Rescoring make sense when we use full precision vector during rescoring. If we uses quantized vector during rescoring, rescoring won't increase recall much. Therefore, rescoring kind of tied to on_disk in that sense. |
thats correct @heemin32 . Here when we are talking about rescoring we are talking about rescoring via full precision vectors only. |
Then, it is "on_disk" right? |
why it will be on_disk then? as Jack mentioned earlier on_disk is just a way to setup some defaults. |
It might confuse user experience.I thought SQ is actually throwing away the original vector and only store quantized vector. Whereas, on_disk will store full precision vector but use quantized vector for in memory index. |
No @heemin32, in Lucene SQ also has full precision vectors stored in a segment file on disk, which we will use to requantize the data if the quantiles changed in that segment |
@navneet1v @shatejas as discussed offline ran some tests by tuning hyper parameters, ef_search(using method_parameters), ef_construction and m. Looking at the results, there isn't much improvement in recall(not close to 0.9) even after bumping up these parameters a lot. So, we must invest some time and add rescoring support to lucene.
|
Description
Add support for Lucene SQ 4 bits
Related Issues
Resolves #2252
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.