From 1d0b4d82a3022278b88e7310c0afea8cab432418 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 14:34:22 +0000 Subject: [PATCH 1/4] ALPRD: assert patch dtype instead of casting; drop execution context ALPRD patch values are always stored as the non-nullable left-parts dtype, so the runtime cast in `canonicalize_patches` (which required an `ExecutionCtx` to materialise a lazy cast and an `all_valid` check) was unnecessary. - Replace `ALPRDData::canonicalize_patches(left_parts, patches, ctx)` with a ctx-free `validate_patches(left_parts, patches)` that asserts the patch values already have the non-nullable left-parts dtype (a non-nullable dtype also guarantees the values contain no nulls, so no `all_valid` execution is needed). Resolves the `TODO(ngates)/TODO(joe): assert the DType, don't cast`. - `validate_parts` likewise asserts the exact non-nullable patch dtype instead of `eq_ignore_nullability` + an `all_valid(LEGACY_SESSION)` execution. - `deserialize` no longer mints a `LEGACY_SESSION` ctx for patch handling. - `take`: `Patches::take` ignores null indices but reports a nullable dtype; drop the spurious nullability with a lazy `cast_values` (no execution at construction) so the patch values satisfy the non-nullable invariant. Net: removes 3 `LEGACY_SESSION` uses from ALPRD construction/validation. ALP already asserts its patch dtype (its patch values are the original floats), so no change was needed there. Signed-off-by: Robert Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Mkrgj5SuJpaKBXK9jydtSc --- encodings/alp/src/alp_rd/array.rs | 63 +++++++++--------------- encodings/alp/src/alp_rd/compute/take.rs | 9 ++-- 2 files changed, 29 insertions(+), 43 deletions(-) diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index 160723e83b2..fba6529efd0 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -17,14 +17,11 @@ use vortex_array::ArrayParts; use vortex_array::ArrayRef; use vortex_array::ArraySlots; use vortex_array::ArrayView; -use vortex_array::Canonical; use vortex_array::EqMode; use vortex_array::ExecutionCtx; use vortex_array::ExecutionResult; use vortex_array::IntoArray; -use vortex_array::LEGACY_SESSION; use vortex_array::TypedArrayRef; -use vortex_array::VortexSessionExecute; use vortex_array::arrays::Primitive; use vortex_array::arrays::PrimitiveArray; use vortex_array::buffer::BufferHandle; @@ -209,13 +206,7 @@ impl VTable for ALPRD { ) }) .transpose()?; - // NOTE: `VTable::deserialize` has a fixed trait signature without `ExecutionCtx`, so we - // cannot plumb a ctx in here. We construct a legacy ctx locally at this trait boundary. - let left_parts_patches = ALPRDData::canonicalize_patches( - &left_parts, - left_parts_patches, - &mut LEGACY_SESSION.create_execution_ctx(), - )?; + let left_parts_patches = ALPRDData::validate_patches(&left_parts, left_parts_patches)?; let slots = ALPRDData::make_slots(&left_parts, &right_parts, left_parts_patches.as_ref()); let data = ALPRDData::new( left_parts_dictionary, @@ -371,8 +362,7 @@ impl ALPRD { ctx: &mut ExecutionCtx, ) -> VortexResult { let len = left_parts.len(); - let left_parts_patches = - ALPRDData::canonicalize_patches(&left_parts, left_parts_patches, ctx)?; + let left_parts_patches = ALPRDData::validate_patches(&left_parts, left_parts_patches)?; let slots = ALPRDData::make_slots(&left_parts, &right_parts, left_parts_patches.as_ref()); let data = ALPRDData::new(left_parts_dictionary, right_bit_width, left_parts_patches); Array::try_from_parts(ArrayParts::new(ALPRD, dtype, len, data).with_slots(slots)) @@ -400,26 +390,27 @@ impl ALPRD { } impl ALPRDData { - fn canonicalize_patches( + /// Validate that `left_parts_patches` are well-formed for an `ALPRDArray` without performing + /// any execution. + /// + /// Patch values must already be stored as the non-nullable `left_parts` dtype. Requiring the + /// non-nullable dtype also guarantees the patch values contain no nulls, so no execution is + /// needed to verify validity. Callers are responsible for constructing patches with the correct + /// value dtype (the encoder, `deserialize`, and the compute kernels all already do so). + fn validate_patches( left_parts: &ArrayRef, left_parts_patches: Option, - ctx: &mut ExecutionCtx, ) -> VortexResult> { - left_parts_patches - .map(|patches| { - if !patches.values().all_valid(ctx)? { - vortex_bail!("patches must be all valid: {}", patches.values()); - } - // TODO(ngates): assert the DType, don't cast it. - // TODO(joe): assert the DType, don't cast it in the next PR. - let mut patches = patches.cast_values(&left_parts.dtype().as_nonnullable())?; - // Force execution of the lazy cast so patch values are materialized - // before serialization. - let canonical = patches.values().clone().execute::(ctx)?; - *patches.values_mut() = canonical.into_array(); - Ok(patches) - }) - .transpose() + if let Some(patches) = &left_parts_patches { + let expected = left_parts.dtype().as_nonnullable(); + vortex_ensure!( + patches.values().dtype() == &expected, + "ALPRD patch values must have dtype {expected} (the non-nullable left-parts dtype), \ + got {}", + patches.values().dtype(), + ); + } + Ok(left_parts_patches) } /// Build a new `ALPRDArray` from components. @@ -548,18 +539,12 @@ fn validate_parts( "patches array_len {} != outer len {len}", patches.array_len(), ); + let expected_patches_dtype = left_parts.dtype().as_nonnullable(); vortex_ensure!( - patches.dtype().eq_ignore_nullability(left_parts.dtype()), - "patches dtype {} does not match left_parts dtype {}", + patches.dtype() == &expected_patches_dtype, + "patches dtype {} does not match expected non-nullable left_parts dtype {}", patches.dtype(), - left_parts.dtype(), - ); - vortex_ensure!( - patches - .values() - .all_valid(&mut LEGACY_SESSION.create_execution_ctx())?, - "patches must be all valid: {}", - patches.values() + expected_patches_dtype, ); } diff --git a/encodings/alp/src/alp_rd/compute/take.rs b/encodings/alp/src/alp_rd/compute/take.rs index 6db6eae2978..40e3d23419a 100644 --- a/encodings/alp/src/alp_rd/compute/take.rs +++ b/encodings/alp/src/alp_rd/compute/take.rs @@ -20,16 +20,17 @@ impl TakeExecute for ALPRD { ctx: &mut ExecutionCtx, ) -> VortexResult> { let taken_left_parts = array.left_parts().take(indices.clone())?; + // `Patches::take` ignores null take-indices, so the resulting patch values are all valid, + // but it reports a nullable dtype. Patch values must be stored as the non-nullable + // left-parts dtype (the invariant `ALPRD::try_new` asserts), so drop the spurious + // nullability with a lazy cast — no execution happens here, it is materialized later. let left_parts_exceptions = array .left_parts_patches() .map(|patches| patches.take(indices, ctx)) .transpose()? .flatten() .map(|p| { - let values_dtype = p - .values() - .dtype() - .with_nullability(taken_left_parts.dtype().nullability()); + let values_dtype = p.values().dtype().as_nonnullable(); p.cast_values(&values_dtype) }) .transpose()?; From a2fb628ae344afc7522947e136df09acc81547f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 15:02:41 +0000 Subject: [PATCH 2/4] ALPRD: assert patch values have no nulls instead of casting Per review: patch values already share the left-parts dtype, so drop the `cast_values` in `take` entirely. Instead assert the invariant directly: patch dtype matches left parts (ignoring nullability) and the values are `definitely_no_nulls` (a static check, no execution). `Patches::take`/`take_search`/`take_map` previously gave the taken value array a nullable validity inherited from the take indices even when nulls were excluded (`include_nulls = false`). Since no null take-indices emit a value in that mode, the value array is now marked `Validity::NonNullable`, so taking a non-nullable patch values array yields non-nullable values and satisfies `definitely_no_nulls` without any cast or execution. Signed-off-by: Robert Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Mkrgj5SuJpaKBXK9jydtSc --- encodings/alp/src/alp_rd/array.rs | 34 +++++++++++++++--------- encodings/alp/src/alp_rd/compute/take.rs | 14 +++------- vortex-array/src/patches.rs | 16 ++++++++--- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index fba6529efd0..b1109cc4a3e 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -393,21 +393,25 @@ impl ALPRDData { /// Validate that `left_parts_patches` are well-formed for an `ALPRDArray` without performing /// any execution. /// - /// Patch values must already be stored as the non-nullable `left_parts` dtype. Requiring the - /// non-nullable dtype also guarantees the patch values contain no nulls, so no execution is - /// needed to verify validity. Callers are responsible for constructing patches with the correct - /// value dtype (the encoder, `deserialize`, and the compute kernels all already do so). + /// Patch values must have the same dtype as `left_parts` (ignoring nullability) and contain no + /// nulls. Both are checked statically — `definitely_no_nulls` requires no execution — so the + /// values never need to be cast. Callers are responsible for constructing patches with the + /// correct value dtype (the encoder, `deserialize`, and the compute kernels all already do so). fn validate_patches( left_parts: &ArrayRef, left_parts_patches: Option, ) -> VortexResult> { if let Some(patches) = &left_parts_patches { - let expected = left_parts.dtype().as_nonnullable(); vortex_ensure!( - patches.values().dtype() == &expected, - "ALPRD patch values must have dtype {expected} (the non-nullable left-parts dtype), \ - got {}", - patches.values().dtype(), + patches.dtype().eq_ignore_nullability(left_parts.dtype()), + "ALPRD patch values dtype {} must match left-parts dtype {}", + patches.dtype(), + left_parts.dtype(), + ); + vortex_ensure!( + patches.values().validity()?.definitely_no_nulls(), + "ALPRD patch values must contain no nulls, got {}", + patches.values(), ); } Ok(left_parts_patches) @@ -539,12 +543,16 @@ fn validate_parts( "patches array_len {} != outer len {len}", patches.array_len(), ); - let expected_patches_dtype = left_parts.dtype().as_nonnullable(); vortex_ensure!( - patches.dtype() == &expected_patches_dtype, - "patches dtype {} does not match expected non-nullable left_parts dtype {}", + patches.dtype().eq_ignore_nullability(left_parts.dtype()), + "patches dtype {} does not match left_parts dtype {}", patches.dtype(), - expected_patches_dtype, + left_parts.dtype(), + ); + vortex_ensure!( + patches.values().validity()?.definitely_no_nulls(), + "patches must contain no nulls: {}", + patches.values(), ); } diff --git a/encodings/alp/src/alp_rd/compute/take.rs b/encodings/alp/src/alp_rd/compute/take.rs index 40e3d23419a..550d45103bf 100644 --- a/encodings/alp/src/alp_rd/compute/take.rs +++ b/encodings/alp/src/alp_rd/compute/take.rs @@ -20,20 +20,14 @@ impl TakeExecute for ALPRD { ctx: &mut ExecutionCtx, ) -> VortexResult> { let taken_left_parts = array.left_parts().take(indices.clone())?; - // `Patches::take` ignores null take-indices, so the resulting patch values are all valid, - // but it reports a nullable dtype. Patch values must be stored as the non-nullable - // left-parts dtype (the invariant `ALPRD::try_new` asserts), so drop the spurious - // nullability with a lazy cast — no execution happens here, it is materialized later. + // `Patches::take` ignores null take-indices, so the resulting patch values contain no + // nulls (they may report a nullable dtype, but `ALPRD::try_new` validates that via + // `definitely_no_nulls` rather than requiring a cast). let left_parts_exceptions = array .left_parts_patches() .map(|patches| patches.take(indices, ctx)) .transpose()? - .flatten() - .map(|p| { - let values_dtype = p.values().dtype().as_nonnullable(); - p.cast_values(&values_dtype) - }) - .transpose()?; + .flatten(); let right_parts = array .right_parts() .take(indices.clone())? diff --git a/vortex-array/src/patches.rs b/vortex-array/src/patches.rs index 5c73aa95d13..77459d4a64a 100644 --- a/vortex-array/src/patches.rs +++ b/vortex-array/src/patches.rs @@ -910,7 +910,12 @@ impl Patches { let new_indices = new_indices.into_array(); let new_array_len = take_indices.len(); - let values_validity = take_indices_validity.take(&new_indices)?; + // When nulls are excluded every taken value is valid, so the values are non-nullable. + let values_validity = if include_nulls { + take_indices_validity.take(&new_indices)? + } else { + Validity::NonNullable + }; Ok(Some(Self { array_len: new_array_len, @@ -1058,8 +1063,13 @@ where } let new_sparse_indices = new_sparse_indices.into_array(); - let values_validity = - Validity::from_mask(take_validity, take_nullability).take(&new_sparse_indices)?; + // When nulls are excluded every emitted value index is valid, so the value-index array is + // non-nullable; only when nulls are included must the take indices' nullability be preserved. + let values_validity = if include_nulls { + Validity::from_mask(take_validity, take_nullability).take(&new_sparse_indices)? + } else { + Validity::NonNullable + }; Ok(Some(( new_sparse_indices, PrimitiveArray::new(value_indices, values_validity).into_array(), From 697806e55837a6f5bcadb19f6719207c6603127c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 15:27:46 +0000 Subject: [PATCH 3/4] ALP: assert patch values have no nulls instead of casting Apply the same treatment as ALPRD to ALP's compute paths: patch values are concrete floats with no nulls, so the `cast_values` in `take`/`mask` (which only adjusted nullability to match the taken/masked array) is unnecessary. - `validate_patches` now asserts the patch dtype matches the float type ignoring nullability, plus `definitely_no_nulls` (a static check), instead of requiring an exact nullability match. - `take`/`mask` drop `cast_values` and use the `Patches::take`/`mask` output directly (both preserve the values' no-null property). - Update `test_mask_alp_with_patches_*` to assert the surviving patch values remain null-free rather than being cast to nullable. Signed-off-by: Robert Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Mkrgj5SuJpaKBXK9jydtSc --- encodings/alp/src/alp/array.rs | 9 +++++++-- encodings/alp/src/alp/compute/mask.rs | 21 +++++++++++++-------- encodings/alp/src/alp/compute/take.rs | 13 ++++--------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/encodings/alp/src/alp/array.rs b/encodings/alp/src/alp/array.rs index 8e581fe51fc..8c0bc346f0f 100644 --- a/encodings/alp/src/alp/array.rs +++ b/encodings/alp/src/alp/array.rs @@ -311,10 +311,15 @@ impl ALPData { let expected_type = DType::Primitive(T::PTYPE, encoded.dtype().nullability()); vortex_ensure!( - patches.dtype() == &expected_type, - "Expected patches type {expected_type}, actual {}", + patches.dtype().eq_ignore_nullability(&expected_type), + "Expected patches type {expected_type} (ignoring nullability), actual {}", patches.dtype(), ); + vortex_ensure!( + patches.values().validity()?.definitely_no_nulls(), + "ALP patch values must contain no nulls, got {}", + patches.values(), + ); Ok(()) } diff --git a/encodings/alp/src/alp/compute/mask.rs b/encodings/alp/src/alp/compute/mask.rs index 796b0fa0813..06859fed814 100644 --- a/encodings/alp/src/alp/compute/mask.rs +++ b/encodings/alp/src/alp/compute/mask.rs @@ -36,16 +36,13 @@ impl MaskKernel for ALP { ) -> VortexResult> { let vortex_mask = Validity::Array(mask.not()?).execute_mask(array.len(), ctx)?; let masked_encoded = array.encoded().clone().mask(mask.clone())?; - let masked_dtype = array - .dtype() - .with_nullability(masked_encoded.dtype().nullability()); + // Patch values are concrete floats with no nulls; `Patches::mask` filters them and + // preserves that, so no cast is needed. let masked_patches = array .patches() .map(|p| p.mask(&vortex_mask, ctx)) .transpose()? - .flatten() - .map(|patches| patches.cast_values(&masked_dtype)) - .transpose()?; + .flatten(); Ok(Some( ALP::new(masked_encoded, array.exponents(), masked_patches).into_array(), )) @@ -99,7 +96,7 @@ mod test { } #[test] - fn test_mask_alp_with_patches_casts_surviving_patch_values_to_nullable() { + fn test_mask_alp_with_patches_keeps_patch_values_without_nulls() { let mut ctx = array_session().create_execution_ctx(); let values = PrimitiveArray::from_iter([1.234f32, f32::NAN, 2.345, f32::INFINITY, 3.456]); let alp = alp_encode(values.as_view(), None, &mut ctx).unwrap(); @@ -113,7 +110,15 @@ mod test { let masked_alp = masked.as_opt::().unwrap(); let masked_patches = masked_alp.patches().unwrap(); + // Masking introduces nulls into the array, but the surviving patch values stay concrete + // (no cast), so they remain free of nulls. assert_eq!(masked.dtype().nullability(), Nullability::Nullable); - assert_eq!(masked_patches.dtype().nullability(), Nullability::Nullable); + assert!( + masked_patches + .values() + .validity() + .unwrap() + .definitely_no_nulls() + ); } } diff --git a/encodings/alp/src/alp/compute/take.rs b/encodings/alp/src/alp/compute/take.rs index d7673455de4..ee54e1007fb 100644 --- a/encodings/alp/src/alp/compute/take.rs +++ b/encodings/alp/src/alp/compute/take.rs @@ -19,19 +19,14 @@ impl TakeExecute for ALP { ctx: &mut ExecutionCtx, ) -> VortexResult> { let taken_encoded = array.encoded().take(indices.clone())?; + // Patch values are concrete floats with no nulls; `Patches::take` preserves that, so no + // cast is needed — `ALP::validate_patches` checks the dtype (ignoring nullability) and + // `definitely_no_nulls`. let taken_patches = array .patches() .map(|p| p.take(indices, ctx)) .transpose()? - .flatten() - .map(|patches| { - patches.cast_values( - &array - .dtype() - .with_nullability(taken_encoded.dtype().nullability()), - ) - }) - .transpose()?; + .flatten(); Ok(Some( ALP::new(taken_encoded, array.exponents(), taken_patches).into_array(), )) From 957ae64ac492aa515fe5c00f2ef27561e6f5e7fa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 17:00:43 +0000 Subject: [PATCH 4/4] ALPRD: drop ExecutionCtx from try_new and encode path Now that `ALPRD::validate_patches` asserts the patch dtype and `definitely_no_nulls` statically (no execution), `ALPRD::try_new` no longer needs an `ExecutionCtx`. Remove the parameter and propagate the removal up through `RDEncoder::encode`/`encode_generic`, the compute kernels (`take`/`filter`/`mask`), btrblocks ALPRD compression, the compat-gen fixtures, and the benches. This also lets `MaskReduce::mask` (whose trait signature lacks an `ExecutionCtx`) stop minting a `LEGACY_SESSION` execution context at the trait boundary. Signed-off-by: Robert Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Mkrgj5SuJpaKBXK9jydtSc --- encodings/alp/benches/alp_compress.rs | 6 ++-- encodings/alp/src/alp_rd/array.rs | 3 +- encodings/alp/src/alp_rd/compute/cast.rs | 14 ++++---- encodings/alp/src/alp_rd/compute/filter.rs | 9 ++---- encodings/alp/src/alp_rd/compute/mask.rs | 11 +------ encodings/alp/src/alp_rd/compute/mod.rs | 22 ++++++------- encodings/alp/src/alp_rd/compute/take.rs | 11 ++----- encodings/alp/src/alp_rd/mod.rs | 11 ++----- encodings/alp/src/alp_rd/ops.rs | 6 ++-- vortex-btrblocks/src/schemes/float/alprd.rs | 3 +- .../arrays/synthetic/encodings/alprd.rs | 32 +++++++------------ vortex/benches/single_encoding_throughput.rs | 8 ++--- 12 files changed, 50 insertions(+), 86 deletions(-) diff --git a/encodings/alp/benches/alp_compress.rs b/encodings/alp/benches/alp_compress.rs index 02cc30e44a6..6e769ffc22e 100644 --- a/encodings/alp/benches/alp_compress.rs +++ b/encodings/alp/benches/alp_compress.rs @@ -146,8 +146,8 @@ fn compress_rd(bencher: Bencher, args: (usize, f64) let encoder = RDEncoder::new(primitive.as_slice::()); bencher - .with_inputs(|| (&primitive, &encoder, SESSION.create_execution_ctx())) - .bench_refs(|(primitive, encoder, ctx)| encoder.encode(primitive.as_view(), ctx)) + .with_inputs(|| (&primitive, &encoder)) + .bench_refs(|(primitive, encoder)| encoder.encode(primitive.as_view())) } #[divan::bench(types = [f32, f64], args = RD_BENCH_ARGS)] @@ -155,7 +155,7 @@ fn decompress_rd(bencher: Bencher, args: (usize, f6 let (n, fraction_patch) = args; let primitive = make_rd_array::(n, fraction_patch); let encoder = RDEncoder::new(primitive.as_slice::()); - let encoded = encoder.encode(primitive.as_view(), &mut SESSION.create_execution_ctx()); + let encoded = encoder.encode(primitive.as_view()); bencher .with_inputs(|| (&encoded, SESSION.create_execution_ctx())) diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index b1109cc4a3e..a4e16f5ba09 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -359,7 +359,6 @@ impl ALPRD { right_parts: ArrayRef, right_bit_width: u8, left_parts_patches: Option, - ctx: &mut ExecutionCtx, ) -> VortexResult { let len = left_parts.len(); let left_parts_patches = ALPRDData::validate_patches(&left_parts, left_parts_patches)?; @@ -657,7 +656,7 @@ mod test { // Pick a seed that we know will trigger lots of patches. let encoder: alp_rd::RDEncoder = alp_rd::RDEncoder::new(&[seed.powi(-2)]); - let rd_array = encoder.encode(real_array.as_view(), &mut ctx); + let rd_array = encoder.encode(real_array.as_view()); let decoded = rd_array .as_array() diff --git a/encodings/alp/src/alp_rd/compute/cast.rs b/encodings/alp/src/alp_rd/compute/cast.rs index 8a651708d3f..d290827355c 100644 --- a/encodings/alp/src/alp_rd/compute/cast.rs +++ b/encodings/alp/src/alp_rd/compute/cast.rs @@ -67,7 +67,7 @@ mod tests { let values = vec![1.0f32, 1.1, 1.2, 1.3, 1.4]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - let alprd = encoder.encode(arr.as_view(), &mut ctx); + let alprd = encoder.encode(arr.as_view()); let casted = alprd .into_array() @@ -92,7 +92,7 @@ mod tests { PrimitiveArray::from_option_iter([Some(10.0f64), None, Some(10.1), Some(10.2), None]); let values = vec![10.0f64, 10.1, 10.2]; let encoder = RDEncoder::new(&values); - let alprd = encoder.encode(arr.as_view(), &mut ctx); + let alprd = encoder.encode(arr.as_view()); // Cast to NonNullable should fail since we have nulls. The failure surfaces during // execution since the reduce path defers when the validity stat is not cached. @@ -122,31 +122,31 @@ mod tests { let values = vec![1.23f32, 4.56, 7.89, 10.11, 12.13]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut array_session().create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::f64({ let values = vec![100.1f64, 200.2, 300.3, 400.4, 500.5]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut array_session().create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::single({ let values = vec![42.42f64]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut array_session().create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::negative({ let values = vec![0.0f32, -1.5, 2.5, -3.5, 4.5]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut array_session().create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::nullable({ let arr = PrimitiveArray::from_option_iter([Some(1.1f32), None, Some(2.2), Some(3.3), None]); let values = vec![1.1f32, 2.2, 3.3]; let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut array_session().create_execution_ctx()) + encoder.encode(arr.as_view()) })] fn test_cast_alprd_conformance(#[case] alprd: crate::alp_rd::ALPRDArray) { test_cast_conformance( diff --git a/encodings/alp/src/alp_rd/compute/filter.rs b/encodings/alp/src/alp_rd/compute/filter.rs index 5d98b4561ce..69c26c53e3b 100644 --- a/encodings/alp/src/alp_rd/compute/filter.rs +++ b/encodings/alp/src/alp_rd/compute/filter.rs @@ -32,7 +32,6 @@ impl FilterKernel for ALPRD { array.right_parts().filter(mask.clone())?, array.right_bit_width(), left_parts_exceptions, - ctx, )? .into_array(), )) @@ -71,7 +70,7 @@ mod test { fn test_filter(#[case] a: T, #[case] b: T, #[case] outlier: T) { let mut ctx = SESSION.create_execution_ctx(); let array = PrimitiveArray::new(buffer![a, b, outlier], Validity::NonNullable); - let encoded = RDEncoder::new(&[a, b]).encode(array.as_view(), &mut ctx); + let encoded = RDEncoder::new(&[a, b]).encode(array.as_view()); // Make sure that we're testing the exception pathway. assert!(encoded.left_parts_patches().is_some()); @@ -90,10 +89,7 @@ mod test { let mut ctx = SESSION.create_execution_ctx(); test_filter_conformance( &RDEncoder::new(&[a, b]) - .encode( - PrimitiveArray::from_iter([a, b, outlier, b, outlier]).as_view(), - &mut ctx, - ) + .encode(PrimitiveArray::from_iter([a, b, outlier, b, outlier]).as_view()) .into_array(), &mut ctx, ); @@ -109,7 +105,6 @@ mod test { .encode( PrimitiveArray::from_option_iter([Some(a), None, Some(outlier), Some(a), None]) .as_view(), - &mut ctx, ) .into_array(), &mut ctx, diff --git a/encodings/alp/src/alp_rd/compute/mask.rs b/encodings/alp/src/alp_rd/compute/mask.rs index 335f18a33cb..b32db4ef642 100644 --- a/encodings/alp/src/alp_rd/compute/mask.rs +++ b/encodings/alp/src/alp_rd/compute/mask.rs @@ -4,8 +4,6 @@ use vortex_array::ArrayRef; use vortex_array::ArrayView; use vortex_array::IntoArray; -use vortex_array::LEGACY_SESSION; -use vortex_array::VortexSessionExecute; use vortex_array::arrays::scalar_fn::ScalarFnFactoryExt; use vortex_array::scalar_fn::EmptyOptions; use vortex_array::scalar_fn::fns::mask::Mask as MaskExpr; @@ -22,8 +20,6 @@ impl MaskReduce for ALPRD { EmptyOptions, [array.left_parts().clone(), mask.clone()], )?; - // NOTE: `MaskReduce::mask` has a fixed trait signature without `ExecutionCtx`, so we - // construct a legacy ctx locally at this trait boundary. Ok(Some( ALPRD::try_new( array.dtype().as_nullable(), @@ -32,7 +28,6 @@ impl MaskReduce for ALPRD { array.right_parts().clone(), array.right_bit_width(), array.left_parts_patches(), - &mut LEGACY_SESSION.create_execution_ctx(), )? .into_array(), )) @@ -58,10 +53,7 @@ mod tests { let mut ctx = array_session().create_execution_ctx(); test_mask_conformance( &RDEncoder::new(&[a, b]) - .encode( - PrimitiveArray::from_iter([a, b, outlier, b, outlier]).as_view(), - &mut ctx, - ) + .encode(PrimitiveArray::from_iter([a, b, outlier, b, outlier]).as_view()) .into_array(), &mut ctx, ); @@ -77,7 +69,6 @@ mod tests { .encode( PrimitiveArray::from_option_iter([Some(a), None, Some(outlier), Some(a), None]) .as_view(), - &mut ctx, ) .into_array(), &mut ctx, diff --git a/encodings/alp/src/alp_rd/compute/mod.rs b/encodings/alp/src/alp_rd/compute/mod.rs index f74b23fc2c2..696e622b873 100644 --- a/encodings/alp/src/alp_rd/compute/mod.rs +++ b/encodings/alp/src/alp_rd/compute/mod.rs @@ -34,47 +34,47 @@ mod tests { let values = vec![1.0f32, 1.1, 1.2, 1.3, 1.4]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::f64_array({ let values = vec![100.0f64, 100.01, 100.02, 100.03, 100.04]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] // Nullable arrays #[case::nullable_f32({ let values = vec![1.0f32, 1.2, 1.3]; let arr = PrimitiveArray::from_option_iter([Some(1.0f32), None, Some(1.2), Some(1.3), None]); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::nullable_f64({ let values = vec![10.0f64, 10.2, 10.3]; let arr = PrimitiveArray::from_option_iter([Some(10.0f64), None, Some(10.2), Some(10.3), None]); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] // Edge cases #[case::single_element({ let values = vec![42.42f64]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] // Arrays with small deltas (good for RD encoding) #[case::small_deltas({ let values = vec![1000.0f32, 1000.001, 1000.002, 1000.003, 1000.004]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] // Large arrays #[case::large_f32({ let values: Vec = (0..1000).map(|i| 100.0 + i as f32 * 0.01).collect(); let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] fn test_alp_rd_consistency(#[case] array: ALPRDArray) { let ctx = &mut SESSION.create_execution_ctx(); @@ -86,25 +86,25 @@ mod tests { let values = vec![1.0f32, 1.1, 1.2, 1.3, 1.4]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::f64_basic({ let values = vec![100.0f64, 100.01, 100.02, 100.03, 100.04]; let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::f32_large({ let values: Vec = (0..100).map(|i| 50.0 + i as f32 * 0.1).collect(); let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] #[case::f64_large({ let values: Vec = (0..100).map(|i| 1000.0 + i as f64 * 0.01).collect(); let arr = PrimitiveArray::from_iter(values.clone()); let encoder = RDEncoder::new(&values); - encoder.encode(arr.as_view(), &mut SESSION.create_execution_ctx()) + encoder.encode(arr.as_view()) })] fn test_alp_rd_binary_numeric(#[case] array: ALPRDArray) { test_binary_numeric_array(&array.into_array(), &mut SESSION.create_execution_ctx()); diff --git a/encodings/alp/src/alp_rd/compute/take.rs b/encodings/alp/src/alp_rd/compute/take.rs index 550d45103bf..5718169042d 100644 --- a/encodings/alp/src/alp_rd/compute/take.rs +++ b/encodings/alp/src/alp_rd/compute/take.rs @@ -43,7 +43,6 @@ impl TakeExecute for ALPRD { right_parts, array.right_bit_width(), left_parts_exceptions, - ctx, )? .into_array(), )) @@ -82,7 +81,7 @@ mod test { let mut ctx = SESSION.create_execution_ctx(); let array = PrimitiveArray::from_iter([a, b, outlier]); - let encoded = RDEncoder::new(&[a, b]).encode(array.as_view(), &mut ctx); + let encoded = RDEncoder::new(&[a, b]).encode(array.as_view()); assert!(encoded.left_parts_patches().is_some()); assert!( @@ -108,7 +107,7 @@ mod test { fn take_with_nulls(#[case] a: T, #[case] b: T, #[case] outlier: T) { let mut ctx = SESSION.create_execution_ctx(); let array = PrimitiveArray::from_iter([a, b, outlier]); - let encoded = RDEncoder::new(&[a, b]).encode(array.as_view(), &mut ctx); + let encoded = RDEncoder::new(&[a, b]).encode(array.as_view()); assert!(encoded.left_parts_patches().is_some()); assert!( @@ -139,10 +138,7 @@ mod test { let mut ctx = SESSION.create_execution_ctx(); test_take_conformance( &RDEncoder::new(&[a, b]) - .encode( - PrimitiveArray::from_iter([a, b, outlier, b, outlier]).as_view(), - &mut ctx, - ) + .encode(PrimitiveArray::from_iter([a, b, outlier, b, outlier]).as_view()) .into_array(), &mut ctx, ); @@ -158,7 +154,6 @@ mod test { .encode( PrimitiveArray::from_option_iter([Some(a), None, Some(outlier), Some(a), None]) .as_view(), - &mut ctx, ) .into_array(), &mut ctx, diff --git a/encodings/alp/src/alp_rd/mod.rs b/encodings/alp/src/alp_rd/mod.rs index a9ac7ca82fe..07afe6b8adf 100644 --- a/encodings/alp/src/alp_rd/mod.rs +++ b/encodings/alp/src/alp_rd/mod.rs @@ -187,15 +187,11 @@ impl RDEncoder { /// /// Each value will be split into a left and right component, which are compressed individually. // TODO(joe): make fallible - pub fn encode(&self, array: ArrayView<'_, Primitive>, ctx: &mut ExecutionCtx) -> ALPRDArray { - match_each_alp_float_ptype!(array.ptype(), |P| { self.encode_generic::

