facebookresearch/faiss
1
0
Fork 0
mirror of https://github.com/facebookresearch/faiss.git synced 2026-10-11 22:50:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix panorama tests: pass D_panorama (not D_regular) to check_ref_knn_with_draws (#5430)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5430

`assert_search_results_equal` in the panorama tests called `check_ref_knn_with_draws(D_regular, I_regular, D_regular, I_panorama, rtol=rtol)`, passing `D_regular` as the 3rd positional argument. That parameter is `Dnew` (the distances compared against the reference `Dref`), so `D_panorama` was intended — the regular/panorama pairing is broken for that argument.

Effect: inside `check_ref_knn_with_draws`, `np.testing.assert_allclose(Dref, Dnew)` degenerated to `assert_allclose(D_regular, D_regular)`, a vacuous self-comparison, and the distance-tie bucketing used only `D_regular` instead of the joint regular/panorama distances. The test still passed overall because the helper already asserts `assert_allclose(D_regular, D_panorama, rtol, atol)` on the preceding line (stricter) and the id comparison (`I_regular` vs `I_panorama`) was correct — so this is a low-severity fix that restores the intended internal check.

Fix: change the 3rd argument from `D_regular` to `D_panorama` at both call sites: `tests/test_refine_panorama.py` and `tests/test_ivf_flat_panorama.py`.

Surfaced by an adversarial verification pass over the faiss `black -l 80` reformat; the bug is pre-existing and unrelated to formatting, so it is a separate diff.

___

Reviewed By: mnorris11

Differential Revision: D112191720

fbshipit-source-id: a49f18eb350f8fe51e77facc5c2b67e1bb2a696e
This commit is contained in:
Tia Lim
2026-07-16 12:09:45 -07:00
committed by meta-codesync[bot]
parent a4b4c0f2de
commit 7cd1ae906c
3 changed files with 5 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
contrib/evaluation.py
+3 -3
View File
@@ -244,10 +244,10 @@ def _cluster_tables_with_tolerance(tab1, tab2, thr):
return idx1, idx2
def check_ref_knn_with_draws(Dref, Iref, Dnew, Inew, rtol=1e-5):
def check_ref_knn_with_draws(Dref, Iref, Dnew, Inew, rtol=1e-5, atol=0):
"""test that knn search results are identical, with possible ties.
Raise if not."""
np.testing.assert_allclose(Dref, Dnew, rtol=rtol)
np.testing.assert_allclose(Dref, Dnew, rtol=rtol, atol=atol)
# here we have to be careful because of draws
testcase = unittest.TestCase() # because it makes nice error messages
for i in range(len(Iref)):
@@ -255,7 +255,7 @@ def check_ref_knn_with_draws(Dref, Iref, Dnew, Inew, rtol=1e-5):
continue
# otherwise collect elements per distance
r = rtol * Dref[i].max()
r = rtol * Dref[i].max() + atol
DrefC, DnewC = _cluster_tables_with_tolerance(Dref[i], Dnew[i], r)
tests/test_ivf_flat_panorama.py
+1 -1
View File
@@ -105,7 +105,7 @@ class TestIndexIVFFlatPanorama(unittest.TestCase):
err_msg="Distances mismatch",
)
check_ref_knn_with_draws(
D_regular, I_regular, D_regular, I_panorama, rtol=rtol
D_regular, I_regular, D_panorama, I_panorama, rtol=rtol, atol=atol
)
def assert_range_results_equal(
tests/test_refine_panorama.py
+1 -1
View File
@@ -91,7 +91,7 @@ class TestIndexRefinePanorama(unittest.TestCase):
err_msg="Distances mismatch",
)
check_ref_knn_with_draws(
D_regular, I_regular, D_regular, I_panorama, rtol=rtol
D_regular, I_regular, D_panorama, I_panorama, rtol=rtol, atol=atol
)
def test_refine_panorama_correctness(self):