From 25f3c3abac8bcc47cd49f1a3c33f2e8540a58d4f Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 15 Apr 2025 11:55:59 +0000 Subject: [PATCH 01/21] add size_t bg_vector_indexing_count; size_t main_vector_indexing_count; --- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index f868cd16d..319f92a4f 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -63,6 +63,8 @@ struct HNSWRepairJob : public AsyncJob { template class TieredHNSWIndex : public VecSimTieredIndex { private: + size_t bg_vector_indexing_count; + size_t main_vector_indexing_count; /// Mappings from id/label to associated jobs, for invalidating and update ids if necessary. // In MULTI, we can have more than one insert job pending per label. // **This map is protected with the flat buffer lock** @@ -633,9 +635,11 @@ TieredHNSWIndex::TieredHNSWIndex(HNSWIndex allocator) : VecSimTieredIndex(hnsw_index, bf_index, tiered_index_params, allocator), + bg_vector_indexing_count(0), main_vector_indexing_count(0), labelToInsertJobs(this->allocator), idToRepairJobs(this->allocator), idToSwapJob(this->allocator), invalidJobs(this->allocator), currInvalidJobId(0), readySwapJobs(0) { + // If the param for swapJobThreshold is 0 use the default value, if it exceeds the maximum // allowed, use the maximum value. this->pendingSwapJobsThreshold = @@ -708,6 +712,10 @@ size_t TieredHNSWIndex::indexLabelCount() const { template int TieredHNSWIndex::addVector(const void *blob, labelType label) { int ret = 1; + if (label == 999999) { + TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, + "bg_vec_count: %zu, main_thread_vector_count: %zu", this->bg_vector_indexing_count, this->main_vector_indexing_count); + } auto hnsw_index = this->getHNSWIndex(); // writeMode is not protected since it is assumed to be called only from the "main thread" // (that is the thread that is exculusively calling add/delete vector). @@ -733,6 +741,10 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l // This will do nothing (and return 0) if this label doesn't exist. Otherwise, it may // remove vector from the flat buffer and/or the HNSW index. ret -= this->deleteVector(label); + if (ret != 1) { + TIERED_LOG(VecSimCommonStrings::LOG_VERBOSE_STRING, + "removed a vector while trying to insert label %zu. ret: %d", label, ret); + } } if (this->frontendIndex->indexSize() >= this->flatBufferLimit) { // We didn't remove a vector from flat buffer due to overwrite, insert the new vector @@ -742,11 +754,13 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l // index. auto storage_blob = this->frontendIndex->preprocessForStorage(blob); this->insertVectorToHNSW(hnsw_index, label, storage_blob.get()); + this->main_vector_indexing_count++; return ret; } // Otherwise, we fall back to the "regular" insertion into the flat buffer // (since it is not full anymore after removing the previous vector stored under the label). } + this->bg_vector_indexing_count++; this->flatIndexGuard.lock(); idType new_flat_id = this->frontendIndex->indexSize(); if (this->frontendIndex->isLabelExists(label) && !this->frontendIndex->isMultiValue()) { From ffe9e81e6a8c7931cd3c739251284a26f1d6f6c2 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Fri, 18 Apr 2025 04:37:55 +0000 Subject: [PATCH 02/21] fix datasize in executeInsertJob --- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 36 ++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 319f92a4f..1c94a8e91 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -538,10 +538,11 @@ void TieredHNSWIndex::executeInsertJob(HNSWInsertJob *job) { HNSWIndex *hnsw_index = this->getHNSWIndex(); // Copy the vector blob from the flat buffer, so we can release the flat lock while we are // indexing the vector into HNSW index. + size_t data_size = this->frontendIndex->getDataSize(); auto blob_copy = this->getAllocator()->allocate_unique(this->frontendIndex->getDataSize()); memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), - this->frontendIndex->getDim() * sizeof(DataType)); + data_size); this->insertVectorToHNSW(hnsw_index, job->label, blob_copy.get()); @@ -703,7 +704,7 @@ size_t TieredHNSWIndex::indexLabelCount() const { this->mainIndexGuard.unlock(); return output.size(); } - +#include "VecSim/vec_sim_debug.h" // In the tiered index, we assume that the blobs are processed by the flat buffer // before being transferred to the HNSW index. // When inserting vectors directly into the HNSW index—such as in VecSim_WriteInPlace mode— or when @@ -712,11 +713,40 @@ size_t TieredHNSWIndex::indexLabelCount() const { template int TieredHNSWIndex::addVector(const void *blob, labelType label) { int ret = 1; + auto hnsw_index = this->getHNSWIndex(); + if (label == 100000) { + std::string res("index connections: {"); + + res += "Entry Point Label: "; + res += std::to_string(hnsw_index->getEntryPointLabel()) + "\n"; + TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, + "%s", res.c_str()); + for (idType id = 0; id < this->backendIndex->indexSize(); id++) { + std::string n_data("Node "); + labelType label = hnsw_index->getExternalLabel(id); + if (label == SIZE_MAX) + continue; // The ID is not in the index + int **neighbors_output; + VecSimDebug_GetElementNeighborsInHNSWGraph(this->backendIndex, label, &neighbors_output); + n_data += std::to_string(label) + ": "; + for (size_t l = 0; neighbors_output[l]; l++) { + n_data += "Level " + std::to_string(l) + " neighbors: "; + auto &neighbours = neighbors_output[l]; + auto neighbours_count = neighbours[0]; + for (size_t j = 1; j <= neighbours_count; j++) { + n_data += std::to_string(neighbours[j]) + ", "; + } + TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, + "%s", n_data.c_str()); + } + VecSimDebug_ReleaseElementNeighborsInHNSWGraph(neighbors_output); + } + } if (label == 999999) { TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, "bg_vec_count: %zu, main_thread_vector_count: %zu", this->bg_vector_indexing_count, this->main_vector_indexing_count); } - auto hnsw_index = this->getHNSWIndex(); + // writeMode is not protected since it is assumed to be called only from the "main thread" // (that is the thread that is exculusively calling add/delete vector). if (this->getWriteMode() == VecSim_WriteInPlace) { From 64b3d3fdb38dcdad5c0b68a61f0fd8a9baa8b16a Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 27 Apr 2025 11:38:59 +0000 Subject: [PATCH 03/21] disable neighbours print --- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 56 ++++++++++++------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 1c94a8e91..44edcfe8c 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -714,34 +714,34 @@ template int TieredHNSWIndex::addVector(const void *blob, labelType label) { int ret = 1; auto hnsw_index = this->getHNSWIndex(); - if (label == 100000) { - std::string res("index connections: {"); - - res += "Entry Point Label: "; - res += std::to_string(hnsw_index->getEntryPointLabel()) + "\n"; - TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, - "%s", res.c_str()); - for (idType id = 0; id < this->backendIndex->indexSize(); id++) { - std::string n_data("Node "); - labelType label = hnsw_index->getExternalLabel(id); - if (label == SIZE_MAX) - continue; // The ID is not in the index - int **neighbors_output; - VecSimDebug_GetElementNeighborsInHNSWGraph(this->backendIndex, label, &neighbors_output); - n_data += std::to_string(label) + ": "; - for (size_t l = 0; neighbors_output[l]; l++) { - n_data += "Level " + std::to_string(l) + " neighbors: "; - auto &neighbours = neighbors_output[l]; - auto neighbours_count = neighbours[0]; - for (size_t j = 1; j <= neighbours_count; j++) { - n_data += std::to_string(neighbours[j]) + ", "; - } - TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, - "%s", n_data.c_str()); - } - VecSimDebug_ReleaseElementNeighborsInHNSWGraph(neighbors_output); - } - } + // if (label == 100000) { + // std::string res("index connections: {"); + + // res += "Entry Point Label: "; + // res += std::to_string(hnsw_index->getEntryPointLabel()) + "\n"; + // TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, + // "%s", res.c_str()); + // for (idType id = 0; id < this->backendIndex->indexSize(); id++) { + // std::string n_data("Node "); + // labelType label = hnsw_index->getExternalLabel(id); + // if (label == SIZE_MAX) + // continue; // The ID is not in the index + // int **neighbors_output; + // VecSimDebug_GetElementNeighborsInHNSWGraph(this->backendIndex, label, &neighbors_output); + // n_data += std::to_string(label) + ": "; + // for (size_t l = 0; neighbors_output[l]; l++) { + // n_data += "Level " + std::to_string(l) + " neighbors: "; + // auto &neighbours = neighbors_output[l]; + // auto neighbours_count = neighbours[0]; + // for (size_t j = 1; j <= neighbours_count; j++) { + // n_data += std::to_string(neighbours[j]) + ", "; + // } + // TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, + // "%s", n_data.c_str()); + // } + // VecSimDebug_ReleaseElementNeighborsInHNSWGraph(neighbors_output); + // } + // } if (label == 999999) { TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, "bg_vec_count: %zu, main_thread_vector_count: %zu", this->bg_vector_indexing_count, this->main_vector_indexing_count); From 94e5789662c47e2d32c8c99d6218ce9294b27c5d Mon Sep 17 00:00:00 2001 From: meiravgri Date: Mon, 28 Apr 2025 14:07:15 +0000 Subject: [PATCH 04/21] replace manual blob size calcaulations format comment out new tests --- .../algorithms/brute_force/brute_force.h | 2 +- .../brute_force/brute_force_multi.h | 2 +- .../brute_force/brute_force_single.h | 2 +- src/VecSim/algorithms/hnsw/hnsw_multi.h | 2 +- src/VecSim/algorithms/hnsw/hnsw_single.h | 2 +- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 21 ++-- src/VecSim/algorithms/svs/svs.h | 4 +- tests/unit/CMakeLists.txt | 2 +- tests/unit/test_int8.cpp | 114 ++++++++++++++++++ 9 files changed, 133 insertions(+), 18 deletions(-) diff --git a/src/VecSim/algorithms/brute_force/brute_force.h b/src/VecSim/algorithms/brute_force/brute_force.h index d4cddfcdf..54dde3a4b 100644 --- a/src/VecSim/algorithms/brute_force/brute_force.h +++ b/src/VecSim/algorithms/brute_force/brute_force.h @@ -350,7 +350,7 @@ BruteForceIndex::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { auto *queryBlobCopy = this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->dim * sizeof(DataType)); + memcpy(queryBlobCopy, queryBlob, this->getDataSize()); this->preprocessQueryInPlace(queryBlobCopy); // Ownership of queryBlobCopy moves to BF_BatchIterator that will free it at the end. return newBatchIterator_Instance(queryBlobCopy, queryParams); diff --git a/src/VecSim/algorithms/brute_force/brute_force_multi.h b/src/VecSim/algorithms/brute_force/brute_force_multi.h index 999f5ac8b..1d0534855 100644 --- a/src/VecSim/algorithms/brute_force/brute_force_multi.h +++ b/src/VecSim/algorithms/brute_force/brute_force_multi.h @@ -45,7 +45,7 @@ class BruteForceIndex_Multi : public BruteForceIndex { for (idType id : ids->second) { auto vec = std::vector(this->dim); - memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType)); + memcpy(vec.data(), this->getDataByInternalId(id), this->getDataSize()); vectors_output.push_back(vec); } } diff --git a/src/VecSim/algorithms/brute_force/brute_force_single.h b/src/VecSim/algorithms/brute_force/brute_force_single.h index 0c27615e9..585c71f21 100644 --- a/src/VecSim/algorithms/brute_force/brute_force_single.h +++ b/src/VecSim/algorithms/brute_force/brute_force_single.h @@ -47,7 +47,7 @@ class BruteForceIndex_Single : public BruteForceIndex { auto id = labelToIdLookup.at(label); auto vec = std::vector(this->dim); - memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType)); + memcpy(vec.data(), this->getDataByInternalId(id), this->getDataSize()); vectors_output.push_back(vec); } #endif diff --git a/src/VecSim/algorithms/hnsw/hnsw_multi.h b/src/VecSim/algorithms/hnsw/hnsw_multi.h index 972c981e4..23922df90 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_multi.h +++ b/src/VecSim/algorithms/hnsw/hnsw_multi.h @@ -201,7 +201,7 @@ HNSWIndex_Multi::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { auto queryBlobCopy = this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->dim * sizeof(DataType)); + memcpy(queryBlobCopy, queryBlob, this->getDataSize()); this->preprocessQueryInPlace(queryBlobCopy); // Ownership of queryBlobCopy moves to HNSW_BatchIterator that will free it at the end. return new (this->allocator) HNSWMulti_BatchIterator( diff --git a/src/VecSim/algorithms/hnsw/hnsw_single.h b/src/VecSim/algorithms/hnsw/hnsw_single.h index c9ef19ead..b8cadd634 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_single.h +++ b/src/VecSim/algorithms/hnsw/hnsw_single.h @@ -161,7 +161,7 @@ HNSWIndex_Single::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { auto queryBlobCopy = this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->dim * sizeof(DataType)); + memcpy(queryBlobCopy, queryBlob, this->getDataSize()); this->preprocessQueryInPlace(queryBlobCopy); // Ownership of queryBlobCopy moves to HNSW_BatchIterator that will free it at the end. return new (this->allocator) HNSWSingle_BatchIterator( diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 44edcfe8c..92cfc5937 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -199,7 +199,7 @@ class TieredHNSWIndex : public VecSimTieredIndex { VecSimDebugInfoIterator *debugInfoIterator() const override; VecSimBatchIterator *newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const override { - size_t blobSize = this->frontendIndex->getDim() * sizeof(DataType); + size_t blobSize = this->frontendIndex->getDataSize(); void *queryBlobCopy = this->allocator->allocate(blobSize); memcpy(queryBlobCopy, queryBlob, blobSize); return new (this->allocator) @@ -539,10 +539,9 @@ void TieredHNSWIndex::executeInsertJob(HNSWInsertJob *job) { // Copy the vector blob from the flat buffer, so we can release the flat lock while we are // indexing the vector into HNSW index. size_t data_size = this->frontendIndex->getDataSize(); - auto blob_copy = this->getAllocator()->allocate_unique(this->frontendIndex->getDataSize()); + auto blob_copy = this->getAllocator()->allocate_unique(data_size); - memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), - data_size); + memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), data_size); this->insertVectorToHNSW(hnsw_index, job->label, blob_copy.get()); @@ -636,7 +635,7 @@ TieredHNSWIndex::TieredHNSWIndex(HNSWIndex allocator) : VecSimTieredIndex(hnsw_index, bf_index, tiered_index_params, allocator), - bg_vector_indexing_count(0), main_vector_indexing_count(0), + bg_vector_indexing_count(0), main_vector_indexing_count(0), labelToInsertJobs(this->allocator), idToRepairJobs(this->allocator), idToSwapJob(this->allocator), invalidJobs(this->allocator), currInvalidJobId(0), readySwapJobs(0) { @@ -727,9 +726,9 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l // if (label == SIZE_MAX) // continue; // The ID is not in the index // int **neighbors_output; - // VecSimDebug_GetElementNeighborsInHNSWGraph(this->backendIndex, label, &neighbors_output); - // n_data += std::to_string(label) + ": "; - // for (size_t l = 0; neighbors_output[l]; l++) { + // VecSimDebug_GetElementNeighborsInHNSWGraph(this->backendIndex, label, + // &neighbors_output); n_data += std::to_string(label) + ": "; for (size_t l = 0; + // neighbors_output[l]; l++) { // n_data += "Level " + std::to_string(l) + " neighbors: "; // auto &neighbours = neighbors_output[l]; // auto neighbours_count = neighbours[0]; @@ -744,7 +743,8 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l // } if (label == 999999) { TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, - "bg_vec_count: %zu, main_thread_vector_count: %zu", this->bg_vector_indexing_count, this->main_vector_indexing_count); + "bg_vec_count: %zu, main_thread_vector_count: %zu", + this->bg_vector_indexing_count, this->main_vector_indexing_count); } // writeMode is not protected since it is assumed to be called only from the "main thread" @@ -773,7 +773,8 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l ret -= this->deleteVector(label); if (ret != 1) { TIERED_LOG(VecSimCommonStrings::LOG_VERBOSE_STRING, - "removed a vector while trying to insert label %zu. ret: %d", label, ret); + "removed a vector while trying to insert label %zu. ret: %d", label, + ret); } } if (this->frontendIndex->indexSize() >= this->flatBufferLimit) { diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 6210f6e83..0cdb1d7f3 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -146,7 +146,7 @@ class SVSIndex : public VecSimIndexAbstract, fl return MemoryUtils::unique_blob{const_cast(original_data), [](void *) {}}; } - const auto data_size = this->dim * sizeof(DataType) * n; + const auto data_size = this->getDataSize() * n; auto processed_blob = MemoryUtils::unique_blob{this->allocator->allocate(data_size), @@ -428,7 +428,7 @@ class SVSIndex : public VecSimIndexAbstract, fl VecSimQueryParams *queryParams) const override { auto *queryBlobCopy = this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->dim * sizeof(DataType)); + memcpy(queryBlobCopy, queryBlob, this->getDataSize()); this->preprocessQueryInPlace(queryBlobCopy); // Ownership of queryBlobCopy moves to VecSimBatchIterator that will free it at the end. if (indexSize() == 0) { diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index f248705d4..63e245c2d 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -79,4 +79,4 @@ if(SVS_SUPPORTED) add_executable(test_svs ../utils/mock_thread_pool.cpp test_svs.cpp unit_test_utils.cpp) target_link_libraries(test_svs PUBLIC gtest_main VectorSimilarity) gtest_discover_tests(test_svs) -endif() \ No newline at end of file +endif() diff --git a/tests/unit/test_int8.cpp b/tests/unit/test_int8.cpp index aaf0f5d51..c377515f7 100644 --- a/tests/unit/test_int8.cpp +++ b/tests/unit/test_int8.cpp @@ -993,3 +993,117 @@ TEST_F(INT8TieredTest, getElementNeighbors) { HNSWParams params = {.dim = 4, .M = 20}; get_element_neighbors(params); } + +// /** +// * This test specifically targets the bug where memcpy in hnsw_tiered.h uses manually computed +// size +// * (dim * sizeof(DataType)) instead of getDataSize(), which doesn't account for additional data +// * stored alongside the vector. In the int8 + cosine configuration, a float norm is appended to +// the +// * vector data, and using the manual size truncates this norm during the copy. +// */ +// TEST_F(INT8TieredTest, TestInt8CosineVectorCopy) { +// // Create a tiered HNSW index with int8 vectors and cosine similarity +// HNSWParams params = { +// .type = VecSimType_INT8, +// .dim = 4, +// .metric = VecSimMetric_Cosine, +// }; + +// // Create the tiered index with a small flat buffer limit to ensure vectors move to HNSW +// quickly TieredIndexParams tiered_params = generate_tiered_params(params, 1, 1); +// SetUp(tiered_params); + +// // Create a test vector +// int8_t vector[4]; +// test_utils::populate_int8_vec(vector, 4, 42); + +// // Add the vector to the index +// VecSimIndex_AddVector(index, vector, 1); + +// // Verify the vector is in the flat buffer +// auto *flat_index = reinterpret_cast*>(CastToBruteForce()); +// auto *hnsw_index = CastToHNSW(); +// ASSERT_EQ(flat_index->indexSize(), 1); +// ASSERT_EQ(hnsw_index->indexSize(), 0); + +// // Get the vector from the flat buffer +// const int8_t *flat_vector = reinterpret_cast(flat_index->getDataByInternalId(0)); float vector_norm = +// spaces::IntegralType_ComputeNorm(flat_vector, dim); ASSERT_EQ(index_vector_norm, +// vector_norm) << "wrong vector norm for vector id:" << i; + +// // Run a thread iteration to move the vector from flat to HNSW +// mock_thread_pool.thread_iteration(); + +// // Verify the vector has been moved to HNSW +// ASSERT_EQ(flat_index->indexSize(), 0); +// ASSERT_EQ(hnsw_index->indexSize(), 1); + +// // Get the vector from HNSW +// const int8_t *hnsw_vector = reinterpret_cast(hnsw_index->getDataByInternalId(0)); + +// // Verify the norm is correctly preserved in the HNSW vector +// float norm_in_hnsw = *(reinterpret_cast(hnsw_vector + params.dim)); + +// // This would fail with the bug, as the norm would be truncated +// ASSERT_NEAR(norm_in_flat, norm_in_hnsw, 1e-5); + +// // Calculate the distance from the vector to itself in HNSW +// // For cosine distance to itself, this should be close to 0 +// // We don't use ASSERT_NEAR here because the distance calculation might have some numerical +// issues +// // that are not related to the bug we're testing +// } + +// /** +// * This test specifically targets the bug fix where memcpy was using the wrong size. +// * It demonstrates the difference between using the correct size (getDataSize()) +// * and the incorrect size (dim * sizeof(DataType)). +// */ +// TEST_F(INT8TieredTest, TestInt8CosineBugFix) { +// // Create a tiered HNSW index with int8 vectors and cosine similarity +// HNSWParams params = { +// .type = VecSimType_INT8, +// .dim = 4, +// .metric = VecSimMetric_Cosine, +// .multi = false +// }; + +// // Create the tiered index +// // TieredIndexParams tiered_params = generate_tiered_params(params); +// SetUp(params); + +// // Create a test vector +// int8_t vector[4]; +// test_utils::populate_int8_vec(vector, 4, 42); + +// // Add the vector to the index +// ASSERT_EQ(this->GenerateAndAddVector(i, i), 1); +// // VecSimIndex_AddVector(index, vector, 1); + +// // Get the flat index and the vector from it +// auto *flat_index = CastToBruteForce(); +// const int8_t *flat_vector = reinterpret_cast(flat_index->getDataByInternalId(0)); + +// // Verify the norm is present in the correct copy +// float norm_in_flat = *(reinterpret_cast(flat_vector + params.dim)); + +// ASSERT_NEAR(norm_in_flat, norm_in_correct_copy, 1e-5); + +// // Run a thread iteration to move the vector from flat to HNSW +// mock_thread_pool.thread_iteration(); + +// // Get the HNSW index and the vector from it +// auto *hnsw_index = CastToHNSW(); +// const int8_t *hnsw_vector = reinterpret_cast(hnsw_index->getDataByInternalId(0)); + +// // Verify the norm is correctly preserved in the HNSW vector +// float norm_in_hnsw = *(reinterpret_cast(hnsw_vector + params.dim)); + +// // This would fail with the bug, as the norm would be truncated +// ASSERT_NEAR(norm_in_flat, norm_in_hnsw, 1e-5); +// } From 352456528a6c8ed054395617a9f5111ea3fb0925 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Wed, 30 Apr 2025 04:24:29 +0000 Subject: [PATCH 05/21] INT8Test::PopulateRandomVector increases seed every call so we will get different vectors. this required a change in metrics_test because now the score is not exactly 0. VecSimIndexAbstract CastToBruteForce()-> rename to GetFlatBufferIndex add BruteForceIndex CastToBruteForce() add getFlatBufferIndexAsBruteForce to tiered index to get BruteForceIndex functions added to hnsw and BF: Formalize getDataByLabel definition: populate the intput empty vector with the vectors data,not including any additional meta data that might present in the vector storage like the norm. add getStoredVectorDataByLabel returns a vector of char vectors. each vector contains the vector as it is stored in the vectors container, including the meta data if exists. --- .../algorithms/brute_force/brute_force.h | 44 ++- .../brute_force/brute_force_multi.h | 22 +- .../brute_force/brute_force_single.h | 22 +- src/VecSim/algorithms/hnsw/hnsw.h | 38 ++- src/VecSim/algorithms/hnsw/hnsw_multi.h | 20 +- src/VecSim/algorithms/hnsw/hnsw_single.h | 17 +- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 12 +- src/VecSim/vec_sim_tiered_index.h | 3 + tests/unit/test_bruteforce.cpp | 4 +- tests/unit/test_int8.cpp | 259 ++++++++++-------- 10 files changed, 297 insertions(+), 144 deletions(-) diff --git a/src/VecSim/algorithms/brute_force/brute_force.h b/src/VecSim/algorithms/brute_force/brute_force.h index 54dde3a4b..42bf12654 100644 --- a/src/VecSim/algorithms/brute_force/brute_force.h +++ b/src/VecSim/algorithms/brute_force/brute_force.h @@ -40,8 +40,8 @@ class BruteForceIndex : public VecSimIndexAbstract { size_t indexSize() const override; size_t indexCapacity() const override; std::unique_ptr getVectorsIterator() const; - DataType *getDataByInternalId(idType id) const { - return (DataType *)this->vectors->getElement(id); + const DataType *getDataByInternalId(idType id) const { + return reinterpret_cast(this->vectors->getElement(id)); } VecSimQueryReply *topKQuery(const void *queryBlob, size_t k, VecSimQueryParams *queryParams) const override; @@ -75,15 +75,41 @@ class BruteForceIndex : public VecSimIndexAbstract { virtual ~BruteForceIndex() = default; #ifdef BUILD_TESTS /** - * @brief Used for testing - store vector(s) data associated with a given label. This function - * copies the vector(s)' data buffer(s) and place it in the output vector + * @brief Used for testing - get only the vector elements associated with a given label. + * This function copies only the vector(s) elements into the output vector, + * without any additional metadata that might be stored with the vector(s). * - * @param label - * @param vectors_output empty vector to be modified, should store the blob(s) associated with - * the label. + * Important: This method returns ONLY the vector elements, even if the stored vector contains + * additional metadata. For example, with int8_t/uint8_t vectors using cosine similarity, + * this method will NOT return the norm that is stored with the vector. + * + * If you need the complete data including any metadata, use getStoredVectorDataByLabel() + * instead. + * + * @param label The label to retrieve vector(s) elements for + * @param vectors_output Empty vector to be filled with vector(s) */ virtual void getDataByLabel(labelType label, std::vector> &vectors_output) const = 0; + + /** + * @brief Used for testing - get the complete raw data associated with a given label. + * This function returns the ENTIRE vector(s) data as stored in the index, including any + * additional metadata that might be stored alongside the vector elements. + * + * For example: + * - For int8_t/uint8_t vectors with cosine similarity, this includes the norm stored at the end + * - For other vector types or future implementations, this will include any additional data + * that might be stored with the vector + * + * Use this method when you need access to the complete vector data as it is stored internally. + * + * @param label The label to retrieve data for + * @return A vector containing the complete vector data (elements + metadata) for the given + * label + */ + virtual std::vector> getStoredVectorDataByLabel(labelType label) const = 0; + void fitMemory() override { if (count == 0) { return; @@ -350,7 +376,9 @@ BruteForceIndex::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { auto *queryBlobCopy = this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->getDataSize()); + memcpy(queryBlobCopy, queryBlob, this->dim * sizeof(DataType)); + + // memcpy(queryBlobCopy, queryBlob, this->getDataSize()); this->preprocessQueryInPlace(queryBlobCopy); // Ownership of queryBlobCopy moves to BF_BatchIterator that will free it at the end. return newBatchIterator_Instance(queryBlobCopy, queryParams); diff --git a/src/VecSim/algorithms/brute_force/brute_force_multi.h b/src/VecSim/algorithms/brute_force/brute_force_multi.h index 1d0534855..bb6dcc2d8 100644 --- a/src/VecSim/algorithms/brute_force/brute_force_multi.h +++ b/src/VecSim/algorithms/brute_force/brute_force_multi.h @@ -45,10 +45,30 @@ class BruteForceIndex_Multi : public BruteForceIndex { for (idType id : ids->second) { auto vec = std::vector(this->dim); - memcpy(vec.data(), this->getDataByInternalId(id), this->getDataSize()); + // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like + // the norm + memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType)); vectors_output.push_back(vec); } } + + std::vector> getStoredVectorDataByLabel(labelType label) const override { + std::vector> vectors_output; + auto ids = labelToIdsLookup.find(label); + + for (idType id : ids->second) { + // Get the data pointer - need to cast to char* for memcpy + const char *data = reinterpret_cast(this->getDataByInternalId(id)); + + // Create a vector with the full data (including any metadata like norms) + std::vector vec(this->getDataSize()); + memcpy(vec.data(), data, this->getDataSize()); + vectors_output.push_back(std::move(vec)); + } + + return vectors_output; + } + #endif private: // inline definitions diff --git a/src/VecSim/algorithms/brute_force/brute_force_single.h b/src/VecSim/algorithms/brute_force/brute_force_single.h index 585c71f21..96f6ef65d 100644 --- a/src/VecSim/algorithms/brute_force/brute_force_single.h +++ b/src/VecSim/algorithms/brute_force/brute_force_single.h @@ -39,17 +39,33 @@ class BruteForceIndex_Single : public BruteForceIndex { // We call this when we KNOW that the label exists in the index. idType getIdOfLabel(labelType label) const { return labelToIdLookup.find(label)->second; } - +// #define BUILD_TESTS #ifdef BUILD_TESTS void getDataByLabel(labelType label, std::vector> &vectors_output) const override { - auto id = labelToIdLookup.at(label); auto vec = std::vector(this->dim); - memcpy(vec.data(), this->getDataByInternalId(id), this->getDataSize()); + // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like the + // norm + memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType)); vectors_output.push_back(vec); } + + std::vector> getStoredVectorDataByLabel(labelType label) const override { + std::vector> vectors_output; + auto id = labelToIdLookup.at(label); + + // Get the data pointer - need to cast to char* for memcpy + const char *data = reinterpret_cast(this->getDataByInternalId(id)); + + // Create a vector with the full data (including any metadata like norms) + std::vector vec(this->getDataSize()); + memcpy(vec.data(), data, this->getDataSize()); + vectors_output.push_back(std::move(vec)); + + return vectors_output; + } #endif protected: // inline definitions diff --git a/src/VecSim/algorithms/hnsw/hnsw.h b/src/VecSim/algorithms/hnsw/hnsw.h index 61ca1fa2d..7510a66df 100644 --- a/src/VecSim/algorithms/hnsw/hnsw.h +++ b/src/VecSim/algorithms/hnsw/hnsw.h @@ -301,15 +301,41 @@ class HNSWIndex : public VecSimIndexAbstract, #ifdef BUILD_TESTS /** - * @brief Used for testing - store vector(s) data associated with a given label. This function - * copies the vector(s)' data buffer(s) and place it in the output vector + * @brief Used for testing - get only the vector elements associated with a given label. + * This function copies only the vector(s) elements into the output vector, + * without any additional metadata that might be stored with the vector. * - * @param label - * @param vectors_output empty vector to be modified, should store the blob(s) associated with - * the label. + * Important: This method returns ONLY the vector elements, even if the stored vector contains + * additional metadata. For example, with int8_t/uint8_t vectors using cosine similarity, + * this method will NOT return the norm that is stored with the vector(s). + * + * If you need the complete data including any metadata, use getStoredVectorDataByLabel() + * instead. + * + * @param label The label to retrieve vector(s) elements for + * @param vectors_output Empty vector to be filled with vector(s) */ virtual void getDataByLabel(labelType label, std::vector> &vectors_output) const = 0; + + /** + * @brief Used for testing - get the complete raw data associated with a given label. + * This function returns the ENTIRE vector(s) data as stored in the index, including any + * additional metadata that might be stored alongside the vector elements. + * + * For example: + * - For int8_t/uint8_t vectors with cosine similarity, this includes the norm stored at the end + * - For other vector types or future implementations, this will include any additional data + * that might be stored with the vector + * + * Use this method when you need access to the complete vector data as it is stored internally. + * + * @param label The label to retrieve data for + * @return A vector containing the complete vector data (elements + metadata) for the given + * label + */ + virtual std::vector> getStoredVectorDataByLabel(labelType label) const = 0; + void fitMemory() override { if (maxElements > 0) { idToMetaData.shrink_to_fit(); @@ -1559,7 +1585,7 @@ void HNSWIndex::insertElementToGraph(idType element_id, for (auto level = static_cast(max_common_level); level >= 0; level--) { candidatesMaxHeap top_candidates = searchLayer(curr_element, vector_data, level, efConstruction); - // If the entry point was marked deleted between iterations, we may recieve an empty + // If the entry point was marked deleted between iterations, we may receive an empty // candidates set. if (!top_candidates.empty()) { curr_element = mutuallyConnectNewElement(element_id, top_candidates, level); diff --git a/src/VecSim/algorithms/hnsw/hnsw_multi.h b/src/VecSim/algorithms/hnsw/hnsw_multi.h index 23922df90..9048e9738 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_multi.h +++ b/src/VecSim/algorithms/hnsw/hnsw_multi.h @@ -72,10 +72,28 @@ class HNSWIndex_Multi : public HNSWIndex { for (idType id : ids->second) { auto vec = std::vector(this->dim); - memcpy(vec.data(), this->getDataByInternalId(id), this->dataSize); + // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like + // the norm + memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType)); vectors_output.push_back(vec); } } + + std::vector> getStoredVectorDataByLabel(labelType label) const override { + std::vector> vectors_output; + auto ids = labelLookup.find(label); + + for (idType id : ids->second) { + const char *data = this->getDataByInternalId(id); + + // Create a vector with the full data (including any metadata like norms) + std::vector vec(this->dataSize); + memcpy(vec.data(), data, this->dataSize); + vectors_output.push_back(std::move(vec)); + } + + return vectors_output; + } #endif ~HNSWIndex_Multi() = default; diff --git a/src/VecSim/algorithms/hnsw/hnsw_single.h b/src/VecSim/algorithms/hnsw/hnsw_single.h index b8cadd634..2993f7c19 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_single.h +++ b/src/VecSim/algorithms/hnsw/hnsw_single.h @@ -48,9 +48,24 @@ class HNSWIndex_Single : public HNSWIndex { auto id = labelLookup.at(label); auto vec = std::vector(this->dim); - memcpy(vec.data(), this->getDataByInternalId(id), this->dataSize); + // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like the + // norm + memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType)); vectors_output.push_back(vec); } + + std::vector> getStoredVectorDataByLabel(labelType label) const override { + std::vector> vectors_output; + auto id = labelLookup.at(label); + const char *data = this->getDataByInternalId(id); + + // Create a vector with the full data (including any metadata like norms) + std::vector vec(this->dataSize); + memcpy(vec.data(), data, this->dataSize); + vectors_output.push_back(std::move(vec)); + + return vectors_output; + } #endif ~HNSWIndex_Single() = default; diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 92cfc5937..a537c35d9 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -235,6 +235,8 @@ class TieredHNSWIndex : public VecSimTieredIndex { #ifdef BUILD_TESTS void getDataByLabel(labelType label, std::vector> &vectors_output) const; + + std::vector> getStoredVectorDataByLabel(labelType label) const; #endif }; @@ -748,7 +750,7 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l } // writeMode is not protected since it is assumed to be called only from the "main thread" - // (that is the thread that is exculusively calling add/delete vector). + // (that is the thread that is exclusively calling add/delete vector). if (this->getWriteMode() == VecSim_WriteInPlace) { // First, check if we need to overwrite the vector in-place for single (from both indexes). if (!this->backendIndex->isMultiValue()) { @@ -885,7 +887,7 @@ int TieredHNSWIndex::deleteVector(labelType label) { // Note that we may remove the same vector that has been removed from the flat index, if it was // being ingested at that time. // writeMode is not protected since it is assumed to be called only from the "main thread" - // (that is the thread that is exculusively calling add/delete vector). + // (that is the thread that is exclusively calling add/delete vector). if (this->getWriteMode() == VecSim_WriteAsync) { num_deleted_vectors += this->deleteLabelFromHNSW(label); // Apply ready swap jobs if number of deleted vectors reached the threshold @@ -1228,4 +1230,10 @@ void TieredHNSWIndex::getDataByLabel( labelType label, std::vector> &vectors_output) const { this->getHNSWIndex()->getDataByLabel(label, vectors_output); } + +template +std::vector> +TieredHNSWIndex::getStoredVectorDataByLabel(labelType label) const { + return this->getHNSWIndex()->getStoredVectorDataByLabel(label); +} #endif diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index f166667e7..559737117 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -101,6 +101,9 @@ class VecSimTieredIndex : public VecSimIndexInterface { inline VecSimIndexAbstract *getFlatBufferIndex() { return this->frontendIndex; } + inline BruteForceIndex *getFlatBufferIndexAsBruteForce() { + return this->frontendIndex; + } inline size_t getFlatBufferLimit() { return this->flatBufferLimit; } virtual void fitMemory() override { diff --git a/tests/unit/test_bruteforce.cpp b/tests/unit/test_bruteforce.cpp index 821af06a9..ac7db607b 100644 --- a/tests/unit/test_bruteforce.cpp +++ b/tests/unit/test_bruteforce.cpp @@ -80,7 +80,7 @@ TYPED_TEST(BruteForceTest, brute_force_vector_update_test) { ASSERT_EQ(bf_index->idToLabelMapping.size(), DEFAULT_BLOCK_SIZE); // Check update. - TEST_DATA_T *vector_data = bf_index->getDataByInternalId(0); + const TEST_DATA_T *vector_data = bf_index->getDataByInternalId(0); for (size_t i = 0; i < dim; ++i) { ASSERT_EQ(*vector_data, 2.0); ++vector_data; @@ -386,7 +386,7 @@ TYPED_TEST(BruteForceTest, test_delete_swap_block) { ASSERT_EQ(deleted_label_id_pair, bf_single_index->labelToIdLookup.end()); // The vector in index1 should hold id5 data. - TEST_DATA_T *vector_data = bf_index->getDataByInternalId(1); + const TEST_DATA_T *vector_data = bf_index->getDataByInternalId(1); for (size_t i = 0; i < dim; ++i) { ASSERT_EQ(*vector_data, 5); ++vector_data; diff --git a/tests/unit/test_int8.cpp b/tests/unit/test_int8.cpp index c377515f7..840e34836 100644 --- a/tests/unit/test_int8.cpp +++ b/tests/unit/test_int8.cpp @@ -7,6 +7,7 @@ #include "VecSim/vec_sim_debug.h" #include "VecSim/spaces/L2/L2.h" #include "VecSim/spaces/IP/IP.h" +#include "VecSim/spaces/normalize/normalize_naive.h" class INT8Test : public ::testing::Test { protected: @@ -38,7 +39,9 @@ class INT8Test : public ::testing::Test { virtual HNSWIndex *CastToHNSW() { return CastIndex>(); } - void PopulateRandomVector(int8_t *out_vec) { test_utils::populate_int8_vec(out_vec, dim); } + void PopulateRandomVector(int8_t *out_vec) { + test_utils::populate_int8_vec(out_vec, dim, current_seed++); + } int PopulateRandomAndAddVector(size_t id, int8_t *out_vec) { PopulateRandomVector(out_vec); return VecSimIndex_AddVector(index, out_vec, id); @@ -92,6 +95,7 @@ class INT8Test : public ::testing::Test { VecSimIndex *index; size_t dim; + int current_seed{0}; }; class INT8HNSWTest : public INT8Test { @@ -173,7 +177,7 @@ class INT8TieredTest : public INT8Test { virtual void TearDown() override {} virtual const void *GetDataByInternalId(idType id) override { - return CastIndex>(CastToBruteForce()) + return CastIndex>(GetFlatBufferIndex()) ->getDataByInternalId(id); } @@ -186,11 +190,16 @@ class INT8TieredTest : public INT8Test { return CastIndex>(CastToHNSW()); } - VecSimIndexAbstract *CastToBruteForce() { + VecSimIndexAbstract *GetFlatBufferIndex() { auto tiered_index = dynamic_cast *>(index); return tiered_index->getFlatBufferIndex(); } + BruteForceIndex *CastToBruteForce() { + auto tiered_index = dynamic_cast *>(index); + return tiered_index->getFlatBufferIndexAsBruteForce(); + } + int GenerateRandomAndAddVector(size_t id) override { int8_t v[dim]; PopulateRandomVector(v); @@ -384,7 +393,7 @@ void INT8Test::metrics_test(params_t index_params) { double expected_score = 0; auto verify_res = [&](size_t id, double score, size_t index) { - ASSERT_EQ(score, expected_score) << "failed at vector id:" << id; + ASSERT_NEAR(score, expected_score, 1e-6f) << "failed at vector id:" << id; }; for (size_t i = 0; i < n; i++) { @@ -729,7 +738,7 @@ void INT8TieredTest::test_info(bool is_multi) { VecSimIndexDebugInfo info = INT8Test::test_info(hnsw_params); ASSERT_EQ(info.commonInfo.basicInfo.algo, VecSimAlgo_HNSWLIB); - VecSimIndexDebugInfo frontendIndexInfo = CastToBruteForce()->debugInfo(); + VecSimIndexDebugInfo frontendIndexInfo = GetFlatBufferIndex()->debugInfo(); VecSimIndexDebugInfo backendIndexInfo = CastToHNSW()->debugInfo(); compareCommonInfo(info.tieredInfo.frontendCommonInfo, frontendIndexInfo.commonInfo); @@ -837,7 +846,7 @@ void INT8TieredTest::test_info_iterator(VecSimMetric metric) { SetUp(params); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); VecSimDebugInfoIterator *infoIter = VecSimIndex_DebugInfoIterator(index); - VecSimIndexDebugInfo frontendIndexInfo = CastToBruteForce()->debugInfo(); + VecSimIndexDebugInfo frontendIndexInfo = GetFlatBufferIndex()->debugInfo(); VecSimIndexDebugInfo backendIndexInfo = CastToHNSW()->debugInfo(); VecSimDebugInfoIterator_Free(infoIter); } @@ -875,7 +884,7 @@ void INT8HNSWTest::test_serialization(bool is_multi) { int8_t data[n * dim]; for (size_t i = 0; i < n * dim; i += dim) { - test_utils::populate_int8_vec(data + i, dim, i); + this->PopulateRandomVector(data + i); } for (size_t j = 0; j < n; ++j) { @@ -994,116 +1003,126 @@ TEST_F(INT8TieredTest, getElementNeighbors) { get_element_neighbors(params); } -// /** -// * This test specifically targets the bug where memcpy in hnsw_tiered.h uses manually computed -// size -// * (dim * sizeof(DataType)) instead of getDataSize(), which doesn't account for additional data -// * stored alongside the vector. In the int8 + cosine configuration, a float norm is appended to -// the -// * vector data, and using the manual size truncates this norm during the copy. -// */ -// TEST_F(INT8TieredTest, TestInt8CosineVectorCopy) { -// // Create a tiered HNSW index with int8 vectors and cosine similarity -// HNSWParams params = { -// .type = VecSimType_INT8, -// .dim = 4, -// .metric = VecSimMetric_Cosine, -// }; - -// // Create the tiered index with a small flat buffer limit to ensure vectors move to HNSW -// quickly TieredIndexParams tiered_params = generate_tiered_params(params, 1, 1); -// SetUp(tiered_params); - -// // Create a test vector -// int8_t vector[4]; -// test_utils::populate_int8_vec(vector, 4, 42); - -// // Add the vector to the index -// VecSimIndex_AddVector(index, vector, 1); - -// // Verify the vector is in the flat buffer -// auto *flat_index = reinterpret_cast*>(CastToBruteForce()); -// auto *hnsw_index = CastToHNSW(); -// ASSERT_EQ(flat_index->indexSize(), 1); -// ASSERT_EQ(hnsw_index->indexSize(), 0); - -// // Get the vector from the flat buffer -// const int8_t *flat_vector = reinterpret_cast(flat_index->getDataByInternalId(0)); float vector_norm = -// spaces::IntegralType_ComputeNorm(flat_vector, dim); ASSERT_EQ(index_vector_norm, -// vector_norm) << "wrong vector norm for vector id:" << i; - -// // Run a thread iteration to move the vector from flat to HNSW -// mock_thread_pool.thread_iteration(); - -// // Verify the vector has been moved to HNSW -// ASSERT_EQ(flat_index->indexSize(), 0); -// ASSERT_EQ(hnsw_index->indexSize(), 1); - -// // Get the vector from HNSW -// const int8_t *hnsw_vector = reinterpret_cast(hnsw_index->getDataByInternalId(0)); - -// // Verify the norm is correctly preserved in the HNSW vector -// float norm_in_hnsw = *(reinterpret_cast(hnsw_vector + params.dim)); - -// // This would fail with the bug, as the norm would be truncated -// ASSERT_NEAR(norm_in_flat, norm_in_hnsw, 1e-5); - -// // Calculate the distance from the vector to itself in HNSW -// // For cosine distance to itself, this should be close to 0 -// // We don't use ASSERT_NEAR here because the distance calculation might have some numerical -// issues -// // that are not related to the bug we're testing -// } - -// /** -// * This test specifically targets the bug fix where memcpy was using the wrong size. -// * It demonstrates the difference between using the correct size (getDataSize()) -// * and the incorrect size (dim * sizeof(DataType)). -// */ -// TEST_F(INT8TieredTest, TestInt8CosineBugFix) { -// // Create a tiered HNSW index with int8 vectors and cosine similarity -// HNSWParams params = { -// .type = VecSimType_INT8, -// .dim = 4, -// .metric = VecSimMetric_Cosine, -// .multi = false -// }; - -// // Create the tiered index -// // TieredIndexParams tiered_params = generate_tiered_params(params); -// SetUp(params); - -// // Create a test vector -// int8_t vector[4]; -// test_utils::populate_int8_vec(vector, 4, 42); - -// // Add the vector to the index -// ASSERT_EQ(this->GenerateAndAddVector(i, i), 1); -// // VecSimIndex_AddVector(index, vector, 1); - -// // Get the flat index and the vector from it -// auto *flat_index = CastToBruteForce(); -// const int8_t *flat_vector = reinterpret_cast(flat_index->getDataByInternalId(0)); - -// // Verify the norm is present in the correct copy -// float norm_in_flat = *(reinterpret_cast(flat_vector + params.dim)); - -// ASSERT_NEAR(norm_in_flat, norm_in_correct_copy, 1e-5); - -// // Run a thread iteration to move the vector from flat to HNSW -// mock_thread_pool.thread_iteration(); - -// // Get the HNSW index and the vector from it -// auto *hnsw_index = CastToHNSW(); -// const int8_t *hnsw_vector = reinterpret_cast(hnsw_index->getDataByInternalId(0)); - -// // Verify the norm is correctly preserved in the HNSW vector -// float norm_in_hnsw = *(reinterpret_cast(hnsw_vector + params.dim)); - -// // This would fail with the bug, as the norm would be truncated -// ASSERT_NEAR(norm_in_flat, norm_in_hnsw, 1e-5); -// } +/** + * Tests int8_t vectors with cosine similarity in a tiered index across three scenarios: + * 1. Verifies vector data correctness when stored in the flat buffer + * 2. Verifies vector data correctness when inserted directly into HNSW (when flat buffer is full) + * 3. Verifies vector data correctness after transfer from flat buffer to HNSW + * + * For each scenario, the test confirms: + * - Vector data matches the expected normalized vector + * - The norm is correctly stored at the end of the vector + * - Search operations (topK, range, batch) return the expected results + */ + +TEST_F(INT8TieredTest, CosineBlobCorrectness) { + // Create TieredHNSW index with cosine metric + constexpr size_t dim = 4; + HNSWParams hnsw_params = {.dim = dim, .metric = VecSimMetric_Cosine}; + // Create tiered index with buffer limit set to 1. + TieredIndexParams tiered_params = this->generate_tiered_params(hnsw_params, 1, 1); + SetUp(tiered_params); + + auto frontend_index = this->CastToBruteForce(); + auto hnsw_index = this->CastToHNSW(); + + int8_t vector[dim]; + PopulateRandomVector(vector); + float vector_norm = spaces::IntegralType_ComputeNorm(vector, dim); + + auto verify_norm = [&](const int8_t *input_vector, float expected_norm) { + float vectors_stored_norm = *(reinterpret_cast(input_vector + dim)); + ASSERT_EQ(vectors_stored_norm, expected_norm) << "wrong vector norm"; + }; + + int8_t normalized_vec[dim + sizeof(float)]; + memcpy(normalized_vec, vector, dim); + spaces::integer_normalizeVector(normalized_vec, dim); + ASSERT_NO_FATAL_FAILURE(verify_norm(normalized_vec, vector_norm)); + + int8_t query[dim + sizeof(float)]; + PopulateRandomVector(query); + float query_norm = spaces::IntegralType_ComputeNorm(query, dim); + + // Calculate the expected score manually. + int ip = 0; + for (size_t i = 0; i < dim; i++) { + ip += vector[i] * query[i]; + } + float expected_score = 1.0 - (float(ip) / (vector_norm * query_norm)); + + auto verify_res = [&](size_t label, double score, size_t result_rank) { + ASSERT_EQ(score, expected_score) << "label: " << label; + }; + + // ============== Scenario 1: + // blob correctness in the flat buffer + + // Add a vector to the flat buffer. + VecSimIndex_AddVector(index, vector, 0); + { + SCOPED_TRACE("Store in the flat buffer"); + // Get the stored vector data including the norm + auto stored_vec = frontend_index->getStoredVectorDataByLabel(0); + const int8_t *stored_vec_data = reinterpret_cast(stored_vec.at(0).data()); + // the vector should be normalized. + ASSERT_NO_FATAL_FAILURE(CompareVectors(stored_vec_data, normalized_vec, dim)); + // The norm should be stored in the last position. + verify_norm(stored_vec_data, vector_norm); + + ASSERT_NO_FATAL_FAILURE(runTopKSearchTest(index, query, 1, verify_res)); + ASSERT_NO_FATAL_FAILURE(runRangeQueryTest(index, query, 0.5, verify_res, 1, BY_SCORE)); + VecSimBatchIterator *batchIterator = VecSimBatchIterator_New(index, query, nullptr); + ASSERT_NO_FATAL_FAILURE(runBatchIteratorSearchTest(batchIterator, 1, verify_res)); + VecSimBatchIterator_Free(batchIterator); + } + + // ============== Scenario 2: + // blob correctness when inserted directly to the hnsw + + // Add another vector and exceed the flat buffer capacity. The vector should be stored directly + // in the hnsw index + VecSimIndex_AddVector(index, vector, 1); + EXPECT_EQ(frontend_index->indexSize(), 1); + EXPECT_EQ(hnsw_index->indexSize(), 1); + { + SCOPED_TRACE("Full buffer; add vector directly to hnsw"); + auto stored_vec = hnsw_index->getStoredVectorDataByLabel(1); + const int8_t *stored_vec_data = reinterpret_cast(stored_vec.at(0).data()); + // the vector should be normalized. + ASSERT_NO_FATAL_FAILURE(CompareVectors(stored_vec_data, normalized_vec, dim)); + // The norm should be stored in the last position. + verify_norm(stored_vec_data, vector_norm); + + size_t k = 2; + ASSERT_NO_FATAL_FAILURE(runTopKSearchTest(index, query, k, verify_res)); + ASSERT_NO_FATAL_FAILURE(runRangeQueryTest(index, query, 100, verify_res, k, BY_SCORE)); + VecSimBatchIterator *batchIterator = VecSimBatchIterator_New(index, query, nullptr); + ASSERT_NO_FATAL_FAILURE(runBatchIteratorSearchTest(batchIterator, k, verify_res)); + VecSimBatchIterator_Free(batchIterator); + } + + // ============== Scenario 3: + // blob correctness after transferred to the hnsw + + // Move the first vector to the hnsw index. + mock_thread_pool.thread_iteration(); + EXPECT_EQ(frontend_index->indexSize(), 0); + EXPECT_EQ(hnsw_index->indexSize(), 2); + { + SCOPED_TRACE("Execute insertion job"); + auto stored_vec = hnsw_index->getStoredVectorDataByLabel(0); + const int8_t *stored_vec_data = reinterpret_cast(stored_vec.at(0).data()); + // the vector should be normalized. + ASSERT_NO_FATAL_FAILURE(CompareVectors(stored_vec_data, normalized_vec, dim)); + // The norm should be stored in the last position. + verify_norm(stored_vec_data, vector_norm); + + size_t k = 2; + ASSERT_NO_FATAL_FAILURE(runTopKSearchTest(index, query, k, verify_res)); + ASSERT_NO_FATAL_FAILURE(runRangeQueryTest(index, query, 100, verify_res, k, BY_SCORE)); + VecSimBatchIterator *batchIterator = VecSimBatchIterator_New(index, query, nullptr); + ASSERT_NO_FATAL_FAILURE(runBatchIteratorSearchTest(batchIterator, k, verify_res)); + VecSimBatchIterator_Free(batchIterator); + } +} From 80d5cec1b8ea72d8a1b51151664279ce45121c50 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Fri, 2 May 2025 05:06:57 +0000 Subject: [PATCH 06/21] build test with VERBOSE=1 --- .github/workflows/event-pull_request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/event-pull_request.yml b/.github/workflows/event-pull_request.yml index a46b04c41..18164e144 100644 --- a/.github/workflows/event-pull_request.yml +++ b/.github/workflows/event-pull_request.yml @@ -28,7 +28,7 @@ jobs: - name: check format run: make check-format - name: unit tests - run: make unit_test + run: make unit_test VERBOSE=1 - name: flow tests run: make flow_test VERBOSE=1 From 3fe0515f7b832abcbd3b37cf28bd95af5d121e5f Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sat, 3 May 2025 04:28:13 +0000 Subject: [PATCH 07/21] verbose cov --- .github/workflows/event-pull_request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/event-pull_request.yml b/.github/workflows/event-pull_request.yml index 18164e144..d85beadc5 100644 --- a/.github/workflows/event-pull_request.yml +++ b/.github/workflows/event-pull_request.yml @@ -33,7 +33,7 @@ jobs: run: make flow_test VERBOSE=1 coverage: - needs: [basic-tests] + # needs: [basic-tests] if: ${{ !github.event.pull_request.draft}} uses: ./.github/workflows/coverage.yml secrets: inherit From b097a206af04280e946e01c33132bb3b3917694a Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sat, 3 May 2025 04:52:32 +0000 Subject: [PATCH 08/21] verbose cov fix assert in FP32SpacesOptimizationTest::FP32InnerProductTest --- .github/workflows/coverage.yml | 4 ++-- tests/unit/test_spaces.cpp | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index f927edbdb..5b02c54d7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -37,11 +37,11 @@ jobs: - name: checkout uses: actions/checkout@v4 with: - submodules: recursive + submodules: recursiveß - name: install dependencies run: sudo .install/install_script.sh - name: run codecov - run: make coverage + run: make coverage VERBOSE=1 - name: Upload coverage to Codecov uses: codecov/codecov-action@v4 with: diff --git a/tests/unit/test_spaces.cpp b/tests/unit/test_spaces.cpp index 0add313ad..fd6375a57 100644 --- a/tests/unit/test_spaces.cpp +++ b/tests/unit/test_spaces.cpp @@ -509,6 +509,8 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { // CPU_FEATURES_ARCH_X86_64 #ifdef OPT_AVX512F if (optimization.avx512f) { + ASSERT_EQ(false, true(dim)) + << "hello i am in FP32InnerProductTest::if (optimization.avx512f) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_FP32_IP_implementation_AVX512F(dim)) @@ -520,6 +522,8 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { #endif #ifdef OPT_AVX if (optimization.avx) { + ASSERT_EQ(false, true(dim)) + << "hello i am in FP32InnerProductTest::if (optimization.avx) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_FP32_IP_implementation_AVX(dim)) @@ -531,6 +535,8 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { #endif #ifdef OPT_SSE if (optimization.sse) { + ASSERT_EQ(false, true(dim)) + << "hello i am in FP32InnerProductTest::if (optimization.sse) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_FP32_IP_implementation_SSE(dim)) From b10dd24790050c64071af92417e81e439380b4b7 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sat, 3 May 2025 14:19:05 +0000 Subject: [PATCH 09/21] fix --- tests/unit/test_spaces.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_spaces.cpp b/tests/unit/test_spaces.cpp index fd6375a57..4ab3f39b0 100644 --- a/tests/unit/test_spaces.cpp +++ b/tests/unit/test_spaces.cpp @@ -509,7 +509,7 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { // CPU_FEATURES_ARCH_X86_64 #ifdef OPT_AVX512F if (optimization.avx512f) { - ASSERT_EQ(false, true(dim)) + ASSERT_EQ(false, true) << "hello i am in FP32InnerProductTest::if (optimization.avx512f) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); @@ -522,7 +522,7 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { #endif #ifdef OPT_AVX if (optimization.avx) { - ASSERT_EQ(false, true(dim)) + ASSERT_EQ(false, true) << "hello i am in FP32InnerProductTest::if (optimization.avx) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); @@ -535,7 +535,7 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { #endif #ifdef OPT_SSE if (optimization.sse) { - ASSERT_EQ(false, true(dim)) + ASSERT_EQ(false, true) << "hello i am in FP32InnerProductTest::if (optimization.sse) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); From 6b2a89e74db8a44b14f24b2c412c49ef0123dc4c Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 4 May 2025 10:31:45 +0000 Subject: [PATCH 10/21] add force_copy to maybeCopyToAlignedMem and preprocessQuery use preprocessQuery with force_copy in batch iterator instead on preprocessQueryInPlace --- .github/workflows/coverage.yml | 4 +-- cmake/svs.cmake | 4 +-- .../algorithms/brute_force/brute_force.h | 11 ++++---- .../brute_force/brute_force_single.h | 2 +- src/VecSim/algorithms/hnsw/hnsw_multi.h | 11 ++++---- src/VecSim/algorithms/hnsw/hnsw_single.h | 11 ++++---- .../computer/preprocessor_container.cpp | 25 +++++++++---------- .../spaces/computer/preprocessor_container.h | 17 +++++++------ src/VecSim/vec_sim_index.h | 10 +++++--- 9 files changed, 50 insertions(+), 45 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 5b02c54d7..f927edbdb 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -37,11 +37,11 @@ jobs: - name: checkout uses: actions/checkout@v4 with: - submodules: recursiveß + submodules: recursive - name: install dependencies run: sudo .install/install_script.sh - name: run codecov - run: make coverage VERBOSE=1 + run: make coverage - name: Upload coverage to Codecov uses: codecov/codecov-action@v4 with: diff --git a/cmake/svs.cmake b/cmake/svs.cmake index 3270c14d2..b583e8e71 100644 --- a/cmake/svs.cmake +++ b/cmake/svs.cmake @@ -62,8 +62,8 @@ if(USE_SVS) find_package(svs REQUIRED) set(SVS_LVQ_HEADER "svs/extensions/vamana/lvq.h") else() - # This file is included from both CMakeLists.txt and python_bindings/CMakeLists.txt - # Set `root` relative to this file, regardless of where it is included from. + # This file is included from both CMakeLists.txt and python_bindings/CMakeLists.txt + # Set `root` relative to this file, regardless of where it is included from. get_filename_component(root ${CMAKE_CURRENT_LIST_DIR}/.. ABSOLUTE) add_subdirectory( ${root}/deps/ScalableVectorSearch diff --git a/src/VecSim/algorithms/brute_force/brute_force.h b/src/VecSim/algorithms/brute_force/brute_force.h index 42bf12654..338fadf18 100644 --- a/src/VecSim/algorithms/brute_force/brute_force.h +++ b/src/VecSim/algorithms/brute_force/brute_force.h @@ -374,14 +374,13 @@ template VecSimBatchIterator * BruteForceIndex::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { - auto *queryBlobCopy = - this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->dim * sizeof(DataType)); + // force_copy == true. + auto queryBlobCopy = this->preprocessQuery(queryBlob, true); - // memcpy(queryBlobCopy, queryBlob, this->getDataSize()); - this->preprocessQueryInPlace(queryBlobCopy); + // take ownership of the blob copy and pass it to the batch iterator. + auto *queryBlobCopyPtr = queryBlobCopy.release(); // Ownership of queryBlobCopy moves to BF_BatchIterator that will free it at the end. - return newBatchIterator_Instance(queryBlobCopy, queryParams); + return newBatchIterator_Instance(queryBlobCopyPtr, queryParams); } template diff --git a/src/VecSim/algorithms/brute_force/brute_force_single.h b/src/VecSim/algorithms/brute_force/brute_force_single.h index 96f6ef65d..7b1bf635f 100644 --- a/src/VecSim/algorithms/brute_force/brute_force_single.h +++ b/src/VecSim/algorithms/brute_force/brute_force_single.h @@ -39,7 +39,7 @@ class BruteForceIndex_Single : public BruteForceIndex { // We call this when we KNOW that the label exists in the index. idType getIdOfLabel(labelType label) const { return labelToIdLookup.find(label)->second; } -// #define BUILD_TESTS + #ifdef BUILD_TESTS void getDataByLabel(labelType label, std::vector> &vectors_output) const override { diff --git a/src/VecSim/algorithms/hnsw/hnsw_multi.h b/src/VecSim/algorithms/hnsw/hnsw_multi.h index 9048e9738..89433ff47 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_multi.h +++ b/src/VecSim/algorithms/hnsw/hnsw_multi.h @@ -217,13 +217,14 @@ template VecSimBatchIterator * HNSWIndex_Multi::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { - auto queryBlobCopy = - this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->getDataSize()); - this->preprocessQueryInPlace(queryBlobCopy); + // force_copy == true. + auto queryBlobCopy = this->preprocessQuery(queryBlob, true); + + // take ownership of the blob copy and pass it to the batch iterator. + auto *queryBlobCopyPtr = queryBlobCopy.release(); // Ownership of queryBlobCopy moves to HNSW_BatchIterator that will free it at the end. return new (this->allocator) HNSWMulti_BatchIterator( - queryBlobCopy, this, queryParams, this->allocator); + queryBlobCopyPtr, this, queryParams, this->allocator); } /** diff --git a/src/VecSim/algorithms/hnsw/hnsw_single.h b/src/VecSim/algorithms/hnsw/hnsw_single.h index 2993f7c19..b1ad68489 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_single.h +++ b/src/VecSim/algorithms/hnsw/hnsw_single.h @@ -174,13 +174,14 @@ template VecSimBatchIterator * HNSWIndex_Single::newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const { - auto queryBlobCopy = - this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->getDataSize()); - this->preprocessQueryInPlace(queryBlobCopy); + // force_copy == true. + auto queryBlobCopy = this->preprocessQuery(queryBlob, true); + + // take ownership of the blob copy and pass it to the batch iterator. + auto *queryBlobCopyPtr = queryBlobCopy.release(); // Ownership of queryBlobCopy moves to HNSW_BatchIterator that will free it at the end. return new (this->allocator) HNSWSingle_BatchIterator( - queryBlobCopy, this, queryParams, this->allocator); + queryBlobCopyPtr, this, queryParams, this->allocator); } /** diff --git a/src/VecSim/spaces/computer/preprocessor_container.cpp b/src/VecSim/spaces/computer/preprocessor_container.cpp index bcf840fdb..b30bfe9c1 100644 --- a/src/VecSim/spaces/computer/preprocessor_container.cpp +++ b/src/VecSim/spaces/computer/preprocessor_container.cpp @@ -18,10 +18,9 @@ PreprocessorsContainerAbstract::preprocessForStorage(const void *original_blob, return wrapWithDummyDeleter(const_cast(original_blob)); } -MemoryUtils::unique_blob -PreprocessorsContainerAbstract::preprocessQuery(const void *original_blob, - size_t processed_bytes_count) const { - return maybeCopyToAlignedMem(original_blob, processed_bytes_count); +MemoryUtils::unique_blob PreprocessorsContainerAbstract::preprocessQuery( + const void *original_blob, size_t processed_bytes_count, bool force_copy) const { + return maybeCopyToAlignedMem(original_blob, processed_bytes_count, force_copy); } void PreprocessorsContainerAbstract::preprocessQueryInPlace(void *blob, @@ -30,15 +29,15 @@ void PreprocessorsContainerAbstract::preprocessQueryInPlace(void *blob, void PreprocessorsContainerAbstract::preprocessStorageInPlace(void *blob, size_t processed_bytes_count) const {} -MemoryUtils::unique_blob -PreprocessorsContainerAbstract::maybeCopyToAlignedMem(const void *original_blob, - size_t blob_bytes_count) const { - if (this->alignment) { - if ((uintptr_t)original_blob % this->alignment) { - auto aligned_mem = this->allocator->allocate_aligned(blob_bytes_count, this->alignment); - memcpy(aligned_mem, original_blob, blob_bytes_count); - return this->wrapAllocated(aligned_mem); - } +MemoryUtils::unique_blob PreprocessorsContainerAbstract::maybeCopyToAlignedMem( + const void *original_blob, size_t blob_bytes_count, bool force_copy) const { + bool needs_copy = + force_copy || (this->alignment && ((uintptr_t)original_blob % this->alignment != 0)); + + if (needs_copy) { + auto aligned_mem = this->allocator->allocate_aligned(blob_bytes_count, this->alignment); + memcpy(aligned_mem, original_blob, blob_bytes_count); + return this->wrapAllocated(aligned_mem); } // Returning a unique_ptr with a no-op deleter diff --git a/src/VecSim/spaces/computer/preprocessor_container.h b/src/VecSim/spaces/computer/preprocessor_container.h index 36547d52b..7262ecdd6 100644 --- a/src/VecSim/spaces/computer/preprocessor_container.h +++ b/src/VecSim/spaces/computer/preprocessor_container.h @@ -27,7 +27,8 @@ class PreprocessorsContainerAbstract : public VecsimBaseObject { size_t processed_bytes_count) const; virtual MemoryUtils::unique_blob preprocessQuery(const void *original_blob, - size_t processed_bytes_count) const; + size_t processed_bytes_count, + bool force_copy = false) const; virtual void preprocessQueryInPlace(void *blob, size_t processed_bytes_count) const; @@ -40,7 +41,8 @@ class PreprocessorsContainerAbstract : public VecsimBaseObject { // Allocate and copy the blob only if the original blob is not aligned. MemoryUtils::unique_blob maybeCopyToAlignedMem(const void *original_blob, - size_t blob_bytes_count) const; + size_t blob_bytes_count, + bool force_copy = false) const; MemoryUtils::unique_blob wrapAllocated(void *blob) const { return MemoryUtils::unique_blob( @@ -85,7 +87,8 @@ class MultiPreprocessorsContainer : public PreprocessorsContainerAbstract { size_t processed_bytes_count) const override; MemoryUtils::unique_blob preprocessQuery(const void *original_blob, - size_t processed_bytes_count) const override; + size_t processed_bytes_count, + bool force_copy = false) const override; void preprocessQueryInPlace(void *blob, size_t processed_bytes_count) const override; @@ -216,7 +219,7 @@ MultiPreprocessorsContainer::preprocessForStorage( template MemoryUtils::unique_blob MultiPreprocessorsContainer::preprocessQuery( - const void *original_blob, size_t processed_bytes_count) const { + const void *original_blob, size_t processed_bytes_count, bool force_copy) const { void *query_blob = nullptr; for (auto pp : preprocessors) { @@ -225,9 +228,9 @@ MemoryUtils::unique_blob MultiPreprocessorsContainer: // modifies the memory in place pp->preprocessQuery(original_blob, query_blob, processed_bytes_count, this->alignment); } - return query_blob - ? std::move(this->wrapAllocated(query_blob)) - : std::move(this->maybeCopyToAlignedMem(original_blob, processed_bytes_count)); + return query_blob ? std::move(this->wrapAllocated(query_blob)) + : std::move(this->maybeCopyToAlignedMem(original_blob, processed_bytes_count, + force_copy)); } template diff --git a/src/VecSim/vec_sim_index.h b/src/VecSim/vec_sim_index.h index 1593c6d53..65eedeb43 100644 --- a/src/VecSim/vec_sim_index.h +++ b/src/VecSim/vec_sim_index.h @@ -143,10 +143,11 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { /** * @brief Preprocess a blob for query. * - * @param queryBlob will be copied. + * @param queryBlob will be copied if preprocessing is required, or if force_copy is set to + * true. * @return unique_ptr of the processed blob. */ - MemoryUtils::unique_blob preprocessQuery(const void *queryBlob) const; + MemoryUtils::unique_blob preprocessQuery(const void *queryBlob, bool force_copy = false) const; /** * @brief Preprocess a blob for storage. @@ -289,8 +290,9 @@ ProcessedBlobs VecSimIndexAbstract::preprocess(const void *b template MemoryUtils::unique_blob -VecSimIndexAbstract::preprocessQuery(const void *queryBlob) const { - return this->preprocessors->preprocessQuery(queryBlob, this->dataSize); +VecSimIndexAbstract::preprocessQuery(const void *queryBlob, + bool force_copy) const { + return this->preprocessors->preprocessQuery(queryBlob, this->dataSize, force_copy); } template From c71876bf6bcdc9f6fc76d8c2bfd3db217f95ce84 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 4 May 2025 10:35:02 +0000 Subject: [PATCH 11/21] also batch in svs --- src/VecSim/algorithms/svs/svs.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index c0a709055..2f41bde47 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -426,17 +426,18 @@ class SVSIndex : public VecSimIndexAbstract, fl VecSimBatchIterator *newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const override { - auto *queryBlobCopy = - this->allocator->allocate_aligned(this->dataSize, this->preprocessors->getAlignment()); - memcpy(queryBlobCopy, queryBlob, this->getDataSize()); - this->preprocessQueryInPlace(queryBlobCopy); + // force_copy == true. + auto queryBlobCopy = this->preprocessQuery(queryBlob, true); + + // take ownership of the blob copy and pass it to the batch iterator. + auto *queryBlobCopyPtr = queryBlobCopy.release(); // Ownership of queryBlobCopy moves to VecSimBatchIterator that will free it at the end. if (indexSize() == 0) { return new (this->getAllocator()) - NullSVS_BatchIterator(queryBlobCopy, queryParams, this->getAllocator()); + NullSVS_BatchIterator(queryBlobCopyPtr, queryParams, this->getAllocator()); } else { return new (this->getAllocator()) SVS_BatchIterator( - queryBlobCopy, impl_.get(), queryParams, this->getAllocator()); + queryBlobCopyPtr, impl_.get(), queryParams, this->getAllocator()); } } From 07f9ec12a540ac431af3779b51a8169f38aba59b Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 4 May 2025 10:44:59 +0000 Subject: [PATCH 12/21] revert redundancies --- .../brute_force/brute_force_single.h | 1 + src/VecSim/algorithms/hnsw/hnsw_tiered.h | 51 ++----------------- tests/unit/test_spaces.cpp | 6 --- 3 files changed, 5 insertions(+), 53 deletions(-) diff --git a/src/VecSim/algorithms/brute_force/brute_force_single.h b/src/VecSim/algorithms/brute_force/brute_force_single.h index 7b1bf635f..992c02aa4 100644 --- a/src/VecSim/algorithms/brute_force/brute_force_single.h +++ b/src/VecSim/algorithms/brute_force/brute_force_single.h @@ -43,6 +43,7 @@ class BruteForceIndex_Single : public BruteForceIndex { #ifdef BUILD_TESTS void getDataByLabel(labelType label, std::vector> &vectors_output) const override { + auto id = labelToIdLookup.at(label); auto vec = std::vector(this->dim); diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index a537c35d9..e781b260f 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -63,8 +63,6 @@ struct HNSWRepairJob : public AsyncJob { template class TieredHNSWIndex : public VecSimTieredIndex { private: - size_t bg_vector_indexing_count; - size_t main_vector_indexing_count; /// Mappings from id/label to associated jobs, for invalidating and update ids if necessary. // In MULTI, we can have more than one insert job pending per label. // **This map is protected with the flat buffer lock** @@ -199,7 +197,9 @@ class TieredHNSWIndex : public VecSimTieredIndex { VecSimDebugInfoIterator *debugInfoIterator() const override; VecSimBatchIterator *newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const override { - size_t blobSize = this->frontendIndex->getDataSize(); + // Here the blob is expected to bo user's input, so it's size doesn't include preprocessing + // overhead, if any. + size_t blobSize = this->frontendIndex->getDim() * sizeof(DataType); void *queryBlobCopy = this->allocator->allocate(blobSize); memcpy(queryBlobCopy, queryBlob, blobSize); return new (this->allocator) @@ -637,11 +637,9 @@ TieredHNSWIndex::TieredHNSWIndex(HNSWIndex allocator) : VecSimTieredIndex(hnsw_index, bf_index, tiered_index_params, allocator), - bg_vector_indexing_count(0), main_vector_indexing_count(0), labelToInsertJobs(this->allocator), idToRepairJobs(this->allocator), idToSwapJob(this->allocator), invalidJobs(this->allocator), currInvalidJobId(0), readySwapJobs(0) { - // If the param for swapJobThreshold is 0 use the default value, if it exceeds the maximum // allowed, use the maximum value. this->pendingSwapJobsThreshold = @@ -705,7 +703,7 @@ size_t TieredHNSWIndex::indexLabelCount() const { this->mainIndexGuard.unlock(); return output.size(); } -#include "VecSim/vec_sim_debug.h" + // In the tiered index, we assume that the blobs are processed by the flat buffer // before being transferred to the HNSW index. // When inserting vectors directly into the HNSW index—such as in VecSim_WriteInPlace mode— or when @@ -715,40 +713,6 @@ template int TieredHNSWIndex::addVector(const void *blob, labelType label) { int ret = 1; auto hnsw_index = this->getHNSWIndex(); - // if (label == 100000) { - // std::string res("index connections: {"); - - // res += "Entry Point Label: "; - // res += std::to_string(hnsw_index->getEntryPointLabel()) + "\n"; - // TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, - // "%s", res.c_str()); - // for (idType id = 0; id < this->backendIndex->indexSize(); id++) { - // std::string n_data("Node "); - // labelType label = hnsw_index->getExternalLabel(id); - // if (label == SIZE_MAX) - // continue; // The ID is not in the index - // int **neighbors_output; - // VecSimDebug_GetElementNeighborsInHNSWGraph(this->backendIndex, label, - // &neighbors_output); n_data += std::to_string(label) + ": "; for (size_t l = 0; - // neighbors_output[l]; l++) { - // n_data += "Level " + std::to_string(l) + " neighbors: "; - // auto &neighbours = neighbors_output[l]; - // auto neighbours_count = neighbours[0]; - // for (size_t j = 1; j <= neighbours_count; j++) { - // n_data += std::to_string(neighbours[j]) + ", "; - // } - // TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, - // "%s", n_data.c_str()); - // } - // VecSimDebug_ReleaseElementNeighborsInHNSWGraph(neighbors_output); - // } - // } - if (label == 999999) { - TIERED_LOG(VecSimCommonStrings::LOG_WARNING_STRING, - "bg_vec_count: %zu, main_thread_vector_count: %zu", - this->bg_vector_indexing_count, this->main_vector_indexing_count); - } - // writeMode is not protected since it is assumed to be called only from the "main thread" // (that is the thread that is exclusively calling add/delete vector). if (this->getWriteMode() == VecSim_WriteInPlace) { @@ -773,11 +737,6 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l // This will do nothing (and return 0) if this label doesn't exist. Otherwise, it may // remove vector from the flat buffer and/or the HNSW index. ret -= this->deleteVector(label); - if (ret != 1) { - TIERED_LOG(VecSimCommonStrings::LOG_VERBOSE_STRING, - "removed a vector while trying to insert label %zu. ret: %d", label, - ret); - } } if (this->frontendIndex->indexSize() >= this->flatBufferLimit) { // We didn't remove a vector from flat buffer due to overwrite, insert the new vector @@ -787,13 +746,11 @@ int TieredHNSWIndex::addVector(const void *blob, labelType l // index. auto storage_blob = this->frontendIndex->preprocessForStorage(blob); this->insertVectorToHNSW(hnsw_index, label, storage_blob.get()); - this->main_vector_indexing_count++; return ret; } // Otherwise, we fall back to the "regular" insertion into the flat buffer // (since it is not full anymore after removing the previous vector stored under the label). } - this->bg_vector_indexing_count++; this->flatIndexGuard.lock(); idType new_flat_id = this->frontendIndex->indexSize(); if (this->frontendIndex->isLabelExists(label) && !this->frontendIndex->isMultiValue()) { diff --git a/tests/unit/test_spaces.cpp b/tests/unit/test_spaces.cpp index 4ab3f39b0..0add313ad 100644 --- a/tests/unit/test_spaces.cpp +++ b/tests/unit/test_spaces.cpp @@ -509,8 +509,6 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { // CPU_FEATURES_ARCH_X86_64 #ifdef OPT_AVX512F if (optimization.avx512f) { - ASSERT_EQ(false, true) - << "hello i am in FP32InnerProductTest::if (optimization.avx512f) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_FP32_IP_implementation_AVX512F(dim)) @@ -522,8 +520,6 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { #endif #ifdef OPT_AVX if (optimization.avx) { - ASSERT_EQ(false, true) - << "hello i am in FP32InnerProductTest::if (optimization.avx) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_FP32_IP_implementation_AVX(dim)) @@ -535,8 +531,6 @@ TEST_P(FP32SpacesOptimizationTest, FP32InnerProductTest) { #endif #ifdef OPT_SSE if (optimization.sse) { - ASSERT_EQ(false, true) - << "hello i am in FP32InnerProductTest::if (optimization.sse) "; unsigned char alignment = 0; arch_opt_func = IP_FP32_GetDistFunc(dim, &alignment, &optimization); ASSERT_EQ(arch_opt_func, Choose_FP32_IP_implementation_SSE(dim)) From a618046bc65c1ddfe0a12dacf2ccc79ec5aad21f Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 4 May 2025 11:17:22 +0000 Subject: [PATCH 13/21] tiered batch iterator doesn't copy the blob --- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index e781b260f..654dfc903 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -163,7 +163,7 @@ class TieredHNSWIndex : public VecSimTieredIndex { inline void filter_irrelevant_results(VecSimQueryResultContainer &); public: - TieredHNSW_BatchIterator(void *query_vector, + TieredHNSW_BatchIterator(const void *query_vector, const TieredHNSWIndex *index, VecSimQueryParams *queryParams, std::shared_ptr allocator); @@ -197,13 +197,9 @@ class TieredHNSWIndex : public VecSimTieredIndex { VecSimDebugInfoIterator *debugInfoIterator() const override; VecSimBatchIterator *newBatchIterator(const void *queryBlob, VecSimQueryParams *queryParams) const override { - // Here the blob is expected to bo user's input, so it's size doesn't include preprocessing - // overhead, if any. - size_t blobSize = this->frontendIndex->getDim() * sizeof(DataType); - void *queryBlobCopy = this->allocator->allocate(blobSize); - memcpy(queryBlobCopy, queryBlob, blobSize); + // The query blob will be processed and copied by the internal indexes's batch iterator. return new (this->allocator) - TieredHNSW_BatchIterator(queryBlobCopy, this, queryParams, this->allocator); + TieredHNSW_BatchIterator(queryBlob, this, queryParams, this->allocator); } inline void setLastSearchMode(VecSearchMode mode) override { return this->backendIndex->setLastSearchMode(mode); @@ -919,9 +915,10 @@ double TieredHNSWIndex::getDistanceFrom_Unsafe(labelType lab template TieredHNSWIndex::TieredHNSW_BatchIterator::TieredHNSW_BatchIterator( - void *query_vector, const TieredHNSWIndex *index, + const void *query_vector, const TieredHNSWIndex *index, VecSimQueryParams *queryParams, std::shared_ptr allocator) - : VecSimBatchIterator(query_vector, queryParams ? queryParams->timeoutCtx : nullptr, + // Tiered batch iterator doesn't hold its own copy of the query vector. + : VecSimBatchIterator(nullptr, queryParams ? queryParams->timeoutCtx : nullptr, std::move(allocator)), index(index), flat_results(this->allocator), hnsw_results(this->allocator), flat_iterator(this->index->frontendIndex->newBatchIterator(query_vector, queryParams)), From a038e1de41eb4eea805cf967be9f9804a2d02bbd Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 4 May 2025 13:11:43 +0000 Subject: [PATCH 14/21] add // TODO: handle original_blob_size != processed_bytes_count --- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 3 ++- src/VecSim/algorithms/svs/svs.h | 1 + src/VecSim/spaces/computer/preprocessor_container.cpp | 1 + src/VecSim/spaces/computer/preprocessors.h | 5 +++++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 654dfc903..55f9fa00b 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -538,7 +538,8 @@ void TieredHNSWIndex::executeInsertJob(HNSWInsertJob *job) { // indexing the vector into HNSW index. size_t data_size = this->frontendIndex->getDataSize(); auto blob_copy = this->getAllocator()->allocate_unique(data_size); - + // Assuming the size of the blob stored in the frontend index matches the size of the blob + // stored in the HNSW index. memcpy(blob_copy.get(), this->frontendIndex->getDataByInternalId(job->id), data_size); this->insertVectorToHNSW(hnsw_index, job->label, blob_copy.get()); diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 2f41bde47..3ab9c1758 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -151,6 +151,7 @@ class SVSIndex : public VecSimIndexAbstract, fl auto processed_blob = MemoryUtils::unique_blob{this->allocator->allocate(data_size), [this](void *ptr) { this->allocator->free_allocation(ptr); }}; + // Assuming original data size equals to processed data size memcpy(processed_blob.get(), original_data, data_size); // Preprocess each vector in place for (size_t i = 0; i < n; i++) { diff --git a/src/VecSim/spaces/computer/preprocessor_container.cpp b/src/VecSim/spaces/computer/preprocessor_container.cpp index b30bfe9c1..329b5998d 100644 --- a/src/VecSim/spaces/computer/preprocessor_container.cpp +++ b/src/VecSim/spaces/computer/preprocessor_container.cpp @@ -36,6 +36,7 @@ MemoryUtils::unique_blob PreprocessorsContainerAbstract::maybeCopyToAlignedMem( if (needs_copy) { auto aligned_mem = this->allocator->allocate_aligned(blob_bytes_count, this->alignment); + // TODO: handle original_blob_size != processed_bytes_count memcpy(aligned_mem, original_blob, blob_bytes_count); return this->wrapAllocated(aligned_mem); } diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 80f6100ce..dabcb7bef 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -55,9 +55,11 @@ class CosinePreprocessor : public PreprocessorInterface { // If one of them is null, allocate memory for it and copy the original_blob to it. if (storage_blob == nullptr) { storage_blob = this->allocator->allocate(processed_bytes_count); + // TODO: handle original_blob_size != processed_bytes_count memcpy(storage_blob, original_blob, processed_bytes_count); } else if (query_blob == nullptr) { query_blob = this->allocator->allocate_aligned(processed_bytes_count, alignment); + // TODO: handle original_blob_size != processed_bytes_count memcpy(query_blob, original_blob, processed_bytes_count); } @@ -68,6 +70,7 @@ class CosinePreprocessor : public PreprocessorInterface { if (query_blob == nullptr) { // If both blobs are null, allocate query_blob and set // storage_blob to point to it. query_blob = this->allocator->allocate_aligned(processed_bytes_count, alignment); + // TODO: handle original_blob_size != processed_bytes_count memcpy(query_blob, original_blob, processed_bytes_count); storage_blob = query_blob; } @@ -80,6 +83,7 @@ class CosinePreprocessor : public PreprocessorInterface { size_t processed_bytes_count) const override { if (blob == nullptr) { blob = this->allocator->allocate(processed_bytes_count); + // TODO: handle original_blob_size != processed_bytes_count memcpy(blob, original_blob, processed_bytes_count); } normalize_func(blob, this->dim); @@ -89,6 +93,7 @@ class CosinePreprocessor : public PreprocessorInterface { unsigned char alignment) const override { if (blob == nullptr) { blob = this->allocator->allocate_aligned(processed_bytes_count, alignment); + // TODO: handle original_blob_size != processed_bytes_count memcpy(blob, original_blob, processed_bytes_count); } normalize_func(blob, this->dim); From 36628953e87895ebc0dcf8ffe97e07b8f0ea3ae0 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 4 May 2025 13:26:51 +0000 Subject: [PATCH 15/21] revert pull request changes --- .github/workflows/event-pull_request.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/event-pull_request.yml b/.github/workflows/event-pull_request.yml index d85beadc5..a46b04c41 100644 --- a/.github/workflows/event-pull_request.yml +++ b/.github/workflows/event-pull_request.yml @@ -28,12 +28,12 @@ jobs: - name: check format run: make check-format - name: unit tests - run: make unit_test VERBOSE=1 + run: make unit_test - name: flow tests run: make flow_test VERBOSE=1 coverage: - # needs: [basic-tests] + needs: [basic-tests] if: ${{ !github.event.pull_request.draft}} uses: ./.github/workflows/coverage.yml secrets: inherit From 5a672eaf3f35b4ec0d296a4a7fd479c6186178e8 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Mon, 5 May 2025 04:51:33 +0000 Subject: [PATCH 16/21] move getDataByLabel and getStoredVectorDataByLabel to VecSimIndexAbstract --- .../algorithms/brute_force/brute_force.h | 38 +----------------- src/VecSim/algorithms/hnsw/hnsw.h | 38 +----------------- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 9 +---- src/VecSim/algorithms/svs/svs.h | 13 +++++- src/VecSim/vec_sim_index.h | 40 ++++++++++++++++++- 5 files changed, 52 insertions(+), 86 deletions(-) diff --git a/src/VecSim/algorithms/brute_force/brute_force.h b/src/VecSim/algorithms/brute_force/brute_force.h index 58d1088fc..77feee052 100644 --- a/src/VecSim/algorithms/brute_force/brute_force.h +++ b/src/VecSim/algorithms/brute_force/brute_force.h @@ -5,7 +5,7 @@ * Licensed under your choice of the Redis Source Available License 2.0 * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the * GNU Affero General Public License v3 (AGPLv3). -*/ + */ #pragma once #include "VecSim/containers/data_block.h" @@ -76,42 +76,6 @@ class BruteForceIndex : public VecSimIndexAbstract { virtual ~BruteForceIndex() = default; #ifdef BUILD_TESTS - /** - * @brief Used for testing - get only the vector elements associated with a given label. - * This function copies only the vector(s) elements into the output vector, - * without any additional metadata that might be stored with the vector(s). - * - * Important: This method returns ONLY the vector elements, even if the stored vector contains - * additional metadata. For example, with int8_t/uint8_t vectors using cosine similarity, - * this method will NOT return the norm that is stored with the vector. - * - * If you need the complete data including any metadata, use getStoredVectorDataByLabel() - * instead. - * - * @param label The label to retrieve vector(s) elements for - * @param vectors_output Empty vector to be filled with vector(s) - */ - virtual void getDataByLabel(labelType label, - std::vector> &vectors_output) const = 0; - - /** - * @brief Used for testing - get the complete raw data associated with a given label. - * This function returns the ENTIRE vector(s) data as stored in the index, including any - * additional metadata that might be stored alongside the vector elements. - * - * For example: - * - For int8_t/uint8_t vectors with cosine similarity, this includes the norm stored at the end - * - For other vector types or future implementations, this will include any additional data - * that might be stored with the vector - * - * Use this method when you need access to the complete vector data as it is stored internally. - * - * @param label The label to retrieve data for - * @return A vector containing the complete vector data (elements + metadata) for the given - * label - */ - virtual std::vector> getStoredVectorDataByLabel(labelType label) const = 0; - void fitMemory() override { if (count == 0) { return; diff --git a/src/VecSim/algorithms/hnsw/hnsw.h b/src/VecSim/algorithms/hnsw/hnsw.h index 2ed6d9f26..54ae973c5 100644 --- a/src/VecSim/algorithms/hnsw/hnsw.h +++ b/src/VecSim/algorithms/hnsw/hnsw.h @@ -5,7 +5,7 @@ * Licensed under your choice of the Redis Source Available License 2.0 * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the * GNU Affero General Public License v3 (AGPLv3). -*/ + */ #pragma once #include "graph_data.h" @@ -302,42 +302,6 @@ class HNSWIndex : public VecSimIndexAbstract, virtual int removeLabel(labelType label) = 0; #ifdef BUILD_TESTS - /** - * @brief Used for testing - get only the vector elements associated with a given label. - * This function copies only the vector(s) elements into the output vector, - * without any additional metadata that might be stored with the vector. - * - * Important: This method returns ONLY the vector elements, even if the stored vector contains - * additional metadata. For example, with int8_t/uint8_t vectors using cosine similarity, - * this method will NOT return the norm that is stored with the vector(s). - * - * If you need the complete data including any metadata, use getStoredVectorDataByLabel() - * instead. - * - * @param label The label to retrieve vector(s) elements for - * @param vectors_output Empty vector to be filled with vector(s) - */ - virtual void getDataByLabel(labelType label, - std::vector> &vectors_output) const = 0; - - /** - * @brief Used for testing - get the complete raw data associated with a given label. - * This function returns the ENTIRE vector(s) data as stored in the index, including any - * additional metadata that might be stored alongside the vector elements. - * - * For example: - * - For int8_t/uint8_t vectors with cosine similarity, this includes the norm stored at the end - * - For other vector types or future implementations, this will include any additional data - * that might be stored with the vector - * - * Use this method when you need access to the complete vector data as it is stored internally. - * - * @param label The label to retrieve data for - * @return A vector containing the complete vector data (elements + metadata) for the given - * label - */ - virtual std::vector> getStoredVectorDataByLabel(labelType label) const = 0; - void fitMemory() override { if (maxElements > 0) { idToMetaData.shrink_to_fit(); diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 90d4fa49a..5f3c3fd2d 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -5,7 +5,7 @@ * Licensed under your choice of the Redis Source Available License 2.0 * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the * GNU Affero General Public License v3 (AGPLv3). -*/ + */ #pragma once @@ -240,8 +240,6 @@ class TieredHNSWIndex : public VecSimTieredIndex { #ifdef BUILD_TESTS void getDataByLabel(labelType label, std::vector> &vectors_output) const; - - std::vector> getStoredVectorDataByLabel(labelType label) const; #endif }; @@ -1195,9 +1193,4 @@ void TieredHNSWIndex::getDataByLabel( this->getHNSWIndex()->getDataByLabel(label, vectors_output); } -template -std::vector> -TieredHNSWIndex::getStoredVectorDataByLabel(labelType label) const { - return this->getHNSWIndex()->getStoredVectorDataByLabel(label); -} #endif diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 5672f0a8d..f90363c2a 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -5,7 +5,7 @@ * Licensed under your choice of the Redis Source Available License 2.0 * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the * GNU Affero General Public License v3 (AGPLv3). -*/ + */ #pragma once #include "VecSim/vec_sim_index.h" @@ -481,6 +481,15 @@ class SVSIndex : public VecSimIndexAbstract, fl } #ifdef BUILD_TESTS - virtual void fitMemory() {}; + void fitMemory() override {} + std::vector> getStoredVectorDataByLabel(labelType label) const override { + assert(nullptr && "Not implemented"); + return {}; + } + void getDataByLabel( + labelType label, + std::vector>> &vectors_output) const override { + assert(nullptr && "Not implemented"); + } #endif }; diff --git a/src/VecSim/vec_sim_index.h b/src/VecSim/vec_sim_index.h index b639c23b9..9730a42ba 100644 --- a/src/VecSim/vec_sim_index.h +++ b/src/VecSim/vec_sim_index.h @@ -5,7 +5,7 @@ * Licensed under your choice of the Redis Source Available License 2.0 * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the * GNU Affero General Public License v3 (AGPLv3). -*/ + */ #pragma once #include "vec_sim_common.h" #include "vec_sim_interface.h" @@ -267,7 +267,6 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { }; return info; } - #ifdef BUILD_TESTS void replacePPContainer(PreprocessorsContainerAbstract *newPPContainer) { delete this->preprocessors; @@ -277,6 +276,43 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { IndexComponents get_components() const { return {.indexCalculator = this->indexCalculator, .preprocessors = this->preprocessors}; } + + /** + * @brief Used for testing - get only the vector elements associated with a given label. + * This function copies only the vector(s) elements into the output vector, + * without any additional metadata that might be stored with the vector. + * + * Important: This method returns ONLY the vector elements, even if the stored vector contains + * additional metadata. For example, with int8_t/uint8_t vectors using cosine similarity, + * this method will NOT return the norm that is stored with the vector(s). + * + * If you need the complete data including any metadata, use getStoredVectorDataByLabel() + * instead. + * + * @param label The label to retrieve vector(s) elements for + * @param vectors_output Empty vector to be filled with vector(s) + */ + virtual void getDataByLabel(labelType label, + std::vector> &vectors_output) const = 0; + + /** + * @brief Used for testing - get the complete raw data associated with a given label. + * This function returns the ENTIRE vector(s) data as stored in the index, including any + * additional metadata that might be stored alongside the vector elements. + * + * For example: + * - For int8_t/uint8_t vectors with cosine similarity, this includes the norm stored at the end + * - For other vector types or future implementations, this will include any additional data + * that might be stored with the vector + * + * Use this method when you need access to the complete vector data as it is stored internally. + * + * @param label The label to retrieve data for + * @return A vector containing the complete vector data (elements + metadata) for the given + * label + */ + virtual std::vector> getStoredVectorDataByLabel(labelType label) const = 0; + #endif protected: From 4fa002a2b99ef0769ccaedde66749964f209e290 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 6 May 2025 12:42:39 +0000 Subject: [PATCH 17/21] add test_index_test_utils currently testing get_stored_vector_data_single_test --- tests/unit/CMakeLists.txt | 3 + tests/unit/test_index_test_utils.cpp | 248 +++++++++++++++++++++++++++ tests/utils/tests_utils.h | 13 ++ 3 files changed, 264 insertions(+) create mode 100644 tests/unit/test_index_test_utils.cpp diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 7b1171faf..4037a269d 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -46,6 +46,7 @@ add_executable(test_bf16 ../utils/mock_thread_pool.cpp test_bf16.cpp unit_test_u add_executable(test_fp16 ../utils/mock_thread_pool.cpp test_fp16.cpp unit_test_utils.cpp) add_executable(test_int8 ../utils/mock_thread_pool.cpp test_int8.cpp unit_test_utils.cpp) add_executable(test_uint8 ../utils/mock_thread_pool.cpp test_uint8.cpp unit_test_utils.cpp) +add_executable(test_index_test_utils ../utils/mock_thread_pool.cpp test_index_test_utils.cpp unit_test_utils.cpp) target_link_libraries(test_hnsw PUBLIC gtest_main VectorSimilarity) target_link_libraries(test_hnsw_parallel PUBLIC gtest_main VectorSimilarity) @@ -59,6 +60,7 @@ target_link_libraries(test_bf16 PUBLIC gtest_main VectorSimilarity) target_link_libraries(test_fp16 PUBLIC gtest_main VectorSimilarity) target_link_libraries(test_int8 PUBLIC gtest_main VectorSimilarity) target_link_libraries(test_uint8 PUBLIC gtest_main VectorSimilarity) +target_link_libraries(test_index_test_utils PUBLIC gtest_main VectorSimilarity) if(USE_SVS) add_executable(test_svs ../utils/mock_thread_pool.cpp test_svs.cpp unit_test_utils.cpp) @@ -79,6 +81,7 @@ gtest_discover_tests(test_bf16 TEST_PREFIX BF16UNIT_) gtest_discover_tests(test_fp16 TEST_PREFIX FP16UNIT_) gtest_discover_tests(test_int8 TEST_PREFIX INT8UNIT_) gtest_discover_tests(test_uint8 TEST_PREFIX UINT8UNIT_) +gtest_discover_tests(test_index_test_utils) if(USE_SVS) gtest_discover_tests(test_svs) diff --git a/tests/unit/test_index_test_utils.cpp b/tests/unit/test_index_test_utils.cpp new file mode 100644 index 000000000..77f91e96b --- /dev/null +++ b/tests/unit/test_index_test_utils.cpp @@ -0,0 +1,248 @@ +/* + * Copyright (c) 2006-Present, Redis Ltd. + * All rights reserved. + * + * Licensed under your choice of the Redis Source Available License 2.0 + * (RSALv2); or (b) the Server Side Public License v1 (SSPLv1); or (c) the + * GNU Affero General Public License v3 (AGPLv3). + */ + +#include "gtest/gtest.h" +#include "VecSim/vec_sim.h" +#include "unit_test_utils.h" +#include "tests_utils.h" +#include "VecSim/algorithms/brute_force/brute_force_multi.h" +#include "VecSim/algorithms/brute_force/brute_force_single.h" +#include "VecSim/algorithms/hnsw/hnsw_multi.h" +#include "VecSim/algorithms/hnsw/hnsw_single.h" +#include "VecSim/spaces/normalize/normalize_naive.h" +#include + +class IndexTestUtilsTest : public testing::TestWithParam> { +protected: + static constexpr size_t dim = 4; + static constexpr size_t labels_count = 5; + + VecSimIndex *index; + bool is_multi = std::get<0>(GetParam()); + VecSimMetric metric = std::get<1>(GetParam()); + size_t vec_per_label = 1; + + void SetUp(HNSWParams ¶ms) { + params.dim = dim; + params.multi = is_multi; + params.metric = metric; + VecSimParams vecsim_params = CreateParams(params); + index = VecSimIndex_New(&vecsim_params); + vec_per_label = this->is_multi ? 3 : 1; + } + + void SetUp(BFParams ¶ms) { + params.dim = dim; + params.multi = is_multi; + params.metric = metric; + VecSimParams vecsim_params = CreateParams(params); + index = VecSimIndex_New(&vecsim_params); + vec_per_label = this->is_multi ? 3 : 1; + } + + void TearDown() { VecSimIndex_Free(index); } + + // id should be unique as it will be used as a seed for the random vector generation + virtual void GenerateRandomAndAddVector(size_t label, size_t id) { + FAIL() << "IndexTestUtilsTest::GenerateRandomAndAddVector this method should be overriden"; + } + + template + void ValidateVectorsImp(std::vector> index_label_vectors, + std::vector> original_vectors, size_t label) { + for (size_t i = 0; i < vec_per_label; i++) { + const DataType *vec = reinterpret_cast(index_label_vectors[i].data()); + for (size_t j = 0; j < dim; j++) { + ASSERT_EQ(vec[j], original_vectors[label * vec_per_label + i][j]); + } + } + } + virtual void ValidateVectors(std::vector> vectors, size_t label) { + FAIL() << "IndexTestUtilsTest::ValidateVectors this method should be overriden"; + } + + virtual std::vector> GetStoredVectorsData(size_t label) { + ADD_FAILURE() + << "IndexTestUtilsTest::GetStoredVectorsData() this method should be overriden"; + return {}; + } + + virtual size_t GetIndexDatasize() { + ADD_FAILURE() << "IndexTestUtilsTest::GetIndexDatasize() this method should be overriden"; + return {}; + } + + // Tests + void get_stored_vector_data_single_test(); +}; + +class Int8IndexTestUtilsTest : public IndexTestUtilsTest { +protected: + std::vector> vectors; + void GenerateRandomAndAddVector(size_t label, size_t id) override { + std::vector v(dim); + test_utils::populate_int8_vec(v.data(), dim, id); + VecSimIndex_AddVector(index, v.data(), label); + + vectors.emplace_back(v); + } + + std::vector> GetStoredVectorsData(size_t label) override { + return (dynamic_cast *>(this->index)) + ->getStoredVectorDataByLabel(label); + } + + size_t GetIndexDatasize() override { + return (dynamic_cast *>(this->index))->getDataSize(); + } + + void ValidateVectors(std::vector> index_vectors, size_t label) override { + IndexTestUtilsTest::ValidateVectorsImp(index_vectors, vectors, label); + } + + void ValidateCosine() { + for (size_t i = 0; i < labels_count; i++) { + auto stored_data = GetStoredVectorsData(i); + for (size_t j = 0; j < stored_data.size(); j++) { + ASSERT_EQ(stored_data[j].size(), dim * sizeof(int8_t) + sizeof(float)); + const int8_t *stored_vec = reinterpret_cast(stored_data[j].data()); + // compute expected norm using the original vector + float expected_norm = + test_utils::integral_compute_norm(vectors[i * vec_per_label + j].data(), dim); + const float *stored_norm = reinterpret_cast(stored_vec + dim); + ASSERT_EQ(*stored_norm, expected_norm) << "wrong vector norm for vector id:" << j; + } + } + } +}; + +class Float32IndexTestUtilsTest : public IndexTestUtilsTest { +protected: + std::vector> vectors; + void GenerateRandomAndAddVector(size_t label, size_t id) override { + std::vector v(dim); + test_utils::populate_float_vec(v.data(), dim, id); + + VecSimIndex_AddVector(index, v.data(), label); + VecSimMetric metric = std::get<1>(GetParam()); + + if (metric == VecSimMetric_Cosine) + VecSim_Normalize(v.data(), dim, VecSimType_FLOAT32); + + vectors.emplace_back(v); + } + + void ValidateVectors(std::vector> index_vectors, size_t label) override { + IndexTestUtilsTest::ValidateVectorsImp(index_vectors, vectors, label); + } + + std::vector> GetStoredVectorsData(size_t label) override { + return (dynamic_cast *>(this->index)) + ->getStoredVectorDataByLabel(label); + } + + size_t GetIndexDatasize() override { + return (dynamic_cast *>(this->index))->getDataSize(); + } +}; + +TEST_P(Int8IndexTestUtilsTest, BF) { + BFParams params = {.type = VecSimType_INT8, .dim = dim}; + SetUp(params); + + EXPECT_NO_FATAL_FAILURE(get_stored_vector_data_single_test()); + VecSimMetric metric = std::get<1>(GetParam()); + if (metric == VecSimMetric_Cosine) { + EXPECT_NO_FATAL_FAILURE(ValidateCosine()); + } +} + +TEST_P(Int8IndexTestUtilsTest, HNSW) { + HNSWParams params = {.type = VecSimType_INT8, .dim = dim}; + SetUp(params); + + EXPECT_NO_FATAL_FAILURE(get_stored_vector_data_single_test()); + VecSimMetric metric = std::get<1>(GetParam()); + if (metric == VecSimMetric_Cosine) { + EXPECT_NO_FATAL_FAILURE(ValidateCosine()); + } +} + +/** Run all Int8IndexTestUtilsTest tests for each {is_multi, VecSimMetric} combination */ +INSTANTIATE_TEST_SUITE_P(Int8IndexTestUtilsTest, Int8IndexTestUtilsTest, + testing::Combine(testing::Values(false, true), // is_multi + testing::Values(VecSimMetric_L2, VecSimMetric_IP, + VecSimMetric_Cosine)), + [](const testing::TestParamInfo &info) { + bool is_multi = std::get<0>(info.param); + const char *metric = VecSimMetric_ToString(std::get<1>(info.param)); + std::string test_name(is_multi ? "Multi_" : "Single_"); + return test_name + "_" + metric; + }); + +TEST_P(Float32IndexTestUtilsTest, BF) { + BFParams params = {.type = VecSimType_FLOAT32, .dim = dim}; + SetUp(params); + + EXPECT_NO_FATAL_FAILURE(get_stored_vector_data_single_test()); + VecSimMetric metric = std::get<1>(GetParam()); +} + +TEST_P(Float32IndexTestUtilsTest, HNSW) { + HNSWParams params = {.type = VecSimType_FLOAT32, .dim = dim}; + SetUp(params); + + EXPECT_NO_FATAL_FAILURE(get_stored_vector_data_single_test()); + VecSimMetric metric = std::get<1>(GetParam()); +} + +/** Run all Float32IndexTestUtilsTest tests for each {is_multi, VecSimMetric} combination */ +INSTANTIATE_TEST_SUITE_P( + Float32IndexTestUtilsTest, Float32IndexTestUtilsTest, + testing::Combine(testing::Values(false, true), // is_multi + testing::Values(VecSimMetric_L2, VecSimMetric_IP, VecSimMetric_Cosine)), + [](const testing::TestParamInfo &info) { + bool is_multi = std::get<0>(info.param); + const char *metric = VecSimMetric_ToString(std::get<1>(info.param)); + std::string test_name(is_multi ? "Multi_" : "Single_"); + return test_name + "_" + metric; + }); + +void IndexTestUtilsTest::get_stored_vector_data_single_test() { + size_t n = this->labels_count * this->vec_per_label; + + // Add vectors to the index + int id = 0; + for (size_t i = 0; i < this->labels_count; i++) { + for (size_t j = 0; j < vec_per_label; j++) { + this->GenerateRandomAndAddVector(i, id++); + } + } + + // Verify the index size + ASSERT_EQ(VecSimIndex_IndexSize(index), n); + + // Get stored vector data for each label + for (size_t i = 0; i < this->labels_count; i++) { + auto stored_data = GetStoredVectorsData(i); + auto stored_vectors_elements = GetStoredVectorsData(i); + + // Should return a vector of vectors for each label + ASSERT_EQ(stored_data.size(), vec_per_label); + + // Get the size of the stored data + size_t data_size = GetIndexDatasize(); + for (size_t j = 0; j < vec_per_label; j++) { + ASSERT_EQ(stored_data[j].size(), data_size); + } + + // Compare the stored data with the original vectors + EXPECT_NO_FATAL_FAILURE(this->ValidateVectors(stored_data, i)); + } +} diff --git a/tests/utils/tests_utils.h b/tests/utils/tests_utils.h index 3f0a6de2d..93a3eaf0b 100644 --- a/tests/utils/tests_utils.h +++ b/tests/utils/tests_utils.h @@ -40,6 +40,19 @@ static void populate_uint8_vec(uint8_t *v, size_t dim, int seed = 1234) { } } +// Assuming v is a memory allocation of size dim * sizeof(float) +static void populate_float_vec(float *v, size_t dim, int seed = 1234) { + + std::mt19937 gen(seed); // Mersenne Twister engine initialized with the fixed seed + + // Define a distribution range for float values between -1.0 and 1.0 + std::uniform_real_distribution dis(-1.0f, 1.0f); + + for (size_t i = 0; i < dim; i++) { + v[i] = dis(gen); + } +} + template float integral_compute_norm(const datatype *vec, size_t dim) { return spaces::IntegralType_ComputeNorm(vec, dim); From 0c3d47dc7b78c7eefd694cea6561c613722a0a02 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 6 May 2025 12:51:39 +0000 Subject: [PATCH 18/21] getFlatBufferIndex returns BruteForceIndex --- src/VecSim/algorithms/hnsw/hnsw_tiered.h | 4 ++++ src/VecSim/vec_sim_tiered_index.h | 7 +------ tests/unit/test_int8.cpp | 14 ++++---------- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/VecSim/algorithms/hnsw/hnsw_tiered.h b/src/VecSim/algorithms/hnsw/hnsw_tiered.h index 5f3c3fd2d..68d3e5d93 100644 --- a/src/VecSim/algorithms/hnsw/hnsw_tiered.h +++ b/src/VecSim/algorithms/hnsw/hnsw_tiered.h @@ -926,6 +926,10 @@ TieredHNSWIndex::TieredHNSW_BatchIterator::TieredHNSW_BatchI const void *query_vector, const TieredHNSWIndex *index, VecSimQueryParams *queryParams, std::shared_ptr allocator) // Tiered batch iterator doesn't hold its own copy of the query vector. + // Instead, each internal batch iterators (flat_iterator and hnsw_iterator) create their own + // copies: flat_iterator copy is created during TieredHNSW_BatchIterator construction When + // TieredHNSW_BatchIterator::getNextResults() is called and hnsw_iterator is not initialized, it + // retrieves the blob from flat_iterator : VecSimBatchIterator(nullptr, queryParams ? queryParams->timeoutCtx : nullptr, std::move(allocator)), index(index), flat_results(this->allocator), hnsw_results(this->allocator), diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index c3c5d5f35..9968fa699 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -107,12 +107,7 @@ class VecSimTieredIndex : public VecSimIndexInterface { static VecSimWriteMode getWriteMode() { return VecSimIndexInterface::asyncWriteMode; } #ifdef BUILD_TESTS - inline VecSimIndexAbstract *getFlatBufferIndex() { - return this->frontendIndex; - } - inline BruteForceIndex *getFlatBufferIndexAsBruteForce() { - return this->frontendIndex; - } + inline BruteForceIndex *getFlatBufferIndex() { return this->frontendIndex; } inline size_t getFlatBufferLimit() { return this->flatBufferLimit; } virtual void fitMemory() override { diff --git a/tests/unit/test_int8.cpp b/tests/unit/test_int8.cpp index a5df911b8..8a58a357e 100644 --- a/tests/unit/test_int8.cpp +++ b/tests/unit/test_int8.cpp @@ -186,8 +186,7 @@ class INT8TieredTest : public INT8Test { virtual void TearDown() override {} virtual const void *GetDataByInternalId(idType id) override { - return CastIndex>(GetFlatBufferIndex()) - ->getDataByInternalId(id); + return CastToBruteForce()->getDataByInternalId(id); } virtual HNSWIndex *CastToHNSW() override { @@ -199,14 +198,9 @@ class INT8TieredTest : public INT8Test { return CastIndex>(CastToHNSW()); } - VecSimIndexAbstract *GetFlatBufferIndex() { - auto tiered_index = dynamic_cast *>(index); - return tiered_index->getFlatBufferIndex(); - } - BruteForceIndex *CastToBruteForce() { auto tiered_index = dynamic_cast *>(index); - return tiered_index->getFlatBufferIndexAsBruteForce(); + return tiered_index->getFlatBufferIndex(); } int GenerateRandomAndAddVector(size_t id) override { @@ -747,7 +741,7 @@ void INT8TieredTest::test_info(bool is_multi) { VecSimIndexDebugInfo info = INT8Test::test_info(hnsw_params); ASSERT_EQ(info.commonInfo.basicInfo.algo, VecSimAlgo_HNSWLIB); - VecSimIndexDebugInfo frontendIndexInfo = GetFlatBufferIndex()->debugInfo(); + VecSimIndexDebugInfo frontendIndexInfo = CastToBruteForce()->debugInfo(); VecSimIndexDebugInfo backendIndexInfo = CastToHNSW()->debugInfo(); compareCommonInfo(info.tieredInfo.frontendCommonInfo, frontendIndexInfo.commonInfo); @@ -855,7 +849,7 @@ void INT8TieredTest::test_info_iterator(VecSimMetric metric) { SetUp(params); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); VecSimDebugInfoIterator *infoIter = VecSimIndex_DebugInfoIterator(index); - VecSimIndexDebugInfo frontendIndexInfo = GetFlatBufferIndex()->debugInfo(); + VecSimIndexDebugInfo frontendIndexInfo = CastToBruteForce()->debugInfo(); VecSimIndexDebugInfo backendIndexInfo = CastToHNSW()->debugInfo(); VecSimDebugInfoIterator_Free(infoIter); } From 2495ae68e2730f424ad7dfbcce14df242e1e80ff Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 6 May 2025 12:56:03 +0000 Subject: [PATCH 19/21] little test name fix --- tests/unit/test_index_test_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_index_test_utils.cpp b/tests/unit/test_index_test_utils.cpp index 77f91e96b..1828242f6 100644 --- a/tests/unit/test_index_test_utils.cpp +++ b/tests/unit/test_index_test_utils.cpp @@ -183,7 +183,7 @@ INSTANTIATE_TEST_SUITE_P(Int8IndexTestUtilsTest, Int8IndexTestUtilsTest, bool is_multi = std::get<0>(info.param); const char *metric = VecSimMetric_ToString(std::get<1>(info.param)); std::string test_name(is_multi ? "Multi_" : "Single_"); - return test_name + "_" + metric; + return test_name + metric; }); TEST_P(Float32IndexTestUtilsTest, BF) { From 1cf9456241442f6548f1786f220e0abdae2f06da Mon Sep 17 00:00:00 2001 From: meiravgri Date: Tue, 6 May 2025 13:02:43 +0000 Subject: [PATCH 20/21] small fix --- tests/unit/test_index_test_utils.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_index_test_utils.cpp b/tests/unit/test_index_test_utils.cpp index 1828242f6..ce1cefba9 100644 --- a/tests/unit/test_index_test_utils.cpp +++ b/tests/unit/test_index_test_utils.cpp @@ -231,7 +231,6 @@ void IndexTestUtilsTest::get_stored_vector_data_single_test() { // Get stored vector data for each label for (size_t i = 0; i < this->labels_count; i++) { auto stored_data = GetStoredVectorsData(i); - auto stored_vectors_elements = GetStoredVectorsData(i); // Should return a vector of vectors for each label ASSERT_EQ(stored_data.size(), vec_per_label); From 17479111f69f9d7b9f3e7ee23e783b4d6ec10978 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Wed, 7 May 2025 07:42:42 +0300 Subject: [PATCH 21/21] fix range --- tests/unit/test_int8.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_int8.cpp b/tests/unit/test_int8.cpp index 8a58a357e..7fa66db38 100644 --- a/tests/unit/test_int8.cpp +++ b/tests/unit/test_int8.cpp @@ -1074,7 +1074,7 @@ TEST_F(INT8TieredTest, CosineBlobCorrectness) { verify_norm(stored_vec_data, vector_norm); ASSERT_NO_FATAL_FAILURE(runTopKSearchTest(index, query, 1, verify_res)); - ASSERT_NO_FATAL_FAILURE(runRangeQueryTest(index, query, 0.5, verify_res, 1, BY_SCORE)); + ASSERT_NO_FATAL_FAILURE(runRangeQueryTest(index, query, 2, verify_res, 1, BY_SCORE)); VecSimBatchIterator *batchIterator = VecSimBatchIterator_New(index, query, nullptr); ASSERT_NO_FATAL_FAILURE(runBatchIteratorSearchTest(batchIterator, 1, verify_res)); VecSimBatchIterator_Free(batchIterator);