Skip to content

Commit 19c3b5a

Browse files
committed
fix(disk): honor explicit PiPNN build selection
1 parent 1d190ef commit 19c3b5a

4 files changed

Lines changed: 13 additions & 190 deletions

File tree

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

diskann-disk/src/build/builder/build.rs

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,26 +76,9 @@ where
7676
index_writer: DiskIndexWriter,
7777
) -> ANNResult<Self> {
7878
#[cfg(feature = "pipnn")]
79-
let disk_build_param = {
80-
let mut disk_build_param = disk_build_param;
81-
if let Some(config) = disk_build_param.pipnn_config() {
82-
config.validate()?;
83-
let estimate = disk_build_param.use_vamana_if_pipnn_exceeds(
84-
index_configuration.max_points,
85-
index_configuration.dim,
86-
std::mem::size_of::<Data::VectorDataType>(),
87-
index_configuration.num_threads,
88-
)?;
89-
let selected = disk_build_param.build_algorithm();
90-
info!(
91-
estimated_peak_bytes = estimate,
92-
memory_limit_bytes = disk_build_param.build_memory_limit().in_bytes(),
93-
algorithm = %selected,
94-
"Selected graph build algorithm"
95-
);
96-
}
97-
disk_build_param
98-
};
79+
if let Some(config) = disk_build_param.pipnn_config() {
80+
config.validate()?;
81+
}
9982

10083
let pq_storage = PQStorage::new(
10184
&(index_writer.get_index_path_prefix() + "_pq_pivots.bin"),

diskann-disk/src/build/builder/build/pipnn/tests.rs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@ use vfs::MemoryFS;
1717

1818
use crate::{
1919
build::{
20-
builder::{
21-
build::DiskIndexBuilder,
22-
core::{determine_build_strategy, IndexBuildStrategy},
23-
},
20+
builder::build::DiskIndexBuilder,
2421
configuration::{MemoryBudget, NumPQChunks, PiPNNParameters},
2522
},
2623
data_model::AdHoc,
@@ -147,15 +144,15 @@ fn pipnn_graph_adapter_writes_real_point_header() {
147144
}
148145

149146
#[test]
150-
fn pipnn_disk_build_falls_back_to_complete_vamana_pipeline() {
147+
fn explicit_pipnn_selection_is_not_replaced_by_memory_budget() {
151148
let storage = VirtualStorageProvider::new_memory();
152149
let (points, dimensions) = (256, 8);
153150
write_data(&storage, points, dimensions);
154151
let mut builder = builder(&storage, points, dimensions, 0.000001, 1.3, pipnn());
155152

156153
assert!(matches!(
157154
builder.disk_build_param.build_algorithm(),
158-
crate::BuildAlgorithm::Vamana
155+
crate::BuildAlgorithm::PiPNN(_)
159156
));
160157
assert_eq!(
161158
builder.disk_build_param.build_quantization(),
@@ -164,21 +161,12 @@ fn pipnn_disk_build_falls_back_to_complete_vamana_pipeline() {
164161
assert_eq!(builder.index_configuration.config.pruned_degree().get(), 32);
165162
assert_eq!(builder.index_configuration.config.l_build().get(), 50);
166163
assert_eq!(builder.index_configuration.config.alpha(), 1.3);
167-
assert!(matches!(
168-
determine_build_strategy::<AdHoc<f32>>(
169-
&builder.index_configuration,
170-
builder.disk_build_param.build_memory_limit().in_bytes() as f64,
171-
builder.disk_build_param.build_quantization(),
172-
),
173-
IndexBuildStrategy::Merged
174-
));
175-
176164
builder.build().unwrap();
177165
assert!(storage.exists(&get_disk_index_file("/index")));
178166
}
179167

180168
#[test]
181-
fn pipnn_disk_build_rejects_invalid_config_before_fallback() {
169+
fn pipnn_disk_build_rejects_invalid_config() {
182170
let storage = VirtualStorageProvider::new_memory();
183171
let invalid = PiPNNParameters {
184172
c_max: 0,
@@ -193,7 +181,7 @@ fn pipnn_disk_build_rejects_invalid_config_before_fallback() {
193181
let writer = DiskIndexWriter::new("/data.fbin".into(), "/index".into(), None, 4096).unwrap();
194182

195183
let error = match DiskIndexBuilder::<AdHoc<f32>, _>::new(&storage, params, config, writer) {
196-
Ok(_) => panic!("invalid PiPNN config must not silently fall back to Vamana"),
184+
Ok(_) => panic!("invalid PiPNN config must be rejected"),
197185
Err(error) => error,
198186
};
199187

diskann-disk/src/build/configuration/disk_index_build_parameter.rs

Lines changed: 5 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
use std::num::NonZeroUsize;
99

1010
use diskann::ANNError;
11-
#[cfg(feature = "pipnn")]
12-
use diskann::ANNResult;
1311
use thiserror::Error;
1412

1513
#[cfg(feature = "pipnn")]
@@ -111,8 +109,8 @@ impl NumPQChunks {
111109
/// Parameters specific for disk index construction.
112110
#[derive(Clone, PartialEq, Debug)]
113111
pub struct DiskIndexBuildParameters {
114-
/// Limit on graph-construction memory. PiPNN falls back to Vamana when its
115-
/// estimated one-shot peak exceeds this value.
112+
/// Memory budget for disk-index pipeline stages that support bounded work.
113+
/// Explicit one-shot PiPNN selection is never silently replaced.
116114
build_memory_limit: MemoryBudget,
117115

118116
/// Number of PQ chunks stored in-memory for search and to be generated during build.
@@ -146,8 +144,8 @@ impl DiskIndexBuildParameters {
146144

147145
/// Create parameters for one-shot PiPNN graph construction.
148146
///
149-
/// PiPNN uses the common search-PQ and disk-layout pipeline. The memory
150-
/// budget selects Vamana when the estimated PiPNN peak does not fit.
147+
/// PiPNN uses the common search-PQ and disk-layout pipeline. Its one-shot
148+
/// graph build is not governed by the pipeline memory budget.
151149
#[cfg(feature = "pipnn")]
152150
pub fn new_pipnn(
153151
build_memory_limit: MemoryBudget,
@@ -204,116 +202,6 @@ impl DiskIndexBuildParameters {
204202
BuildAlgorithm::Vamana => None,
205203
}
206204
}
207-
208-
#[cfg(feature = "pipnn")]
209-
pub(crate) fn use_vamana_if_pipnn_exceeds(
210-
&mut self,
211-
npoints: usize,
212-
dimensions: usize,
213-
element_size: usize,
214-
num_threads: usize,
215-
) -> ANNResult<usize> {
216-
let parameters = match &self.build_algorithm {
217-
BuildAlgorithm::PiPNN(parameters) => parameters,
218-
BuildAlgorithm::Vamana => {
219-
return Err(ANNError::log_index_error(
220-
"memory selection requires PiPNN parameters",
221-
));
222-
}
223-
};
224-
let estimate =
225-
estimate_pipnn_peak_memory(parameters, npoints, dimensions, element_size, num_threads);
226-
if estimate.is_none_or(|bytes| bytes > self.build_memory_limit.in_bytes()) {
227-
self.build_algorithm = BuildAlgorithm::Vamana;
228-
}
229-
Ok(estimate.unwrap_or(usize::MAX))
230-
}
231-
}
232-
233-
#[cfg(feature = "pipnn")]
234-
fn estimate_pipnn_peak_memory(
235-
config: &PiPNNParameters,
236-
npoints: usize,
237-
dimensions: usize,
238-
element_size: usize,
239-
num_threads: usize,
240-
) -> Option<usize> {
241-
const PROPORTIONAL_HEADROOM_PERCENT: u128 = 108;
242-
const PROCESS_HEADROOM: u128 = 16 * 1024 * 1024;
243-
const PER_WORKER_HEADROOM: u128 = 28 * 1024 * 1024;
244-
245-
let workers = if num_threads == 0 {
246-
std::thread::available_parallelism().map_or(1, NonZeroUsize::get)
247-
} else {
248-
num_threads
249-
} as u128;
250-
let copies = config
251-
.fanout
252-
.iter()
253-
.try_fold(config.replicas as u128, |copies, &fanout| {
254-
copies.checked_mul(fanout as u128)
255-
})?;
256-
let dataset = (dimensions as u128).checked_mul(element_size as u128)?;
257-
let leaf_ids = copies.checked_mul(size_of::<u32>() as u128)?;
258-
let offered = copies.checked_mul(config.k as u128)?.checked_mul(2)?;
259-
let candidate_capacity = offered.checked_next_power_of_two()?;
260-
let candidate_storage = candidate_capacity.checked_mul(size_of::<u32>() as u128)?;
261-
let candidate_metadata =
262-
size_of::<std::sync::Mutex<diskann::graph::AdjacencyList<u32>>>() as u128;
263-
264-
let partition_per_point = dataset
265-
.checked_add(16)?
266-
.checked_add(leaf_ids.checked_mul(2)?)?;
267-
let leaf_per_point = dataset
268-
.checked_add(candidate_metadata)?
269-
.checked_add(candidate_storage)?
270-
.checked_add(leaf_ids)?;
271-
let finalization_per_point = dataset
272-
.checked_add(candidate_metadata)?
273-
.checked_add(size_of::<Vec<u32>>() as u128)?
274-
.checked_add(candidate_storage)?;
275-
276-
let leaf_size = config.c_max.min(npoints).max(1) as u128;
277-
let leaf_scratch = leaf_size
278-
.checked_mul(dimensions as u128)?
279-
.checked_mul(size_of::<f32>() as u128)?
280-
.checked_add(leaf_size.checked_mul(leaf_size)?.checked_mul(5)?)?
281-
.checked_add(leaf_size.checked_mul(config.k as u128)?.checked_mul(24)?)?
282-
.checked_add(leaf_size.checked_mul(20)?)?
283-
.checked_add(68)?;
284-
let partition_rows = (npoints as u128).min(1_024);
285-
let partition_leaders = (npoints as u128).min(1_000);
286-
let partition_scratch = partition_leaders
287-
.checked_mul(dimensions as u128)?
288-
.checked_mul(size_of::<f32>() as u128)?
289-
.checked_add(
290-
partition_rows
291-
.checked_mul(dimensions as u128)?
292-
.checked_mul(size_of::<f32>() as u128)?,
293-
)?
294-
.checked_add(
295-
partition_rows
296-
.checked_mul(partition_leaders)?
297-
.checked_mul(size_of::<f32>() as u128)?,
298-
)?;
299-
300-
let points = npoints as u128;
301-
let structural = points
302-
.checked_mul(partition_per_point)?
303-
.checked_add(workers.checked_mul(partition_scratch)?)?
304-
.max(
305-
points
306-
.checked_mul(leaf_per_point)?
307-
.checked_add(workers.checked_mul(leaf_scratch)?)?,
308-
)
309-
.max(points.checked_mul(finalization_per_point)?);
310-
let proportional = structural
311-
.checked_mul(PROPORTIONAL_HEADROOM_PERCENT)?
312-
.div_ceil(100);
313-
let allocator_floor = structural
314-
.checked_add(PROCESS_HEADROOM)?
315-
.checked_add(workers.checked_mul(PER_WORKER_HEADROOM)?)?;
316-
usize::try_from(proportional.max(allocator_floor)).ok()
317205
}
318206

319207
#[cfg(test)]
@@ -407,47 +295,12 @@ mod dataset_test {
407295
let params = DiskIndexBuildParameters::new_pipnn(budget, pq, parameters);
408296

409297
assert_eq!(params.pipnn_config(), Some(config));
298+
assert_eq!(params.build_memory_limit(), budget);
410299
assert_eq!(params.search_pq_chunks(), pq);
411300
assert_eq!(
412301
params.data_compression_chunk_vector_count(),
413302
DEFAULT_DATA_COMPRESSION_CHUNK_VECTOR_COUNT
414303
);
415304
assert!(matches!(params.build_algorithm(), BuildAlgorithm::PiPNN(_)));
416305
}
417-
418-
#[cfg(feature = "pipnn")]
419-
#[test]
420-
fn pipnn_memory_estimate_covers_measured_bigann10m_peak() {
421-
const MEASURED_PEAK: usize = 8_656_004 * 1024;
422-
let parameters = PiPNNParameters {
423-
c_max: 512,
424-
c_min: 64,
425-
p_samp: 0.01,
426-
fanout: vec![10, 3],
427-
k: 2,
428-
replicas: 1,
429-
};
430-
431-
let estimate = estimate_pipnn_peak_memory(&parameters, 10_000_000, 128, 2, 16).unwrap();
432-
433-
assert!(estimate >= MEASURED_PEAK);
434-
assert!(estimate <= MEASURED_PEAK * 120 / 100);
435-
}
436-
437-
#[cfg(feature = "pipnn")]
438-
#[test]
439-
fn pipnn_falls_back_to_vamana_above_budget() {
440-
let budget = MemoryBudget::try_from_gb(1.0).unwrap();
441-
let pq = NumPQChunks::new_with(1, 128).unwrap();
442-
let mut params =
443-
DiskIndexBuildParameters::new_pipnn(budget, pq, PiPNNParameters::default());
444-
445-
let estimate = params
446-
.use_vamana_if_pipnn_exceeds(10_000_000, 128, 2, 16)
447-
.unwrap();
448-
449-
assert!(estimate > budget.in_bytes());
450-
assert!(matches!(params.build_algorithm(), BuildAlgorithm::Vamana));
451-
assert_eq!(params.build_quantization(), &QuantizationType::FP);
452-
}
453306
}

0 commit comments

Comments
 (0)