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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -107,5 +107,5 @@ zigzag = "0.1.0"
warnings = "deny"

[workspace.lints.clippy]
all = "deny"
all = { level = "deny", priority = -1 }
or_fun_call = "deny"
4 changes: 2 additions & 2 deletions bench-vortex/benches/compress_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion};

fn vortex_compress_taxi(c: &mut Criterion) {
taxi_data_parquet();
let mut group = c.benchmark_group("end to end");
let mut group = c.benchmark_group("end to end - taxi");
group.sample_size(10);
group.bench_function("compress", |b| b.iter(|| black_box(compress_taxi_data())));
group.finish()
Expand All @@ -16,7 +16,7 @@ fn vortex_compress_taxi(c: &mut Criterion) {
fn vortex_compress_medicare1(c: &mut Criterion) {
let dataset = BenchmarkDatasets::PBI(Medicare1);
dataset.as_uncompressed();
let mut group = c.benchmark_group("end to end");
let mut group = c.benchmark_group("end to end - medicare");
group.sample_size(10);
group.bench_function("compress", |b| {
b.iter(|| black_box(dataset.compress_to_vortex()))
Expand Down
39 changes: 1 addition & 38 deletions vortex-alp/src/compute.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
use vortex::compute::as_contiguous::AsContiguousFn;
use vortex::compute::scalar_at::{scalar_at, ScalarAtFn};
use vortex::compute::slice::{slice, SliceFn};
use vortex::compute::take::{take, TakeFn};
use vortex::compute::ArrayCompute;
use vortex::{impl_default_as_contiguous_fn, Array, ArrayDType, IntoArray};
use vortex::{Array, ArrayDType, IntoArray};
use vortex_error::VortexResult;
use vortex_scalar::Scalar;

use crate::{match_each_alp_float_ptype, ALPArray};

impl_default_as_contiguous_fn!(ALPArray);

impl ArrayCompute for ALPArray {
fn scalar_at(&self) -> Option<&dyn ScalarAtFn> {
Some(self)
Expand All @@ -23,10 +20,6 @@ impl ArrayCompute for ALPArray {
fn take(&self) -> Option<&dyn TakeFn> {
Some(self)
}

fn as_contiguous(&self) -> Option<&dyn AsContiguousFn> {
Some(self)
}
}

impl ScalarAtFn for ALPArray {
Expand Down Expand Up @@ -68,33 +61,3 @@ impl SliceFn for ALPArray {
.into_array())
}
}

#[cfg(test)]
mod test {
use vortex::array::primitive::PrimitiveArray;
use vortex::compute::as_contiguous::AsContiguousFn;
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);
}
}
27 changes: 0 additions & 27 deletions vortex-array/src/array/bool/compute/as_contiguous.rs

This file was deleted.

6 changes: 0 additions & 6 deletions vortex-array/src/array/bool/compute/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::array::bool::BoolArray;
use crate::compute::as_arrow::AsArrowArray;
use crate::compute::as_contiguous::AsContiguousFn;
use crate::compute::compare::CompareFn;
use crate::compute::fill::FillForwardFn;
use crate::compute::scalar_at::ScalarAtFn;
Expand All @@ -9,7 +8,6 @@ use crate::compute::take::TakeFn;
use crate::compute::ArrayCompute;

mod as_arrow;
mod as_contiguous;
mod compare;
mod fill;
mod flatten;
Expand All @@ -22,10 +20,6 @@ impl ArrayCompute for BoolArray {
Some(self)
}

fn as_contiguous(&self) -> Option<&dyn AsContiguousFn> {
Some(self)
}

fn compare(&self) -> Option<&dyn CompareFn> {
Some(self)
}
Expand Down
23 changes: 2 additions & 21 deletions vortex-array/src/array/chunked/compute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,21 @@ use vortex_error::VortexResult;
use vortex_scalar::Scalar;

use crate::array::chunked::ChunkedArray;
use crate::compute::as_contiguous::{as_contiguous, AsContiguousFn};
use crate::compute::scalar_at::{scalar_at, ScalarAtFn};
use crate::compute::scalar_subtract::SubtractScalarFn;
use crate::compute::slice::SliceFn;
use crate::compute::take::TakeFn;
use crate::compute::ArrayCompute;
use crate::Array;

mod slice;
mod take;

impl ArrayCompute for ChunkedArray {
fn as_contiguous(&self) -> Option<&dyn AsContiguousFn> {
fn scalar_at(&self) -> Option<&dyn ScalarAtFn> {
Some(self)
}

fn scalar_at(&self) -> Option<&dyn ScalarAtFn> {
fn subtract_scalar(&self) -> Option<&dyn SubtractScalarFn> {
Some(self)
}

Expand All @@ -29,23 +27,6 @@ impl ArrayCompute for ChunkedArray {
fn take(&self) -> Option<&dyn TakeFn> {
Some(self)
}

fn subtract_scalar(&self) -> Option<&dyn SubtractScalarFn> {
Some(self)
}
}

impl AsContiguousFn for ChunkedArray {
fn as_contiguous(&self, arrays: &[Array]) -> VortexResult<Array> {
// Combine all the chunks into one, then call as_contiguous again.
let mut chunks = Vec::with_capacity(self.nchunks());
for array in arrays {
for chunk in Self::try_from(array).unwrap().chunks() {
chunks.push(chunk);
}
}
as_contiguous(&chunks)
}
}

impl ScalarAtFn for ChunkedArray {
Expand Down
16 changes: 5 additions & 11 deletions vortex-array/src/array/chunked/compute/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ impl TakeFn for ChunkedArray {

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

use crate::array::chunked::ChunkedArray;
use crate::compute::as_contiguous::as_contiguous;
use crate::compute::take::take;
use crate::{ArrayDType, ArrayTrait, AsArray, IntoArray};

Expand All @@ -68,14 +65,11 @@ mod test {
assert_eq!(arr.len(), 9);
let indices = vec![0, 0, 6, 4].into_array();

let result = as_contiguous(
&ChunkedArray::try_from(take(arr.as_array_ref(), &indices).unwrap())
.unwrap()
.chunks()
.collect_vec(),
)
.unwrap()
.into_primitive();
let result = &ChunkedArray::try_from(take(arr.as_array_ref(), &indices).unwrap())
.unwrap()
.into_array()
.flatten_primitive()
.unwrap();
assert_eq!(result.typed_data::<i32>(), &[1, 1, 1, 2]);
}
}
Loading