fix: relax test_query_delta_indices assertion for approximate index#7622
Open
XuQianJin-Stars wants to merge 1 commit into
Open
fix: relax test_query_delta_indices assertion for approximate index#7622XuQianJin-Stars wants to merge 1 commit into
XuQianJin-Stars wants to merge 1 commit into
Conversation
The test_query_delta_indices test asserts that the top-2 nearest neighbors of a query vector are exactly [0, 1000], but this only holds for exact indexes (e.g. IvfPq / IvfFlat). For approximate indexes such as IvfHnswSq, scalar quantization can cause the returned ids to differ from the ground truth (observed on Windows CI: got [1000, 1486]). The core intent of the test is to verify that both delta indices are queried (i.e. one result comes from each delta segment). Relax the assertion accordingly for approximate indexes while keeping the exact assertion for exact indexes.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
Fix a flaky assertion in
test_query_delta_indicesthat fails on Windows CI when the underlying index type is approximate (e.g.IvfHnswSq).Background
test_query_delta_indicesasserts that the top-2 nearest neighbors of the query vector are exactly[0, 1000]. This assumption only holds for exact indexes such asIvfPq/IvfFlat.For approximate indexes such as
IvfHnswSq, scalar quantization (SQ) introduces small numerical differences that can cause the returned row ids to diverge from the exact ground truth. This has been observed on Windows CI, where the test returned[1000, 1486]instead of[0, 1000], producing a flaky failure unrelated to the delta-index logic being tested.Reference failure log:
test index::append::tests::test_query_delta_indices<IvfHnswSq>on Windows CI[0, 1000][1000, 1486]What this PR does
The core intent of the test is to verify that both delta index segments are queried — i.e. one result comes from the first delta segment (row ids
< 1000) and the other from the second delta segment (row ids>= 1000).This PR relaxes the assertion so that:
IvfPq,IvfFlat, ...): keep the strict assertionrow_ids == [0, 1000].IvfHnswSq, ...): assert only the structural invariant — one result comes from each delta segment — instead of asserting the exact row ids.This preserves the original intent of the test while eliminating the false positive caused by SQ quantization error.
Changes
rust/lance/src/index/append.rs: relax the assertion intest_query_delta_indicesfor approximate index types.Testing
cargo fmt --check✅cargo clippy -p lance --lib --tests --all-features -- -D warnings✅cargo test -p lance index::append::tests::test_query_delta_indices✅ (all variants pass)Risk
Minimal — this is a test-only change that does not alter any production code path. It relaxes an over-strict assertion to match the actual guarantees provided by approximate indexes.