From bc4d1d3f1b2015fe737f5aea21525a84d23fed73 Mon Sep 17 00:00:00 2001 From: meiravgri <109056284+meiravgri@users.noreply.github.com> Date: Fri, 13 Dec 2024 16:40:53 +0200 Subject: [PATCH] [MOD-8244] fix TieredFactory::EstimateInitialSize (#567) * initailize all bf params in tiered EstimateInitialSize (metric was missing, leading to a miss computation of the components memory size estimation) * add is_normalized to EstimateComponentsMemory (passed to EstimatePreprocessorsContainerMemory) * addHNSWTieredIndexTest:: testIndexesAttributes checkes tierred index indexes attributes and componenets * remove comment (cherry picked from commit a586edf1993a894aaaabcb20f6d40e9870006dd7) --- .../index_factories/brute_force_factory.cpp | 10 ++-- .../index_factories/brute_force_factory.h | 2 +- .../components/components_factory.h | 4 +- .../components/preprocessors_factory.h | 10 +++- src/VecSim/index_factories/hnsw_factory.cpp | 10 ++-- src/VecSim/index_factories/hnsw_factory.h | 2 +- src/VecSim/index_factories/tiered_factory.cpp | 30 ++++++---- .../spaces/computer/preprocessor_container.h | 6 ++ src/VecSim/vec_sim_index.h | 4 ++ tests/unit/test_hnsw_tiered.cpp | 55 ++++++++++++++++++- 10 files changed, 105 insertions(+), 28 deletions(-) diff --git a/src/VecSim/index_factories/brute_force_factory.cpp b/src/VecSim/index_factories/brute_force_factory.cpp index 8d777cf64..e2919d75f 100644 --- a/src/VecSim/index_factories/brute_force_factory.cpp +++ b/src/VecSim/index_factories/brute_force_factory.cpp @@ -98,7 +98,7 @@ inline size_t EstimateInitialSize_ChooseMultiOrSingle(bool is_multi) { return sizeof(BruteForceIndex_Single); } -size_t EstimateInitialSize(const BFParams *params) { +size_t EstimateInitialSize(const BFParams *params, bool is_normalized) { size_t allocations_overhead = VecSimAllocator::getAllocationOverheadSize(); @@ -106,16 +106,16 @@ size_t EstimateInitialSize(const BFParams *params) { size_t est = sizeof(VecSimAllocator) + allocations_overhead; if (params->type == VecSimType_FLOAT32) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } else if (params->type == VecSimType_FLOAT64) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } else if (params->type == VecSimType_BFLOAT16) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } else if (params->type == VecSimType_FLOAT16) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } diff --git a/src/VecSim/index_factories/brute_force_factory.h b/src/VecSim/index_factories/brute_force_factory.h index e0821d550..cb075b530 100644 --- a/src/VecSim/index_factories/brute_force_factory.h +++ b/src/VecSim/index_factories/brute_force_factory.h @@ -25,7 +25,7 @@ VecSimIndex *NewIndex(const VecSimParams *params, bool is_normalized = false); VecSimIndex *NewIndex(const BFParams *bfparams, bool is_normalized = false); VecSimIndex *NewIndex(const BFParams *bfparams, const AbstractIndexInitParams &abstractInitParams, bool is_normalized); -size_t EstimateInitialSize(const BFParams *params); +size_t EstimateInitialSize(const BFParams *params, bool is_normalized = false); size_t EstimateElementSize(const BFParams *params); }; // namespace BruteForceFactory diff --git a/src/VecSim/index_factories/components/components_factory.h b/src/VecSim/index_factories/components/components_factory.h index 5846192e4..6f4e984d1 100644 --- a/src/VecSim/index_factories/components/components_factory.h +++ b/src/VecSim/index_factories/components/components_factory.h @@ -28,13 +28,13 @@ CreateIndexComponents(std::shared_ptr allocator, VecSimMetric m } template -size_t EstimateComponentsMemory(VecSimMetric metric) { +size_t EstimateComponentsMemory(VecSimMetric metric, bool is_normalized) { size_t allocations_overhead = VecSimAllocator::getAllocationOverheadSize(); // Currently we have only one distance calculator implementation size_t est = allocations_overhead + sizeof(DistanceCalculatorCommon); - est += EstimatePreprocessorsContainerMemory(metric); + est += EstimatePreprocessorsContainerMemory(metric, is_normalized); return est; } diff --git a/src/VecSim/index_factories/components/preprocessors_factory.h b/src/VecSim/index_factories/components/preprocessors_factory.h index 3f0bdf4fe..ede609886 100644 --- a/src/VecSim/index_factories/components/preprocessors_factory.h +++ b/src/VecSim/index_factories/components/preprocessors_factory.h @@ -35,10 +35,16 @@ CreatePreprocessorsContainer(std::shared_ptr allocator, } template -size_t EstimatePreprocessorsContainerMemory(VecSimMetric metric) { +size_t EstimatePreprocessorsContainerMemory(VecSimMetric metric, bool is_normalized = false) { size_t allocations_overhead = VecSimAllocator::getAllocationOverheadSize(); + VecSimMetric pp_metric; + if (is_normalized && metric == VecSimMetric_Cosine) { + pp_metric = VecSimMetric_IP; + } else { + pp_metric = metric; + } - if (metric == VecSimMetric_Cosine) { + if (pp_metric == VecSimMetric_Cosine) { constexpr size_t n_preprocessors = 1; // One entry in preprocessors array size_t est = diff --git a/src/VecSim/index_factories/hnsw_factory.cpp b/src/VecSim/index_factories/hnsw_factory.cpp index 3c70c7964..58f7091f6 100644 --- a/src/VecSim/index_factories/hnsw_factory.cpp +++ b/src/VecSim/index_factories/hnsw_factory.cpp @@ -98,21 +98,21 @@ inline size_t EstimateInitialSize_ChooseMultiOrSingle(bool is_multi) { return sizeof(HNSWIndex_Single); } -size_t EstimateInitialSize(const HNSWParams *params) { +size_t EstimateInitialSize(const HNSWParams *params, bool is_normalized) { size_t allocations_overhead = VecSimAllocator::getAllocationOverheadSize(); size_t est = sizeof(VecSimAllocator) + allocations_overhead; if (params->type == VecSimType_FLOAT32) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } else if (params->type == VecSimType_FLOAT64) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } else if (params->type == VecSimType_BFLOAT16) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } else if (params->type == VecSimType_FLOAT16) { - est += EstimateComponentsMemory(params->metric); + est += EstimateComponentsMemory(params->metric, is_normalized); est += EstimateInitialSize_ChooseMultiOrSingle(params->multi); } return est; diff --git a/src/VecSim/index_factories/hnsw_factory.h b/src/VecSim/index_factories/hnsw_factory.h index e5fefde51..3f99a560c 100644 --- a/src/VecSim/index_factories/hnsw_factory.h +++ b/src/VecSim/index_factories/hnsw_factory.h @@ -22,7 +22,7 @@ namespace HNSWFactory { */ VecSimIndex *NewIndex(const VecSimParams *params, bool is_normalized = false); VecSimIndex *NewIndex(const HNSWParams *params, bool is_normalized = false); -size_t EstimateInitialSize(const HNSWParams *params); +size_t EstimateInitialSize(const HNSWParams *params, bool is_normalized = false); size_t EstimateElementSize(const HNSWParams *params); #ifdef BUILD_TESTS diff --git a/src/VecSim/index_factories/tiered_factory.cpp b/src/VecSim/index_factories/tiered_factory.cpp index d56f3e4a0..68635e4ca 100644 --- a/src/VecSim/index_factories/tiered_factory.cpp +++ b/src/VecSim/index_factories/tiered_factory.cpp @@ -18,6 +18,18 @@ using float16 = vecsim_types::float16; namespace TieredFactory { namespace TieredHNSWFactory { + +static inline BFParams NewBFParams(const TieredIndexParams *params) { + auto hnsw_params = params->primaryIndexParams->algoParams.hnswParams; + BFParams bf_params = {.type = hnsw_params.type, + .dim = hnsw_params.dim, + .metric = hnsw_params.metric, + .multi = hnsw_params.multi, + .blockSize = hnsw_params.blockSize}; + + return bf_params; +} + template inline VecSimIndex *NewIndex(const TieredIndexParams *params) { @@ -27,11 +39,7 @@ inline VecSimIndex *NewIndex(const TieredIndexParams *params) { HNSWFactory::NewIndex(params->primaryIndexParams, true)); // initialize brute force index - BFParams bf_params = {.type = params->primaryIndexParams->algoParams.hnswParams.type, - .dim = params->primaryIndexParams->algoParams.hnswParams.dim, - .metric = params->primaryIndexParams->algoParams.hnswParams.metric, - .multi = params->primaryIndexParams->algoParams.hnswParams.multi, - .blockSize = params->primaryIndexParams->algoParams.hnswParams.blockSize}; + BFParams bf_params = NewBFParams(params); std::shared_ptr flat_allocator = VecSimAllocator::newVecsimAllocator(); AbstractIndexInitParams abstractInitParams = {.allocator = flat_allocator, @@ -52,11 +60,12 @@ inline VecSimIndex *NewIndex(const TieredIndexParams *params) { hnsw_index, frontendIndex, *params, management_layer_allocator); } -inline size_t EstimateInitialSize(const TieredIndexParams *params, BFParams &bf_params_output) { +inline size_t EstimateInitialSize(const TieredIndexParams *params) { HNSWParams hnsw_params = params->primaryIndexParams->algoParams.hnswParams; // Add size estimation of VecSimTieredIndex sub indexes. - size_t est = HNSWFactory::EstimateInitialSize(&hnsw_params); + // Normalization is done by the frontend index. + size_t est = HNSWFactory::EstimateInitialSize(&hnsw_params, true); // Management layer allocator overhead. size_t allocations_overhead = VecSimAllocator::getAllocationOverheadSize(); @@ -72,8 +81,6 @@ inline size_t EstimateInitialSize(const TieredIndexParams *params, BFParams &bf_ } else if (hnsw_params.type == VecSimType_FLOAT16) { est += sizeof(TieredHNSWIndex); } - bf_params_output.type = hnsw_params.type; - bf_params_output.multi = hnsw_params.multi; return est; } @@ -107,10 +114,11 @@ size_t EstimateInitialSize(const TieredIndexParams *params) { BFParams bf_params{}; if (params->primaryIndexParams->algo == VecSimAlgo_HNSWLIB) { - est += TieredHNSWFactory::EstimateInitialSize(params, bf_params); + est += TieredHNSWFactory::EstimateInitialSize(params); + bf_params = TieredHNSWFactory::NewBFParams(params); } - est += BruteForceFactory::EstimateInitialSize(&bf_params); + est += BruteForceFactory::EstimateInitialSize(&bf_params, false); return est; } diff --git a/src/VecSim/spaces/computer/preprocessor_container.h b/src/VecSim/spaces/computer/preprocessor_container.h index 03fcbb911..8fdbe5e87 100644 --- a/src/VecSim/spaces/computer/preprocessor_container.h +++ b/src/VecSim/spaces/computer/preprocessor_container.h @@ -87,6 +87,12 @@ class MultiPreprocessorsContainer : public PreprocessorsContainerAbstract { void preprocessQueryInPlace(void *blob, size_t processed_bytes_count) const override; +#ifdef BUILD_TESTS + std::array getPreprocessors() const { + return preprocessors; + } +#endif + private: using Base = PreprocessorsContainerAbstract; }; diff --git a/src/VecSim/vec_sim_index.h b/src/VecSim/vec_sim_index.h index 669b6a7bd..08599951b 100644 --- a/src/VecSim/vec_sim_index.h +++ b/src/VecSim/vec_sim_index.h @@ -250,6 +250,10 @@ struct VecSimIndexAbstract : public VecSimIndexInterface { delete this->preprocessors; this->preprocessors = newPPContainer; } + + IndexComponents get_components() const { + return {.indexCalculator = this->indexCalculator, .preprocessors = this->preprocessors}; + } #endif protected: diff --git a/tests/unit/test_hnsw_tiered.cpp b/tests/unit/test_hnsw_tiered.cpp index 4b1df107a..ee5fef7d1 100644 --- a/tests/unit/test_hnsw_tiered.cpp +++ b/tests/unit/test_hnsw_tiered.cpp @@ -120,6 +120,59 @@ TYPED_TEST(HNSWTieredIndexTest, CreateIndexInstance) { ASSERT_EQ(tiered_index->labelToInsertJobs.at(vector_label).size(), 0); } +TYPED_TEST(HNSWTieredIndexTest, testIndexesAttributes) { + // Create TieredHNSW index instance with a mock queue. + HNSWParams params = {.type = TypeParam::get_index_type(), + .dim = 4, + .metric = VecSimMetric_Cosine, + .multi = TypeParam::isMulti()}; + VecSimParams hnsw_params = CreateParams(params); + auto mock_thread_pool = tieredIndexMock(); + auto *tiered_index = this->CreateTieredHNSWIndex(hnsw_params, mock_thread_pool); + + HNSWIndex *hnsw_index = this->CastToHNSW(tiered_index); + BruteForceIndex *bf_index = this->GetFlatIndex(tiered_index); + + VecSimIndexBasicInfo hnsw_info = VecSimIndex_Info(hnsw_index).commonInfo.basicInfo; + VecSimIndexBasicInfo bf_info = VecSimIndex_Info(bf_index).commonInfo.basicInfo; + + // assert metric + ASSERT_EQ(hnsw_info.metric, params.metric); + ASSERT_EQ(bf_info.metric, params.metric); + // assert type + ASSERT_EQ(hnsw_info.type, params.type); + ASSERT_EQ(bf_info.type, params.type); + // assert dim + ASSERT_EQ(hnsw_info.dim, params.dim); + ASSERT_EQ(bf_info.dim, params.dim); + // assert multi + ASSERT_EQ(hnsw_info.isMulti, params.multi); + ASSERT_EQ(bf_info.isMulti, params.multi); + + // assert containers + + // bf - multi with cosine + IndexComponents bf_components = bf_index->get_components(); + PreprocessorsContainerAbstract *bf_preprocessors = bf_components.preprocessors; + const std::type_info &bf_pp_container_expected_type = + typeid(MultiPreprocessorsContainer); + const std::type_info &bf_pp_container_actual_type = typeid(*bf_preprocessors); + ASSERT_EQ(bf_pp_container_actual_type, bf_pp_container_expected_type); + + std::array pp_arr = + dynamic_cast *>(bf_preprocessors) + ->getPreprocessors(); + const std::type_info &bf_pp_expected_type = typeid(CosinePreprocessor); + const std::type_info &bf_pp_actual_type = typeid(*pp_arr[0]); + ASSERT_EQ(bf_pp_actual_type, bf_pp_expected_type); + + // hnsw - simple + IndexComponents hnsw_components = hnsw_index->get_components(); + const std::type_info &hnsw_pp_expected_type = typeid(PreprocessorsContainerAbstract); + const std::type_info &hnsw_pp_actual_type = typeid(*hnsw_components.preprocessors); + ASSERT_EQ(hnsw_pp_expected_type, hnsw_pp_actual_type); +} + TYPED_TEST(HNSWTieredIndexTest, testSizeEstimation) { size_t dim = 128; size_t n = DEFAULT_BLOCK_SIZE; @@ -129,7 +182,7 @@ TYPED_TEST(HNSWTieredIndexTest, testSizeEstimation) { HNSWParams hnsw_params = {.type = TypeParam::get_index_type(), .dim = dim, - .metric = VecSimMetric_L2, + .metric = VecSimMetric_Cosine, .multi = isMulti, .M = M}; VecSimParams vecsim_hnsw_params = CreateParams(hnsw_params);