Skip to content

Commit 5b72ea8

Browse files
committed
ACE: harden npartitions check
- 0 means auto. - 1 is invalid since at least 2 paritions are required. Increase to 2. - > nrows is invalid and an error is thrown.
1 parent 64cdf7b commit 5b72ea8

7 files changed

Lines changed: 136 additions & 14 deletions

File tree

cpp/src/neighbors/detail/cagra/cagra_build.cuh

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
#pragma once
@@ -959,6 +959,30 @@ bool ace_check_use_disk_mode(raft::resources const& res,
959959
return use_disk_mode;
960960
}
961961

962+
// Resolve the ACE partition count while preserving 0 as the auto-selection sentinel.
963+
inline size_t ace_resolve_partition_count(size_t n_partitions)
964+
{
965+
if (n_partitions == 0) { return 2; }
966+
if (n_partitions == 1) {
967+
RAFT_LOG_WARN(
968+
"ACE: Requested 1 partition; adjusted to 2 before applying partitioning heuristics");
969+
return 2;
970+
}
971+
return n_partitions;
972+
}
973+
974+
// Validate the structural ACE partition-count invariants required by the labeler.
975+
inline void ace_validate_partition_count(size_t n_partitions,
976+
size_t dataset_size,
977+
bool adjusted_for_memory = false)
978+
{
979+
RAFT_EXPECTS(n_partitions <= dataset_size,
980+
adjusted_for_memory
981+
? "ACE: configured memory limit is unsatisfiable because the requested partition "
982+
"count cannot exceed dataset size"
983+
: "ACE: number of partitions cannot exceed dataset size");
984+
}
985+
962986
// Validate and adjust partitions for disk mode memory requirements
963987
template <typename T, typename IdxT>
964988
void ace_validate_disk_mode_partitions(raft::resources const& res,
@@ -1154,11 +1178,9 @@ index<T, IdxT> build_ace(raft::resources const& res,
11541178
"ACE: Intermediate graph degree must be greater than 0");
11551179
RAFT_EXPECTS(params.graph_degree > 0, "ACE: Graph degree must be greater than 0");
11561180

1157-
size_t n_partitions = npartitions;
1158-
if (n_partitions == 0) {
1159-
// Default: start with 2 partitions and increase if needed (minimum for ACE to make sense).
1160-
n_partitions = 2;
1161-
}
1181+
size_t n_partitions = ace_resolve_partition_count(npartitions);
1182+
1183+
ace_validate_partition_count(n_partitions, dataset_size);
11621184

11631185
size_t min_required_per_partition = 1000;
11641186
if (n_partitions > dataset_size / min_required_per_partition) {
@@ -1211,6 +1233,7 @@ index<T, IdxT> build_ace(raft::resources const& res,
12111233
graph_degree,
12121234
params.guarantee_connectivity,
12131235
mem);
1236+
ace_validate_partition_count(n_partitions, dataset_size, true);
12141237
}
12151238

12161239
// Preallocate space for files for better performance and fail early if not enough space.

cpp/tests/neighbors/ann_hnsw_ace.cuh

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
#pragma once
@@ -262,6 +262,31 @@ class AnnHnswAceTest : public ::testing::TestWithParam<AnnHnswAceInputs> {
262262
std::filesystem::remove_all(temp_dir);
263263
}
264264

265+
void testHnswAceRejectsTooManyPartitions()
266+
{
267+
auto database_host = raft::make_host_matrix<DataT, int64_t>(ps.n_rows, ps.dim);
268+
raft::copy(database_host.data_handle(), database_dev.data(), ps.n_rows * ps.dim, stream_);
269+
raft::resource::sync_stream(handle_);
270+
271+
hnsw::index_params hnsw_params;
272+
hnsw_params.metric = ps.metric;
273+
hnsw_params.hierarchy = hnsw::HnswHierarchy::GPU;
274+
hnsw_params.M = 32;
275+
hnsw_params.ef_construction = ps.ef_construction;
276+
277+
auto ace_params = graph_build_params::ace_params();
278+
ace_params.npartitions = static_cast<size_t>(ps.n_rows) + 1;
279+
hnsw_params.graph_build_params = ace_params;
280+
281+
try {
282+
[[maybe_unused]] auto hnsw_index =
283+
hnsw::build(handle_, hnsw_params, raft::make_const_mdspan(database_host.view()));
284+
FAIL() << "ACE accepted more partitions than dataset rows";
285+
} catch (const std::exception& e) {
286+
EXPECT_NE(std::string(e.what()).find("cannot exceed dataset size"), std::string::npos);
287+
}
288+
}
289+
265290
// Verify the in-memory CAGRA -> HNSW conversion spills to disk when the resulting HNSW
266291
// index would not fit in (an artificially constrained) host memory. This exercises
267292
// serialize_to_hnswlib_from_inmem and the batched serializer core, covering:
@@ -472,7 +497,7 @@ inline std::vector<AnnHnswAceInputs> generate_hnsw_ace_inputs()
472497
{5000}, // n_rows
473498
{64, 128}, // dim
474499
{10}, // k
475-
{2, 4}, // npartitions
500+
{0, 1, 2, 4}, // npartitions (auto, adjusted, and explicit)
476501
{100}, // ef_construction
477502
{false, true}, // use_disk (test both modes)
478503
{cuvs::distance::DistanceType::L2Expanded,
@@ -502,6 +527,19 @@ inline std::vector<AnnHnswAceInputs> generate_hnsw_ace_memory_fallback_inputs()
502527
};
503528
}
504529

