Skip to content

test(index): fix flaky IVF_RQ recall test with multi-bit RaBitQ#7679

Merged
BubbleCal merged 2 commits into
lance-format:mainfrom
wombatu-kun:fix/deterministic-ivf-rq-recall-test
Jul 9, 2026
Merged

test(index): fix flaky IVF_RQ recall test with multi-bit RaBitQ#7679
BubbleCal merged 2 commits into
lance-format:mainfrom
wombatu-kun:fix/deterministic-ivf-rq-recall-test

Conversation

@wombatu-kun

@wombatu-kun wombatu-kun commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What

test_build_ivf_rq (rust/lance/src/index/vector/ivf/v2.rs) is flaky: it builds an IVF_RQ index over seeded random vectors and asserts recall >= 0.5, but recall sits close enough to the bar that it occasionally dips below. It surfaced in CI on #7371 as recall: 0.49 for the case_2::rotation_type_1_RQRotationType__Fast permutation (nlist=1, Cosine, Fast rotation) - see the failed linux-build job: https://github.com/lance-format/lance/actions/runs/28875488220/job/85649319644. It is a pre-existing flake, not caused by any recent change to the search path.

Root cause

test_recall issues its queries with nprobes == nlist, so every partition is scanned and the measured recall reflects RaBitQ quantization error alone. The test builds with num_bits = 1, which means ex_bits = 0: only the sign of each rotated coordinate survives. Over 512 uniformly random, L2-normalized 32-dim vectors the top-100 neighbours are barely distinguishable at that resolution, so recall lands near the bar rather than far above it.

Measured across 336 samples (3 runs x 6 rstest cases x both rotation types x the f32/f64/remap/multivec recall sites):

num_bits ex_bits mean recall stdev min
1 0 0.667 0.041 0.560
3 2 0.904 0.019 0.860
4 3 0.941 0.017 0.890
5 4 0.974 0.011 0.950

At num_bits = 1 the 0.5 bar sits at mean - 4.1 * stdev. The dataset itself is already deterministic (generate_random_array_with_range seeds StdRng::from_seed([13; 32])), but the build draws a fresh random rotation every run from an unseeded entropy RNG, and the IVF k-means init is unseeded too, so that tail is reachable. Seeding those would only fix one draw of a distribution whose mean is 0.667; it would not make the assertion mean anything.

For comparison, the sibling tests in the same file assert IVF_FLAT 1.0, IVF_PQ 0.9 / 0.9 / 0.85, IVF_SQ 0.85 / 0.85 / 0.75.

Fix

Build with num_bits = 5 instead of 1 and raise the assertion from 0.5 to 0.9, which lands at mean - 6.9 * stdev. No production change; the diff is confined to #[cfg(test)] mod tests.

ex_bits = 4 also exercises a FastScan ex-code kernel that test_build_ivf_rq_multi_bit_persists_split_codes_and_searches never reaches: its num_bits 4 and 6 cases have ex_bits 3 and 5, which take the bit-plane repack path, and its num_bits = 9 case has ex_bits = 8. supports_ex_fastscan accepts 2 | 4 | 8.

Both rotation types stay in the matrix. The commented-out #[ignore = "Temporarily skipping flaky 4-bit IVF_RQ tests"] line is removed: it was dead, and it referred to 4-bit tests when the test was 1-bit.

Verification

All 12 test_build_ivf_rq permutations pass over 10 consecutive runs (120 invocations), including the previously-failing case_2::rotation_type_1_RQRotationType__Fast. The rest of index::vector::ivf::v2::tests passes, which covers the shared test_index / test_remap / test_index_multivec helpers used by the SQ, PQ, flat and HNSW tests. cargo fmt --all is clean in all three workspaces and cargo clippy -p lance --tests -- -D warnings is clean.

@github-actions github-actions Bot added the chore label Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@westonpace westonpace requested a review from BubbleCal July 8, 2026 15:07

@BubbleCal BubbleCal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just modifying the test to use more bits (3~5 bits should work well for high recall)

Comment thread rust/lance/src/index/vector/ivf/v2.rs Outdated
/// `random_orthogonal` (private to `lance-index`); we reproduce an equivalent
/// deterministically in-crate. The value type must match the dataset because
/// the quantize path downcasts `rotate_mat` to the dataset's primitive type.
fn deterministic_orthogonal_fsl(value_type: &DataType, seed: u64) -> FixedSizeListArray {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are not longer using dense matrix for rotating.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 6fde16e. The helper is gone along with the rest of the determinization scaffolding; the test now builds with num_bits = 5 instead of pinning a rotation.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Test expectations in test_build_ivf_rq were updated to require higher recall thresholds (0.9 instead of 0.5) for L2, Cosine, and Dot distance types, along with an updated rotation-related parameter value passed to RQBuildParams::with_rotation_type.

Changes

IVF_RQ Test Threshold Update

Layer / File(s) Summary
Recall thresholds and rotation parameter
rust/lance/src/index/vector/ivf/v2.rs
Parameterized test cases for test_build_ivf_rq now require recall of 0.9 (up from 0.5) for L2, Cosine, and Dot distance types at nlist 1 and 4, with updated comments, and the rotation parameter passed to RQBuildParams::with_rotation_type changed from 1 to 5.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: fixing a flaky IVF_RQ recall test, and it matches the test-only scope of the PR.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@wombatu-kun

Copy link
Copy Markdown
Contributor Author

Done 6fde16e. num_bits = 5, recall bar raised from 0.5 to 0.9.

Measured over 336 samples (3 runs x 6 rstest cases x both rotation types x the f32/f64/remap/multivec recall sites):

num_bits ex_bits mean recall stdev min
1 0 0.667 0.041 0.560
3 2 0.904 0.019 0.860
4 3 0.941 0.017 0.890
5 4 0.974 0.011 0.950

At 1 bit the old 0.5 bar sat at mean - 4.1 * stdev, which is why it flaked. At 5 bits the new 0.9 bar sits at mean - 6.9 * stdev. I picked 5 over 3 because ex_bits = 4 also covers a FastScan ex-code kernel that test_build_ivf_rq_multi_bit_persists_split_codes_and_searches never reaches: its num_bits 4 and 6 cases have ex_bits 3 and 5, which take the bit-plane repack path, and its num_bits = 9 case has ex_bits = 8.

12 permutations over 10 consecutive runs, all green.

@wombatu-kun wombatu-kun changed the title test(index): make flaky IVF_RQ recall test deterministic test(index): fix flaky IVF_RQ recall test with multi-bit RaBitQ Jul 9, 2026
@wombatu-kun wombatu-kun requested a review from BubbleCal July 9, 2026 15:18
@BubbleCal BubbleCal merged commit fc882fb into lance-format:main Jul 9, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants