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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 5, 2025
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
88b1481
Return NULL Index if SVS LVQ is requested but not supported.
rfsaliev Apr 17, 2025
fb75a0c
Add tests for non-supported SVS cases.
rfsaliev Apr 17, 2025
cd7cd57
Fix SVS LVQ mode support detection.
rfsaliev Apr 17, 2025
74d23d9
Simplify ASSERT_INDEX() macro in svs_test
rfsaliev Apr 17, 2025
1eb1cbf
Enable quantization fallback to non-quantized mode
rfsaliev Apr 25, 2025
6d567ba
Added comments according to code review requests.
rfsaliev Apr 30, 2025
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
View file
Open in desktop
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
View file
Open in desktop
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
35 changes: 35 additions & 0 deletions src/VecSim/algorithms/svs/svs_utils.h
View file
Open in desktop
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,35 @@ 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 Qquantization mode
// @param quant_bits requested SVS quantization mode
// @return pair<fallbackMode, bool>
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
auto fallBack = supported ? quant_bits : VecSimSvsQuant_NONE;

// And always return true, as far as non-quantized mode is always supported
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 mark in TODO that the actual fallback should be the basic quantisation if check_cpuid() == false && quant_bits != VecSimSvsQuant_NONE once it is available in svs

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 comments

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
View file
Open in desktop
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
23 changes: 21 additions & 2 deletions src/VecSim/index_factories/svs_factory.cpp
View file
Open in desktop
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,11 @@ size_t EstimateInitialSize(const SVSParams *params, bool is_normalized) {
}

} // namespace SVSFactory

#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
View file
Open in desktop
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