530+
inline std::vector<AnnHnswAceInputs> generate_hnsw_ace_invalid_partition_inputs()
531+
{
532+
return {{10,
533+
5000,
534+
64,
535+
10,
536+
0, // Set to n_rows + 1 by testHnswAceRejectsTooManyPartitions.
537+
100,
538+
false,
539+
cuvs::distance::DistanceType::L2Expanded,
540+
0.0}};
541+
}
542+
505543
// Inputs for testing the in-memory CAGRA -> HNSW disk-spill conversion path.
506544
inline std::vector<AnnHnswAceInputs> generate_hnsw_inmem_spill_inputs()
507545
{
@@ -524,6 +562,8 @@ inline std::vector<AnnHnswAceInputs> generate_hnsw_inmem_spill_inputs()
524562
const std::vector<AnnHnswAceInputs> hnsw_ace_inputs = generate_hnsw_ace_inputs();
525563
const std::vector<AnnHnswAceInputs> hnsw_ace_memory_fallback_inputs =
526564
generate_hnsw_ace_memory_fallback_inputs();
565+
const std::vector<AnnHnswAceInputs> hnsw_ace_invalid_partition_inputs =
566+
generate_hnsw_ace_invalid_partition_inputs();
527567
const std::vector<AnnHnswAceInputs> hnsw_inmem_spill_inputs = generate_hnsw_inmem_spill_inputs();
528568

529569
} // namespace cuvs::neighbors::hnsw

cpp/tests/neighbors/ann_hnsw_ace/test_float_uint32_t.cu

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -12,6 +12,16 @@ TEST_P(AnnHnswAceTest_float, AnnHnswAceBuild) { this->testHnswAceBuild(); }
1212

1313
INSTANTIATE_TEST_CASE_P(AnnHnswAceTest, AnnHnswAceTest_float, ::testing::ValuesIn(hnsw_ace_inputs));
1414

15+
typedef AnnHnswAceTest<float, float, uint32_t> AnnHnswAceInvalidPartitionTest_float;
16+
TEST_P(AnnHnswAceInvalidPartitionTest_float, RejectsTooManyPartitions)
17+
{
18+
this->testHnswAceRejectsTooManyPartitions();
19+
}
20+
21+
INSTANTIATE_TEST_CASE_P(AnnHnswAceInvalidPartitionTest,
22+
AnnHnswAceInvalidPartitionTest_float,
23+
::testing::ValuesIn(hnsw_ace_invalid_partition_inputs));
24+
1525
// Test for memory limit fallback to disk mode
1626
typedef AnnHnswAceTest<float, float, uint32_t> AnnHnswAceMemoryFallbackTest_float;
1727
TEST_P(AnnHnswAceMemoryFallbackTest_float, AnnHnswAceMemoryLimitFallback)

cpp/tests/neighbors/ann_hnsw_ace/test_half_uint32_t.cu

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -12,6 +12,16 @@ TEST_P(AnnHnswAceTest_half, AnnHnswAceBuild) { this->testHnswAceBuild(); }
1212

1313
INSTANTIATE_TEST_CASE_P(AnnHnswAceTest, AnnHnswAceTest_half, ::testing::ValuesIn(hnsw_ace_inputs));
1414

15+
typedef AnnHnswAceTest<float, half, uint32_t> AnnHnswAceInvalidPartitionTest_half;
16+
TEST_P(AnnHnswAceInvalidPartitionTest_half, RejectsTooManyPartitions)
17+
{
18+
this->testHnswAceRejectsTooManyPartitions();
19+
}
20+
21+
INSTANTIATE_TEST_CASE_P(AnnHnswAceInvalidPartitionTest,
22+
AnnHnswAceInvalidPartitionTest_half,
23+
::testing::ValuesIn(hnsw_ace_invalid_partition_inputs));
24+
1525
// Test for memory limit fallback to disk mode
1626
typedef AnnHnswAceTest<float, half, uint32_t> AnnHnswAceMemoryFallbackTest_half;
1727
TEST_P(AnnHnswAceMemoryFallbackTest_half, AnnHnswAceMemoryLimitFallback)

