From af404c19386696be26255d654c67d7f63f3c77c2 Mon Sep 17 00:00:00 2001 From: Zirui Song Date: Thu, 3 Sep 2026 08:09:50 +0000 Subject: [PATCH 1/2] Build the quantized indexes from an mmapped codes CSR Signed-off-by: Zirui Song --- .../disk_seismic_scalar_quantized_index.cpp | 10 --- nsparse/disk_seismic_scalar_quantized_index.h | 11 ++- nsparse/mmap_index.h | 30 +++++--- nsparse/seismic_scalar_quantized_index.cpp | 10 --- nsparse/seismic_scalar_quantized_index.h | 12 +-- nsparse/utils/csr_layout.h | 16 +++- tests/csr_interchange_test_util.h | 43 +++++++++++ ...sk_seismic_scalar_quantized_index_test.cpp | 58 ++++++++++++++ tests/disk_seismic_test_util.h | 1 + tests/id_map_index_test.cpp | 51 +++++++++++++ tests/seismic_scalar_quantized_index_test.cpp | 76 +++++++++++++++++-- 11 files changed, 267 insertions(+), 51 deletions(-) diff --git a/nsparse/disk_seismic_scalar_quantized_index.cpp b/nsparse/disk_seismic_scalar_quantized_index.cpp index 7eb4201..48040a9 100644 --- a/nsparse/disk_seismic_scalar_quantized_index.cpp +++ b/nsparse/disk_seismic_scalar_quantized_index.cpp @@ -77,16 +77,6 @@ DiskSeismicScalarQuantizedIndex::DiskSeismicScalarQuantizedIndex( SeismicClusterParameters parameter, int dim) : DiskSeismicIndexBase(dim, parameter), sq_(quantizer_type, vmin, vmax) {} -void DiskSeismicScalarQuantizedIndex::read_csr(const char* file_path, - Residency residency) { - if (residency == Residency::kMmap) { - throw std::invalid_argument( - "mmap residency is not available for a quantized index: a mapped " - "CSR is borrowed as float, and this index searches over codes"); - } - MmapIndex::read_csr(file_path, residency); -} - size_t DiskSeismicScalarQuantizedIndex::code_element_size() const { return sq_.bytes_per_value(); } diff --git a/nsparse/disk_seismic_scalar_quantized_index.h b/nsparse/disk_seismic_scalar_quantized_index.h index fd344da..7d774ab 100644 --- a/nsparse/disk_seismic_scalar_quantized_index.h +++ b/nsparse/disk_seismic_scalar_quantized_index.h @@ -68,17 +68,16 @@ class DiskSeismicScalarQuantizedIndex : public DiskSeismicIndexBase { const char* index_file, size_t pos); - // Only the copying residency. A mapped CSR is borrowed at the width it was - // written in, which is float, whereas this index searches over codes: the - // values have to pass through add() to be quantized. - void read_csr(const char* file_path, - Residency residency = Residency::kInMemory) override; - private: [[nodiscard]] uint32_t format_version() const override { return kFormatVersion; } [[nodiscard]] size_t code_element_size() const override; + // The value width read_csr(kMmap) borrows a native CSR at: this index's + // code width, so a codes CSR is borrowed in place rather than a float one. + [[nodiscard]] size_t mmap_element_size() const override { + return sq_.bytes_per_value(); + } const uint8_t* encode_values(const float* values, size_t nnz, std::vector& scratch) const override; const uint8_t* encode_query( diff --git a/nsparse/mmap_index.h b/nsparse/mmap_index.h index 2a2e117..11bba5b 100644 --- a/nsparse/mmap_index.h +++ b/nsparse/mmap_index.h @@ -72,10 +72,13 @@ class MmapIndex : public Index { // so they are always gone before the spill is released. detail::ClusteredListsSpill batch_spill_; + // The value width read_mcsr borrows a native CSR's values at. Float for the + // unquantized types (the default); a quantizing index overrides it to its + // code width so the same mapped-CSR path yields codes borrowed in place + // rather than floats. + [[nodiscard]] virtual size_t mmap_element_size() const { return U32; } + private: - // Values are borrowed at their stored width, so a quantizing index cannot - // use this path. - static constexpr size_t kMmapElementSize = U32; bool is_mmap_index_ = false; // Points at the native layout (csr_layout.h) rather than copying it. Terms @@ -114,10 +117,17 @@ class MmapIndex : public Index { file_path); } + // The value width the borrow reinterprets in place: float for the + // unquantized types, a quantizer's code width for a quantizing one. A + // file written at a different width fails the size check below, so a + // codes CSR handed to a float index (or vice versa) is rejected rather + // than misread. + const size_t element_size = mmap_element_size(); + const size_t indptr_size = static_cast(num_rows) + 1; const auto nnz_size = static_cast(nnz); if (file.size() != - csr_layout::native_file_size(indptr_size, nnz_size)) { + csr_layout::native_file_size(indptr_size, nnz_size, element_size)) { throw std::invalid_argument( std::string("CSR file is not in the native layout (convert it " "with csr_layout::convert): ") + @@ -128,17 +138,19 @@ class MmapIndex : public Index { const auto* indices = cursor.read_array(nnz_size); cursor.skip(csr_layout::native_values_offset(indptr_size, nnz_size) - cursor.pos()); - const auto* values = cursor.read_array(nnz_size); + // Borrowed as raw bytes at the value width. The values offset is padded + // to alignof(float), which satisfies any code width, so the in-place + // reinterpret downstream stays aligned. + const auto* values = cursor.read_array(nnz_size * element_size); // Validates before borrowing, so a corrupt file throws here rather // than faulting during search. auto vectors = std::make_unique(SparseVectors::map_vectors( - {.element_size = kMmapElementSize, + {.element_size = element_size, .dimension = static_cast(dimension_)}, - indptr, indptr_size, indices, nnz_size, - reinterpret_cast(values), - nnz_size * kMmapElementSize)); + indptr, indptr_size, indices, nnz_size, values, + nnz_size * element_size)); // Committed last, so a rejected file leaves the index untouched. mapped_file_ = std::move(file); diff --git a/nsparse/seismic_scalar_quantized_index.cpp b/nsparse/seismic_scalar_quantized_index.cpp index 100f702..01f4b73 100644 --- a/nsparse/seismic_scalar_quantized_index.cpp +++ b/nsparse/seismic_scalar_quantized_index.cpp @@ -146,16 +146,6 @@ SeismicScalarQuantizedIndex::SeismicScalarQuantizedIndex( sq_(quantizer_type, vmin, vmax), cluster_parameter_(parameter) {} -void SeismicScalarQuantizedIndex::read_csr(const char* file_path, - Residency residency) { - if (residency == Residency::kMmap) { - throw std::invalid_argument( - "mmap residency is not available for a quantized index: a mapped " - "CSR is borrowed as float, and this index searches over codes"); - } - MmapIndex::read_csr(file_path, residency); -} - void SeismicScalarQuantizedIndex::add(idx_t n, const idx_t* indptr, const term_t* indices, const float* values) { diff --git a/nsparse/seismic_scalar_quantized_index.h b/nsparse/seismic_scalar_quantized_index.h index c58b0ff..ff5a8d2 100644 --- a/nsparse/seismic_scalar_quantized_index.h +++ b/nsparse/seismic_scalar_quantized_index.h @@ -63,13 +63,13 @@ class SeismicScalarQuantizedIndex : public MmapIndex, public IndexIO { const char* index_file, size_t pos); - // Only the copying residency. A mapped CSR is borrowed at the width it was - // written in, which is float, whereas this index searches over codes: the - // values have to pass through add() to be quantized. - void read_csr(const char* file_path, - Residency residency = Residency::kInMemory) override; - private: + // The value width read_csr(kMmap) borrows a native CSR at: this index's + // code width, so a codes CSR is borrowed in place rather than a float one. + [[nodiscard]] size_t mmap_element_size() const override { + return sq_.bytes_per_value(); + } + // interfaces of IndexIO [[nodiscard]] uint32_t format_version() const override { return kFormatVersion; diff --git a/nsparse/utils/csr_layout.h b/nsparse/utils/csr_layout.h index 94ffc8d..baf18da 100644 --- a/nsparse/utils/csr_layout.h +++ b/nsparse/utils/csr_layout.h @@ -22,12 +22,16 @@ // int64 indptr[r + 1] idx_t indptr[r + 1] // int32 indices[nnz] term_t indices[nnz] // -// float values[nnz] float values[nnz] +// float values[nnz] value values[nnz] // // Interchange is what scipy writes and what Index::read_csr narrows while // copying. Native stores in-memory widths so MmapIndex can borrow the arrays in // place; values is padded because term_t is 2 bytes, and an odd nnz would leave -// the floats misaligned for the mapped reader's in-place reinterpret. +// the floats misaligned for the mapped reader's in-place reinterpret. The +// native value width is the index's own: 4-byte float for the unquantized +// types, or 1-/2-byte scalar codes for a quantizing one (see native_file_size's +// element_size). The pad is to alignof(float), which also satisfies the smaller +// code widths. // // File size cannot tell the layouts apart (rows=1, nnz=4 is 64 bytes either // way), so conversion is explicit and the results are kept apart by suffix. @@ -53,8 +57,12 @@ constexpr size_t native_values_offset(size_t indptr_size, size_t nnz) { return unaligned + padding(unaligned); } -constexpr size_t native_file_size(size_t indptr_size, size_t nnz) { - return native_values_offset(indptr_size, nnz) + nnz * sizeof(float); +// `element_size` is the native value width: sizeof(float) for the unquantized +// types (the default), or the quantizer's byte width (1 or 2) for a quantizing +// index whose CSR holds codes rather than floats. +constexpr size_t native_file_size(size_t indptr_size, size_t nnz, + size_t element_size = sizeof(float)) { + return native_values_offset(indptr_size, nnz) + nnz * element_size; } std::string native_path(const std::string& path); diff --git a/tests/csr_interchange_test_util.h b/tests/csr_interchange_test_util.h index 976181a..52692c3 100644 --- a/tests/csr_interchange_test_util.h +++ b/tests/csr_interchange_test_util.h @@ -14,11 +14,13 @@ #include #include #include +#include #include #include #include #include "nsparse/types.h" +#include "nsparse/utils/csr_layout.h" // Shared helpers for the mmap-CSR build path, used by both the regular and the // disk-resident index suites: write a corpus as an interchange CSR (the layout @@ -49,6 +51,47 @@ void write_interchange_csr(const std::string& path, const Corpus& c, static_cast(c.values.size() * sizeof(float))); } +// Writes a corpus of pre-quantized codes as a NATIVE CSR -- the layout read_mcsr +// borrows when a quantizing index maps it: int64 header {rows, num_cols, nnz}, +// idx_t indptr[rows+1], term_t indices[nnz], pad to alignof(float), then +// `element_size`-byte codes[nnz]. `codes` holds nnz*element_size bytes, +// row-aligned with `indices`. This is the code-width analog of +// csr_layout::convert's output, written directly since the codes path has no +// interchange form. +inline void write_native_codes_csr(const std::string& path, + const std::vector& indptr, + const std::vector& indices, + const std::vector& codes, + int64_t num_cols, size_t element_size) { + // The declared width must describe the buffer, or the file would not match + // the layout read_mcsr validates -- a mismatch here is a test-authoring bug. + if (codes.size() != indices.size() * element_size) { + throw std::invalid_argument( + "write_native_codes_csr: codes size does not match indices * " + "element_size"); + } + std::ofstream out(path, std::ios::binary); + const std::array header = { + static_cast(indptr.size()) - 1, num_cols, + static_cast(indices.size())}; + out.write(reinterpret_cast(header.data()), + header.size() * sizeof(int64_t)); + out.write(reinterpret_cast(indptr.data()), + static_cast(indptr.size() * sizeof(idx_t))); + out.write(reinterpret_cast(indices.data()), + static_cast(indices.size() * sizeof(term_t))); + const size_t values_pos = csr_layout::kHeaderBytes + + indptr.size() * sizeof(idx_t) + + indices.size() * sizeof(term_t); + const std::array pad{}; + if (const size_t pad_bytes = csr_layout::padding(values_pos); + pad_bytes > 0) { + out.write(pad.data(), static_cast(pad_bytes)); + } + out.write(reinterpret_cast(codes.data()), + static_cast(codes.size())); +} + // Writes the id-map file that IDMapIndex::read_csr_and_ids reads: // [int64 count][idx_t external_id x count]. Row-aligned with the CSR, so // external_ids[i] is the external id of CSR row i. diff --git a/tests/disk_seismic_scalar_quantized_index_test.cpp b/tests/disk_seismic_scalar_quantized_index_test.cpp index e80222c..e7d94d9 100644 --- a/tests/disk_seismic_scalar_quantized_index_test.cpp +++ b/tests/disk_seismic_scalar_quantized_index_test.cpp @@ -331,4 +331,62 @@ TEST(DiskSeismicSQIndex, FactoryCreatesIt) { QuantizerType::QT_16bit); } +// Building the disk quantized index from a native codes CSR borrowed via mmap +// must match the add()-fed build: the upstream writes codes at the quantizer's +// width, read_csr(kMmap) borrows them, and build -> persist -> mmap-reload -> +// search is bit-exact to feeding the same corpus (quantized by add()) then +// building. Both see identical codes because add() runs the same +// ScalarQuantizer::encode the test wrote. Also asserts a width-mismatched codes +// file is rejected rather than misread. +TEST(DiskSeismicSQIndex, MmapCodesCsrBuildMatchesAddBuild) { + const CSR corpus = make_corpus(1500, /*seed=*/1); + const CSR queries = make_corpus(40, /*seed=*/2); + DiskSeismicSearchParameters params(/*cut=*/25, /*k_prime=*/32); + + // Reference: float corpus fed through add() (which quantizes), reloaded. + DiskSeismicScalarQuantizedIndex added(QuantizerType::QT_8bit, 0.0F, 1.0F, + cluster_params(), kDim); + add_corpus(added, corpus); + added.build(); + TempIndexFile added_file("nsparse_dssq_addbuild.idx"); + write_index(&added, added_file.c_str()); + std::unique_ptr added_mapped( + read_index(added_file.c_str(), IndexIoFlag::kUseMmap)); + ASSERT_NE(added_mapped, nullptr); + const ScoreIds fresh = search_all(*added_mapped, queries, 10, ¶ms); + + // Under test: the same corpus pre-quantized to codes, borrowed via mmap. + const ScalarQuantizer sq(QuantizerType::QT_8bit, 0.0F, 1.0F); + const size_t element_size = sq.bytes_per_value(); + std::vector codes(corpus.indices.size() * element_size); + sq.encode(corpus.values.data(), codes.data(), corpus.indices.size()); + TempCsrFiles csr("nsparse_dssq_codes"); + write_native_codes_csr(csr.native(), corpus.indptr, corpus.indices, codes, + kDim, element_size); + + DiskSeismicScalarQuantizedIndex mapped(QuantizerType::QT_8bit, 0.0F, 1.0F, + cluster_params(), kDim); + mapped.read_csr(csr.native().c_str(), Residency::kMmap); + ASSERT_EQ(mapped.num_vectors(), static_cast(corpus.n)); + mapped.build(); + TempIndexFile built_file("nsparse_dssq_mmapbuild.idx"); + write_index(&mapped, built_file.c_str()); + std::unique_ptr built_mapped( + read_index(built_file.c_str(), IndexIoFlag::kUseMmap)); + ASSERT_NE(built_mapped, nullptr); + EXPECT_EQ(built_mapped->num_vectors(), static_cast(corpus.n)); + expect_same_results(search_all(*built_mapped, queries, 10, ¶ms), fresh); + + // A codes file whose width does not match the index's quantizer is rejected + // by the native-layout size check: 16-bit-wide values fed to an 8-bit index. + std::vector wide_codes(corpus.indices.size() * 2); + TempCsrFiles wrong("nsparse_dssq_wrongwidth"); + write_native_codes_csr(wrong.native(), corpus.indptr, corpus.indices, + wide_codes, kDim, /*element_size=*/2); + DiskSeismicScalarQuantizedIndex eight(QuantizerType::QT_8bit, 0.0F, 1.0F, + cluster_params(), kDim); + EXPECT_THROW(eight.read_csr(wrong.native().c_str(), Residency::kMmap), + std::invalid_argument); +} + } // namespace nsparse diff --git a/tests/disk_seismic_test_util.h b/tests/disk_seismic_test_util.h index 9835c9d..4d15fd1 100644 --- a/tests/disk_seismic_test_util.h +++ b/tests/disk_seismic_test_util.h @@ -87,6 +87,7 @@ inline void add_corpus(Index& index, const CSR& c) { // re-exported here so the disk suite reaches them via `disk_seismic_test`. using csr_test::TempCsrFiles; using csr_test::write_interchange_csr; +using csr_test::write_native_codes_csr; // Index file removed on destruction. write_index/read_index take char*. class TempIndexFile { diff --git a/tests/id_map_index_test.cpp b/tests/id_map_index_test.cpp index 11c121f..1014e56 100644 --- a/tests/id_map_index_test.cpp +++ b/tests/id_map_index_test.cpp @@ -25,8 +25,10 @@ #include "nsparse/io/buffered_io.h" #include "nsparse/io/index_io.h" #include "nsparse/seismic_index.h" +#include "nsparse/seismic_scalar_quantized_index.h" #include "nsparse/types.h" #include "nsparse/utils/csr_layout.h" +#include "nsparse/utils/scalar_quantizer.h" #include "tests/csr_interchange_test_util.h" namespace { @@ -514,6 +516,55 @@ TEST(IDMapReadCsrAndId, MatchesAddWithIdsBuild) { EXPECT_TRUE(saw_external) << "labels should be translated to external ids"; } +// read_csr_and_ids over a QUANTIZED delegate: since CR-2 the quantized types +// accept a mmapped (codes) CSR, so the id-map wrapper works over them too -- +// this used to throw. The upstream writes codes at the quantizer's width; the +// build is bit-exact to add_with_ids feeding the same corpus (which add() +// quantizes with the same ScalarQuantizer the test wrote). +TEST(IDMapReadCsrAndId, MatchesAddWithIdsBuildQuantized) { + 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; + auto delegate = [] { + return new nsparse::SeismicScalarQuantizedIndex( + nsparse::QuantizerType::QT_8bit, 0.0F, 1.0F, kClusterParams, kDim); + }; + + // Reference: add_with_ids() feeds floats, which the delegate quantizes. + nsparse::IDMapIndex added(delegate()); + 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 pre-quantized to codes, borrowed via mmap. + const nsparse::ScalarQuantizer sq(nsparse::QuantizerType::QT_8bit, 0.0F, + 1.0F); + const size_t element_size = sq.bytes_per_value(); + std::vector codes(corpus.indices.size() * element_size); + sq.encode(corpus.values.data(), codes.data(), corpus.indices.size()); + nsparse::csr_test::TempCsrFiles csr("nsparse_idmap_sq_src"); + nsparse::csr_test::write_native_codes_csr(csr.native(), corpus.indptr, + corpus.indices, codes, kDim, + element_size); + TempIdFile idfile("nsparse_idmap_sq_src.ids"); + nsparse::csr_test::write_id_map_file(idfile.path(), ids); + + nsparse::IDMapIndex mapped(delegate()); + mapped.read_csr_and_ids(csr.native().c_str(), idfile.path().c_str(), + nsparse::Residency::kMmap); + 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) { diff --git a/tests/seismic_scalar_quantized_index_test.cpp b/tests/seismic_scalar_quantized_index_test.cpp index 8202df8..4961072 100644 --- a/tests/seismic_scalar_quantized_index_test.cpp +++ b/tests/seismic_scalar_quantized_index_test.cpp @@ -32,6 +32,8 @@ #include "nsparse/io/index_io.h" #include "nsparse/sparse_vectors.h" #include "nsparse/types.h" +#include "nsparse/utils/scalar_quantizer.h" +#include "tests/csr_interchange_test_util.h" namespace nsparse { namespace { @@ -1354,13 +1356,75 @@ TEST(SeismicSQIndexMmapIOSingle, mapped_index_stays_valid_for_its_whole_life) { loaded.reset(); // must not fault, and must not double-unmap } -// A mapped CSR is borrowed at the width it was written in, which is float, so -// this index cannot take that path: its values have to be quantized by add(). -TEST(SeismicSQIndexMmapIOSingle, read_csr_rejects_the_mapped_residency) { - SeismicScalarQuantizedIndex index(QuantizerType::QT_8bit, 0.0F, 1.0F, +// Building from a native codes CSR borrowed via mmap must match building the +// same corpus fed through add(): the upstream writes codes at the quantizer's +// width, read_csr(kMmap) borrows them in place, and build/write/reload/search +// is bit-exact to the add()-fed build. This is the quantized analog of the +// unquantized mmap-CSR build; the two builds see identical codes because add() +// runs the same ScalarQuantizer::encode the test wrote into the file. +TEST(SeismicSQIndexMmapIOSingle, mmap_codes_csr_build_matches_add_build) { + constexpr int kDim = 5; + const SeismicClusterParameters params{ + .lambda = 10, .beta = 2, .alpha = 0.5F, .seed = 42}; + // A small reproducible corpus, held as raw CSR arrays so it can be fed both + // ways from the same bytes. + const std::vector indptr = {0, 2, 4, 6, 9}; + const std::vector indices = {0, 2, 1, 3, 0, 4, 2, 3, 4}; + const std::vector values = {1.0F, 0.9F, 0.5F, 0.7F, 0.3F, + 0.8F, 0.6F, 0.4F, 0.2F}; + const idx_t n = static_cast(indptr.size()) - 1; + + // Reference: the float corpus fed through add(), which quantizes it. + SeismicScalarQuantizedIndex added(QuantizerType::QT_8bit, 0.0F, 1.0F, params, + kDim); + added.add(n, indptr.data(), indices.data(), values.data()); + added.build(); + + // Under test: the same corpus pre-quantized to codes and borrowed via mmap. + const ScalarQuantizer sq(QuantizerType::QT_8bit, 0.0F, 1.0F); + const size_t element_size = sq.bytes_per_value(); + std::vector codes(indices.size() * element_size); + sq.encode(values.data(), codes.data(), indices.size()); + + csr_test::TempCsrFiles csr("nsparse_sesq_codes"); + csr_test::write_native_codes_csr(csr.native(), indptr, indices, codes, kDim, + element_size); + + SeismicScalarQuantizedIndex mapped(QuantizerType::QT_8bit, 0.0F, 1.0F, + params, kDim); + mapped.read_csr(csr.native().c_str(), Residency::kMmap); + ASSERT_EQ(mapped.num_vectors(), static_cast(n)); + mapped.build(); + + TempIndexFile file("nsparse_sesq_mmapbuild.idx"); + write_index(&mapped, file.c_str()); + std::unique_ptr reloaded( + read_index(file.c_str(), IndexIoFlag::kUseMmap)); + ASSERT_NE(reloaded, nullptr); + + for (term_t term = 0; term < kDim; ++term) { + EXPECT_EQ(search_scored(reloaded.get(), term, 4), + search_scored(&added, term, 4)) + << "mismatch at term " << term; + } +} + +// A codes CSR whose value width does not match the index's quantizer is +// rejected by read_mcsr's native-layout size check rather than misread: here +// 16-bit-wide values (element_size 2) are fed to an 8-bit index. +TEST(SeismicSQIndexMmapIOSingle, mmap_codes_csr_wrong_width_is_rejected) { + constexpr int kDim = 5; + const std::vector indptr = {0, 2, 4}; + const std::vector indices = {0, 2, 1, 3}; + std::vector wide_codes(indices.size() * 2); + csr_test::TempCsrFiles csr("nsparse_sesq_wrongwidth"); + csr_test::write_native_codes_csr(csr.native(), indptr, indices, wide_codes, + kDim, /*element_size=*/2); + + SeismicScalarQuantizedIndex eight(QuantizerType::QT_8bit, 0.0F, 1.0F, {.lambda = 10, .beta = 2, .alpha = 0.5F}, - 5); - EXPECT_THROW(index.read_csr("does_not_matter.csr", Residency::kMmap), + kDim); + EXPECT_THROW(eight.read_csr(csr.native().c_str(), Residency::kMmap), std::invalid_argument); } From 51441c2bc33ab6016736429432f41249a9cd0bfd Mon Sep 17 00:00:00 2001 From: Zirui Song Date: Thu, 3 Sep 2026 09:51:14 +0000 Subject: [PATCH 2/2] Address Liyun's comment Signed-off-by: Zirui Song --- nsparse/disk_seismic_index_base.h | 6 ++++-- nsparse/disk_seismic_scalar_quantized_index.h | 5 ----- nsparse/mmap_index.h | 14 ++++++++------ nsparse/seismic_scalar_quantized_index.h | 6 +++--- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/nsparse/disk_seismic_index_base.h b/nsparse/disk_seismic_index_base.h index be2f53f..25d1b79 100644 --- a/nsparse/disk_seismic_index_base.h +++ b/nsparse/disk_seismic_index_base.h @@ -64,8 +64,10 @@ class DiskSeismicIndexBase : public MmapIndex, public IndexIO { // --- Hooks the concrete indexes implement. --- - // Stored value width in bytes: 4 (float) or 1/2 (quantized codes). - [[nodiscard]] virtual size_t code_element_size() const = 0; + // Stored value width in bytes: 4 (float) or 1/2 (quantized codes). Kept + // pure (overriding MmapIndex's default) so each concrete disk type must + // state its width. + [[nodiscard]] size_t code_element_size() const override = 0; // Encode nnz float values to the stored width, returning a pointer to // code_element_size()-byte-per-value data. `scratch` backs the result when diff --git a/nsparse/disk_seismic_scalar_quantized_index.h b/nsparse/disk_seismic_scalar_quantized_index.h index 7d774ab..002bb03 100644 --- a/nsparse/disk_seismic_scalar_quantized_index.h +++ b/nsparse/disk_seismic_scalar_quantized_index.h @@ -73,11 +73,6 @@ class DiskSeismicScalarQuantizedIndex : public DiskSeismicIndexBase { return kFormatVersion; } [[nodiscard]] size_t code_element_size() const override; - // The value width read_csr(kMmap) borrows a native CSR at: this index's - // code width, so a codes CSR is borrowed in place rather than a float one. - [[nodiscard]] size_t mmap_element_size() const override { - return sq_.bytes_per_value(); - } const uint8_t* encode_values(const float* values, size_t nnz, std::vector& scratch) const override; const uint8_t* encode_query( diff --git a/nsparse/mmap_index.h b/nsparse/mmap_index.h index 11bba5b..0e9173b 100644 --- a/nsparse/mmap_index.h +++ b/nsparse/mmap_index.h @@ -72,11 +72,13 @@ class MmapIndex : public Index { // so they are always gone before the spill is released. detail::ClusteredListsSpill batch_spill_; - // The value width read_mcsr borrows a native CSR's values at. Float for the - // unquantized types (the default); a quantizing index overrides it to its - // code width so the same mapped-CSR path yields codes borrowed in place - // rather than floats. - [[nodiscard]] virtual size_t mmap_element_size() const { return U32; } + // The stored value width, in bytes, which is also what read_mcsr borrows a + // native CSR's values at: float (the default) for the unquantized types, or + // the quantizer's code width for a quantizing index, so the same mapped-CSR + // path yields codes borrowed in place rather than floats. Non-pure because + // MmapIndex is instantiated directly (e.g. TestMmapIndex); the disk family + // re-declares it pure so each concrete type must state its width. + [[nodiscard]] virtual size_t code_element_size() const { return U32; } private: bool is_mmap_index_ = false; @@ -122,7 +124,7 @@ class MmapIndex : public Index { // file written at a different width fails the size check below, so a // codes CSR handed to a float index (or vice versa) is rejected rather // than misread. - const size_t element_size = mmap_element_size(); + const size_t element_size = code_element_size(); const size_t indptr_size = static_cast(num_rows) + 1; const auto nnz_size = static_cast(nnz); diff --git a/nsparse/seismic_scalar_quantized_index.h b/nsparse/seismic_scalar_quantized_index.h index ff5a8d2..f760e15 100644 --- a/nsparse/seismic_scalar_quantized_index.h +++ b/nsparse/seismic_scalar_quantized_index.h @@ -64,9 +64,9 @@ class SeismicScalarQuantizedIndex : public MmapIndex, public IndexIO { size_t pos); private: - // The value width read_csr(kMmap) borrows a native CSR at: this index's - // code width, so a codes CSR is borrowed in place rather than a float one. - [[nodiscard]] size_t mmap_element_size() const override { + // Stored value width: this index's code width, so read_mcsr borrows a codes + // CSR in place (rather than a float one) via MmapIndex::code_element_size. + [[nodiscard]] size_t code_element_size() const override { return sq_.bytes_per_value(); }