Fix flaky IVF/Refine Panorama kNN tests under AVX512_SPR dispatch#5420
Closed
alibeklfc wants to merge 3 commits into
Closed
Fix flaky IVF/Refine Panorama kNN tests under AVX512_SPR dispatch#5420alibeklfc wants to merge 3 commits into
alibeklfc wants to merge 3 commits into
Conversation
…entroids Summary: `ResidualQuantizer::compute_codes_add_centroids` dispatches on `use_beam_LUT` with an if/else-if chain covering values 0 (lut0, direct L2 residuals) and 1 (lut1, LUT-accelerated encoding). Before this diff, any other value fell through silently: the chunk loop executed but wrote nothing to `codes_out`, leaving the output buffer uninitialized with no exception, no return code, and no warning. `use_beam_LUT` is a public `int` field with no enum constraint. A caller that sets it to 2 — expecting a future mode, or by mistake — would get silently garbage-filled codes from all index types backed by ResidualQuantizer (RQ, PRQ, IVF-RQ, IVF-PRQ) with no way to detect the failure. Fix: Add `FAISS_THROW_IF_NOT_FMT` validation **before the per-batch loop**, checking `use_beam_LUT == 0 || use_beam_LUT == 1`. Placing the guard at method entry (matching the pattern at line 437) ensures it fires for all n — including n=0, where the loop body never executes. The validation uses the correct FAISS_THROW_IF_NOT_FMT idiom (condition expression as first argument, not `false`) so the error message is clean: "use_beam_LUT=X is not supported (must be 0 or 1)". Additionally: - Updated the `use_beam_LUT` field comment in `ResidualQuantizer.h` to document valid values (0 and 1) and the no-other-value constraint. - Updated the corresponding `__init__.pyi` stub comment to match, so Python IDE users see the constraint at the point of declaration. Differential Revision: D110754282
Summary:
D110175030 (June 30, unpublished) replaced 1e20 distance argmin sentinels
in six sites but was never published and remained as an orphaned local commit.
This diff completes the sweep with all nine remaining non-HUGE_VALF sentinels
across the entire faiss codebase.
Sites fixed:
1. fvec_madd_and_argmin<SIMDLevel::NONE> (distances_simd.cpp:156): float
vmin = 1e20 -> HUGE_VALF. Scalar fallback used on devservers and in CI.
2. fvec_madd_and_argmin_sse_ref (distances_sse-inl.h:313): __m128 vmin4 =
_mm_set_ps1(1e20) -> _mm_set_ps1(HUGE_VALF). Called by both the AVX2
and AVX-512 dispatch paths for fvec_madd_and_argmin.
3. fvec_madd_and_argmin<SIMDLevel::ARM_NEON> (distances_aarch64.cpp:109):
vdupq_n_f32(1e20) -> vdupq_n_f32(HUGE_VALF).
4. fvec_madd_and_argmin<SIMDLevel::ARM_SVE> (distances_arm_sve.cpp:106):
float vmin = 1e20 -> HUGE_VALF.
5. ProductQuantizer::compute_code_from_distance_table (ProductQuantizer.cpp:
340): float mindis = 1e20 -> HUGE_VALF. Inner loop selects nearest PQ
centroid from precomputed L2 distance table; wrong sentinel returns code 0
instead of the true argmin for any LUT distance above 1e20.
6. EnumeratedVectors::find_nn (lattice_Zn.cpp:303): distances[i] = -1e20 ->
-HUGE_VALF. Tracks maximum inner product per query; symmetric to the L2
argmin case.
7. ranklist_handle_ties (utils.cpp:279): float prev_dis = -1e38 ->
-HUGE_VALF. Used as a comparison seed for tie-group detection. The -1e38
sentinel is wrong for any actual IP distance below -1e38 (possible with
large unnormalized embeddings); -HUGE_VALF is the correct lower bound.
8-9. HeapWithBucketsGenericCMaxFloat and HeapWithBucketsCMaxFloat bucket
initialization (approx_topk/generic.h:75, approx_topk/simdlib256-inl.h:
54): std::numeric_limits<float>::max() (FLT_MAX = 3.4e38) -> HUGE_VALF
(+Inf). Empty buckets initialized with FLT_MAX instead of HUGE_VALF pass
the CMax::cmp2 gate (HUGE_VALF > FLT_MAX) and get pushed to the heap as
spurious (FLT_MAX, 0) entries when k exceeds the number of filled buckets.
HUGE_VALF is not less than itself so the gate rejects empty buckets.
All other sentinel initializations in faiss already use HUGE_VALF. The 1e20
value fails for any distance in [1e20, +Inf), which can occur with unnormalized
high-dimensional embeddings (d=2048, L2 norms near sqrt(2048) * large_value).
Differential Revision: D110878686
Summary:
D110092203 fixed the same flakiness in test_flat_panorama.py (36% flake rate
on AVX512_SPR dispatch). The root cause — brittle overlap_rate ID comparison
that breaks when SIMD dispatch changes float reduction order and reorders
near-tied neighbors — is identical in test_ivf_flat_panorama.py and
test_refine_panorama.py, which both use for_all_simd_levels.
Replace assert_search_results_equal in both files:
- Remove overlap_rate = np.mean(I_regular == I_panorama) and assertGreater
- Add check_ref_knn_with_draws for tie-aware set comparison
- Relax atol 1e-7 -> 1e-4 (consistent with the flat_panorama fix)
- Pass D_regular as both distance arrays to check_ref_knn_with_draws;
distances already verified by assert_allclose(atol=atol), and using
D_regular for tie-grouping avoids atol=0 failures on near-zero L2
distances (e.g., test_ratio_dims_scanned exact-match scenario)
assert_range_results_equal in test_ivf_flat_panorama.py is left unchanged;
IVF range search has inherently approximate recall (nprobe), so the
intersection-based overlap check there is intentionally looser.
Differential Revision: D111028010
Contributor
|
@alibeklfc has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111028010. |
Contributor
|
This pull request has been merged in ba6af9f. |
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:
D110092203 fixed the same flakiness in test_flat_panorama.py (36% flake rate
on AVX512_SPR dispatch). The root cause — brittle overlap_rate ID comparison
that breaks when SIMD dispatch changes float reduction order and reorders
near-tied neighbors — is identical in test_ivf_flat_panorama.py and
test_refine_panorama.py, which both use for_all_simd_levels.
Replace assert_search_results_equal in both files:
distances already verified by assert_allclose(atol=atol), and using
D_regular for tie-grouping avoids atol=0 failures on near-zero L2
distances (e.g., test_ratio_dims_scanned exact-match scenario)
assert_range_results_equal in test_ivf_flat_panorama.py is left unchanged;
IVF range search has inherently approximate recall (nprobe), so the
intersection-based overlap check there is intentionally looser.
Differential Revision: D111028010