Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cmake/svs.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/VecSim/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions src/VecSim/algorithms/svs/svs_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,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>
Expand Down Expand Up @@ -133,6 +139,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>
Expand Down
8 changes: 0 additions & 8 deletions src/VecSim/index_factories/index_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -33,9 +31,7 @@ VecSimIndex *NewIndex(const VecSimParams *params) {
break;
}
case VecSimAlgo_SVS: {
#if HAVE_SVS
index = SVSFactory::NewIndex(params);
#endif
break;
}
}
Expand All @@ -54,9 +50,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;
}
Expand All @@ -70,9 +64,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;
}
Expand Down
26 changes: 24 additions & 2 deletions src/VecSim/index_factories/svs_factory.cpp
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -35,7 +37,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:
Expand Down Expand Up @@ -91,7 +98,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:
Expand Down Expand Up @@ -169,3 +180,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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment that this is temporary, and we should never get here - once svs public repo will allow building for all platforms and we address platforms with GCC < 11

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment

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
12 changes: 3 additions & 9 deletions tests/unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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)
Loading
Loading