Skip to content

Commit

Permalink
feat: re-enable ALP-RD as top-level float compressor (#1725)
Browse files Browse the repository at this point in the history
Fixes issue seen in #1723

In #1000, changes were made to remove ALP-RD as a top-level compressor
and instead to only use it for patches. However, it seems that it was
not getting selected anymore, whether that was due to the patching cost
overhead or something else. This was noticed by a user, and confirmed by
me in a Python shell.

<img width="946" alt="image"
src="https://github.com/user-attachments/assets/c42caed5-12e3-448b-aea6-3f33a7c97bfc"
/>

After this change, ALP-RD indeed does get selected again.

<img width="907" alt="image"
src="https://github.com/user-attachments/assets/bbb996fc-5223-43ed-9b4c-4b0262a417dc"
/>
  • Loading branch information
a10y authored Dec 19, 2024
1 parent fd632a7 commit 5b6a968
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
2 changes: 2 additions & 0 deletions vortex-sampling-compressor/src/compressors/alp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl EncodingCompressor for ALPCompressor {
.excluding(self)
.compress(&encoded, like.as_ref().and_then(|l| l.child(0)))?;

// Attempt to compress patches with ALP-RD encoding
let compressed_patches = patches
.map(|p| {
ctx.auxiliary("patches")
Expand All @@ -76,6 +77,7 @@ impl EncodingCompressor for ALPCompressor {
fn used_encodings(&self) -> HashSet<EncodingRef> {
HashSet::from([
&ALPEncoding as EncodingRef,
// ALP-RD + BitPacking possibly used for patches
&ALPRDEncoding,
&BitPackedEncoding,
])
Expand Down
11 changes: 8 additions & 3 deletions vortex-sampling-compressor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ mod sampling_compressor;

pub use sampling_compressor::*;

pub const DEFAULT_COMPRESSORS: [CompressorRef; 15] = [
use crate::compressors::alp_rd::ALPRDCompressor;

pub const DEFAULT_COMPRESSORS: [CompressorRef; 16] = [
&ALPCompressor as CompressorRef,
&ALPRDCompressor,
&BITPACK_WITH_PATCHES,
&DEFAULT_CHUNKED_COMPRESSOR,
&ConstantCompressor,
Expand All @@ -72,8 +75,9 @@ pub const DEFAULT_COMPRESSORS: [CompressorRef; 15] = [
];

#[cfg(not(target_arch = "wasm32"))]
pub const ALL_COMPRESSORS: [CompressorRef; 18] = [
pub const ALL_COMPRESSORS: [CompressorRef; 19] = [
&ALPCompressor as CompressorRef,
&ALPRDCompressor,
&BITPACK_WITH_PATCHES,
&DEFAULT_CHUNKED_COMPRESSOR,
&ConstantCompressor,
Expand All @@ -94,8 +98,9 @@ pub const ALL_COMPRESSORS: [CompressorRef; 18] = [
];

#[cfg(target_arch = "wasm32")]
pub const ALL_COMPRESSORS: [CompressorRef; 16] = [
pub const ALL_COMPRESSORS: [CompressorRef; 17] = [
&ALPCompressor as CompressorRef,
&ALPRDCompressor,
&BITPACK_WITH_PATCHES,
&DEFAULT_CHUNKED_COMPRESSOR,
&ConstantCompressor,
Expand Down
25 changes: 25 additions & 0 deletions vortex-sampling-compressor/src/sampling_compressor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,28 @@ pub(crate) fn find_best_compression<'a>(

Ok(best)
}

#[cfg(test)]
mod tests {
use itertools::Itertools;
use vortex_alp::ALPRDEncoding;
use vortex_array::array::PrimitiveArray;
use vortex_array::encoding::Encoding;
use vortex_array::IntoArrayData;

use crate::SamplingCompressor;

#[test]
fn test_default() {
let values = (0..4096)
.map(|x| (x as f64) / 1234567890.0f64)
.collect_vec();
let array = PrimitiveArray::from(values).into_array();

let compressed = SamplingCompressor::default()
.compress(&array, None)
.unwrap()
.into_array();
assert_eq!(compressed.encoding().id(), ALPRDEncoding::ID);
}
}

0 comments on commit 5b6a968

Please sign in to comment.