Repository navigation
SOLR-18424: add oversampling + raw dense vector reranking - #4919
liangkaiwen wants to merge 6 commits into
Conversation
|
Exploring a bug where Lucene's RescoreTopNQuery can get an index out of bounds exception when the inner query returns 0 documents |
…out of bounds exception when docs is zero
There was a problem hiding this comment.
I spent some time digging various aspects of this pull request, that is fine overall.
I have doubts on the explicit check conditions on the vectorEncoding, specifically the 'BYTE' is not supported. So possibly some of my previous comments lose sense overall if we remove that part
Generally speaking both the binary and scalar-quantised dense vector field don't support vectorEncoding='BYTE ' (logically) and from some investigation they will silently skip quantisation entirely (in this class there are various checks on the vectorEncoding and when not FLOAT32 it skips various instructions):
only FLOAT32 fields get the quantizing writer. This is the key line. lucene104/Lucene104ScalarQuantizedVectorsWriter.java, lines 127–137: }
So, my conclusion and ideal outcome is:
- oversampling is useful only with binary or scalar quantised vector fields, so we log a warning/throw exception when used with anything different
-on a separate ticket maybe we can add a check for both the scalar and binary quantised fields that reject the 'BYTE' encoding telling the user this is logically incompatible with the quantisation itself (and the underlying lucene quantisation engine).
Some may argue (including myself) that binary quantisation should be compatible with a 'BYTE' vectorEncoding, but it's a different discussion and investigation, I would say (because from what I quickly saw, this is not permitted by Lucene implementation)
| vectorBuilder.getByteVector(), | ||
| acceptedChildren, | ||
| topK, | ||
| candidateTopK, |
There was a problem hiding this comment.
here candidateTopK could be misleading as BYTE encoding won't be supported in this contribution?
| fieldName, | ||
| vectorBuilder.getByteVector(), | ||
| topK, | ||
| candidateTopK, |
There was a problem hiding this comment.
same comment as below, isn't this a bit confusing given the fact in this PR BYTE encoding won't be supported?
| if (rewrittenInner instanceof MatchNoDocsQuery) { | ||
| return rewrittenInner; | ||
| } | ||
| // Delegate using the already rewritten inner query rather than calling super.rewrite(), which |
There was a problem hiding this comment.
I suspect this is a 'if... else...' maybe better to make it explicit?
| "{!knn f=" + quantizedField + " topK=5 rerankOversample=5}[1.0, 2.0, 3.0, 4.0]", | ||
| "fl", | ||
| "id"), | ||
| EXPECTED_EXACT_TOP_5); |
There was a problem hiding this comment.
should we add here an assertion with the same query but no oversampling? showing the difference?
| "//result/doc[4]/str[@name='id'][.='10']", | ||
| "//result/doc[5]/str[@name='id'][.='3']"); | ||
| } | ||
|
|
There was a problem hiding this comment.
What's the differentiator between some of these tests and the dedicated oversampling KNN tests? I get the throw/not throw exception, but the others feel similar to the ones in the other class.
| |Optional |Default: 1 | ||
| |=== | ||
| + | ||
| If provided, the query will retrieve a candidate pool of documents equal to topK multiplied by the rerankOversample value provided. The candidate set of documents will then be rescored with their raw vector values, and reranked down to a topK result set. |
There was a problem hiding this comment.
I would specify at the moment that is compatible only with scalar quantised or binary quantised vector fields
https://issues.apache.org/jira/browse/SOLR-18424
Description
Solr 10 introduced Binary Quantization, however the recall drop-off is fairly sharp when used as-is. This can be partially offset by oversampling (retrieving more total documents from HNSW graph) and reranking (using raw non-quantized vector values from .vec). While reranking is possible through the standard Solr reranking feature, it is cumbersome syntactically and easy to get wrong. This aims to offer a cleaner and performant oversampling/reranking natively in the KNN query parser
Solution
Code mostly written by Claude Opus 5
Tests
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.