From e974c5d87f9c044ba098e294743575864fe22eb9 Mon Sep 17 00:00:00 2001 From: Liyun Xiu Date: Fri, 4 Sep 2026 05:54:20 +0000 Subject: [PATCH] Sync InvertedIndex's vector count when a CSR is mapped InvertedIndex overrides num_vectors() with a member that only add() and build() maintain. read_mcsr, the kMmap path, populates vectors_ without going through add(), so a mapped build left the count at 0. IDMapIndex::read_csr_and_ids cross-checks that count against the id file's as soon as the delegate's read_csr returns -- before build() runs -- so every id map over an inverted delegate was rejected with "id map count (N) does not match the CSR vector count (0)", making the mmapped CSR build path unusable for the unquantized inverted index. Override read_csr to sync the count after the base read, as DiskSeismicIndexBase already does for the same reason. No perf claim: the added work is one assignment per index build. Signed-off-by: Liyun Xiu --- nsparse/inverted_index.h | 17 +++++++++++++++ tests/id_map_index_test.cpp | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/nsparse/inverted_index.h b/nsparse/inverted_index.h index b114292..10d08ad 100644 --- a/nsparse/inverted_index.h +++ b/nsparse/inverted_index.h @@ -34,6 +34,23 @@ class InvertedIndex : public MmapIndex, public IndexIO { const float* values) override; void build() override; size_t num_vectors() const override { return num_vectors_; } + + // read_mcsr (the kMmap path) borrows vectors_ from the mapping without + // going through add(), which is otherwise the only thing besides build() + // that maintains num_vectors_. IDMapIndex::read_csr_and_ids cross-checks + // this count against the id file's as soon as the delegate's read_csr + // returns -- before build() runs -- so a mapped build reported 0 vectors + // and every id map was rejected. Sync it here, as DiskSeismicIndexBase + // does. (kInMemory routes through add(), which already set it, so + // re-reading get_vectors() is then a harmless no-op.) + void read_csr(const char* file_path, + Residency residency = Residency::kInMemory) override { + MmapIndex::read_csr(file_path, residency); + const auto* vectors = get_vectors(); + if (vectors != nullptr) { + num_vectors_ = vectors->num_vectors(); + } + } std::array id() const override { return name; } static constexpr std::array name = {'I', 'N', 'V', 'T'}; // Bump whenever write_index's payload layout changes. diff --git a/tests/id_map_index_test.cpp b/tests/id_map_index_test.cpp index 1014e56..2ab3930 100644 --- a/tests/id_map_index_test.cpp +++ b/tests/id_map_index_test.cpp @@ -565,6 +565,49 @@ TEST(IDMapReadCsrAndId, MatchesAddWithIdsBuildQuantized) { } } +// read_csr_and_ids over an UNQUANTIZED, non-seismic delegate. InvertedIndex +// overrides num_vectors() with a member that only add() and build() maintain, +// so the mapped path -- which populates vectors_ without going through add() -- +// left it at 0 and read_csr_and_ids rejected every id map with a count +// mismatch. The build is bit-exact to add_with_ids over the same corpus. +TEST(IDMapReadCsrAndId, MatchesAddWithIdsBuildInverted) { + const Corpus corpus = make_corpus(300, kDim, /*seed=*/1); + const Corpus queries = make_corpus(20, kDim, /*seed=*/2); + const std::vector ids = make_external_ids(corpus.n); + constexpr int k = 10; + + // Reference: add_with_ids() then build(). + nsparse::IDMapIndex added(new nsparse::InvertedIndex(kDim)); + added.add_with_ids(corpus.n, corpus.indptr.data(), corpus.indices.data(), + corpus.values.data(), ids.data()); + added.build(); + const auto expected = search_corpus(added, queries, k); + + // Under test: the same corpus borrowed from a mapped native CSR. Values + // stay float here, unlike the quantized case -- an inverted index reports + // the default 4-byte code_element_size and searches over floats. + nsparse::csr_test::TempCsrFiles csr("nsparse_idmap_inverted_src"); + nsparse::csr_test::write_interchange_csr(csr.interchange(), corpus, kDim); + nsparse::csr_layout::convert(csr.interchange(), csr.native()); + TempIdFile idfile("nsparse_idmap_inverted_src.ids"); + nsparse::csr_test::write_id_map_file(idfile.path(), ids); + + nsparse::IDMapIndex mapped(new nsparse::InvertedIndex(kDim)); + mapped.read_csr_and_ids(csr.native().c_str(), idfile.path().c_str(), + nsparse::Residency::kMmap); + // The count read_csr_and_ids checks the id file against, and what used to + // be 0 here. + ASSERT_EQ(mapped.num_vectors(), static_cast(corpus.n)); + mapped.build(); + const auto got = search_corpus(mapped, queries, k); + + EXPECT_EQ(got.first, expected.first) << "external-id labels differ"; + ASSERT_EQ(got.second.size(), expected.second.size()); + for (size_t i = 0; i < got.second.size(); ++i) { + EXPECT_FLOAT_EQ(got.second[i], expected.second[i]) << "score at " << i; + } +} + // The id map is row-aligned with the CSR, so a count that disagrees with the // delegate's vector count is rejected. TEST(IDMapReadCsrAndId, CountMismatchThrows) {