leaves12138 commented on code in PR #62:
URL:
https://github.com/apache/paimon-vector-index/pull/62#discussion_r3651534641
##########
core/src/ivfflat_io.rs:
##########
@@ -229,79 +378,175 @@ impl<R: SeekRead> IVFFlatIndexReader<R> {
return Ok(());
}
- let mut cursor = PreadCursor::new(&mut self.reader,
IVFFLAT_HEADER_SIZE as u64);
- self.quantizer_centroids =
- read_f32_vec(&mut cursor, checked_section_size(self.nlist,
self.d)?)?;
+ let centroid_count = checked_section_size(self.nlist, self.d)?;
+ let centroid_bytes = centroid_count.checked_mul(4).ok_or_else(|| {
+ io::Error::new(
+ io::ErrorKind::InvalidData,
+ "IVF-FLAT centroid bytes overflow",
+ )
+ })?;
+ let table_bytes = self.nlist.checked_mul(16).ok_or_else(|| {
+ io::Error::new(io::ErrorKind::InvalidData, "IVF-FLAT offset table
overflow")
+ })?;
+ let mut metadata = vec![
+ 0u8;
+ centroid_bytes.checked_add(table_bytes).ok_or_else(|| {
+ io::Error::new(
+ io::ErrorKind::InvalidData,
+ "IVF-FLAT metadata size overflow",
+ )
+ })?
+ ];
+ self.reader
+ .pread(&mut [ReadRequest::new(IVFFLAT_HEADER_SIZE as u64, &mut
metadata)])?;
+ self.quantizer_centroids =
bytes_to_f32_vec(&metadata[..centroid_bytes])?;
self.list_offsets = vec![0; self.nlist];
self.list_counts = vec![0; self.nlist];
self.list_id_bytes_lens = vec![0; self.nlist];
+ let mut actual_total = 0i64;
for list_id in 0..self.nlist {
- self.list_offsets[list_id] = read_i64_le(&mut cursor)?;
- let count = read_i32_le(&mut cursor)?;
+ let base = centroid_bytes + list_id * 16;
+ self.list_offsets[list_id] =
+ i64::from_le_bytes(metadata[base..base +
8].try_into().unwrap());
+ let count = i32::from_le_bytes(metadata[base + 8..base +
12].try_into().unwrap());
if count < 0 {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
format!("negative list count {} at list {}", count,
list_id),
));
}
self.list_counts[list_id] = count;
- let id_bytes_len = read_i32_le(&mut cursor)?;
+ actual_total = actual_total.checked_add(count as
i64).ok_or_else(|| {
+ io::Error::new(io::ErrorKind::InvalidData, "IVF-FLAT vector
count overflow")
+ })?;
+ let id_bytes_len =
+ i32::from_le_bytes(metadata[base + 12..base +
16].try_into().unwrap());
if id_bytes_len < 0 {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
format!("negative id_bytes_len {} at list {}",
id_bytes_len, list_id),
));
}
+ if count > 0 && id_bytes_len == 0 {
+ return Err(io::Error::new(
+ io::ErrorKind::InvalidData,
+ format!("missing delta ID bytes for non-empty IVF-FLAT
list {list_id}"),
+ ));
+ }
self.list_id_bytes_lens[list_id] = id_bytes_len;
}
+ if actual_total != self.total_vectors {
+ return Err(io::Error::new(
+ io::ErrorKind::InvalidData,
+ format!(
+ "IVF-FLAT header vector count {} does not match list total
{actual_total}",
+ self.total_vectors
+ ),
+ ));
+ }
self.loaded = true;
Ok(())
}
pub fn read_inverted_list(&mut self, list_id: usize) ->
io::Result<(Vec<i64>, Vec<f32>)> {
+ let mut lists = self.read_inverted_lists(&[list_id])?;
+ let list = lists.pop().expect("one requested list has one result");
+ let vectors = list.vectors().to_vec();
+ Ok((list.ids, vectors))
+ }
+
+ fn read_inverted_lists(&mut self, list_ids: &[usize]) ->
io::Result<Vec<FlatListData>> {
self.ensure_loaded()?;
- if list_id >= self.nlist {
+ if !self.delta_ids {
return Err(io::Error::new(
- io::ErrorKind::InvalidInput,
- format!("list_id {} out of range (nlist={})", list_id,
self.nlist),
+ io::ErrorKind::InvalidData,
+ "IVF-FLAT reader only supports delta IDs",
));
}
- let count = self.list_counts[list_id] as usize;
- if count == 0 {
- return Ok((Vec::new(), Vec::new()));
- }
-
- let offset = checked_list_offset(self.list_offsets[list_id], list_id)?;
- let vector_bytes = checked_list_bytes(count, self.d * 4)?;
- if self.delta_ids {
+ let mut results = (0..list_ids.len()).map(|_|
None).collect::<Vec<_>>();
+ let mut metas = Vec::new();
+ let mut payloads = Vec::new();
+ for (input_index, &list_id) in list_ids.iter().enumerate() {
+ if list_id >= self.nlist {
+ return Err(io::Error::new(
+ io::ErrorKind::InvalidInput,
+ format!("list_id {} out of range (nlist={})", list_id,
self.nlist),
+ ));
+ }
+ let count = self.list_counts[list_id] as usize;
+ if count == 0 {
+ results[input_index] = Some(FlatListData {
+ list_id,
+ ids: Vec::new(),
+ payload: AlignedFlatPayload::empty(),
+ });
+ continue;
+ }
+ let offset = checked_list_offset(self.list_offsets[list_id],
list_id)?;
+ let vector_bytes = checked_list_bytes(count, self.d * 4)?;
let id_bytes_len = self.list_id_bytes_lens[list_id] as usize;
let payload_len = 12usize
.checked_add(id_bytes_len)
.and_then(|len| len.checked_add(vector_bytes))
.ok_or_else(|| {
io::Error::new(io::ErrorKind::InvalidData, "IVF-FLAT list
payload overflow")
})?;
- let mut payload = vec![0u8; payload_len];
- self.reader
- .pread(&mut [ReadRequest::new(offset, &mut payload)])?;
- let base_id =
i64::from_le_bytes(payload[0..8].try_into().unwrap());
- let encoded_len =
i32::from_le_bytes(payload[8..12].try_into().unwrap());
- if encoded_len < 0 || encoded_len as usize != id_bytes_len {
- return Err(io::Error::new(
- io::ErrorKind::InvalidData,
- "IVF-FLAT id_bytes_len mismatch",
- ));
+ metas.push(FlatListRead {
+ input_index,
+ list_id,
+ count,
+ id_bytes_len,
+ offset,
+ });
+ payloads.push(AlignedFlatPayload::new(
Review Comment:
Thanks for adding aggregate payload batching. One oversized list is still
unbounded, though: `bounded_ivf_payload_batch_end` deliberately returns the
first payload even when it is larger than 64 MiB, and each reader allocates
that whole list before issuing the positional read. A one-list IVF-FLAT index,
or a highly skewed list, can therefore still allocate `count * dimension * 4`
bytes (potentially multiple GiB), so the original OOM case is only fixed for
the sum of multiple lists. Could we stream a single large list in chunks, or
otherwise reject/cap it, while keeping the progress guarantee?
##########
core/src/diskann.rs:
##########
@@ -0,0 +1,798 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::distance::{preprocess_vectors, MetricType};
+use crate::pq::ProductQuantizer;
+use crate::vamana::{
+ estimate_sharded_vamana_memory_bytes, estimate_vamana_memory_bytes,
VamanaGraph,
+};
+use rand::rngs::StdRng;
+use rand::{Rng, SeedableRng};
+use std::borrow::Cow;
+use std::collections::VecDeque;
+use std::io;
+use std::time::{Duration, Instant};
+
+pub(crate) const DISKANN_ADJACENCY_LOCATOR_NODE_BYTES: usize = 4;
+pub(crate) const DISKANN_ADJACENCY_LOCATOR_BLOCK_NODES: usize = 16;
+/// Match the proven DiskANN training bound: more samples materially increase
+/// memory and training time without consistently improving the codebook.
+pub const DISKANN_MAX_PQ_TRAINING_VECTORS: usize = 50_000;
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DiskAnnStorageLayout {
+ /// Keep compressed adjacency pages and dense raw-vector records in
separate sections.
+ Compact,
+ /// Store each raw vector immediately before its compressed adjacency list.
+ Interleaved,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(u32)]
+pub enum DiskAnnRawVectorEncoding {
+ /// Preserve indexed vectors and final distances as little-endian `f32`.
+ F32 = 1,
+ /// Store little-endian IEEE 754 binary16 values for approximate final
reranking.
+ F16 = 2,
+}
+
+impl DiskAnnRawVectorEncoding {
+ pub(crate) const fn element_size(self) -> usize {
+ match self {
+ Self::F32 => size_of::<f32>(),
+ Self::F16 => size_of::<u16>(),
+ }
+ }
+
+ pub(crate) const fn from_code(code: u32) -> Option<Self> {
+ match code {
+ 1 => Some(Self::F32),
+ 2 => Some(Self::F16),
+ _ => None,
+ }
+ }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DiskAnnBuildDistance {
+ /// Use full-precision distances for graph traversal and robust pruning.
+ FullPrecision,
+ /// Use PQ distances for graph traversal and full precision for robust
pruning.
+ ProductQuantized,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub struct DiskAnnBuildParams {
+ pub max_degree: usize,
+ pub build_search_list_size: usize,
+ pub alpha: f32,
+ pub seed: u64,
+ pub memory_budget_bytes: usize,
+ pub storage_layout: DiskAnnStorageLayout,
+ pub raw_vector_encoding: DiskAnnRawVectorEncoding,
+ pub build_distance: DiskAnnBuildDistance,
+}
+
+impl Default for DiskAnnBuildParams {
+ fn default() -> Self {
+ Self {
+ max_degree: 64,
+ build_search_list_size: 100,
+ alpha: 1.2,
+ seed: 42,
+ memory_budget_bytes: 8 * 1024 * 1024 * 1024,
+ storage_layout: DiskAnnStorageLayout::Compact,
+ raw_vector_encoding: DiskAnnRawVectorEncoding::F16,
+ build_distance: DiskAnnBuildDistance::ProductQuantized,
+ }
+ }
+}
+
+pub(crate) fn validate_diskann_format_configuration(
+ dimension: usize,
+ pq_m: usize,
+ pq_bits: usize,
+ build: DiskAnnBuildParams,
+) -> io::Result<()> {
+ if dimension == 0 {
+ return Err(invalid_input("DiskANN dimension must be greater than 0"));
+ }
+ if dimension > 1024 {
+ return Err(invalid_input("DiskANN v1 dimension must be at most 1024"));
+ }
+ if pq_m == 0 {
+ return Err(invalid_input("DiskANN pq.m must be greater than 0"));
+ }
+ if pq_m > dimension {
+ return Err(invalid_input(format!(
+ "DiskANN pq.m {} must not exceed dimension {}",
+ pq_m, dimension
+ )));
+ }
+ if !matches!(pq_bits, 4 | 8) {
+ return Err(invalid_input("DiskANN pq.bits must be 4 or 8"));
+ }
+ if build.max_degree == 0 {
+ return Err(invalid_input(
+ "DiskANN maximum degree must be greater than 0",
+ ));
+ }
+ if build.max_degree > 1023 {
+ return Err(invalid_input(format!(
+ "DiskANN adjacency list size {} exceeds the v1 1023-neighbor page
limit",
+ build.max_degree.saturating_mul(size_of::<u32>())
+ )));
+ }
+ if build.build_search_list_size < build.max_degree {
+ return Err(invalid_input(format!(
+ "DiskANN build search-list size {} must be at least maximum degree
{}",
+ build.build_search_list_size, build.max_degree
+ )));
+ }
+ if u32::try_from(build.build_search_list_size).is_err() {
+ return Err(invalid_input("DiskANN build search-list size exceeds
u32"));
+ }
+ if !build.alpha.is_finite() || build.alpha < 1.0 {
+ return Err(invalid_input("DiskANN alpha must be at least 1 and
finite"));
+ }
+ let interleaved_record_bytes = dimension
+ .checked_mul(build.raw_vector_encoding.element_size())
+ .and_then(|vector_bytes| {
+ build
+ .max_degree
+ .checked_mul(size_of::<u32>())
+ .and_then(|adjacency_bytes|
vector_bytes.checked_add(adjacency_bytes))
+ });
+ if build.storage_layout == DiskAnnStorageLayout::Interleaved
+ && interleaved_record_bytes.is_none_or(|record_bytes| record_bytes >
4096)
+ {
+ return Err(invalid_input(
+ "DiskANN interleaved raw vector and maximum adjacency list must
fit in one page",
+ ));
+ }
+ Ok(())
+}
+
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub struct DiskAnnBuildStats {
+ /// One for the normal parallel build; greater than one when the memory
+ /// budget selected overlapping shard construction.
+ pub graph_shards: usize,
+ pub total: Duration,
+ pub pq_encoding: Duration,
+ pub vamana_initialization: Duration,
+ pub vamana_pass_one: Duration,
+ pub vamana_pass_two: Duration,
+ pub connectivity_repair: Duration,
+ pub locality_remap: Duration,
+ pub resident_serialization: Duration,
+ pub adjacency_serialization: Duration,
+ pub vector_serialization: Duration,
+}
+
+impl DiskAnnBuildStats {
+ pub fn accounted_duration(self) -> Duration {
+ [
+ self.pq_encoding,
+ self.vamana_initialization,
+ self.vamana_pass_one,
+ self.vamana_pass_two,
+ self.connectivity_repair,
+ self.locality_remap,
+ self.resident_serialization,
+ self.adjacency_serialization,
+ self.vector_serialization,
+ ]
+ .into_iter()
+ .sum()
+ }
+}
+
+pub struct DiskAnnIndex {
+ pub d: usize,
+ pub metric: MetricType,
+ pub pq: ProductQuantizer,
+ pub build_params: DiskAnnBuildParams,
+ pub ids: Vec<i64>,
+ pub vectors: Vec<f32>,
+}
+
+impl DiskAnnIndex {
+ pub fn new(
+ d: usize,
+ metric: MetricType,
+ pq_m: usize,
+ build_params: DiskAnnBuildParams,
+ ) -> Self {
+ Self::with_pq_bits(d, metric, pq_m, 8, build_params)
+ }
+
+ pub fn with_pq_bits(
+ d: usize,
+ metric: MetricType,
+ pq_m: usize,
+ pq_bits: usize,
+ build_params: DiskAnnBuildParams,
+ ) -> Self {
+ Self {
+ d,
+ metric,
+ pq: ProductQuantizer::with_nbits_balanced(d, pq_m, pq_bits),
+ build_params,
+ ids: Vec::new(),
+ vectors: Vec::new(),
+ }
+ }
+
+ pub fn train(&mut self, data: &[f32], n: usize) {
Review Comment:
Thanks for budgeting the per-task PQ scratch and parallelism. The retained
trainer reservoir is still outside this plan when downsampling is required.
`VectorIndexTrainer` always keeps up to 50,000 raw vectors; at `d=1024` that is
204,800,000 bytes. With a 128 MiB budget, `pq_training_plan` succeeds with
`sample_count=27,892`, after which `bounded_pq_training_sample` allocates
another sampled buffer while the original 50,000-vector reservoir remains
alive. Cosine training can add a normalized owned copy as well. Since
`peak_for` counts only one `sample_bytes` buffer, the accepted budget can still
be exceeded before KMeans scratch is considered. Could the trainer reservoir
limit be derived from the budget, or could the plan include the retained,
downsampled, and normalization buffers that coexist?
##########
core/src/pq.rs:
##########
@@ -105,63 +182,85 @@ impl ProductQuantizer {
let m = self.m;
let d = self.d;
- let dsub = self.dsub;
let ksub = self.ksub;
+ let chunk_offsets = &self.chunk_offsets;
// Train all M sub-quantizers in parallel
let sub_results: Vec<Vec<f32>> = (0..m)
.into_par_iter()
.map(|sub| {
- let offset = sub * dsub;
+ let start = chunk_offsets[sub];
+ let stop = chunk_offsets[sub + 1];
+ let chunk_dim = stop - start;
- let mut sub_data = vec![0.0f32; n * dsub];
+ let mut sub_data = vec![0.0f32; n * chunk_dim];
for i in 0..n {
- sub_data[i * dsub..(i + 1) * dsub]
- .copy_from_slice(&data[i * d + offset..i * d + offset
+ dsub]);
+ sub_data[i * chunk_dim..(i + 1) * chunk_dim]
+ .copy_from_slice(&data[i * d + start..i * d + stop]);
}
let init: Option<Vec<f32>> = prev_centroids.as_ref().map(|pc| {
- let src = sub * ksub * dsub;
- pc[src..src + ksub * dsub].to_vec()
+ let src = start * ksub;
+ pc[src..src + ksub * chunk_dim].to_vec()
});
- kmeans::kmeans_train_with_init(km_config, &sub_data, n, dsub,
ksub, init.as_deref())
+ kmeans::kmeans_train_with_init(
+ km_config,
+ &sub_data,
+ n,
+ chunk_dim,
+ ksub,
+ init.as_deref(),
+ )
})
.collect();
- self.centroids = vec![0.0f32; m * ksub * dsub];
+ self.centroids = vec![0.0f32; d * ksub];
for (sub, sub_centroids) in sub_results.into_iter().enumerate() {
- let dst_offset = sub * ksub * dsub;
- self.centroids[dst_offset..dst_offset + ksub *
dsub].copy_from_slice(&sub_centroids);
+ let chunk_dim = self.chunk_dim(sub);
+ let dst_offset = self.centroid_chunk_base(sub);
+ self.centroids[dst_offset..dst_offset + ksub * chunk_dim]
+ .copy_from_slice(&sub_centroids);
}
self.rebuild_norms_cache();
}
/// Rebuild the centroid norms cache. Called after training or loading
centroids.
pub fn rebuild_norms_cache(&mut self) {
- self.centroid_norms_cache = vec![0.0f32; self.m * self.ksub];
+ self.try_rebuild_norms_cache()
+ .expect("PQ centroid norms allocation failed");
+ }
+
+ pub fn try_rebuild_norms_cache(&mut self) -> Result<(),
std::collections::TryReserveError> {
+ let mut norms = Vec::new();
+ norms.try_reserve_exact(self.m * self.ksub)?;
+ norms.resize(self.m * self.ksub, 0.0f32);
for sub in 0..self.m {
- let c_base = sub * self.ksub * self.dsub;
+ let chunk_dim = self.chunk_dim(sub);
+ let c_base = self.centroid_chunk_base(sub);
for j in 0..self.ksub {
- let c_off = c_base + j * self.dsub;
- self.centroid_norms_cache[sub * self.ksub + j] =
- fvec_norm_l2sqr(&self.centroids[c_off..c_off + self.dsub]);
+ let c_off = c_base + j * chunk_dim;
+ norms[sub * self.ksub + j] =
+ fvec_norm_l2sqr(&self.centroids[c_off..c_off + chunk_dim]);
}
}
+ self.centroid_norms_cache = norms;
+ Ok(())
}
/// Bytes per encoded vector.
pub fn code_size(&self) -> usize {
if self.nbits == 4 {
- self.m / 2
+ self.m.div_ceil(2)
Review Comment:
The persistence path now rejects odd `m`, but the public in-memory API is
still incorrect. `IVFPQIndex::with_nbits(..., 4, ...)` accepts odd `m`; both
`scan_4bit_simd` and the transposed/FastScan path use `m / 2`, so the final
subquantizer is ignored. I reproduced this with `m=3` and two codes that differ
only in the third subquantizer: `ProductQuantizer::distance_from_table`
distinguishes them, while `IVFPQIndex::search` ranks them as equal, both before
and after `build_search_structures()`. Please either reject odd 4-bit `m` at
construction/configuration time or teach all scanners to process the final low
nibble.
##########
python/paimon_vindex/__init__.py:
##########
@@ -401,15 +447,54 @@ def __del__(self):
class VectorIndexReader:
- def __init__(self, input):
+ def __init__(
+ self,
+ input,
+ storage_profile: StorageProfile = StorageProfile.AUTO,
+ memory_budget_bytes: int = 4 * 1024 * 1024 * 1024,
+ ):
self._input = input
self._closed = False
+ profile_names = {
+ "auto": StorageProfile.AUTO,
+ "memory": StorageProfile.MEMORY,
+ "local_storage": StorageProfile.LOCAL_STORAGE,
+ "remote_storage": StorageProfile.REMOTE_STORAGE,
+ "object_store": StorageProfile.OBJECT_STORE,
+ }
+ try:
+ storage_profile = profile_names.get(storage_profile,
storage_profile)
+ storage_profile = StorageProfile(storage_profile)
+ except ValueError as exc:
+ raise ValueError(f"invalid storage_profile: {storage_profile}")
from exc
+ if memory_budget_bytes < 0:
+ raise ValueError("memory_budget_bytes must be non-negative")
+
self._read_ranges_callback = _make_read_ranges_callback(self._input)
input_file = _ffi.PaimonVindexInputFile()
input_file.ctx = None
input_file.read_ranges_fn = self._read_ranges_callback
- self._handle = lib.paimon_vindex_reader_open(input_file)
+ capability_names = (
+ "preferred_alignment_bytes",
+ "preferred_window_bytes",
+ "max_ranges_per_read",
+ )
+ capabilities = {
+ name: int(getattr(self._input, name, 0)) for name in
capability_names
+ }
+ if any(value < 0 for value in capabilities.values()):
+ raise ValueError("input read capabilities must be non-negative")
+ input_file.preferred_alignment_bytes = capabilities[
+ "preferred_alignment_bytes"
+ ]
+ input_file.preferred_window_bytes =
capabilities["preferred_window_bytes"]
+ input_file.max_ranges_per_read = capabilities["max_ranges_per_read"]
+ options = _ffi.PaimonVindexReaderOptions(
+ int(storage_profile),
+ memory_budget_bytes,
+ )
+ self._handle = lib.paimon_vindex_reader_open_with_options(input_file,
options)
Review Comment:
The cross-thread race is fixed, but `threading.RLock` leaves a reentrant
use-after-free path. `search` holds the lock while native code invokes
`input.pread_many`; if that callback calls `reader.close()` (or another native
reader method), it runs on the same thread and re-acquires the RLock,
freeing/re-entering the handle while the original Rust call still owns `&mut
VectorIndexReader`. I reproduced this with an IVF-FLAT input whose first
search-time `pread_many` calls `reader.close()`; the Python process segfaults
with exit status 139. The Java wrapper already tracks the native-handle owner
and rejects reentry. Could the Python wrappers similarly reject all reentrant
native-handle operations rather than permitting them? The writer output
callbacks have the same pattern.
##########
python/paimon_vindex/__init__.py:
##########
@@ -401,15 +497,55 @@ def __del__(self):
class VectorIndexReader:
- def __init__(self, input):
+ def __init__(
+ self,
+ input,
+ storage_profile: StorageProfile = StorageProfile.AUTO,
+ memory_budget_bytes: int = 4 * 1024 * 1024 * 1024,
+ ):
+ self._native_handle_lock = threading.RLock()
self._input = input
self._closed = False
+ profile_names = {
+ "auto": StorageProfile.AUTO,
+ "memory": StorageProfile.MEMORY,
+ "local_storage": StorageProfile.LOCAL_STORAGE,
+ "remote_storage": StorageProfile.REMOTE_STORAGE,
+ "object_store": StorageProfile.OBJECT_STORE,
+ }
+ try:
+ storage_profile = profile_names.get(storage_profile,
storage_profile)
+ storage_profile = StorageProfile(storage_profile)
+ except ValueError as exc:
+ raise ValueError(f"invalid storage_profile: {storage_profile}")
from exc
+ if memory_budget_bytes < 0:
Review Comment:
Could we apply the same platform-width validation added for `SearchParams`
here? This only rejects negative values, so on a 64-bit build
`memory_budget_bytes = ctypes.c_size_t(-1).value + 1` is silently converted to
zero by `PaimonVindexReaderOptions`. The non-negative read-capability hints and
the `l_search` / calibration `top_k` arguments have the same unchecked
`c_size_t` conversion. Using `operator.index` and validating against `SIZE_MAX`
before constructing the ctypes values would keep caller errors from changing
the requested behavior.
##########
tools/convert_ann_benchmarks.py:
##########
@@ -0,0 +1,149 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Convert an ANN-Benchmarks dense HDF5 dataset to fvecs/ivecs files."""
+
+import argparse
+from pathlib import Path
+
+import h5py
+import numpy as np
+
+
+def write_fvecs(
+ dataset: h5py.Dataset,
+ path: Path,
+ batch_rows: int,
+ row_limit: int | None = None,
Review Comment:
The package declares `requires-python = ">=3.9"`, but these `int | None`
annotations require Python 3.10 when annotations are evaluated normally. On
Python 3.9 the script fails during function definition before the CLI can run.
Please use `Optional[int]` or add `from __future__ import annotations`, and
ideally include the minimum supported Python version in the tool smoke test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
