diff --git a/src/enums/operators.rs b/src/enums/operators.rs index cffb126..177310b 100644 --- a/src/enums/operators.rs +++ b/src/enums/operators.rs @@ -25,18 +25,33 @@ pub enum ArithmeticOperator { Multiply, /// Division (`lhs / rhs`) /// - /// For integers, division by zero panics in unmasked arrays and nullifies in masked arrays. - /// For floating-point, follows IEEE 754 (yields ±Inf or NaN). + /// Division is true division: `7 / 2` is `3.5`. The `Scalar` arms + /// return a `Float64` scalar for integer operands, and callers dividing + /// integer arrays cast their operands to `f64` before dispatch so the + /// float kernels produce the float result. + /// + /// The integer slice kernels themselves serve `FloorDiv`, so `Divide` + /// handed raw integer slices at the kernel level behaves as floor + /// division. Cast to float first for a true-division result. + /// + /// Division by zero on floats follows IEEE 754: a nonzero value over + /// zero yields Inf with the operands' sign, and zero over zero yields + /// NaN. On raw integer slices it panics in unmasked arrays and + /// nullifies in masked arrays. Divide, /// Modulus/remainder operation (`lhs % rhs`) /// - /// Behaviour matches Rust's `%` operator. Division by zero handling follows same - /// rules as `Divide` operation. + /// Behaviour matches Rust's `%` operator: the result keeps the + /// dividend's sign, so `-7 % 2` is `-1`. Division by zero handling + /// follows same rules as `Divide` operation. Remainder, /// Exponentiation (`lhs ^ rhs`) /// - /// For integers, uses repeated multiplication. For floating-point, uses `pow()` function. - /// Negative exponents on integers may yield zero due to truncation. + /// For integers, exponentiation by squaring with wrapping + /// multiplication, so overflow wraps like the other integer arms. The + /// exponent must convert to `u32`: a negative or larger exponent + /// returns an error advising a cast to float. For floating-point, uses + /// logarithmic computation. Power, /// Floor division (`lhs // rhs`) /// diff --git a/src/enums/value/mod.rs b/src/enums/value/mod.rs index 5f42024..530365d 100644 --- a/src/enums/value/mod.rs +++ b/src/enums/value/mod.rs @@ -160,7 +160,7 @@ impl Value { Value::Array(a) => a.len(), #[cfg(feature = "views")] - Value::ArrayView(av) => av.array.len(), + Value::ArrayView(av) => av.len(), Value::FieldArray(fa) => fa.array.len(), @@ -376,3 +376,61 @@ impl Value { } // Also see typed accessors in ./conversions.rs and trait impls in ./impls.rs + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use super::*; + use crate::{ + Array, ArrayV, ArrowType, Field, FieldArray, IntegerArray, MaskedArray, NumericArray, Table, + }; + + fn seq_array(n: usize) -> Array { + let mut arr = IntegerArray::::default(); + for i in 0..n { + arr.push(i as i64); + } + Array::NumericArray(NumericArray::Int64(Arc::new(arr))) + } + + /// `len` counts the rows a view covers, so callers can pass the count + /// straight to `slice`. + #[cfg(feature = "views")] + #[test] + fn len_of_an_array_view_counts_the_window() { + let view = ArrayV::new(seq_array(100), 30, 10); + let value = Value::ArrayView(Arc::new(view)); + + assert_eq!(value.len(), 10, "the window, not the backing array"); + } + + /// The two view variants agree, so an operator sizing its work from + /// `len` behaves the same whichever shape it was handed. + #[cfg(feature = "views")] + #[test] + fn len_agrees_across_the_view_variants() { + let array = Value::ArrayView(Arc::new(ArrayV::new(seq_array(100), 30, 10))); + + let mut table = Table::new("t".to_string(), None); + table.add_col(FieldArray::new( + Field::new("v", ArrowType::Int64, false, None), + seq_array(100), + )); + let table = Value::Table(Arc::new(table)).slice(30, 10); + + assert_eq!(array.len(), table.len()); + } + + /// `len` bounds a `slice` on the same value. The two are read together + /// wherever work is split into row ranges, so a `len` drawn from the + /// backing array would run a window past its own end. + #[cfg(feature = "views")] + #[test] + fn len_bounds_a_slice_of_the_same_value() { + let value = Value::ArrayView(Arc::new(ArrayV::new(seq_array(100), 30, 10))); + + let whole = value.slice(0, value.len()); + assert_eq!(whole.len(), 10); + } +} diff --git a/src/kernels/arithmetic/mod.rs b/src/kernels/arithmetic/mod.rs index 62c3116..5a362fe 100644 --- a/src/kernels/arithmetic/mod.rs +++ b/src/kernels/arithmetic/mod.rs @@ -290,6 +290,81 @@ mod tests { apply_int_u64 ); + // At the slice-kernel level, integer division rounds towards negative + // infinity. Rust's `/` truncates towards zero, so the two differ on + // every negative inexact quotient: -7 / 2 floors to -4 where truncation + // gives -3. Divide and FloorDiv therefore agree on integer slices. + // Remainder keeps the dividend's sign per Rust's `%`. + + #[test] + fn int_divide_floors_towards_negative_infinity_dense() { + let lhs = vec64![-7i64, 7, -7, 8, -8]; + let rhs = vec64![2i64, 2, -2, 2, 2]; + + let out = apply_int_i64(&lhs, &rhs, ArithmeticOperator::Divide, None).unwrap(); + assert_int(&out, &[-4, 3, 3, 4, -4], None); + + let floor = apply_int_i64(&lhs, &rhs, ArithmeticOperator::FloorDiv, None).unwrap(); + assert_eq!( + out.data.as_slice(), + floor.data.as_slice(), + "Divide and FloorDiv agree on integers" + ); + } + + #[test] + fn int_divide_floors_towards_negative_infinity_masked() { + let lhs = vec64![-7i64, 8, 9]; + let rhs = vec64![2i64, 0, 3]; + let mask = bitmask(&[true, true, true]); + + let out = apply_int_i64(&lhs, &rhs, ArithmeticOperator::Divide, Some(&mask)).unwrap(); + assert_int(&out, &[-4, 0, 3], Some(&[true, false, true])); + } + + #[test] + fn int_divide_unsigned_never_rounds() { + // Unsigned quotients never round below zero, so the floor rule + // leaves them identical to truncation. + let lhs = vec64![7u64, 9]; + let rhs = vec64![2u64, 4]; + + let out = apply_int_u64(&lhs, &rhs, ArithmeticOperator::Divide, None).unwrap(); + assert_int(&out, &[3, 2], None); + } + + #[test] + fn int_remainder_keeps_dividend_sign() { + let lhs = vec64![-7i64, 7, -7]; + let rhs = vec64![2i64, -2, -2]; + + let out = apply_int_i64(&lhs, &rhs, ArithmeticOperator::Remainder, None).unwrap(); + assert_int(&out, &[-1, 1, -1], None); + } + + #[cfg(feature = "scalar_type")] + #[test] + fn scalar_divide_is_true_division() { + use crate::Scalar; + use crate::kernels::routing::arithmetic::scalar_arithmetic; + + let out = scalar_arithmetic( + Scalar::Int64(-7), + Scalar::Int64(2), + ArithmeticOperator::Divide, + ) + .unwrap(); + assert_eq!(out, Scalar::Float64(-3.5)); + + let out = scalar_arithmetic( + Scalar::UInt32(7), + Scalar::UInt32(2), + ArithmeticOperator::Divide, + ) + .unwrap(); + assert_eq!(out, Scalar::Float64(3.5)); + } + macro_rules! float_kernel_suite { ($test_fn:ident, $ty:ty, $apply_fn:ident, $eps:expr) => { #[test] diff --git a/src/kernels/arithmetic/simd.rs b/src/kernels/arithmetic/simd.rs index 81983c2..a2152f8 100644 --- a/src/kernels/arithmetic/simd.rs +++ b/src/kernels/arithmetic/simd.rs @@ -44,12 +44,14 @@ use crate::Bitmask; use num_traits::{One, PrimInt, ToPrimitive, WrappingAdd, WrappingMul, WrappingSub, Zero}; use crate::enums::operators::ArithmeticOperator; +use crate::kernels::arithmetic::std::wrapping_pow_u32; use crate::kernels::bitmask::simd::all_true_mask_simd; use crate::utils::{simd_mask, write_simd_mask_bits}; /// SIMD integer arithmetic kernel for dense arrays (no nulls). /// Vectorised operations with scalar fallback for power operations and array tails. -/// Panics on division/remainder by zero (consistent with scalar behaviour). +/// Panics on division/remainder by zero, or on a `Power` exponent outside `u32` +/// range (consistent with scalar behaviour). #[inline(always)] pub fn int_dense_body_simd( op: ArithmeticOperator, @@ -57,7 +59,15 @@ pub fn int_dense_body_simd( rhs: &[T], out: &mut [T], ) where - T: Copy + One + PrimInt + ToPrimitive + Zero + SimdElement + WrappingMul, + T: Copy + + One + + PrimInt + + ToPrimitive + + Zero + + SimdElement + + WrappingAdd + + WrappingMul + + WrappingSub, Simd: Add> + Sub> + Mul> @@ -65,8 +75,12 @@ pub fn int_dense_body_simd( + Rem>, { let n = lhs.len(); + // The scalar tail takes its own closure because SIMD integer lanes wrap + // by definition while the raw scalar operators panic on overflow in a + // debug build, so the tail wraps through the same wrapping calls as the + // scalar kernels to keep one overflow policy across the whole input. macro_rules! run { - ($vec_op:tt) => {{ + ($vec_op:tt, $scalar:expr) => {{ let vectorisable = n / LANES * LANES; let mut i = 0; while i < vectorisable { @@ -77,31 +91,32 @@ pub fn int_dense_body_simd( } // Scalar tail for idx in vectorisable..n { - out[idx] = lhs[idx] $vec_op rhs[idx]; + out[idx] = $scalar(lhs[idx], rhs[idx]); } }}; } match op { - ArithmeticOperator::Add => run!(+), - ArithmeticOperator::Subtract => run!(-), - ArithmeticOperator::Multiply => run!(*), - ArithmeticOperator::Divide => run!(/), // Panics if divisor is zero - ArithmeticOperator::Remainder => run!(%), // Panics if divisor is zero - // Power and floor division run per element on the whole input. + ArithmeticOperator::Add => run!(+, |x: T, y: T| x.wrapping_add(&y)), + ArithmeticOperator::Subtract => run!(-, |x: T, y: T| x.wrapping_sub(&y)), + ArithmeticOperator::Multiply => run!(*, |x: T, y: T| x.wrapping_mul(&y)), + ArithmeticOperator::Remainder => run!(%, |x: T, y: T| x % y), // Panics if divisor is zero + // Power and division run per element on the whole input. There is no + // hardware SIMD integer divide, so the per-element loop costs nothing + // over the lane form. Integer division rounds towards negative + // infinity, so Divide and FloorDiv share one arm. Panics if the + // divisor is zero. ArithmeticOperator::Power => { for idx in 0..n { - let mut acc = T::one(); - let exp = rhs[idx].to_u32().unwrap_or(0); - for _ in 0..exp { - acc = acc.wrapping_mul(&lhs[idx]); - } - out[idx] = acc; + out[idx] = match rhs[idx].to_u32() { + Some(e) => wrapping_pow_u32(lhs[idx], e), + None => panic!("Power exponent out of u32 range"), + }; } } - ArithmeticOperator::FloorDiv => { + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => { for idx in 0..n { out[idx] = if rhs[idx] == T::zero() { - panic!("Floor division by zero") + panic!("Division by zero") } else { let d = lhs[idx] / rhs[idx]; let r = lhs[idx] % rhs[idx]; @@ -117,7 +132,8 @@ pub fn int_dense_body_simd( } /// SIMD integer arithmetic kernel with null mask support. -/// Division/remainder by zero produces null results (mask=false) rather than panicking. +/// Division/remainder by zero, and a `Power` exponent outside `u32` range, +/// produce null results (mask=false) rather than panicking. #[inline(always)] pub fn int_masked_body_simd( op: ArithmeticOperator, @@ -192,27 +208,34 @@ pub fn int_masked_body_simd( ArithmeticOperator::Add => run!(+), ArithmeticOperator::Subtract => run!(-), ArithmeticOperator::Multiply => run!(*), - ArithmeticOperator::Divide => run_div!(/), ArithmeticOperator::Remainder => run_div!(%), + // An exponent outside u32 range nulls its lane. ArithmeticOperator::Power => { let mut i = 0; while i < vectorisable { let a = Simd::::from_slice(&lhs[i..i + LANES]); let b = Simd::::from_slice(&rhs[i..i + LANES]); let mut tmp = [T::zero(); LANES]; + let mut exp_ok = [false; LANES]; for l in 0..LANES { - tmp[l] = a[l].pow(b[l].to_u32().unwrap_or(0)); + if let Some(e) = b[l].to_u32() { + tmp[l] = wrapping_pow_u32(a[l], e); + exp_ok[l] = true; + } } Simd::::from_array(tmp).copy_to_slice(&mut out[i..i + LANES]); write_simd_mask_bits( out_mask, i, - Mask::<::Mask, LANES>::splat(true), + Mask::<::Mask, LANES>::from_array(exp_ok), ); i += LANES; } } - ArithmeticOperator::FloorDiv => { + // Integer `/` truncates toward zero, so this arm corrects the + // result to floor towards negative infinity, giving Divide and + // FloorDiv the same result. A zero divisor nulls its lane. + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => { let mut i = 0; while i < vectorisable { let a = Simd::::from_slice(&lhs[i..i + LANES]); @@ -259,29 +282,35 @@ pub fn int_masked_body_simd( } } ArithmeticOperator::Power => { - out[idx] = lhs[idx].pow(rhs[idx].to_u32().unwrap_or(0)); - unsafe { - out_mask.set_unchecked(idx, true); + if let Some(e) = rhs[idx].to_u32() { + out[idx] = wrapping_pow_u32(lhs[idx], e); + unsafe { + out_mask.set_unchecked(idx, true); + } + } else { + out[idx] = T::zero(); + unsafe { + out_mask.set_unchecked(idx, false); + } } } - ArithmeticOperator::Divide | ArithmeticOperator::Remainder => { + ArithmeticOperator::Remainder => { if rhs[idx] == T::zero() { out[idx] = T::zero(); unsafe { out_mask.set_unchecked(idx, false); } } else { - out[idx] = match op { - ArithmeticOperator::Divide => lhs[idx] / rhs[idx], - ArithmeticOperator::Remainder => lhs[idx] % rhs[idx], - _ => unreachable!(), - }; + out[idx] = lhs[idx] % rhs[idx]; unsafe { out_mask.set_unchecked(idx, true); } } } - ArithmeticOperator::FloorDiv => { + // Integer `/` truncates toward zero, so this arm corrects + // the result to floor towards negative infinity, giving + // Divide and FloorDiv the same result. + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => { if rhs[idx] == T::zero() { out[idx] = T::zero(); unsafe { @@ -339,8 +368,8 @@ pub fn int_masked_body_simd( ArithmeticOperator::Add => run!(+), ArithmeticOperator::Subtract => run!(-), ArithmeticOperator::Multiply => run!(*), - ArithmeticOperator::Divide => run_div!(/), ArithmeticOperator::Remainder => run_div!(%), + // An exponent outside u32 range nulls its lane. ArithmeticOperator::Power => { while i + LANES <= n { let a = Simd::::from_slice(&lhs[i..i + LANES]); @@ -348,17 +377,25 @@ pub fn int_masked_body_simd( let m_src: Mask<::Mask, LANES> = simd_mask(mask, i, n); // scalar per-lane power let mut tmp = [T::zero(); LANES]; + let mut exp_ok = [false; LANES]; for l in 0..LANES { - tmp[l] = a[l].pow(b[l].to_u32().unwrap_or(0)); + if let Some(e) = b[l].to_u32() { + tmp[l] = wrapping_pow_u32(a[l], e); + exp_ok[l] = true; + } } + let exp_ok = Mask::<::Mask, LANES>::from_array(exp_ok); let selected = m_src.select(Simd::::from_array(tmp), Simd::splat(T::zero())); selected.copy_to_slice(&mut out[i..i + LANES]); - write_simd_mask_bits(out_mask, i, m_src); + write_simd_mask_bits(out_mask, i, m_src & exp_ok); i += LANES; } } - ArithmeticOperator::FloorDiv => { + // Integer `/` truncates toward zero, so this arm corrects the + // result to floor towards negative infinity, giving Divide and + // FloorDiv the same result. A zero divisor nulls its lane. + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => { while i + LANES <= n { let a = Simd::::from_slice(&lhs[i..i + LANES]); let b = Simd::::from_slice(&rhs[i..i + LANES]); @@ -394,13 +431,6 @@ pub fn int_masked_body_simd( ArithmeticOperator::Add => (lhs[j].wrapping_add(&rhs[j]), true), ArithmeticOperator::Subtract => (lhs[j].wrapping_sub(&rhs[j]), true), ArithmeticOperator::Multiply => (lhs[j].wrapping_mul(&rhs[j]), true), - ArithmeticOperator::Divide => { - if rhs[j] == T::zero() { - (T::zero(), false) // division by zero -> invalid - } else { - (lhs[j] / rhs[j], true) - } - } ArithmeticOperator::Remainder => { if rhs[j] == T::zero() { (T::zero(), false) // remainder by zero -> invalid @@ -408,10 +438,16 @@ pub fn int_masked_body_simd( (lhs[j] % rhs[j], true) } } - ArithmeticOperator::Power => (lhs[j].pow(rhs[j].to_u32().unwrap_or(0)), true), - ArithmeticOperator::FloorDiv => { + ArithmeticOperator::Power => match rhs[j].to_u32() { + Some(e) => (wrapping_pow_u32(lhs[j], e), true), + None => (T::zero(), false), // exponent out of u32 range -> null + }, + // Integer `/` truncates toward zero, so this arm corrects + // the result to floor towards negative infinity, giving + // Divide and FloorDiv the same result. + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => { if rhs[j] == T::zero() { - (T::zero(), false) + (T::zero(), false) // division by zero -> invalid } else { let d = lhs[j] / rhs[j]; let r = lhs[j] % rhs[j]; diff --git a/src/kernels/arithmetic/std.rs b/src/kernels/arithmetic/std.rs index 287ee9d..09b0e7c 100644 --- a/src/kernels/arithmetic/std.rs +++ b/src/kernels/arithmetic/std.rs @@ -30,15 +30,38 @@ //! branch-free loop per operation //! - Intentionally avoids parallelisation to allow higher-level chunking strategies //! - Wrapping arithmetic for integers to prevent overflow panics +//! - `Divide` and `FloorDiv` share one arm: integer `/` truncates toward zero, +//! so the result is corrected to floor towards negative infinity //! - Division by zero handling: panics for integers, produces Inf/NaN for floats use crate::Bitmask; use crate::enums::operators::ArithmeticOperator; use num_traits::{Float, PrimInt, ToPrimitive, WrappingAdd, WrappingMul, WrappingSub}; +/// Wrapping integer exponentiation by squaring. +/// +/// Overflow wraps modulo the lane width, matching the wrapping +/// add/subtract/multiply arms, so a large power never panics. Wrapping +/// multiplication is associative modulo `2^n`, so squaring produces the same +/// result as repeated multiplication. +#[inline(always)] +pub fn wrapping_pow_u32(base: T, exp: u32) -> T { + let mut acc = T::one(); + let mut base = base; + let mut exp = exp; + while exp > 0 { + if exp & 1 == 1 { + acc = acc.wrapping_mul(&base); + } + base = base.wrapping_mul(&base); + exp >>= 1; + } + acc +} + /// Scalar integer arithmetic kernel for dense arrays (no nulls). /// Performs element-wise operations using wrapping arithmetic to prevent overflow panics. -/// Panics on division/remainder by zero. +/// Panics on division/remainder by zero, or on a `Power` exponent outside `u32` range. #[inline(always)] pub fn int_dense_body_std( op: ArithmeticOperator, @@ -60,24 +83,12 @@ pub fn int_dense_body_std run!(|x, y| x.wrapping_add(&y)), ArithmeticOperator::Subtract => run!(|x, y| x.wrapping_sub(&y)), ArithmeticOperator::Multiply => run!(|x, y| x.wrapping_mul(&y)), - ArithmeticOperator::Divide => run!(|x, y| { + // Integer `/` truncates toward zero, so this arm corrects the result + // to floor towards negative infinity, giving Divide and FloorDiv the + // same result and letting them share one arm. + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => run!(|x, y| { if y == T::zero() { panic!("Division by zero") - } else { - x / y - } - }), - ArithmeticOperator::Remainder => run!(|x, y| { - if y == T::zero() { - panic!("Remainder by zero") - } else { - x % y - } - }), - ArithmeticOperator::Power => run!(|x, y| x.pow(y.to_u32().unwrap_or(0))), - ArithmeticOperator::FloorDiv => run!(|x, y| { - if y == T::zero() { - panic!("Floor division by zero") } else { let d = x / y; let r = x % y; @@ -89,12 +100,24 @@ pub fn int_dense_body_std run!(|x, y| { + if y == T::zero() { + panic!("Remainder by zero") + } else { + x % y + } + }), + ArithmeticOperator::Power => run!(|x, y| match y.to_u32() { + Some(e) => wrapping_pow_u32(x, e), + None => panic!("Power exponent out of u32 range"), + }), } } /// Scalar integer arithmetic kernel with null mask support. /// Handles division by zero gracefully by marking results as null instead of panicking. -/// Invalid inputs (mask=false) and zero division produce null outputs. +/// A cleared input bit, a zero divisor, and a `Power` exponent outside `u32` +/// range all produce a null output rather than panicking. #[inline(always)] pub fn int_masked_body_std( op: ArithmeticOperator, @@ -130,11 +153,20 @@ pub fn int_masked_body_std run!(|x, y| (x.wrapping_add(&y), true)), ArithmeticOperator::Subtract => run!(|x, y| (x.wrapping_sub(&y), true)), ArithmeticOperator::Multiply => run!(|x, y| (x.wrapping_mul(&y), true)), - ArithmeticOperator::Divide => run!(|x, y| { + // Integer `/` truncates toward zero, so this arm corrects the result + // to floor towards negative infinity, giving Divide and FloorDiv the + // same result and letting them share one arm. + ArithmeticOperator::Divide | ArithmeticOperator::FloorDiv => run!(|x, y| { if y == T::zero() { (T::zero(), false) // division by zero -> invalid } else { - (x / y, true) + let d = x / y; + let r = x % y; + if r != T::zero() && (x ^ y) < T::zero() { + (d - T::one(), true) + } else { + (d, true) + } } }), ArithmeticOperator::Remainder => run!(|x, y| { @@ -144,19 +176,9 @@ pub fn int_masked_body_std run!(|x, y| (x.pow(y.to_u32().unwrap_or(0)), true)), - ArithmeticOperator::FloorDiv => run!(|x, y| { - if y == T::zero() { - (T::zero(), false) - } else { - let d = x / y; - let r = x % y; - if r != T::zero() && (x ^ y) < T::zero() { - (d - T::one(), true) - } else { - (d, true) - } - } + ArithmeticOperator::Power => run!(|x, y| match y.to_u32() { + Some(e) => (wrapping_pow_u32(x, e), true), + None => (T::zero(), false), // exponent out of u32 range -> null }), } } diff --git a/src/kernels/routing/arithmetic.rs b/src/kernels/routing/arithmetic.rs index b80aaac..68662e9 100644 --- a/src/kernels/routing/arithmetic.rs +++ b/src/kernels/routing/arithmetic.rs @@ -44,13 +44,15 @@ pub fn scalar_arithmetic( (Scalar::Int32(l), Scalar::Int32(r), Add) => Scalar::Int32(l + r), (Scalar::Int32(l), Scalar::Int32(r), Subtract) => Scalar::Int32(l - r), (Scalar::Int32(l), Scalar::Int32(r), Multiply) => Scalar::Int32(l * r), - (Scalar::Int32(l), Scalar::Int32(r), Divide) => Scalar::Int32(l / r), + // Integer scalar division is true division and returns a Float64 + // scalar, so 7 / 2 is 3.5 and division by zero follows IEEE 754. + (Scalar::Int32(l), Scalar::Int32(r), Divide) => Scalar::Float64(l as f64 / r as f64), // Int64 operations (Scalar::Int64(l), Scalar::Int64(r), Add) => Scalar::Int64(l + r), (Scalar::Int64(l), Scalar::Int64(r), Subtract) => Scalar::Int64(l - r), (Scalar::Int64(l), Scalar::Int64(r), Multiply) => Scalar::Int64(l * r), - (Scalar::Int64(l), Scalar::Int64(r), Divide) => Scalar::Int64(l / r), + (Scalar::Int64(l), Scalar::Int64(r), Divide) => Scalar::Float64(l as f64 / r as f64), // Float32 operations (Scalar::Float32(l), Scalar::Float32(r), Add) => Scalar::Float32(l + r), @@ -86,7 +88,7 @@ pub fn scalar_arithmetic( #[cfg(feature = "extended_numeric_types")] (Scalar::Int8(l), Scalar::Int8(r), Multiply) => Scalar::Int8(l * r), #[cfg(feature = "extended_numeric_types")] - (Scalar::Int8(l), Scalar::Int8(r), Divide) => Scalar::Int8(l / r), + (Scalar::Int8(l), Scalar::Int8(r), Divide) => Scalar::Float64(l as f64 / r as f64), // Int16 #[cfg(feature = "extended_numeric_types")] @@ -96,7 +98,7 @@ pub fn scalar_arithmetic( #[cfg(feature = "extended_numeric_types")] (Scalar::Int16(l), Scalar::Int16(r), Multiply) => Scalar::Int16(l * r), #[cfg(feature = "extended_numeric_types")] - (Scalar::Int16(l), Scalar::Int16(r), Divide) => Scalar::Int16(l / r), + (Scalar::Int16(l), Scalar::Int16(r), Divide) => Scalar::Float64(l as f64 / r as f64), // UInt8 #[cfg(feature = "extended_numeric_types")] @@ -106,7 +108,7 @@ pub fn scalar_arithmetic( #[cfg(feature = "extended_numeric_types")] (Scalar::UInt8(l), Scalar::UInt8(r), Multiply) => Scalar::UInt8(l * r), #[cfg(feature = "extended_numeric_types")] - (Scalar::UInt8(l), Scalar::UInt8(r), Divide) => Scalar::UInt8(l / r), + (Scalar::UInt8(l), Scalar::UInt8(r), Divide) => Scalar::Float64(l as f64 / r as f64), // UInt16 #[cfg(feature = "extended_numeric_types")] @@ -116,19 +118,19 @@ pub fn scalar_arithmetic( #[cfg(feature = "extended_numeric_types")] (Scalar::UInt16(l), Scalar::UInt16(r), Multiply) => Scalar::UInt16(l * r), #[cfg(feature = "extended_numeric_types")] - (Scalar::UInt16(l), Scalar::UInt16(r), Divide) => Scalar::UInt16(l / r), + (Scalar::UInt16(l), Scalar::UInt16(r), Divide) => Scalar::Float64(l as f64 / r as f64), // UInt32 (Scalar::UInt32(l), Scalar::UInt32(r), Add) => Scalar::UInt32(l + r), (Scalar::UInt32(l), Scalar::UInt32(r), Subtract) => Scalar::UInt32(l - r), (Scalar::UInt32(l), Scalar::UInt32(r), Multiply) => Scalar::UInt32(l * r), - (Scalar::UInt32(l), Scalar::UInt32(r), Divide) => Scalar::UInt32(l / r), + (Scalar::UInt32(l), Scalar::UInt32(r), Divide) => Scalar::Float64(l as f64 / r as f64), // UInt64 (Scalar::UInt64(l), Scalar::UInt64(r), Add) => Scalar::UInt64(l + r), (Scalar::UInt64(l), Scalar::UInt64(r), Subtract) => Scalar::UInt64(l - r), (Scalar::UInt64(l), Scalar::UInt64(r), Multiply) => Scalar::UInt64(l * r), - (Scalar::UInt64(l), Scalar::UInt64(r), Divide) => Scalar::UInt64(l / r), + (Scalar::UInt64(l), Scalar::UInt64(r), Divide) => Scalar::Float64(l as f64 / r as f64), // String concatenation (Scalar::String32(l), Scalar::String32(r), Add) => Scalar::String32(format!("{}{}", l, r)), #[cfg(feature = "large_string")] diff --git a/src/structs/views/bitmask_view.rs b/src/structs/views/bitmask_view.rs index a1086a7..5403e93 100644 --- a/src/structs/views/bitmask_view.rs +++ b/src/structs/views/bitmask_view.rs @@ -82,7 +82,7 @@ use crate::{Array, Bitmask, BitmaskVT, BooleanArray}; /// assert!(view.get(1)); /// assert!(view.get(2)); /// ``` -#[derive(Clone, PartialEq)] +#[derive(Clone, Copy, PartialEq)] pub struct BitmaskV<'a> { /// The **outer bitmask** that this view is derived from - we retain a reference to it. /// Importantly, this is the ***full bitmask*** - not the *view*, and thus should not be @@ -133,6 +133,21 @@ impl<'a> BitmaskV<'a> { self.bitmask.get(self.offset + i) } + /// Returns the value at logical index `i` within the view, skipping the + /// bounds check. + /// + /// Element-wise kernels read a flagged null_mask (valid) bit per row, so the checked + /// [`get`](Self::get) costs a comparison in the innermost loop. This is + /// the unchecked counterpart for loops that have already established + /// `i < len`. + /// + /// # Safety + /// `i` must be less than the view's length. + #[inline] + pub unsafe fn get_unchecked(&self, i: usize) -> bool { + unsafe { self.bitmask.get_unchecked(self.offset + i) } + } + /// Returns a slice of the bitmask’s bytes /// Due to the booleans being bitpacked in a u8, /// the slice retains:\ diff --git a/src/structs/views/collections/numeric_array_view.rs b/src/structs/views/collections/numeric_array_view.rs index 4beea63..8032f58 100644 --- a/src/structs/views/collections/numeric_array_view.rs +++ b/src/structs/views/collections/numeric_array_view.rs @@ -52,7 +52,7 @@ use crate::structs::views::bitmask_view::BitmaskV; use crate::traits::concatenate::Concatenate; use crate::traits::print::MAX_PREVIEW; use crate::traits::shape::Shape; -use crate::{Array, ArrayV, Bitmask, FieldArray, MaskedArray, NumericArray}; +use crate::{Array, ArrayV, FieldArray, MaskedArray, NumericArray}; /// # NumericArrayView /// @@ -301,23 +301,24 @@ impl NumericArrayV { /// that still reference the original will cast independently when they /// reach this call, so it generally is best avoided in such contexts as it would /// clone for every independent window view. - pub fn guarantee_f64(&mut self) -> (&[f64], Option<&Bitmask>, Option) { + pub fn guarantee_f64(&mut self) -> (&[f64], Option>, Option) { if !matches!(&self.array, NumericArray::Float64(_)) { // Take the old array out, leaving Null as placeholder let old = std::mem::take(&mut self.array); self.array = old.cow_into_f64(); } - // Safe: the branch above guarantees Float64 at this point - let NumericArray::Float64(arr) = &self.array else { - unreachable!() - }; - let slice = &arr.data.as_slice()[self.offset..self.offset + self.len]; - let mask = arr.null_mask.as_ref(); - let nc = if mask.is_some() { + let nc = if self.array.null_mask().is_some() { Some(self.null_count()) } else { None }; + let (offset, len) = (self.offset, self.len); + // Safe: the branch above guarantees Float64 at this point + let NumericArray::Float64(arr) = &self.array else { + unreachable!() + }; + let slice = &arr.data.as_slice()[offset..offset + len]; + let mask = arr.null_mask.as_ref().map(|m| m.view(offset, len)); (slice, mask, nc) } } @@ -470,7 +471,52 @@ mod tests { use std::sync::Arc; use super::*; - use crate::{Array, Bitmask, IntegerArray, NumericArray, vec64}; + use crate::{Array, Bitmask, FloatArray, IntegerArray, NumericArray, vec64}; + + #[test] + fn guarantee_f64_windows_the_null_mask_with_the_slice() { + let mut arr = FloatArray::::default(); + for v in [10.0, 20.0, 30.0, 40.0, 50.0] { + arr.push(v); + } + let mut mask = Bitmask::new_set_all(5, true); + mask.set(0, false); + mask.set(1, false); + arr.null_mask = Some(mask); + + let mut view = NumericArrayV::new(NumericArray::Float64(Arc::new(arr)), 2, 3); + let (slice, mask, nc) = view.guarantee_f64(); + + assert_eq!(slice, &[30.0, 40.0, 50.0]); + let mask = mask.expect("the backing array carries a null mask"); + assert_eq!(mask.len(), 3); + // Reading the parent's bits from zero would report the first two + // window rows as null. + assert!((0..3).all(|i| mask.get(i)), "every window row is valid"); + assert_eq!(nc, Some(0)); + } + + #[test] + fn guarantee_f64_windows_the_null_mask_after_casting() { + let mut arr = IntegerArray::::default(); + for v in [1, 2, 3, 4, 5] { + arr.push(v); + } + let mut mask = Bitmask::new_set_all(5, true); + mask.set(1, false); + mask.set(3, false); + arr.null_mask = Some(mask); + + let mut view = NumericArrayV::new(NumericArray::Int32(Arc::new(arr)), 2, 3); + let (slice, mask, _) = view.guarantee_f64(); + + assert_eq!(slice, &[3.0, 4.0, 5.0]); + let mask = mask.expect("the backing array carries a null mask"); + // Window row 1 is parent row 3, the null one. + assert!(mask.get(0)); + assert!(!mask.get(1)); + assert!(mask.get(2)); + } #[test] fn test_numeric_array_view_basic_indexing_and_slice() {