From 88b14815025019564cc8b03833d29de6337ed3d0 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Thu, 17 Apr 2025 11:44:07 +0200 Subject: [PATCH 1/6] Return NULL Index if SVS LVQ is requested but not supported. --- src/VecSim/algorithms/svs/svs_utils.h | 22 ++++++++++++++++++++++ src/VecSim/index_factories/svs_factory.cpp | 4 ++++ 2 files changed, 26 insertions(+) diff --git a/src/VecSim/algorithms/svs/svs_utils.h b/src/VecSim/algorithms/svs/svs_utils.h index 94f7b422f..4b41a80c9 100644 --- a/src/VecSim/algorithms/svs/svs_utils.h +++ b/src/VecSim/algorithms/svs/svs_utils.h @@ -6,6 +6,11 @@ #include "svs/lib/float16.h" #include "svs/index/vamana/dynamic_index.h" +#include +#include +#include +#include + namespace svs_details { // VecSim->SVS data type conversion template @@ -133,6 +138,23 @@ inline svs::lib::PowerOfTwo SVSBlockSize(size_t bs, size_t elem_size) { return svs_bs; } +bool check_cpuid() { + uint32_t eax, ebx, ecx, edx; + __cpuid(0, eax, ebx, ecx, edx); + std::string vendor_id = std::string((const char*)&ebx, 4) + + std::string((const char*)&edx, 4) + + std::string((const char*)&ecx, 4); + return (vendor_id == "GenuineIntel"); +} + +bool isSVSLVQModeSupported(VecSimSvsQuantBits quant_bits) { +#if HAVE_SVS_LVQ + // Check if the CPU supports SVS LVQ + return check_cpuid(); +#endif + return quant_bits == VecSimSvsQuant_NONE; +} + } // namespace svs_details template diff --git a/src/VecSim/index_factories/svs_factory.cpp b/src/VecSim/index_factories/svs_factory.cpp index 9191920a6..a32e7f2e1 100644 --- a/src/VecSim/index_factories/svs_factory.cpp +++ b/src/VecSim/index_factories/svs_factory.cpp @@ -35,6 +35,10 @@ VecSimIndex *NewIndexImpl(const VecSimParams *params, bool is_normalized) { template VecSimIndex *NewIndexImpl(const VecSimParams *params, bool is_normalized) { + if (!svs_details::isSVSLVQModeSupported(params->algoParams.svsParams.quantBits)) { + return NULL; + } + switch (params->algoParams.svsParams.quantBits) { case VecSimSvsQuant_NONE: return NewIndexImpl(params, is_normalized); From fb75a0c3ec9840ee2642f65335801c83be7dcaf7 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Thu, 17 Apr 2025 15:39:46 +0200 Subject: [PATCH 2/6] Add tests for non-supported SVS cases. --- cmake/svs.cmake | 1 - src/VecSim/CMakeLists.txt | 2 +- src/VecSim/algorithms/svs/svs_utils.h | 6 +- src/VecSim/index_factories/index_factory.cpp | 8 -- src/VecSim/index_factories/svs_factory.cpp | 10 ++ tests/unit/CMakeLists.txt | 12 +-- tests/unit/test_svs.cpp | 96 +++++++++++++++----- 7 files changed, 92 insertions(+), 43 deletions(-) diff --git a/cmake/svs.cmake b/cmake/svs.cmake index 3270c14d2..065137224 100644 --- a/cmake/svs.cmake +++ b/cmake/svs.cmake @@ -38,7 +38,6 @@ if(USE_SVS) message(STATUS "SVS support enabled") # Configure SVS build add_compile_definitions("HAVE_SVS=1") - set(svs_factory_file "index_factories/svs_factory.cpp") # detect if build environment is using glibc include(CheckSymbolExists) diff --git a/src/VecSim/CMakeLists.txt b/src/VecSim/CMakeLists.txt index cb23e46fe..557851018 100644 --- a/src/VecSim/CMakeLists.txt +++ b/src/VecSim/CMakeLists.txt @@ -29,7 +29,7 @@ add_library(VectorSimilarity ${VECSIM_LIBTYPE} index_factories/brute_force_factory.cpp index_factories/hnsw_factory.cpp index_factories/tiered_factory.cpp - ${svs_factory_file} + index_factories/svs_factory.cpp index_factories/index_factory.cpp index_factories/components/components_factory.cpp algorithms/hnsw/visited_nodes_handler.cpp diff --git a/src/VecSim/algorithms/svs/svs_utils.h b/src/VecSim/algorithms/svs/svs_utils.h index 4b41a80c9..fcd982a41 100644 --- a/src/VecSim/algorithms/svs/svs_utils.h +++ b/src/VecSim/algorithms/svs/svs_utils.h @@ -138,7 +138,8 @@ inline svs::lib::PowerOfTwo SVSBlockSize(size_t bs, size_t elem_size) { return svs_bs; } -bool check_cpuid() { +// clang-format off +inline bool check_cpuid() { uint32_t eax, ebx, ecx, edx; __cpuid(0, eax, ebx, ecx, edx); std::string vendor_id = std::string((const char*)&ebx, 4) + @@ -146,8 +147,9 @@ bool check_cpuid() { std::string((const char*)&ecx, 4); return (vendor_id == "GenuineIntel"); } +//clang-format on -bool isSVSLVQModeSupported(VecSimSvsQuantBits quant_bits) { +inline bool isSVSLVQModeSupported(VecSimSvsQuantBits quant_bits) { #if HAVE_SVS_LVQ // Check if the CPU supports SVS LVQ return check_cpuid(); diff --git a/src/VecSim/index_factories/index_factory.cpp b/src/VecSim/index_factories/index_factory.cpp index f5de71b70..3fd3b70ee 100644 --- a/src/VecSim/index_factories/index_factory.cpp +++ b/src/VecSim/index_factories/index_factory.cpp @@ -9,9 +9,7 @@ #include "hnsw_factory.h" #include "brute_force_factory.h" #include "tiered_factory.h" -#if HAVE_SVS #include "svs_factory.h" -#endif namespace VecSimFactory { VecSimIndex *NewIndex(const VecSimParams *params) { @@ -33,9 +31,7 @@ VecSimIndex *NewIndex(const VecSimParams *params) { break; } case VecSimAlgo_SVS: { -#if HAVE_SVS index = SVSFactory::NewIndex(params); -#endif break; } } @@ -54,9 +50,7 @@ size_t EstimateInitialSize(const VecSimParams *params) { case VecSimAlgo_TIERED: return TieredFactory::EstimateInitialSize(¶ms->algoParams.tieredParams); case VecSimAlgo_SVS:; // empty statement if svs not available -#if HAVE_SVS return SVSFactory::EstimateInitialSize(¶ms->algoParams.svsParams); -#endif } return -1; } @@ -70,9 +64,7 @@ size_t EstimateElementSize(const VecSimParams *params) { case VecSimAlgo_TIERED: return TieredFactory::EstimateElementSize(¶ms->algoParams.tieredParams); case VecSimAlgo_SVS:; // empty statement if svs not available -#if HAVE_SVS return SVSFactory::EstimateElementSize(¶ms->algoParams.svsParams); -#endif } return -1; } diff --git a/src/VecSim/index_factories/svs_factory.cpp b/src/VecSim/index_factories/svs_factory.cpp index a32e7f2e1..7a46c73f9 100644 --- a/src/VecSim/index_factories/svs_factory.cpp +++ b/src/VecSim/index_factories/svs_factory.cpp @@ -1,4 +1,6 @@ #include "VecSim/index_factories/svs_factory.h" + +#if HAVE_SVS #include "VecSim/memory/vecsim_malloc.h" #include "VecSim/vec_sim_index.h" #include "VecSim/algorithms/svs/svs.h" @@ -173,3 +175,11 @@ size_t EstimateInitialSize(const SVSParams *params, bool is_normalized) { } } // namespace SVSFactory + +#else // HAVE_SVS +namespace SVSFactory { +VecSimIndex *NewIndex(const VecSimParams *params, bool is_normalized) { return NULL; } +size_t EstimateInitialSize(const SVSParams *params, bool is_normalized) { return -1; } +size_t EstimateElementSize(const SVSParams *params) { return -1; } +}; // namespace SVSFactory +#endif // HAVE_SVS \ No newline at end of file diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 7b1171faf..0631ce926 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_svs ../utils/mock_thread_pool.cpp test_svs.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,11 +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) - -if(USE_SVS) - 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) -endif() +target_link_libraries(test_svs PUBLIC gtest_main VectorSimilarity) include(GoogleTest) @@ -79,7 +76,4 @@ 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_) - -if(USE_SVS) - gtest_discover_tests(test_svs) -endif() +gtest_discover_tests(test_svs) diff --git a/tests/unit/test_svs.cpp b/tests/unit/test_svs.cpp index 1706040ff..52331bc77 100644 --- a/tests/unit/test_svs.cpp +++ b/tests/unit/test_svs.cpp @@ -1,23 +1,23 @@ #include "gtest/gtest.h" #include "VecSim/vec_sim.h" #include "unit_test_utils.h" -#include "VecSim/algorithms/svs/svs.h" -#include "cpu_features_macros.h" #include -#include -#include -template -class SVSTest : public ::testing::Test { - bool _checkCPU() { - uint32_t eax, ebx, ecx, edx; - __cpuid(0, eax, ebx, ecx, edx); - std::string vendor_id = std::string((const char *)&ebx, 4) + - std::string((const char *)&edx, 4) + - std::string((const char *)&ecx, 4); - return (vendor_id == "GenuineIntel"); +#if HAVE_SVS +#include "VecSim/algorithms/svs/svs.h" + +#define ASSERT_INDEX(index) \ + if (index == nullptr) { \ + const auto quantBits = TypeParam::get_quant_bits(); \ + if (quantBits != VecSimSvsQuant_NONE && !svs_details::isSVSLVQModeSupported(quantBits)) { \ + GTEST_SKIP() << "SVS LVQ is not supported."; \ + } else { \ + GTEST_FAIL() << "Failed to create SVS index"; \ + } \ } +template +class SVSTest : public ::testing::Test { public: using data_t = typename index_type_t::data_t; @@ -34,13 +34,6 @@ class SVSTest : public ::testing::Test { assert(indexBase != nullptr); return indexBase; } - - void SetUp() override { - if constexpr (index_type_t::get_quant_bits() != VecSimSvsQuant_NONE) - if (!_checkCPU()) { - GTEST_SKIP() << "SVS LVQ is not supported on non-Intel hardware."; - } - } }; // TEST_DATA_T and TEST_DIST_T are defined in test_utils.h @@ -54,9 +47,7 @@ struct SVSIndexType { // clang-format off using SVSDataTypeSet = ::testing::Types -#if HAVE_SVS_LVQ ,SVSIndexType -#endif >; // clang-format on @@ -79,6 +70,7 @@ TYPED_TEST(SVSTest, svs_vector_add_test) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); EXPECT_EQ(VecSimIndex_IndexSize(index), 0); @@ -106,6 +98,7 @@ TYPED_TEST(SVSTest, svs_vector_update_test) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); auto *svs_index = this->CastToSVS(index); @@ -146,6 +139,7 @@ TYPED_TEST(SVSTest, svs_vector_search_by_id_test) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, i); @@ -177,6 +171,7 @@ TYPED_TEST(SVSTest, svs_bulk_vectors_add_delete_test) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); auto svs_index = this->CastToSVS(index); // CAST_TO_SVS(index, svs::distance::DistanceL2); @@ -231,6 +226,7 @@ TYPED_TEST(SVSTest, svs_get_distance) { for (size_t i = 0; i < numIndex; i++) { params.metric = (VecSimMetric)i; index[i] = this->CreateNewIndex(params); + ASSERT_INDEX(index[i]); VecSimIndex_AddVector(index[i], v1, 1); VecSimIndex_AddVector(index[i], v2, 2); VecSimIndex_AddVector(index[i], v3, 3); @@ -300,6 +296,7 @@ TYPED_TEST(SVSTest, svs_indexing_same_vector) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, @@ -335,6 +332,7 @@ TYPED_TEST(SVSTest, svs_reindexing_same_vector) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // SVSIndex *bf_index = this->CastToBF(index); @@ -388,6 +386,7 @@ TYPED_TEST(SVSTest, svs_reindexing_same_vector_different_id) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, @@ -444,6 +443,8 @@ TYPED_TEST(SVSTest, svs_batch_iterator) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); + for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, i); } @@ -498,6 +499,7 @@ TYPED_TEST(SVSTest, svs_batch_iterator_non_unique_scores) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, i / 10); @@ -559,6 +561,7 @@ TYPED_TEST(SVSTest, svs_batch_iterator_reset) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, i / 10); @@ -617,6 +620,7 @@ TYPED_TEST(SVSTest, svs_batch_iterator_corner_cases) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // Query for (n,n,...,n) vector (recall that n is the largest id in te index). TEST_DATA_T query[dim]; @@ -686,6 +690,7 @@ TYPED_TEST(SVSTest, resizeIndex) { SVSParams params = {.dim = dim, .metric = VecSimMetric_L2, .blockSize = bs}; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // Add up to n. for (size_t i = 0; i < n; i++) { @@ -724,6 +729,7 @@ TYPED_TEST(SVSTest, svs_empty_index) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); ASSERT_EQ(VecSimIndex_IndexSize(index), 0); @@ -772,6 +778,7 @@ TYPED_TEST(SVSTest, test_delete_vector) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // Delete from empty index ASSERT_EQ(VecSimIndex_DeleteVector(index, 111), 0); @@ -817,6 +824,7 @@ TYPED_TEST(SVSTest, sanity_reinsert_1280) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); auto *vectors = new TEST_DATA_T[n * d]; @@ -869,6 +877,7 @@ TYPED_TEST(SVSTest, test_svs_info) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); ASSERT_EQ(info.commonInfo.basicInfo.algo, VecSimAlgo_SVS); @@ -883,6 +892,7 @@ TYPED_TEST(SVSTest, test_svs_info) { params.blockSize = 1; index = this->CreateNewIndex(params); + ASSERT_INDEX(index); info = VecSimIndex_DebugInfo(index); ASSERT_EQ(info.commonInfo.basicInfo.algo, VecSimAlgo_SVS); @@ -928,6 +938,7 @@ TYPED_TEST(SVSTest, test_basic_svs_info_iterator) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); VecSimDebugInfoIterator *infoIter = VecSimIndex_DebugInfoIterator(index); @@ -954,6 +965,7 @@ TYPED_TEST(SVSTest, test_dynamic_svs_info_iterator) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); VecSimDebugInfoIterator *infoIter = VecSimIndex_DebugInfoIterator(index); @@ -1040,6 +1052,7 @@ TYPED_TEST(SVSTest, svs_vector_search_test_ip) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); ASSERT_EQ(info.commonInfo.basicInfo.algo, VecSimAlgo_SVS); @@ -1085,6 +1098,7 @@ TYPED_TEST(SVSTest, svs_vector_search_test_l2) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimIndexDebugInfo info = VecSimIndex_DebugInfo(index); ASSERT_EQ(info.commonInfo.basicInfo.algo, VecSimAlgo_SVS); @@ -1127,6 +1141,7 @@ TYPED_TEST(SVSTest, svs_search_empty_index) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); ASSERT_EQ(VecSimIndex_IndexSize(index), 0); @@ -1181,6 +1196,7 @@ TYPED_TEST(SVSTest, svs_test_inf_score) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); TEST_DATA_T inf_val = GetInfVal(params.type); ASSERT_FALSE(std::isinf(inf_val)); @@ -1244,6 +1260,7 @@ TYPED_TEST(SVSTest, preferAdHocOptimization) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // Set the index size artificially to be the required one. // (this->CastToBF(index))->count = index_size; @@ -1276,6 +1293,7 @@ TYPED_TEST(SVSTest, preferAdHocOptimization) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); ASSERT_TRUE(VecSimIndex_PreferAdHocSearch(index, 0, 50, true)); @@ -1303,6 +1321,7 @@ TYPED_TEST(SVSTest, batchIteratorSwapIndices) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); TEST_DATA_T close_vec[] = {1.0, 1.0, 1.0, 1.0}; TEST_DATA_T further_vec[] = {2.0, 2.0, 2.0, 2.0}; @@ -1371,6 +1390,7 @@ TYPED_TEST(SVSTest, svs_vector_search_test_cosine) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // To meet accurary in LVQ case we have to add bulk of vectors at once. std::vector v(n); @@ -1464,6 +1484,7 @@ TYPED_TEST(SVSTest, testSizeEstimation) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // EstimateInitialSize is called after CreateNewIndex because params struct is // changed in CreateNewIndex. size_t estimation = EstimateInitialSize(params); @@ -1501,6 +1522,7 @@ TYPED_TEST(SVSTest, testInitialSizeEstimation) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // EstimateInitialSize is called after CreateNewIndex because params struct is // changed in CreateNewIndex. size_t estimation = EstimateInitialSize(params); @@ -1529,6 +1551,7 @@ TYPED_TEST(SVSTest, testTimeoutReturn_topK) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSim_SetTimeoutCallbackFunction([](void *ctx) { return 1; }); // Always times out @@ -1564,6 +1587,7 @@ TYPED_TEST(SVSTest, testTimeoutReturn_range) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSim_SetTimeoutCallbackFunction([](void *ctx) { return 1; }); // Always times out @@ -1601,6 +1625,7 @@ TYPED_TEST(SVSTest, testTimeoutReturn_batch_iterator) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, i); @@ -1659,6 +1684,7 @@ TYPED_TEST(SVSTest, rangeQuery) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); for (size_t i = 0; i < n; i++) { GenerateAndAddVector(index, dim, i, i); @@ -1728,6 +1754,7 @@ TYPED_TEST(SVSTest, rangeQueryCosine) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); // To meet accurary in LVQ case we have to add bulk of vectors at once. std::vector v(n); @@ -1787,6 +1814,7 @@ TYPED_TEST(SVSTest, FitMemoryTest) { }; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); size_t initial_memory = index->getAllocationSize(); index->fitMemory(); @@ -1807,6 +1835,7 @@ TYPED_TEST(SVSTest, resolve_ws_search_runtime_params) { SVSParams params = {.dim = 4, .metric = VecSimMetric_L2}; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimQueryParams qparams, zero; bzero(&zero, sizeof(VecSimQueryParams)); @@ -1900,6 +1929,7 @@ TYPED_TEST(SVSTest, resolve_use_search_history_runtime_params) { SVSParams params = {.dim = 4, .metric = VecSimMetric_L2}; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimQueryParams qparams, zero; bzero(&zero, sizeof(VecSimQueryParams)); @@ -2008,6 +2038,7 @@ TYPED_TEST(SVSTest, resolve_epsilon_runtime_params) { SVSParams params = {.dim = 4, .metric = VecSimMetric_L2}; VecSimIndex *index = this->CreateNewIndex(params); + ASSERT_INDEX(index); VecSimQueryParams qparams, zero; bzero(&zero, sizeof(VecSimQueryParams)); @@ -2078,3 +2109,24 @@ TYPED_TEST(SVSTest, resolve_epsilon_runtime_params) { VecSimIndex_Free(index); } + +#else // HAVE_SVS + +TEST(SVSTest, svs_not_supported) { + SVSParams params = { + .type = VecSimType_FLOAT32, + .dim = 16, + .metric = VecSimMetric_IP, + }; + auto index_params = CreateParams(params); + auto index = VecSimIndex_New(&index_params); + ASSERT_EQ(index, nullptr); + + auto size = VecSimIndex_EstimateInitialSize(&index_params); + ASSERT_EQ(size, -1); + + auto size2 = VecSimIndex_EstimateElementSize(&index_params); + ASSERT_EQ(size2, -1); +} + +#endif \ No newline at end of file From cd7cd5783ea7586416ebfbfb6c00ba80dba93c3f Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Thu, 17 Apr 2025 16:32:45 +0200 Subject: [PATCH 3/6] Fix SVS LVQ mode support detection. --- src/VecSim/algorithms/svs/svs_utils.h | 9 ++++----- src/VecSim/index_factories/svs_factory.cpp | 2 +- tests/unit/test_svs.cpp | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_utils.h b/src/VecSim/algorithms/svs/svs_utils.h index fcd982a41..498904a24 100644 --- a/src/VecSim/algorithms/svs/svs_utils.h +++ b/src/VecSim/algorithms/svs/svs_utils.h @@ -147,16 +147,15 @@ inline bool check_cpuid() { std::string((const char*)&ecx, 4); return (vendor_id == "GenuineIntel"); } -//clang-format on +// clang-format on inline bool isSVSLVQModeSupported(VecSimSvsQuantBits quant_bits) { + return quant_bits == VecSimSvsQuant_NONE #if HAVE_SVS_LVQ - // Check if the CPU supports SVS LVQ - return check_cpuid(); + || check_cpuid() // Check if the CPU supports SVS LVQ #endif - return quant_bits == VecSimSvsQuant_NONE; + ; } - } // namespace svs_details template diff --git a/src/VecSim/index_factories/svs_factory.cpp b/src/VecSim/index_factories/svs_factory.cpp index 7a46c73f9..502c7c511 100644 --- a/src/VecSim/index_factories/svs_factory.cpp +++ b/src/VecSim/index_factories/svs_factory.cpp @@ -182,4 +182,4 @@ VecSimIndex *NewIndex(const VecSimParams *params, bool is_normalized) { return N size_t EstimateInitialSize(const SVSParams *params, bool is_normalized) { return -1; } size_t EstimateElementSize(const SVSParams *params) { return -1; } }; // namespace SVSFactory -#endif // HAVE_SVS \ No newline at end of file +#endif // HAVE_SVS diff --git a/tests/unit/test_svs.cpp b/tests/unit/test_svs.cpp index 52331bc77..cdffbf5c6 100644 --- a/tests/unit/test_svs.cpp +++ b/tests/unit/test_svs.cpp @@ -2129,4 +2129,4 @@ TEST(SVSTest, svs_not_supported) { ASSERT_EQ(size2, -1); } -#endif \ No newline at end of file +#endif From 74d23d9367a35f06aefe17ce721aeb79ea7489ae Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Thu, 17 Apr 2025 17:19:06 +0200 Subject: [PATCH 4/6] Simplify ASSERT_INDEX() macro in svs_test --- tests/unit/test_svs.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_svs.cpp b/tests/unit/test_svs.cpp index cdffbf5c6..00808a29e 100644 --- a/tests/unit/test_svs.cpp +++ b/tests/unit/test_svs.cpp @@ -8,11 +8,10 @@ #define ASSERT_INDEX(index) \ if (index == nullptr) { \ - const auto quantBits = TypeParam::get_quant_bits(); \ - if (quantBits != VecSimSvsQuant_NONE && !svs_details::isSVSLVQModeSupported(quantBits)) { \ - GTEST_SKIP() << "SVS LVQ is not supported."; \ - } else { \ + if (svs_details::isSVSLVQModeSupported(TypeParam::get_quant_bits())) { \ GTEST_FAIL() << "Failed to create SVS index"; \ + } else { \ + GTEST_SKIP() << "SVS LVQ is not supported."; \ } \ } From 1eb1cbf5b80c9c16430ca33fc6aef727c479e74e Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 25 Apr 2025 14:50:45 +0200 Subject: [PATCH 5/6] Enable quantization fallback to non-quantized mode --- src/VecSim/algorithms/svs/svs_utils.h | 18 +++++++++++++++--- src/VecSim/index_factories/svs_factory.cpp | 15 ++++++++++----- tests/unit/test_svs.cpp | 12 +++++++++--- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_utils.h b/src/VecSim/algorithms/svs/svs_utils.h index 498904a24..dcb0a8179 100644 --- a/src/VecSim/algorithms/svs/svs_utils.h +++ b/src/VecSim/algorithms/svs/svs_utils.h @@ -10,6 +10,7 @@ #include #include #include +#include namespace svs_details { // VecSim->SVS data type conversion @@ -149,12 +150,23 @@ inline bool check_cpuid() { } // clang-format on -inline bool isSVSLVQModeSupported(VecSimSvsQuantBits quant_bits) { - return quant_bits == VecSimSvsQuant_NONE +// Check if the SVS implementation supports Qquantization mode +// @param quant_bits requested SVS quantization mode +// @return pair +inline std::pair isSVSQuantBitsSupported(VecSimSvsQuantBits quant_bits) { + // If HAVE_SVS_LVQ is not defined, we don't support any quantization mode + // else we check if the CPU supports SVS LVQ + bool supported = quant_bits == VecSimSvsQuant_NONE #if HAVE_SVS_LVQ - || check_cpuid() // Check if the CPU supports SVS LVQ + || check_cpuid() // Check if the CPU supports SVS LVQ #endif ; + + // If the quantization mode is not supported, we fallback to non-quantized mode + auto fallBack = supported ? quant_bits : VecSimSvsQuant_NONE; + + // And always return true, as far as non-quantized mode is always supported + return std::make_pair(fallBack, true); } } // namespace svs_details diff --git a/src/VecSim/index_factories/svs_factory.cpp b/src/VecSim/index_factories/svs_factory.cpp index 502c7c511..1641bf960 100644 --- a/src/VecSim/index_factories/svs_factory.cpp +++ b/src/VecSim/index_factories/svs_factory.cpp @@ -37,11 +37,12 @@ VecSimIndex *NewIndexImpl(const VecSimParams *params, bool is_normalized) { template VecSimIndex *NewIndexImpl(const VecSimParams *params, bool is_normalized) { - if (!svs_details::isSVSLVQModeSupported(params->algoParams.svsParams.quantBits)) { - return NULL; - } + // Ignore the 'supported' flag because we always fallback at least to the non-quantized mode + // elsewhere we got code coverage failure for the `supported==false` case + auto quantBits = + std::get<0>(svs_details::isSVSQuantBitsSupported(params->algoParams.svsParams.quantBits)); - switch (params->algoParams.svsParams.quantBits) { + switch (quantBits) { case VecSimSvsQuant_NONE: return NewIndexImpl(params, is_normalized); case VecSimSvsQuant_8: @@ -97,7 +98,11 @@ constexpr size_t QuantizedVectorSize(size_t dims, size_t alignment = 0) { template size_t QuantizedVectorSize(VecSimSvsQuantBits quant_bits, size_t dims, size_t alignment = 0) { - switch (quant_bits) { + // Ignore the 'supported' flag because we always fallback at least to the non-quantized mode + // elsewhere we got code coverage failure for the `supported==false` case + auto quantBits = std::get<0>(svs_details::isSVSQuantBitsSupported(quant_bits)); + + switch (quantBits) { case VecSimSvsQuant_NONE: return QuantizedVectorSize(dims, alignment); case VecSimSvsQuant_8: diff --git a/tests/unit/test_svs.cpp b/tests/unit/test_svs.cpp index 00808a29e..daf4a93f9 100644 --- a/tests/unit/test_svs.cpp +++ b/tests/unit/test_svs.cpp @@ -8,7 +8,7 @@ #define ASSERT_INDEX(index) \ if (index == nullptr) { \ - if (svs_details::isSVSLVQModeSupported(TypeParam::get_quant_bits())) { \ + if (std::get<1>(svs_details::isSVSQuantBitsSupported(TypeParam::get_quant_bits()))) { \ GTEST_FAIL() << "Failed to create SVS index"; \ } else { \ GTEST_SKIP() << "SVS LVQ is not supported."; \ @@ -698,7 +698,10 @@ TYPED_TEST(SVSTest, resizeIndex) { // Initial capacity is rounded up to the block size. size_t extra_cap = n % bs == 0 ? 0 : bs - n % bs; - if constexpr (TypeParam::get_quant_bits() > 0) { + auto quantBits = TypeParam::get_quant_bits(); + // Get the fallback quantization mode + quantBits = std::get<0>(svs_details::isSVSQuantBitsSupported(quantBits)); + if (quantBits != VecSimSvsQuant_NONE) { // LVQDataset does not provide a capacity method extra_cap = 0; } @@ -1460,7 +1463,10 @@ TYPED_TEST(SVSTest, testSizeEstimation) { // converted then to a number of elements. // IMHO, would be better to always interpret block size to a number of elements // rather than conversion to-from number of bytes - if (TypeParam::get_quant_bits() > 0) { + auto quantBits = TypeParam::get_quant_bits(); + // Get the fallback quantization mode + quantBits = std::get<0>(svs_details::isSVSQuantBitsSupported(quantBits)); + if (quantBits != VecSimSvsQuant_NONE) { // Extra data in LVQ vector const auto lvq_vector_extra = sizeof(svs::quantization::lvq::ScalarBundle); dim -= (lvq_vector_extra * 8) / TypeParam::get_quant_bits(); From 6d567baa654a67de30c9a9412daa2bab1e90b4b6 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Wed, 30 Apr 2025 14:28:01 +0200 Subject: [PATCH 6/6] Added comments according to code review requests. --- src/VecSim/algorithms/svs/svs_utils.h | 12 ++++++++++-- src/VecSim/index_factories/svs_factory.cpp | 3 +++ tests/unit/test_svs.cpp | 4 ++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_utils.h b/src/VecSim/algorithms/svs/svs_utils.h index dcb0a8179..172a0e7a1 100644 --- a/src/VecSim/algorithms/svs/svs_utils.h +++ b/src/VecSim/algorithms/svs/svs_utils.h @@ -150,9 +150,13 @@ inline bool check_cpuid() { } // clang-format on -// Check if the SVS implementation supports Qquantization mode +// Check if the SVS implementation supports Quantization mode // @param quant_bits requested SVS quantization mode // @return pair +// @note even if VecSimSvsQuantBits is a simple enum value, +// in theory, it can be a complex type with a combination of modes: +// - primary bits, secondary/residual bits, dimesionality reduction, etc. +// which can be incompatible to each-other. inline std::pair isSVSQuantBitsSupported(VecSimSvsQuantBits quant_bits) { // If HAVE_SVS_LVQ is not defined, we don't support any quantization mode // else we check if the CPU supports SVS LVQ @@ -163,9 +167,13 @@ inline std::pair isSVSQuantBitsSupported(VecSimSvsQuan ; // If the quantization mode is not supported, we fallback to non-quantized mode + // - this is temporary solution until we have a basic quantization mode in SVS + // TODO: use basic SVS quantization as a fallback for unsupported modes auto fallBack = supported ? quant_bits : VecSimSvsQuant_NONE; - // And always return true, as far as non-quantized mode is always supported + // And always return true, as far as fallback mode is always supported + // Upon further decision changes, some cases should treated as not-supported + // So we will need return false. return std::make_pair(fallBack, true); } } // namespace svs_details diff --git a/src/VecSim/index_factories/svs_factory.cpp b/src/VecSim/index_factories/svs_factory.cpp index 1641bf960..4a29052cb 100644 --- a/src/VecSim/index_factories/svs_factory.cpp +++ b/src/VecSim/index_factories/svs_factory.cpp @@ -181,6 +181,9 @@ size_t EstimateInitialSize(const SVSParams *params, bool is_normalized) { } // namespace SVSFactory +// This is a temporary solution to avoid breaking the build when SVS is not available +// and to allow the code to compile without SVS support. +// TODO: remove HAVE_SVS when SVS will support all Redis platfoms and compilers #else // HAVE_SVS namespace SVSFactory { VecSimIndex *NewIndex(const VecSimParams *params, bool is_normalized) { return NULL; } diff --git a/tests/unit/test_svs.cpp b/tests/unit/test_svs.cpp index daf4a93f9..2923bed7e 100644 --- a/tests/unit/test_svs.cpp +++ b/tests/unit/test_svs.cpp @@ -6,6 +6,10 @@ #if HAVE_SVS #include "VecSim/algorithms/svs/svs.h" +// There are possible cases when SVS Index cannot be created with the requested quantization mode +// due to platform and/or hardware limitations or combination of requested 'compression' modes. +// This assert handle those cases and skip a test if the mode is not supported. +// Elsewhere, test will fail if the index creation failed with no reason explained above. #define ASSERT_INDEX(index) \ if (index == nullptr) { \ if (std::get<1>(svs_details::isSVSQuantBitsSupported(TypeParam::get_quant_bits()))) { \