cpp/tests/neighbors/ann_hnsw_ace/test_int8_t_uint32_t.cu

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -14,6 +14,16 @@ INSTANTIATE_TEST_CASE_P(AnnHnswAceTest,
1414
AnnHnswAceTest_int8_t,
1515
::testing::ValuesIn(hnsw_ace_inputs));
1616

17+
typedef AnnHnswAceTest<float, int8_t, uint32_t> AnnHnswAceInvalidPartitionTest_int8_t;
18+
TEST_P(AnnHnswAceInvalidPartitionTest_int8_t, RejectsTooManyPartitions)
19+
{
20+
this->testHnswAceRejectsTooManyPartitions();
21+
}
22+
23+
INSTANTIATE_TEST_CASE_P(AnnHnswAceInvalidPartitionTest,
24+
AnnHnswAceInvalidPartitionTest_int8_t,
25+
::testing::ValuesIn(hnsw_ace_invalid_partition_inputs));
26+
1727
// Test for memory limit fallback to disk mode
1828
typedef AnnHnswAceTest<float, int8_t, uint32_t> AnnHnswAceMemoryFallbackTest_int8_t;
1929
TEST_P(AnnHnswAceMemoryFallbackTest_int8_t, AnnHnswAceMemoryLimitFallback)

cpp/tests/neighbors/ann_hnsw_ace/test_uint8_t_uint32_t.cu

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -14,6 +14,16 @@ INSTANTIATE_TEST_CASE_P(AnnHnswAceTest,
1414
AnnHnswAceTest_uint8_t,
1515
::testing::ValuesIn(hnsw_ace_inputs));
1616

17+
typedef AnnHnswAceTest<float, uint8_t, uint32_t> AnnHnswAceInvalidPartitionTest_uint8_t;
18+
TEST_P(AnnHnswAceInvalidPartitionTest_uint8_t, RejectsTooManyPartitions)
19+
{
20+
this->testHnswAceRejectsTooManyPartitions();
21+
}
22+
23+
INSTANTIATE_TEST_CASE_P(AnnHnswAceInvalidPartitionTest,
24+
AnnHnswAceInvalidPartitionTest_uint8_t,
25+
::testing::ValuesIn(hnsw_ace_invalid_partition_inputs));
26+
1727
// Test for memory limit fallback to disk mode
1828
typedef AnnHnswAceTest<float, uint8_t, uint32_t> AnnHnswAceMemoryFallbackTest_uint8_t;
1929
TEST_P(AnnHnswAceMemoryFallbackTest_uint8_t, AnnHnswAceMemoryLimitFallback)

python/cuvs/cuvs/tests/test_cagra_ace.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION.
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
# SPDX-License-Identifier: Apache-2.0
33
#
44

@@ -163,7 +163,7 @@ def test_cagra_ace_dtypes_and_metrics(dtype, metric, use_disk):
163163
)
164164

165165

166-
@pytest.mark.parametrize("npartitions", [2, 3, 8])
166+
@pytest.mark.parametrize("npartitions", [0, 1, 2, 3, 8])
167167
def test_cagra_ace_partitions(npartitions):
168168
"""Test ACE with different partition sizes (disk mode only)."""
169169
run_cagra_ace_build_search_test(
@@ -172,6 +172,25 @@ def test_cagra_ace_partitions(npartitions):
172172
)
173173

174174

175+
@pytest.mark.parametrize(
176+
("npartitions", "message"),
177+
[
178+
(33, "cannot exceed dataset size"),
179+
],
180+
)
181+
def test_cagra_ace_rejects_invalid_partition_count(npartitions, message):
182+
"""ACE rejects partition counts that would make partition labeling invalid."""
183+
dataset = np.zeros((32, 8), dtype=np.float32)
184+
ace_params = cagra.AceParams(npartitions=npartitions, use_disk=False)
185+
build_params = cagra.IndexParams(
186+
build_algo="ace",
187+
ace_params=ace_params,
188+
)
189+
190+
with pytest.raises(Exception, match=message):
191+
cagra.build(build_params, dataset)
192+
193+
175194
@pytest.mark.parametrize("ef_construction", [50, 100, 200])
176195
def test_cagra_ace_ef_construction(ef_construction):
177196
"""Test ACE with different ef_construction values (disk mode only)."""

0 commit comments

Comments
 (0)