Match merged vectors by document id, not by ordinal position - #2556
Open
imotov wants to merge 1 commit into
Open
Match merged vectors by document id, not by ordinal position#2556imotov wants to merge 1 commit into
imotov wants to merge 1 commit into
Conversation
testMergeTwoSegsWithASingleDocPerSeg and testTwoVectorFieldsPerDoc asserted that ordinal i of the merged segment holds the i-th document's vector. Lucene does not offer that: MockRandomMergePolicy shuffles the segments of a forced merge on purpose, so the document committed second can land at ordinal 0. Stock Lucene99HnswVectorsFormat fails the same tests on the same seeds, so no cuVS writer is involved. The assertions now resolve each ordinal to its document and check that the document kept its own vector, which leaves the randomized merge policy in play. ordToDoc is checked to be increasing so the id lookup cannot agree with a mapping that is itself garbled. Applied to the quantized and GPU-search formats too, which carried the same assumption unreported. Closes NVIDIA#2550
imotov
added
non-breaking
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
testMergeTwoSegsWithASingleDocPerSegandtestTwoVectorFieldsPerDocasserted that ordinal i of the merged segment holds the i-th document's vector. Lucene does not offer that:MockRandomMergePolicyshuffles the segments of a forced merge on purpose, so the document committed second can land at ordinal 0. StockLucene99HnswVectorsFormatfails the same tests on the same seeds, so no cuVS writer is involved.The assertions now resolve each ordinal to its document and check that the document kept its own vector, which leaves the randomized merge policy in play. ordToDoc is checked to be increasing so the id lookup cannot agree with a mapping that is itself garbled. Applied to the quantized and GPU-search formats too, which carried the same assumption unreported.
Closes #2550