[MOD-9384] [SVS] Fix CI builds #658
Conversation
* Add CPU detection for SVS * Add MKL installation for SVS shared library
This reverts commit 1a10a0a.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #658 +/- ##
==========================================
+ Coverage 96.25% 96.33% +0.07%
==========================================
Files 107 112 +5
Lines 5772 6225 +453
==========================================
+ Hits 5556 5997 +441
- Misses 216 228 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rfsaliev
force-pushed
the
rfsaliev/install-mkl
branch
3 times, most recently
from
08d6ce0 to
6b8485e
Compare
rfsaliev
force-pushed
the
rfsaliev/install-mkl
branch
from
6b8485e to
92eae96
Compare
rfsaliev
force-pushed
the
rfsaliev/install-mkl
branch
from
9b08296 to
8223385
Compare
This reverts commit 7b9c5ad.
disable skipping
This reverts commit 79c5682.
There was a problem hiding this comment.
Thank you :)
see my comments along the code
Adiitionaly, as discussed, we should have tests for un supported platforms, verifying that vecsim fatory should returns NULL, including mac (since case VecSimAlgo_SVS: is reachble on this platform)
The tests should simulate the user exprience.
Thanks again
| find_package(svs REQUIRED) | ||
| set(SVS_LVQ_HEADER "svs/extensions/vamana/lvq.h") | ||
| set(SVS_TARGET_NAME "svs::svs_shared_library") | ||
| else() |
There was a problem hiding this comment.
in which case we expect to enter the else?
There was a problem hiding this comment.
when SVS_SHARED_LIB=OFF
e.g. when user provided -DSVS_SHARED_LIB=OFF in CMAKE_FLAGS or MKL library is not installed on the system.
There was a problem hiding this comment.
so why do we need to compile this in this case? what capabilities are added to the library ?
add_subdirectory(
${root}/deps/ScalableVectorSearch
deps/ScalableVectorSearch
)
set(SVS_LVQ_HEADER "svs/quantization/lvq/impl/lvq_impl.h")
There was a problem hiding this comment.
This option allows to compile SVSIndex without shared library from public sources without LVQ.
But also allows to compile with LVQ using SVS private sources which contain lvq_impl.h.
| find_package(svs REQUIRED) | ||
| set(SVS_LVQ_HEADER "svs/extensions/vamana/lvq.h") | ||
| set(SVS_TARGET_NAME "svs::svs_shared_library") | ||
| else() |
There was a problem hiding this comment.
so why do we need to compile this in this case? what capabilities are added to the library ?
add_subdirectory(
${root}/deps/ScalableVectorSearch
deps/ScalableVectorSearch
)
set(SVS_LVQ_HEADER "svs/quantization/lvq/impl/lvq_impl.h")
| # Valgrind does not support AVX512 and Valgrind in running in Debug | ||
| # so disable it if we are in Debug mode | ||
| string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) | ||
| if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG") |
There was a problem hiding this comment.
Added status message.
Enable and fix SVS Index build
A clear and concise description of what the PR is solving.
Which issues this PR fixes
svs.hcompilation error introduced by Refactor Info Report - [MOD-9321] #650Main objects this PR modified
cmake/svs.cmake- which is included inCMakeLists.txtand enables SVS Index support depending on OS/Platform, CPU and compiler compatibilities:ON, forced toOFFfor non-compatible platformsON, forced toOFFifUSE_SVSisOFFor non-GLIBC build environment (e.g. Alpine Linux with libc MUSL).SVS_SHARED_LIBis off, then SVSIndex is compiled using ScalableVectorSearch public code without LVQ/LeanVec support.Mark if applicable