(array, ctx) }) + pub fn encode(&self, array: ArrayView<'_, Primitive>) -> ALPRDArray { + match_each_alp_float_ptype!(array.ptype(), |P| { self.encode_generic::

(array) }) } - fn encode_generic( - &self, - array: ArrayView<'_, Primitive>, - ctx: &mut ExecutionCtx, - ) -> ALPRDArray + fn encode_generic(&self, array: ArrayView<'_, Primitive>) -> ALPRDArray where T: ALPRDFloat + NativePType, T::UINT: NativePType, @@ -294,7 +290,6 @@ impl RDEncoder { packed_right, self.right_bit_width, exceptions, - ctx, ) .vortex_expect("ALPRDArray construction in encode") } diff --git a/encodings/alp/src/alp_rd/ops.rs b/encodings/alp/src/alp_rd/ops.rs index be8d8f88948..0fc0dc40fa1 100644 --- a/encodings/alp/src/alp_rd/ops.rs +++ b/encodings/alp/src/alp_rd/ops.rs @@ -90,7 +90,7 @@ mod test { fn test_slice(#[case] a: T, #[case] b: T, #[case] outlier: T) { let mut ctx = SESSION.create_execution_ctx(); let array = PrimitiveArray::from_iter([a, b, outlier]); - let encoded = RDEncoder::new(&[a, b]).encode(array.as_view(), &mut ctx); + let encoded = RDEncoder::new(&[a, b]).encode(array.as_view()); assert!(encoded.left_parts_patches().is_some()); assert_arrays_eq!(encoded, array, &mut ctx); @@ -106,7 +106,7 @@ mod test { ) { let mut ctx = SESSION.create_execution_ctx(); let array = PrimitiveArray::from_iter([a, b, outlier]); - let encoded = RDEncoder::new(&[a, b]).encode(array.as_view(), &mut ctx); + let encoded = RDEncoder::new(&[a, b]).encode(array.as_view()); assert!(encoded.left_parts_patches().is_some()); assert_arrays_eq!(encoded, array, &mut ctx); } @@ -118,7 +118,7 @@ mod test { let b = 0.2f64; let outlier = 3e100f64; let array = PrimitiveArray::from_option_iter([Some(a), Some(b), Some(outlier)]); - let encoded = RDEncoder::new(&[a, b]).encode(array.as_view(), &mut ctx); + let encoded = RDEncoder::new(&[a, b]).encode(array.as_view()); assert!(encoded.left_parts_patches().is_some()); assert_arrays_eq!( encoded, diff --git a/vortex-btrblocks/src/schemes/float/alprd.rs b/vortex-btrblocks/src/schemes/float/alprd.rs index f97ae077882..969afabad49 100644 --- a/vortex-btrblocks/src/schemes/float/alprd.rs +++ b/vortex-btrblocks/src/schemes/float/alprd.rs @@ -66,7 +66,7 @@ impl Scheme for ALPRDScheme { ptype => vortex_panic!("cannot ALPRD compress ptype {ptype}"), }; - let alp_rd = encoder.encode(primitive_array, exec_ctx); + let alp_rd = encoder.encode(primitive_array); let dtype = alp_rd.dtype().clone(); let right_bit_width = alp_rd.right_bit_width(); let mut parts = ALPRDArrayOwnedExt::into_data_parts(alp_rd); @@ -82,7 +82,6 @@ impl Scheme for ALPRDScheme { parts.right_parts, right_bit_width, parts.left_parts_patches, - exec_ctx, )? .into_array()) } diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/alprd.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/alprd.rs index a073dfc6693..ef41fbf8a58 100644 --- a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/alprd.rs +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/alprd.rs @@ -46,7 +46,7 @@ impl FlatLayoutFixture for AlprdFixture { vec![ALPRD.id()] } - fn build(&self, ctx: &mut ExecutionCtx) -> VortexResult { + fn build(&self, _ctx: &mut ExecutionCtx) -> VortexResult { let sensor: PrimitiveArray = (0..N) .map(|i| { let noise = ((i * 7 + 13) % 100) as f64 / 1000.0; @@ -151,31 +151,21 @@ impl FlatLayoutFixture for AlprdFixture { "nullable_specials", ]), vec![ - sensor_enc.encode(sensor.as_view(), ctx).into_array(), - drift_enc.encode(drift.as_view(), ctx).into_array(), - constant_enc - .encode(constant_series.as_view(), ctx) - .into_array(), - decreasing_enc - .encode(decreasing.as_view(), ctx) - .into_array(), - oscillating_enc - .encode(oscillating.as_view(), ctx) - .into_array(), + sensor_enc.encode(sensor.as_view()).into_array(), + drift_enc.encode(drift.as_view()).into_array(), + constant_enc.encode(constant_series.as_view()).into_array(), + decreasing_enc.encode(decreasing.as_view()).into_array(), + oscillating_enc.encode(oscillating.as_view()).into_array(), periodic_resets_enc - .encode(periodic_resets.as_view(), ctx) - .into_array(), - nullable_enc - .encode(sensor_nullable.as_view(), ctx) - .into_array(), - special_enc - .encode(special_values.as_view(), ctx) + .encode(periodic_resets.as_view()) .into_array(), + nullable_enc.encode(sensor_nullable.as_view()).into_array(), + special_enc.encode(special_values.as_view()).into_array(), boundary_enc - .encode(boundary_specials.as_view(), ctx) + .encode(boundary_specials.as_view()) .into_array(), nullable_special_enc - .encode(nullable_specials.as_view(), ctx) + .encode(nullable_specials.as_view()) .into_array(), ], N, diff --git a/vortex/benches/single_encoding_throughput.rs b/vortex/benches/single_encoding_throughput.rs index a20777af583..b75f832a7a1 100644 --- a/vortex/benches/single_encoding_throughput.rs +++ b/vortex/benches/single_encoding_throughput.rs @@ -299,10 +299,10 @@ fn bench_alp_rd_compress_f64(bencher: Bencher) { let (_, _, float_array) = setup_primitive_arrays(); with_byte_counter(bencher, NUM_VALUES * 8) - .with_inputs(|| (&float_array, SESSION.create_execution_ctx())) - .bench_refs(|(a, ctx)| { + .with_inputs(|| &float_array) + .bench_refs(|a| { let encoder = RDEncoder::new(a.as_slice::()); - encoder.encode(a.as_view(), ctx) + encoder.encode(a.as_view()) }); } @@ -310,7 +310,7 @@ fn bench_alp_rd_compress_f64(bencher: Bencher) { fn bench_alp_rd_decompress_f64(bencher: Bencher) { let (_, _, float_array) = setup_primitive_arrays(); let encoder = RDEncoder::new(float_array.as_slice::()); - let compressed = encoder.encode(float_array.as_view(), &mut SESSION.create_execution_ctx()); + let compressed = encoder.encode(float_array.as_view()); with_byte_counter(bencher, NUM_VALUES * 8) .with_inputs(|| (&compressed, SESSION.create_execution_ctx()))