Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove AsContiguousFn #346

Merged
merged 14 commits into from
Jun 11, 2024
Prev Previous commit
Next Next commit
fix tests + benchmarks
a10y committed Jun 11, 2024
commit 272352cb816ee5fb99acfd97dc5132546fe0ce59
29 changes: 0 additions & 29 deletions vortex-alp/src/compute.rs
Original file line number Diff line number Diff line change
@@ -61,32 +61,3 @@ impl SliceFn for ALPArray {
.into_array())
}
}

#[cfg(test)]
mod test {
use vortex::array::primitive::PrimitiveArray;
use vortex::compute::scalar_at::scalar_at;
use vortex::validity::Validity;
use vortex::IntoArray;

use crate::ALPArray;

#[test]
fn test_as_contiguous() {
let values = vec![1.0, 2.0, 3.0];
let primitives = PrimitiveArray::from_vec(values, Validity::NonNullable);
let encoded = ALPArray::encode(primitives.into_array()).unwrap();
let alp = ALPArray::try_from(&encoded).unwrap();

let flat = alp.as_contiguous(&[encoded]).unwrap();

let a: f64 = scalar_at(&flat, 0).unwrap().try_into().unwrap();
let b: f64 = scalar_at(&flat, 1).unwrap().try_into().unwrap();

let c: f64 = scalar_at(&flat, 2).unwrap().try_into().unwrap();

assert_eq!(a, 1.0);
assert_eq!(b, 2.0);
assert_eq!(c, 3.0);
}
}
12 changes: 4 additions & 8 deletions vortex-array/src/array/chunked/compute/take.rs
Original file line number Diff line number Diff line change
@@ -52,8 +52,6 @@ impl TakeFn for ChunkedArray {

#[cfg(test)]
mod test {
use itertools::Itertools;

use crate::array::chunked::ChunkedArray;
use crate::compute::take::take;
use crate::{ArrayDType, ArrayTrait, AsArray, IntoArray};
@@ -67,14 +65,12 @@ mod test {
assert_eq!(arr.len(), 9);
let indices = vec![0, 0, 6, 4].into_array();

let result = as_contiguous(
let result =
&ChunkedArray::try_from(take(arr.as_array_ref(), &indices).unwrap())
.unwrap()
.chunks()
.collect_vec(),
)
.unwrap()
.into_primitive();
.into_array()
.flatten_primitive()
.unwrap();
assert_eq!(result.typed_data::<i32>(), &[1, 1, 1, 2]);
}
}
2 changes: 1 addition & 1 deletion vortex-array/src/array/chunked/flatten.rs
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ impl ArrayFlatten for ChunkedArray {
}
}

pub fn try_flatten_chunks(chunks: Vec<Array>, dtype: DType) -> VortexResult<Flattened> {
pub(crate) fn try_flatten_chunks(chunks: Vec<Array>, dtype: DType) -> VortexResult<Flattened> {
let mismatched = chunks.iter()
.filter(|chunk| !chunk.dtype().eq(&dtype))
.collect::<Vec<_>>();
34 changes: 0 additions & 34 deletions vortex-array/src/array/sparse/compute/mod.rs
Original file line number Diff line number Diff line change
@@ -121,7 +121,6 @@ mod test {
use crate::array::primitive::PrimitiveArray;
use crate::array::sparse::compute::take_map;
use crate::array::sparse::SparseArray;
use crate::compute::slice::slice;
use crate::compute::take::take;
use crate::validity::Validity;

@@ -181,39 +180,6 @@ mod test {
assert_eq!(taken.len(), 2);
}

#[test]
fn take_slices_and_reassemble() {
let sparse = sparse_array();
let slices = (0..10)
.map(|i| slice(&sparse, i * 10, (i + 1) * 10).unwrap())
.collect_vec();

let taken = slices
.iter()
.map(|s| take(s, &(0u64..10).collect_vec().into_array()).unwrap())
.collect_vec();
for i in [1, 2, 5, 6, 7, 8] {
assert_eq!(SparseArray::try_from(&taken[i]).unwrap().indices().len(), 0);
}
for i in [0, 3, 4, 9] {
assert_eq!(SparseArray::try_from(&taken[i]).unwrap().indices().len(), 1);
}

let contiguous = SparseArray::try_from(as_contiguous(&taken).unwrap()).unwrap();
assert_eq!(
contiguous.indices().into_primitive().typed_data::<u64>(),
[0u64, 7, 7, 9] // relative offsets
);
assert_eq!(
contiguous.values().into_primitive().typed_data::<f64>(),
SparseArray::try_from(sparse)
.unwrap()
.values()
.into_primitive()
.typed_data::<f64>()
);
}

#[test]
fn test_take_map() {
let sparse = SparseArray::try_from(sparse_array()).unwrap();
14 changes: 11 additions & 3 deletions vortex-array/src/validity.rs
Original file line number Diff line number Diff line change
@@ -199,9 +199,17 @@ impl FromIterator<LogicalValidity> for Validity {
// Else, construct the boolean buffer
let mut buffer = BooleanBufferBuilder::new(validities.iter().map(|v| v.len()).sum());
for validity in validities {
let present = validity.to_present_null_buffer()
.expect("Validity should expose NullBuffer")
.into_inner();
let present = match validity {
LogicalValidity::AllValid(count) => {
BooleanBuffer::new_set(count)
}
LogicalValidity::AllInvalid(count) => {
BooleanBuffer::new_unset(count)
}
LogicalValidity::Array(array) => {
array.into_array().flatten_bool().expect("validity must flatten to BoolArray").boolean_buffer()
}
};
buffer.append_buffer(&present);
}
let bool_array = BoolArray::try_new(buffer.finish(), Validity::NonNullable)
6 changes: 6 additions & 0 deletions vortex-fastlanes/src/for/compress.rs
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ use vortex::array::primitive::PrimitiveArray;
use vortex::compress::{CompressConfig, Compressor, EncodingCompression};
use vortex::stats::{ArrayStatistics, Stat};
use vortex::{Array, ArrayDType, ArrayTrait, IntoArray};
use vortex::validity::ArrayValidity;
use vortex_dtype::{match_each_integer_ptype, NativePType, PType};
use vortex_error::{vortex_err, VortexResult};
use vortex_scalar::Scalar;
@@ -29,6 +30,11 @@ impl EncodingCompression for FoREncoding {
return None;
}

// For all-null, cannot encode.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this, arrays that have all-nulls would fail on the compute_as_cast a few lines below

if parray.logical_validity().all_invalid() {
return None;
}

// Nothing for us to do if the min is already zero and tz == 0
let shift = trailing_zeros(array);
let min = parray.statistics().compute_as_cast::<i64>(Stat::Min)?;