From 6c117e08db33e57e9cc49e693721368e8c27f1d2 Mon Sep 17 00:00:00 2001 From: sophisid Date: Sun, 11 Oct 2026 22:01:19 +0200 Subject: [PATCH] Revert "Fix MESSI top-k returning duplicate series" This reverts commit 3871490. Simple CI (ctest) has failed on every PR since this commit; reverting until the failing test is identified. The fix is being reworked in #77. Co-Authored-By: Claude Sonnet 5.5 --- lib/algos/Messi.cpp | 8 +-- tests/CMakeLists.txt | 23 --------- tests/test_Messi_TopK_Distinct.cpp | 79 ------------------------------ 3 files changed, 4 insertions(+), 106 deletions(-) delete mode 100644 tests/test_Messi_TopK_Distinct.cpp diff --git a/lib/algos/Messi.cpp b/lib/algos/Messi.cpp index 10f9f0c..14e7cd3 100644 --- a/lib/algos/Messi.cpp +++ b/lib/algos/Messi.cpp @@ -137,7 +137,7 @@ namespace daisy pos = *node->buffer->full_position_buffer[i] / index->settings->timeseries_size; } pthread_rwlock_wrlock(lock_queue); - pqueue_bsf_insert_no_dup_position(pq_bsf, dist, static_cast(pos), node); + pqueue_bsf_insert(pq_bsf, dist, static_cast(pos), node); pthread_rwlock_unlock(lock_queue); } } @@ -156,7 +156,7 @@ namespace daisy pos = *node->buffer->tmp_full_position_buffer[i] / index->settings->timeseries_size; } pthread_rwlock_wrlock(lock_queue); - pqueue_bsf_insert_no_dup_position(pq_bsf, dist, static_cast(pos), node); + pqueue_bsf_insert(pq_bsf, dist, static_cast(pos), node); pthread_rwlock_unlock(lock_queue); } } @@ -178,7 +178,7 @@ namespace daisy { pthread_rwlock_wrlock(lock_queue); - pqueue_bsf_insert_no_dup_position(pq_bsf, dist, *node->buffer->partial_position_buffer[i] / index->settings->timeseries_size, node); + pqueue_bsf_insert(pq_bsf, dist, *node->buffer->partial_position_buffer[i] / index->settings->timeseries_size, node); pthread_rwlock_unlock(lock_queue); } @@ -229,7 +229,7 @@ namespace daisy if (dist <= pq_bsf->knn[pq_bsf->k - 1]) { pthread_rwlock_wrlock(lock_queue); - pqueue_bsf_insert_no_dup_position(pq_bsf, dist, *node->buffer->partial_position_buffer[i] / index->settings->timeseries_size, node); + pqueue_bsf_insert(pq_bsf, dist, *node->buffer->partial_position_buffer[i] / index->settings->timeseries_size, node); pthread_rwlock_unlock(lock_queue); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6167f04..8dad2f7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -370,29 +370,6 @@ gtest_discover_tests(test_LbBruteforce_Streaming WORKING_DIRECTORY ${CMAKE_SOURC message(STATUS "Tests discovered for test_Messi_L2Square.") endif() -# ////// MESSI top-k distinct results ////// -add_executable( - test_Messi_TopK_Distinct - test_Messi_TopK_Distinct.cpp -) -target_link_libraries( - test_Messi_TopK_Distinct - PRIVATE - GTest::gtest_main - dino_lib - commons_lib - stdc++fs -) -target_include_directories(test_Messi_TopK_Distinct - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR}/../lib - ${CMAKE_CURRENT_SOURCE_DIR}/../commons -) -gtest_discover_tests( - test_Messi_TopK_Distinct - WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} -) - # ////// MESSI streaming ////// add_executable( test_Messi_Streaming diff --git a/tests/test_Messi_TopK_Distinct.cpp b/tests/test_Messi_TopK_Distinct.cpp deleted file mode 100644 index a85f2c7..0000000 --- a/tests/test_Messi_TopK_Distinct.cpp +++ /dev/null @@ -1,79 +0,0 @@ -#include - -#include -#include - -#include -#include -#include -#include - -// Regression test: MESSI's approximate phase scans one leaf to seed the -// best-so-far queue, then the parallel phase scans every leaf again. Without -// de-duplication by position the seeded leaf's series were inserted twice, so -// the k-NN list contained the same series several times and displaced real -// neighbours. -namespace -{ - constexpr daisy::idx_t kRows = 20000; - constexpr daisy::idx_t kDim = 128; - constexpr daisy::idx_t kQueries = 50; - constexpr daisy::idx_t kK = 10; - - // 50 Gaussian clusters with isotropic noise: dense neighbourhoods, so the - // leaf scanned by the approximate phase holds members of the final top-k. - std::vector clusteredData(daisy::idx_t rows, unsigned seed) - { - std::mt19937 centre_rng(42); - std::normal_distribution normal; - std::vector centres(50 * kDim); - for (auto &v : centres) - v = 6.0F * normal(centre_rng); - std::mt19937 rng(seed); - std::uniform_int_distribution pick(0, 49); - std::vector data(static_cast(rows) * kDim); - for (daisy::idx_t r = 0; r < rows; ++r) - { - const float *centre = centres.data() + pick(rng) * kDim; - for (daisy::idx_t d = 0; d < kDim; ++d) - data[static_cast(r) * kDim + d] = centre[d] + normal(rng); - } - return data; - } -} - -TEST(MessiTopKDistinctTest, ReturnsDistinctIdsAndMatchesBruteForce) -{ - auto database = clusteredData(kRows, 1); - auto queries = clusteredData(kQueries, 2); - - daisy::BruteForceSearch brute(daisy::DistanceType::L2_SQUARED); - brute.buildIndex(database.data(), kRows, kDim); - std::vector expected_ids(kQueries * kK); - std::vector expected_distances(kQueries * kK); - brute.searchIndex(queries.data(), kQueries, kK, expected_ids.data(), - expected_distances.data()); - - daisy::MessiConfig config; - config.search_workers = 4; - config.index_workers = 2; - daisy::Messi messi(daisy::DistanceType::L2_SQUARED, config); - messi.setNormalized(false); - messi.buildIndex(database.data(), kRows, kDim); - std::vector ids(kQueries * kK); - std::vector distances(kQueries * kK); - messi.searchIndex(queries.data(), kQueries, kK, ids.data(), distances.data()); - - size_t found = 0; - for (daisy::idx_t q = 0; q < kQueries; ++q) - { - std::set returned(ids.begin() + q * kK, ids.begin() + (q + 1) * kK); - EXPECT_EQ(returned.size(), static_cast(kK)) - << "query " << q << " returned duplicate ids"; - EXPECT_EQ(ids[q * kK], expected_ids[q * kK]) << "nearest neighbour differs, query " << q; - for (daisy::idx_t j = 0; j < kK; ++j) - found += returned.count(expected_ids[q * kK + j]); - } - const double recall = static_cast(found) / (kQueries * kK); - EXPECT_GE(recall, 0.99) << "MESSI top-" << kK << " recall vs brute force"; -}