From bec2d2c6019c3e687bbb0bf56bec4d12ab39f184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mihai=20Capot=C4=83?= Date: Thu, 22 Oct 2026 16:35:31 -0700 Subject: [PATCH] Fix find_medioid returning an element that fails the predicate When no element is closer to the medioid than the sentinel, `utils::find_medioid` now returns the first element that satisfies the predicate, or throws if none does, instead of returning element 0. New "Find Medioid with Extreme Values" and "Dynamic Vamana Index Compact After Far-Away Point" tests fail without the fix and pass with it. Future work: guard against calling `consolidate` or `compact` on an empty index. --- include/svs/core/medioid.h | 15 +++++++- tests/svs/core/medioid.cpp | 44 ++++++++++++++++++++++ tests/svs/index/vamana/dynamic_index_2.cpp | 44 ++++++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) diff --git a/include/svs/core/medioid.h b/include/svs/core/medioid.h index 0acf49c98..8bd95786e 100644 --- a/include/svs/core/medioid.h +++ b/include/svs/core/medioid.h @@ -18,6 +18,7 @@ #include "svs/concepts/data.h" #include "svs/lib/array.h" +#include "svs/lib/exception.h" #include "svs/lib/misc.h" #include "svs/lib/neighbor.h" #include "svs/lib/threads.h" @@ -341,8 +342,20 @@ size_t find_medioid( } }); + // When every distance is beyond float range, Infinity, or NaN, nothing beats the + // sentinel. Callers use the result as a graph entry point, so it must still satisfy + // the predicate: a deleted entry point breaks consolidation and compaction of dynamic + // indexes. if (global_min.id() >= data.size()) { - global_min = Neighbor(0, global_min.distance()); + for (size_t i = 0, imax = data.size(); i < imax; ++i) { + if (predicate(i)) { + return i; + } + } + throw ANNEXCEPTION( + "Cannot find a medioid: none of the {} elements satisfies the predicate.", + data.size() + ); } return global_min.id(); diff --git a/tests/svs/core/medioid.cpp b/tests/svs/core/medioid.cpp index 79a16ab69..76d51a3f3 100644 --- a/tests/svs/core/medioid.cpp +++ b/tests/svs/core/medioid.cpp @@ -25,6 +25,11 @@ // catch2 #include "catch2/catch_test_macros.hpp" +#include "catch2/matchers/catch_matchers_string.hpp" + +// stl +#include +#include ///// helpers namespace { @@ -207,3 +212,42 @@ CATCH_TEST_CASE("Testing Medioid Computation", "[core][medioid]") { )); } } + +CATCH_TEST_CASE("Find Medioid with Extreme Values", "[core][medioid]") { + // Element 0 is excluded by the predicate in every section: it stands in for the + // deleted entry point of a dynamic index, which must never be returned. + auto skip_first = [](const size_t i) { return i != 0; }; + svs::threads::DefaultThreadPool threadpool(2); + + CATCH_SECTION("Squared distances beyond float range") { + // One element at 3e38 moves the medioid to about 3.3e37, so every squared + // distance to it exceeds the float range and nothing beats the sentinel. The + // first element that satisfies the predicate is returned. + auto data = svs::data::SimpleData(10, 2); + for (size_t i = 0; i < 9; ++i) { + data.set_datum(i, std::vector{float(i), float(i)}); + } + data.set_datum(9, std::vector{3e38f, 3e38f}); + CATCH_REQUIRE(svs::utils::find_medioid(data, threadpool, skip_first) == 1); + } + + CATCH_SECTION("No finite distance") { + auto data = svs::data::SimpleData(4, 2); + auto nan = std::numeric_limits::quiet_NaN(); + for (size_t i = 0; i < data.size(); ++i) { + data.set_datum(i, std::vector{nan, nan}); + } + // Nothing compares smaller than the sentinel, so the first element that + // satisfies the predicate is returned. + CATCH_REQUIRE(svs::utils::find_medioid(data, threadpool, skip_first) == 1); + } + + CATCH_SECTION("No element satisfies the predicate") { + auto data = svs::data::SimpleData(4, 2); + auto none = [](const size_t) { return false; }; + CATCH_REQUIRE_THROWS_WITH( + svs::utils::find_medioid(data, threadpool, none), + Catch::Matchers::ContainsSubstring("none of the 4 elements") + ); + } +} diff --git a/tests/svs/index/vamana/dynamic_index_2.cpp b/tests/svs/index/vamana/dynamic_index_2.cpp index e590ae547..2daeda29e 100644 --- a/tests/svs/index/vamana/dynamic_index_2.cpp +++ b/tests/svs/index/vamana/dynamic_index_2.cpp @@ -675,3 +675,47 @@ CATCH_TEST_CASE("Dynamic Vamana Index Default Parameters", "[parameter][vamana]" ); } } + +CATCH_TEST_CASE( + "Dynamic Vamana Index Compact After Far-Away Point", "[graph_index][dynamic_index]" +) { + // A point whose squared distances overflow float made consolidate pick a deleted + // point as the new entry point, and compact then failed with "Couldn't find key." + const size_t dims = 8; + const size_t num_points = 500; + std::mt19937 rng(1234); + std::uniform_real_distribution uniform(0, 1); + auto data = svs::data::BlockedData(num_points, dims); + std::vector datum(dims); + for (size_t i = 0; i < num_points; ++i) { + std::generate(datum.begin(), datum.end(), [&]() { return uniform(rng); }); + data.set_datum(i, datum); + } + std::vector ids(num_points); + std::iota(ids.begin(), ids.end(), 0); + + svs::index::vamana::VamanaBuildParameters parameters{1.2, 32, 64, 1000, 28, true}; + auto index = svs::index::vamana::MutableVamanaIndex( + parameters, std::move(data), ids, Distance(), 2 + ); + + auto far_point = svs::data::SimpleData(1, dims); + far_point.set_datum(0, std::vector(dims, 3e38f)); + index.add_points(far_point, std::vector{num_points}); + + // Delete the entry point, so consolidate must choose a new one, and point 0, which + // consolidate used to fall back to when no distance was below the float sentinel. + size_t entry_point = index.translate_internal_id(index.entry_point()); + std::vector to_delete{entry_point}; + if (entry_point != 0) { + to_delete.push_back(0); + } + index.delete_entries(to_delete); + index.consolidate(); + CATCH_REQUIRE(!index.is_deleted(index.entry_point())); + + index.compact(); + index.debug_check_invariants(false); + CATCH_REQUIRE(index.size() == num_points + 1 - to_delete.size()); + CATCH_REQUIRE(index.has_id(num_points)); +}