-
Notifications
You must be signed in to change notification settings - Fork 73
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
ANN_BENCH #130
ANN_BENCH #130
Changes from 76 commits
4416b0b
118906c
7814d35
f8d8b68
946e106
7c6644e
0619207
9313d1f
2e61d61
8c6f9fe
2c3f5e4
4d5ff0c
545032b
53caff9
d2c1dad
9f31182
292e947
f1297da
9445708
2401fcd
0d214c0
5efe1c6
4b9ec70
4418cf7
09c1d73
5dcaecb
55924cf
8c37b94
8837d2d
c0fbae5
a84659a
c7d1bb6
669fc0e
eac32f0
a6dcbd0
8e2b62a
bb6dbb2
d1969ce
971623c
aafc748
e412122
9823831
f310971
c1136c5
bb87c97
8506c6a
3a6e776
42aa3eb
68f490b
95f6064
3e22b13
fd66402
b867488
d82e497
4f67adc
a96dcb9
2eaf0c6
9e90d8d
48f8353
0fbf825
6a20121
1e64d05
baf8863
c047931
887a656
001108d
449be8c
a71dcb0
8887225
a32abd5
c22035c
512cff6
c8ae918
3fd97f6
3daa597
10171cf
398a808
d7c25bd
660c40d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
#!/usr/bin/env bash | ||
# Copyright (c) 2022-2024, NVIDIA CORPORATION. | ||
|
||
./build.sh tests --allgpuarch --no-nvtx --build-metrics=tests_bench --incl-cache-stats | ||
./build.sh tests bench-ann --allgpuarch --no-nvtx --build-metrics=tests_bench --incl-cache-stats | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Over in RAFT, For my own understanding in reviewing this... why is this being added to the Having these separate things be their own packages can be helpful for parallelizing development, limiting the potential impact of packaging changes, and speeding up debugging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that benchmarks should probably be a separate package. I'm only hesitating to add this because I'm not very familiar with this conda/CI setup. I hoped to postpone this question till a follow-on PR together with the naming decision (#160 (comment)). @benfred, @cjnolet , what do you think? |
||
cmake --install cpp/build --component testing |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,7 @@ option(BUILD_SHARED_LIBS "Build cuvs shared libraries" ON) | |
option(BUILD_TESTS "Build cuvs unit-tests" ON) | ||
option(BUILD_C_LIBRARY "Build raft C API library" OFF) | ||
option(BUILD_C_TESTS "Build raft C API tests" OFF) | ||
option(BUILD_ANN_BENCH "Build cuVS ann benchmarks" OFF) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add an option to build this in CI? It looks to me that this is off by default (which is fine), but that does mean that we don't have any guarantee that any of this is compiling. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We enable this currently together with tests in build.sh (see #130 (comment)) |
||
option(CUDA_ENABLE_KERNELINFO "Enable kernel resource usage info" OFF) | ||
option(CUDA_ENABLE_LINEINFO | ||
"Enable the -lineinfo option for nvcc (useful for cuda-memcheck / profiler)" OFF | ||
|
@@ -92,6 +93,7 @@ include(CMakeDependentOption) | |
|
||
message(VERBOSE "cuVS: Build cuVS unit-tests: ${BUILD_TESTS}") | ||
message(VERBOSE "cuVS: Build CPU only components: ${BUILD_CPU_ONLY}") | ||
message(VERBOSE "cuVS: Build ANN benchmarks: ${BUILD_ANN_BENCH}") | ||
message(VERBOSE "cuVS: Enable detection of conda environment for dependencies: ${DETECT_CONDA_ENV}") | ||
message(VERBOSE "cuVS: Disable depreaction warnings " ${DISABLE_DEPRECATION_WARNINGS}) | ||
message(VERBOSE "cuVS: Disable OpenMP: ${DISABLE_OPENMP}") | ||
|
@@ -184,6 +186,11 @@ endif() | |
|
||
include(cmake/thirdparty/get_cutlass.cmake) | ||
|
||
if(BUILD_ANN_BENCH) | ||
include(${rapids-cmake-dir}/cpm/gbench.cmake) | ||
rapids_cpm_gbench(BUILD_STATIC) | ||
endif() | ||
|
||
# ################################################################################################## | ||
# * cuvs --------------------------------------------------------------------- | ||
|
||
|
@@ -663,3 +670,10 @@ if(BUILD_TESTS OR BUILD_C_TESTS) | |
include(internal/CMakeLists.txt) | ||
include(test/CMakeLists.txt) | ||
endif() | ||
|
||
# ################################################################################################## | ||
# * build ann benchmark executable ----------------------------------------------- | ||
|
||
if(BUILD_ANN_BENCH) | ||
include(bench/ann/CMakeLists.txt) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this to benchmarks, maybe? I'm thinking maybe we could start building one benchmarks suite but maybe have different entrypoints for the ANN grid searches vs clustering benchmarking, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like the benchmark infrastructure differs significantly between vector search vs clustering.
What about having multiple entry points but renaming them to better reflect what we're benchmarking? e.g.
bench-search, bench-clustering.Update: bench-ann currently handles search and indexing, so probably "bench-search" is not very appropriate. Maybe keep this
bench-ann
and add thebench-clustering
,bench-dists
later?