diff --git a/.github/actions/check-files/action.yml b/.github/actions/check-files/action.yml index a4f1e61..303191a 100644 --- a/.github/actions/check-files/action.yml +++ b/.github/actions/check-files/action.yml @@ -24,7 +24,7 @@ runs: echo "combined_files=$COMBINED_FILES" >> $GITHUB_OUTPUT - name: Get changed files id: changed-files - uses: tj-actions/changed-files@v47.0.0 + uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0 with: files: ${{ steps.combine-files.outputs.combined_files }} files_separator: "," diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 7cba0d5..db13712 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -26,7 +26,7 @@ jobs: Get-CI-Image-Tag: needs: check-files if: needs.check-files.outputs.RUN_BUILD_AND_TEST == 'true' - uses: opensearch-project/opensearch-build/.github/workflows/get-ci-image-tag.yml@c2498b758c08fb7bc48476509a5fc1b8dd5f7493 # main + uses: opensearch-project/opensearch-build/.github/workflows/get-ci-image-tag.yml@6631dc102f1e64c182eccfc48a319a87bc9079f6 # main with: product: opensearch diff --git a/nsparse/inverted_index.cpp b/nsparse/inverted_index.cpp index 7813bca..806e837 100644 --- a/nsparse/inverted_index.cpp +++ b/nsparse/inverted_index.cpp @@ -135,6 +135,10 @@ void score_essential_terms(std::vector& scorers, if (doc >= window_end) { break; } + if (doc < window_base) { + ++scorer.current_index; + continue; + } int slot = doc - window_base; bitmap[slot >> 6] |= (1ULL << (slot & 63)); window_scores[slot] += diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2f58059..2d59fa7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -15,6 +15,7 @@ set(NSPARSE_TEST_SRC index_factory_test.cpp index_io_test.cpp inverted_index_test.cpp + inverted_index_recall_test.cpp inverted_list_clusters_test.cpp inverted_lists_test.cpp kmeans_utils_test.cpp diff --git a/tests/inverted_index_recall_test.cpp b/tests/inverted_index_recall_test.cpp new file mode 100644 index 0000000..80f0529 --- /dev/null +++ b/tests/inverted_index_recall_test.cpp @@ -0,0 +1,266 @@ +/** + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "nsparse/inverted_index.h" + +#include + +#include +#include +#include +#include +#include +#include + +#include "nsparse/types.h" + +namespace nsparse { +namespace { + +// Brute-force top-k: compute dot product for all docs and return top-k doc ids. +std::vector brute_force_top_k( + const std::vector>>& docs, + const std::vector>& query, int k) { + std::vector> scores; + for (idx_t d = 0; d < static_cast(docs.size()); ++d) { + float score = 0.0F; + // Build a quick lookup for doc terms. + std::map doc_map; + for (auto& [t, v] : docs[d]) { + doc_map[t] = v; + } + for (auto& [qt, qv] : query) { + auto it = doc_map.find(qt); + if (it != doc_map.end()) { + score += qv * it->second; + } + } + if (score > 0.0F) { + scores.emplace_back(score, d); + } + } + // Sort descending by score, then ascending by doc_id for tie-breaking. + std::sort(scores.begin(), scores.end(), + [](const auto& a, const auto& b) { + if (a.first != b.first) return a.first > b.first; + return a.second < b.second; + }); + std::vector result; + for (int i = 0; i < k && i < static_cast(scores.size()); ++i) { + result.push_back(scores[i].second); + } + return result; +} + +// Generate random sparse documents. +struct RandomSparseData { + int dim; + int n_docs; + int n_queries; + int avg_nnz; + std::vector>> docs; + std::vector>> queries; +}; + +RandomSparseData generate_random_data(int dim, int n_docs, int n_queries, + int avg_nnz, unsigned seed = 42) { + std::mt19937 rng(seed); + std::uniform_int_distribution nnz_dist( + std::max(1, avg_nnz / 2), avg_nnz * 2); + std::uniform_int_distribution term_dist(0, dim - 1); + std::uniform_real_distribution val_dist(0.01F, 2.0F); + + RandomSparseData data; + data.dim = dim; + data.n_docs = n_docs; + data.n_queries = n_queries; + data.avg_nnz = avg_nnz; + + // Generate documents. + for (int d = 0; d < n_docs; ++d) { + int nnz = nnz_dist(rng); + std::set used_terms; + std::vector> doc; + for (int j = 0; j < nnz; ++j) { + term_t t = static_cast(term_dist(rng)); + if (used_terms.insert(t).second) { + doc.emplace_back(t, val_dist(rng)); + } + } + // Sort by term for consistent ordering. + std::sort(doc.begin(), doc.end()); + data.docs.push_back(std::move(doc)); + } + + // Generate queries. + for (int q = 0; q < n_queries; ++q) { + int nnz = nnz_dist(rng); + std::set used_terms; + std::vector> query; + for (int j = 0; j < nnz; ++j) { + term_t t = static_cast(term_dist(rng)); + if (used_terms.insert(t).second) { + query.emplace_back(t, val_dist(rng)); + } + } + std::sort(query.begin(), query.end()); + data.queries.push_back(std::move(query)); + } + + return data; +} + +void add_docs_from_data( + InvertedIndex& index, + const std::vector>>& docs) { + std::vector indptr; + std::vector indices; + std::vector values; + + indptr.push_back(0); + for (const auto& doc : docs) { + for (const auto& [term, value] : doc) { + indices.push_back(term); + values.push_back(value); + } + indptr.push_back(static_cast(indices.size())); + } + + index.add(static_cast(docs.size()), indptr.data(), indices.data(), + values.data()); +} + +float compute_recall(const std::vector& predicted, + const std::vector& ground_truth) { + std::set gt_set(ground_truth.begin(), ground_truth.end()); + int hits = 0; + for (auto id : predicted) { + if (id != INVALID_IDX && gt_set.count(id)) { + ++hits; + } + } + return ground_truth.empty() + ? 1.0F + : static_cast(hits) / static_cast(gt_set.size()); +} + +// Test recall with various dataset sizes and k values. +class InvertedIndexRecallTest + : public ::testing::TestWithParam> {}; + +TEST_P(InvertedIndexRecallTest, recall_should_be_perfect) { + auto [n_docs, dim, avg_nnz, k] = GetParam(); + int n_queries = 50; + + auto data = generate_random_data(dim, n_docs, n_queries, avg_nnz); + + InvertedIndex index(dim); + add_docs_from_data(index, data.docs); + index.build(); + Index* idx = &index; + + float total_recall = 0.0F; + int n_valid_queries = 0; + + for (int q = 0; q < n_queries; ++q) { + const auto& query = data.queries[q]; + if (query.empty()) continue; + + // Build query CSR. + std::vector q_indptr = {0, + static_cast(query.size())}; + std::vector q_indices; + std::vector q_values; + for (auto& [t, v] : query) { + q_indices.push_back(t); + q_values.push_back(v); + } + + std::vector labels(k, INVALID_IDX); + std::vector distances(k, -1.0F); + + idx->search(1, q_indptr.data(), q_indices.data(), q_values.data(), k, + distances.data(), labels.data()); + + // Brute-force ground truth. + auto gt = brute_force_top_k(data.docs, query, k); + + float recall = compute_recall( + std::vector(labels.begin(), labels.end()), gt); + + total_recall += recall; + ++n_valid_queries; + + // Print failing queries for debugging. + if (recall < 1.0F) { + std::cout << "Query " << q << ": recall=" << recall + << " predicted=["; + for (int i = 0; i < k; ++i) { + if (i > 0) std::cout << ","; + std::cout << labels[i]; + } + std::cout << "] gt=["; + for (size_t i = 0; i < gt.size(); ++i) { + if (i > 0) std::cout << ","; + std::cout << gt[i]; + } + std::cout << "]" << std::endl; + + // Print scores for debugging. + std::cout << " predicted_scores=["; + for (int i = 0; i < k; ++i) { + if (i > 0) std::cout << ","; + std::cout << distances[i]; + } + std::cout << "]" << std::endl; + + // Print ground truth scores. + std::cout << " gt_scores=["; + for (size_t i = 0; i < gt.size(); ++i) { + if (i > 0) std::cout << ","; + // Compute score for gt[i]. + float score = 0.0F; + std::map doc_map; + for (auto& [t, v] : data.docs[gt[i]]) doc_map[t] = v; + for (auto& [qt, qv] : query) { + auto it = doc_map.find(qt); + if (it != doc_map.end()) score += qv * it->second; + } + std::cout << score; + } + std::cout << "]" << std::endl; + } + } + + float avg_recall = + n_valid_queries > 0 ? total_recall / n_valid_queries : 1.0F; + std::cout << "Average recall: " << avg_recall << " (n_docs=" << n_docs + << ", dim=" << dim << ", avg_nnz=" << avg_nnz << ", k=" << k + << ")" << std::endl; + + // InvertedIndex should have PERFECT recall (it's an exact algorithm). + EXPECT_FLOAT_EQ(avg_recall, 1.0F) + << "InvertedIndex recall should be 1.0 but got " << avg_recall; +} + +INSTANTIATE_TEST_SUITE_P( + RecallTests, InvertedIndexRecallTest, + ::testing::Values( + // (n_docs, dim, avg_nnz, k) + // Small: single window, no pruning. + std::make_tuple(100, 500, 10, 10), + // Medium: multiple windows, pruning active. + std::make_tuple(5000, 1000, 20, 10), + // Large: many windows, aggressive pruning. + std::make_tuple(10000, 2000, 30, 10), + // Large with higher k. + std::make_tuple(10000, 2000, 30, 50), + // Dense queries, many terms. + std::make_tuple(5000, 500, 50, 10), + // Very sparse queries and docs. + std::make_tuple(10000, 5000, 5, 10))); + +} // namespace +} // namespace nsparse diff --git a/tests/inverted_index_test.cpp b/tests/inverted_index_test.cpp index 427d9d2..95f379d 100644 --- a/tests/inverted_index_test.cpp +++ b/tests/inverted_index_test.cpp @@ -411,5 +411,58 @@ TEST(InvertedIndexIO, roundtrip_preserves_fourcc) { delete loaded; } +// Regression test: score_essential_terms must skip docs before window_base. +// When scorers span multiple windows (>4096 docs) and re-partitioning changes +// first_essential, a newly-essential scorer may have its cursor pointing at +// docs before the current window. Without the bounds check, this produces a +// negative slot index causing SIGBUS / out-of-bounds write. +TEST(InvertedIndexSearch, search_multi_window_no_crash) { + // Use 2 terms. term0 has docs in early range, term1 has docs spread across + // multiple windows (>4096). This forces the BMW algorithm to process + // multiple scoring windows and potentially re-partition essential terms. + constexpr int kDim = 2; + constexpr int kNumDocs = 6000; // > kScoreWindowSize (4096) + InvertedIndex index(kDim); + + // Build docs: every doc has term0, every 3rd doc also has term1. + // This creates different posting list lengths that affect max_score + // partitioning. + std::vector> docs; + docs.reserve(kNumDocs); + for (int i = 0; i < kNumDocs; ++i) { + std::map doc; + doc[0] = 0.1F + static_cast(i % 10) * 0.01F; + if (i % 3 == 0) { + doc[1] = 0.5F + static_cast(i % 7) * 0.05F; + } + docs.push_back(doc); + } + add_docs(index, docs); + index.build(); + + // Query both terms — forces multi-term scoring across multiple windows. + std::vector query_indptr = {0, 2}; + std::vector query_indices = {0, 1}; + std::vector query_values = {1.0F, 1.0F}; + + constexpr int kTopK = 10; + std::vector labels(kTopK, -1); + std::vector distances(kTopK, -1.0F); + + Index* idx = &index; + // The main assertion is that this does not crash (SIGBUS). + idx->search(1, query_indptr.data(), query_indices.data(), + query_values.data(), kTopK, distances.data(), labels.data()); + + // Verify we got valid results. + EXPECT_GE(labels[0], 0); + EXPECT_GT(distances[0], 0.0F); + + // Results should be sorted by score descending. + for (int i = 1; i < kTopK; ++i) { + EXPECT_GE(distances[i - 1], distances[i]); + } +} + } // namespace } // namespace nsparse