Fix SVS factory issue on non-SVS and non-LVQ platforms [MOD-9413] (#664)

* Return NULL Index if SVS LVQ is requested but not supported.

* Add tests for non-supported SVS cases.

* Fix SVS LVQ mode support detection.

* Simplify ASSERT_INDEX() macro in svs_test

* Enable quantization fallback to non-quantized mode

* Added comments according to code review requests.

Author: rfsaliev

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)

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

src/VecSim/algorithms/svs/svs_utils.h

@@ -15,6 +15,12 @@
 #include "svs/lib/float16.h"
 #include "svs/index/vamana/dynamic_index.h"
 
+#include <cpuid.h>
+#include <cstdint>
+#include <cstdlib>
+#include <string>
+#include <utility>
+
 namespace svs_details {
 // VecSim->SVS data type conversion
 template <typename T>
@@ -142,6 +148,43 @@ inline svs::lib::PowerOfTwo SVSBlockSize(size_t bs, size_t elem_size) {
     return svs_bs;
 }
 
+// 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) +
+                            std::string((const char*)&edx, 4) +
+                            std::string((const char*)&ecx, 4);
+    return (vendor_id == "GenuineIntel");
+}
+// clang-format on
+
+// Check if the SVS implementation supports Quantization mode
+// @param quant_bits requested SVS quantization mode
+// @return pair<fallbackMode, bool>
+// @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<VecSimSvsQuantBits, bool> 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
+#endif
+        ;
+
+    // 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 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
 
 template <typename DataType, size_t QuantBits, size_t ResidualBits, class Enable = void>

src/VecSim/index_factories/index_factory.cpp

@@ -11,9 +11,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) {
@@ -35,9 +33,7 @@ VecSimIndex *NewIndex(const VecSimParams *params) {
             break;
         }
         case VecSimAlgo_SVS: {
-#if HAVE_SVS
             index = SVSFactory::NewIndex(params);
-#endif
             break;
         }
         }
@@ -56,9 +52,7 @@ size_t EstimateInitialSize(const VecSimParams *params) {
     case VecSimAlgo_TIERED:
         return TieredFactory::EstimateInitialSize(&params->algoParams.tieredParams);
     case VecSimAlgo_SVS:; // empty statement if svs not available
-#if HAVE_SVS
         return SVSFactory::EstimateInitialSize(&params->algoParams.svsParams);
-#endif
     }
     return -1;
 }
@@ -72,9 +66,7 @@ size_t EstimateElementSize(const VecSimParams *params) {
     case VecSimAlgo_TIERED:
         return TieredFactory::EstimateElementSize(&params->algoParams.tieredParams);
     case VecSimAlgo_SVS:; // empty statement if svs not available
-#if HAVE_SVS
         return SVSFactory::EstimateElementSize(&params->algoParams.svsParams);
-#endif
     }
     return -1;
 }

src/VecSim/index_factories/svs_factory.cpp

@@ -8,6 +8,8 @@
  */
 
 #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"
@@ -44,7 +46,12 @@ VecSimIndex *NewIndexImpl(const VecSimParams *params, bool is_normalized) {
 
 template <typename MetricType, typename DataType>
 VecSimIndex *NewIndexImpl(const VecSimParams *params, bool is_normalized) {
-    switch (params->algoParams.svsParams.quantBits) {
+    // 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 (quantBits) {
     case VecSimSvsQuant_NONE:
         return NewIndexImpl<MetricType, DataType, 0>(params, is_normalized);
     case VecSimSvsQuant_8:
@@ -100,7 +107,11 @@ constexpr size_t QuantizedVectorSize(size_t dims, size_t alignment = 0) {
 
 template <typename DataType>
 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<DataType, 0>(dims, alignment);
     case VecSimSvsQuant_8:
@@ -178,3 +189,14 @@ 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; }
+size_t EstimateInitialSize(const SVSParams *params, bool is_normalized) { return -1; }
+size_t EstimateElementSize(const SVSParams *params) { return -1; }
+}; // namespace SVSFactory
+#endif // HAVE_SVS

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)