Skip to content

fix(index): make IVF-PQ test training deterministic - #8767

Open
lance-gatefixer[bot] wants to merge 8 commits into
mainfrom
gatekeeper/fix-8764-1
Open

fix(index): make IVF-PQ test training deterministic#8767
lance-gatefixer[bot] wants to merge 8 commits into
mainfrom
gatekeeper/fix-8764-1

Conversation

@lance-gatefixer

Copy link
Copy Markdown
Contributor

Summary

  • add an optional seed for k-means centroid initialization while preserving OS entropy by default
  • propagate the seed through standard and streaming IVF training and PQ codebook training
  • seed the affected IVF-PQ and IVF-HNSW-PQ fixtures and add regression coverage for seeded initialization

Root cause

The vector fixtures seeded their input data, but k-means still initialized centroids from OS entropy. Different centroid choices could move the small recall samples below the 0.5 assertion threshold.

Validation

  • cargo test -p lance-index test_seeded_initialization_is_reproducible --lib
  • cargo test -p lance test_build_ivf_pq_4bit --lib
  • cargo test -p lance test_create_ivf_hnsw_pq_4bit --lib
  • cargo test -p lance test_create_ivf_hnsw_pq_multivec --lib
  • cargo clippy --all --tests --benches -- -D warnings
  • cargo check --manifest-path python/Cargo.toml
  • cargo clippy --tests --manifest-path java/lance-jni/Cargo.toml -- -D warnings
  • cargo fmt --all and JNI cargo fmt

Fixes #8764

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer A-java Java bindings + JNI bug Something isn't working labels Aug 26, 2026
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: The current-head Typo checker fails on an unchanged typo inherited from main.

Remote head cbe9bad contains current base dafa464, and the failing “Spell Check with Typos” Check reports python/src/object_store.rs:154 (mis-named); that file has no PR diff. I fetched both refs, verified base ancestry and local/remote head identity, inspected the failing log, and confirmed the flagged line and its originating base commit, so there is no attributable repair to push. The smallest action is to correct the typo on main and rerun this Check; alternatively, explicitly authorize this PR to carry the unrelated one-line correction.

lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Aug 26, 2026
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: The current-head Spell Check with Typos check fails on an unchanged typo inherited from main.

Remote head a1b17be contains current base dafa464. The live check reports python/src/object_store.rs:154 (mis-named), and that file has no PR diff. I addressed the actionable review feedback, ran the affected Rust test matrices plus full workspace Clippy and formatting, pushed the verified repair head, and confirmed this remaining failure is outside that repair. The smallest action is to correct the typo on main and rerun the check; alternatively, explicitly authorize this PR to carry the unrelated one-line correction.

@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: The current head is blocked by an inherited typo-check failure and Gatekeeper’s explicit manual-review requirement.

Remote head a1b17be contains current base dafa464. “Spell Check with Typos” reports python/src/object_store.rs:154 (mis-named), which is absent from the PR diff, while the “Lance Gatekeeper” Check concludes action_required and states that no automated Review was produced and a maintainer must assess the change. I fetched and synchronized both refs, verified base ancestry, inspected the live diagnostics, and confirmed every other completed Check succeeded, so there is no attributable repair to push. The smallest action is to correct the typo on main and have a maintainer review this PR; alternatively, authorize this PR to carry the unrelated one-line typo correction and trigger new review-relevant activity for another admission assessment.

@u70b3

u70b3 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Does this also cover the python torch one-pass IVF_PQ path? test_torch_index_with_nans (python/python/tests/test_vector_index.py:548, num_sub_vectors=4, one_pass_ivfpq=True, torch CPU) has been failing with StopIteration across several independent PRs today (runs 32951304277, 32966813333, 32968165571) — the torch sampler draws 256 valid residuals from 312 non-NaN rows and intermittently exhausts. The seed propagation here covers the Rust kmeans/PQ training, but the python torch sampling path isn't in the file list — wondering whether this fixes that test too, or whether it's a separate root cause. Consolidated in #8789.

@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

No code change: this PR does not cover the Python Torch one-pass sampler. The StopIteration occurs at next(iter(ds_init)) in unchanged Python code, before Rust PQ construction, and is tracked separately in #8789.

@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Aug 27, 2026

@lance-gatekeeper lance-gatekeeper Bot 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.

Gate recommendation: approve.

The remaining nondeterministic fixture path is closed: IVF training now scans the complete lightweight fixture before seeded k-means, and repeated end-to-end builds verify stable IVF and PQ models. This preserves OS entropy by default while making the affected seeded fixtures reproducible end to end.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 27, 2026
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: The current-head Python Linux 3.13 x86_64 check fails on the unrelated tracked Torch sampler flake.

Remote head cd3699c contains current base 87378db. The completed check reports that only test_torch_index_with_nans[V3] failed with StopIteration at lance/vector.py:164, while 1,379 tests passed. I fetched both refs, verified base ancestry and head identity, read the terminal job log, confirmed the failing Python files are identical to main and absent from this PR diff, and matched the failure to #8789, so there is no attributable repair to push. The smallest action is to land the #8789 Torch sampler fix and rerun the Python check; alternatively, rerun the failed job because this failure is intermittent.

@wombatu-kun

Copy link
Copy Markdown
Contributor

This lands the mechanism, but one fixture family with the same root cause is not covered by it: index::vector::ivf::v2::tests::test_legacy_ivf_pq_cosine_multivec_smoke, which builds through pq_matrix_params rather than lightweight_pq_params*.

On 2026-08-27 it was the most frequent Rust failure on main, ahead of io_uring:

Run Job Head Recall
33125150016 linux-build 7fc1231 0.48
33108104729 windows-build 474ae88 0.48
33067105290 linux-build 0ac7d22 0.48

Same shape as #8764. The input is already deterministic, generate_random_array_with_range being StdRng::from_seed([13; 32]) at rust/lance-testing/src/datagen.rs:244, the bar is recall >= 0.5 with k = 100, and the variance is centroid initialization.

Two things would be needed on top of this PR:

  1. kmeans_seed: Some(42) on both param structs in pq_matrix_params (rust/lance/src/index/vector/ivf/v2.rs:4413).
  2. The same whole-fixture scan this PR already applies to the lightweight path. pq_matrix_params sets ivf_params.sample_rate = PQ_MATRIX_NUM_ROWS (320), but the multivec fixture is NUM_ROWS = 512 rows of 3 vectors each, so 1536 vectors against a 320 budget still enters the OS-seeded sampler at rust/lance/src/dataset.rs:1953. Note that pq_matrix_params is shared with test_pq_matrix_case, whose fixture is 320 single vectors, so the sample budget probably wants to depend on the caller rather than move for both.

Worth confirming that the seeded draw actually clears 0.5 for this test before relying on it, the same way the validation section here does for the lightweight family.

@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Addressed in d692d5a: seeded both IVF and PQ training in pq_matrix_params and made the legacy multivector caller scan all 1,536 fixture vectors; the unchanged 0.5 recall assertion passed in repeated focused runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer A-java Java bindings + JNI bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: IVF_HNSW_PQ recall assertions fail intermittently because k-means init is unseeded

2 participants