Skip to content

Commit f980d75

Browse files
authored
fix: Python default VamanaBuildParameters (#237)
Python default build parameters were not usable because of a type mismatch (pybind11 does not automatically convert float to size_t). Also use a C++ VamanaBuildParameters instance to set Python defaults instead of repeating the default constants. Finally, add an extra C++ test to catch inadvertent default constructor changes.
1 parent 49bf383 commit f980d75

File tree

4 files changed

+39
-9
lines changed

4 files changed

+39
-9
lines changed

bindings/python/src/vamana.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,7 @@ void wrap(py::module& m) {
414414
m, "VamanaBuildParameters", "Build parameters for Vamana index construction."
415415
);
416416

417+
svs::index::vamana::VamanaBuildParameters default_parameters;
417418
parameters
418419
.def(
419420
py::init([](float alpha,
@@ -430,13 +431,12 @@ void wrap(py::module& m) {
430431
prune_to,
431432
use_full_search_history};
432433
}),
433-
py::arg("alpha") = svs::FLOAT_PLACEHOLDER,
434-
py::arg("graph_max_degree") = svs::VAMANA_GRAPH_MAX_DEGREE_DEFAULT,
435-
py::arg("window_size") = svs::VAMANA_WINDOW_SIZE_DEFAULT,
436-
py::arg("max_candidate_pool_size") = svs::UNSIGNED_INTEGER_PLACEHOLDER,
437-
py::arg("prune_to") = svs::UNSIGNED_INTEGER_PLACEHOLDER,
438-
py::arg("use_full_search_history") =
439-
svs::VAMANA_USE_FULL_SEARCH_HISTORY_DEFAULT,
434+
py::arg("alpha") = default_parameters.alpha,
435+
py::arg("graph_max_degree") = default_parameters.graph_max_degree,
436+
py::arg("window_size") = default_parameters.window_size,
437+
py::arg("max_candidate_pool_size") = default_parameters.max_candidate_pool_size,
438+
py::arg("prune_to") = default_parameters.prune_to,
439+
py::arg("use_full_search_history") = default_parameters.use_full_search_history,
440440
R"(
441441
Construct a new instance from keyword arguments.
442442

bindings/python/tests/test_vamana.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,3 +378,23 @@ def test_build(self):
378378
self._test_build(loader, svs.DistanceType.L2, matcher)
379379
self._test_build(loader, svs.DistanceType.MIP, matcher)
380380
self._test_build(loader, svs.DistanceType.Cosine, matcher)
381+
382+
def test_vamana_build_parameters(self):
383+
"""Test VamanaBuildParameters construction and member accessors."""
384+
385+
params = svs.VamanaBuildParameters(
386+
alpha = 1.5,
387+
graph_max_degree = 64,
388+
prune_to = 32,
389+
window_size = 128,
390+
max_candidate_pool_size = 256
391+
)
392+
393+
self.assertEqual(params.alpha, 1.5)
394+
self.assertEqual(params.graph_max_degree, 64)
395+
self.assertEqual(params.prune_to, 32)
396+
self.assertEqual(params.window_size, 128)
397+
self.assertEqual(params.max_candidate_pool_size, 256)
398+
399+
# Test instatiation with default parameter values
400+
params = svs.VamanaBuildParameters()

include/svs/lib/preprocessor.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ namespace svs {
176176
// Maximum values used as default initializers
177177
inline constexpr size_t UNSIGNED_INTEGER_PLACEHOLDER = std::numeric_limits<size_t>::max();
178178
inline constexpr float FLOAT_PLACEHOLDER = std::numeric_limits<float>::max();
179-
inline constexpr float VAMANA_GRAPH_MAX_DEGREE_DEFAULT = 32;
180-
inline constexpr float VAMANA_WINDOW_SIZE_DEFAULT = 200;
179+
inline constexpr size_t VAMANA_GRAPH_MAX_DEGREE_DEFAULT = 32;
180+
inline constexpr size_t VAMANA_WINDOW_SIZE_DEFAULT = 200;
181181
inline constexpr bool VAMANA_USE_FULL_SEARCH_HISTORY_DEFAULT = true;
182182
inline constexpr float VAMANA_ALPHA_MINIMIZE_DEFAULT = 1.2;
183183
inline constexpr float VAMANA_ALPHA_MAXIMIZE_DEFAULT = 0.95;

tests/svs/index/vamana/build_parameters.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ window_size = 200
4242

4343
CATCH_TEST_CASE("VamanaBuildParameters", "[index][vamana]") {
4444
CATCH_SECTION("Constructors") {
45+
svs::index::vamana::VamanaBuildParameters empty;
46+
CATCH_REQUIRE(empty.alpha == svs::FLOAT_PLACEHOLDER);
47+
CATCH_REQUIRE(empty.graph_max_degree == svs::VAMANA_GRAPH_MAX_DEGREE_DEFAULT);
48+
CATCH_REQUIRE(empty.window_size == svs::VAMANA_WINDOW_SIZE_DEFAULT);
49+
CATCH_REQUIRE(empty.max_candidate_pool_size == svs::UNSIGNED_INTEGER_PLACEHOLDER);
50+
CATCH_REQUIRE(empty.prune_to == svs::UNSIGNED_INTEGER_PLACEHOLDER);
51+
CATCH_REQUIRE(
52+
empty.use_full_search_history == svs::VAMANA_USE_FULL_SEARCH_HISTORY_DEFAULT
53+
);
54+
4555
auto p = svs::index::vamana::VamanaBuildParameters{1.2f, 64, 128, 750, 60, true};
4656
CATCH_REQUIRE(p.alpha == 1.2f);
4757
CATCH_REQUIRE(p.graph_max_degree == 64);

0 commit comments

Comments
 (0)