Skip to content

Commit 2477685

Browse files
lwwmanningclaude
andcommitted
fix[turboquant]: address PR review comments from AdamGS
- Replace Mutex<HashMap> centroid cache with DashMap for lock-free concurrent reads - Replace OnceLock with LazyLock for the cache static - Use branchless base.max(0.0).powf(exponent) in pdf_unnormalized instead of an if-return branch - Add debug_assert that boundaries are sorted in find_nearest_centroid - Use .iter_mut() instead of &mut for iterator style consistency Signed-off-by: Will Manning <will@spiraldb.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Will Manning <will@willmanning.io>
1 parent b2bcd38 commit 2477685

4 files changed

Lines changed: 13 additions & 19 deletions

File tree

Cargo.lock

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

encodings/turboquant/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ version = { workspace = true }
1717
workspace = true
1818

1919
[dependencies]
20+
dashmap = { workspace = true }
2021
prost = { workspace = true }
2122
rand = { workspace = true }
2223
vortex-array = { workspace = true }
@@ -25,7 +26,6 @@ vortex-error = { workspace = true }
2526
vortex-fastlanes = { workspace = true }
2627
vortex-session = { workspace = true }
2728
vortex-utils = { workspace = true }
28-
parking_lot = { workspace = true }
2929

3030
[dev-dependencies]
3131
rand_distr = { workspace = true }

encodings/turboquant/src/centroids.rs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@
99
//! which converges to `N(0, 1/d)`. The Max-Lloyd algorithm finds optimal quantization centroids
1010
//! that minimize MSE for this distribution.
1111
12-
use std::sync::OnceLock;
12+
use std::sync::LazyLock;
1313

14-
use parking_lot::Mutex;
14+
use dashmap::DashMap;
1515
use vortex_error::VortexResult;
1616
use vortex_error::vortex_bail;
17-
use vortex_utils::aliases::hash_map::HashMap;
1817

1918
/// Number of numerical integration points for computing conditional expectations.
2019
const INTEGRATION_POINTS: usize = 1000;
@@ -25,10 +24,8 @@ const CONVERGENCE_EPSILON: f64 = 1e-12;
2524
/// Maximum iterations for Max-Lloyd algorithm.
2625
const MAX_ITERATIONS: usize = 200;
2726

28-
type CentroidCache = Mutex<HashMap<(u32, u8), Vec<f32>>>;
29-
3027
/// Global centroid cache keyed by (dimension, bit_width).
31-
static CENTROID_CACHE: OnceLock<CentroidCache> = OnceLock::new();
28+
static CENTROID_CACHE: LazyLock<DashMap<(u32, u8), Vec<f32>>> = LazyLock::new(DashMap::new);
3229

3330
/// Get or compute cached centroids for the given dimension and bit width.
3431
///
@@ -43,15 +40,12 @@ pub fn get_centroids(dimension: u32, bit_width: u8) -> VortexResult<Vec<f32>> {
4340
vortex_bail!("TurboQuant dimension must be >= 2, got {dimension}");
4441
}
4542

46-
let cache = CENTROID_CACHE.get_or_init(|| Mutex::new(HashMap::default()));
47-
let mut guard = cache.lock();
48-
49-
if let Some(centroids) = guard.get(&(dimension, bit_width)) {
43+
if let Some(centroids) = CENTROID_CACHE.get(&(dimension, bit_width)) {
5044
return Ok(centroids.clone());
5145
}
5246

5347
let centroids = max_lloyd_centroids(dimension, bit_width);
54-
guard.insert((dimension, bit_width), centroids.clone());
48+
CENTROID_CACHE.insert((dimension, bit_width), centroids.clone());
5549
Ok(centroids)
5650
}
5751

@@ -140,11 +134,7 @@ fn conditional_mean(lo: f64, hi: f64, exponent: f64) -> f64 {
140134
/// Unnormalized PDF of the coordinate distribution: `(1 - x^2)^exponent`.
141135
#[inline]
142136
fn pdf_unnormalized(x_val: f64, exponent: f64) -> f64 {
143-
let base = 1.0 - x_val * x_val;
144-
if base <= 0.0 {
145-
return 0.0;
146-
}
147-
base.powf(exponent)
137+
(1.0 - x_val * x_val).max(0.0).powf(exponent)
148138
}
149139

150140
/// Precompute decision boundaries (midpoints between adjacent centroids).
@@ -164,6 +154,10 @@ pub fn compute_boundaries(centroids: &[f32]) -> Vec<f32> {
164154
#[inline]
165155
#[allow(clippy::cast_possible_truncation)]
166156
pub fn find_nearest_centroid(value: f32, boundaries: &[f32]) -> u8 {
157+
debug_assert!(
158+
boundaries.windows(2).all(|w| w[0] <= w[1]),
159+
"boundaries must be sorted"
160+
);
167161
boundaries.partition_point(|&b| b < value) as u8
168162
}
169163

encodings/turboquant/src/compress.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ pub fn turboquant_encode_qjl(
241241

242242
rotation.inverse_rotate(&dequantized_rotated, &mut dequantized);
243243
if norm > 0.0 {
244-
for val in &mut dequantized {
244+
for val in dequantized.iter_mut() {
245245
*val *= norm;
246246
}
247247
}

0 commit comments

Comments
 (0)