diff --git a/vortex-array/src/scalar/typed_view/primitive/pvalue.rs b/vortex-array/src/scalar/typed_view/primitive/pvalue.rs index 02640d2e416..7fa287b6928 100644 --- a/vortex-array/src/scalar/typed_view/primitive/pvalue.rs +++ b/vortex-array/src/scalar/typed_view/primitive/pvalue.rs @@ -8,7 +8,6 @@ use std::cmp::Ordering; use std::hash::Hash; use std::hash::Hasher; -use num_traits::NumCast; use num_traits::ToPrimitive; use num_traits::Zero; use paste::paste; @@ -16,7 +15,6 @@ use vortex_error::VortexError; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; -use vortex_error::vortex_ensure; use vortex_error::vortex_err; use crate::dtype::NativePType; @@ -485,134 +483,3 @@ impl Display for PValue { } } } - -/// Coercion trait for widening or reinterpreting a [`PValue`] into a concrete type. -pub(super) trait CoercePValue: Sized { - /// Coerce value from a compatible bit representation using into given type. - /// - /// Integers can be widened from narrower type - /// Floats stored as integers will be reinterpreted as bit representation of the float - fn coerce(value: PValue) -> VortexResult; -} - -/// Implements [`CoercePValue`] for an integer type. -macro_rules! int_coerce { - ($T:ty) => { - impl CoercePValue for $T { - #[inline] - fn coerce(value: PValue) -> VortexResult { - Self::try_from(value) - } - } - }; -} - -int_coerce!(u8); -int_coerce!(u16); -int_coerce!(u32); -int_coerce!(u64); -int_coerce!(i8); -int_coerce!(i16); -int_coerce!(i32); -int_coerce!(i64); - -impl CoercePValue for f16 { - #[expect( - clippy::cast_possible_truncation, - reason = "truncation is intentional and checked where needed" - )] - fn coerce(value: PValue) -> VortexResult { - // F16 coercion behavior: - // - U8/U16/U32/U64: Interpreted as the bit representation of an f16 value. - // Only the lower 16 bits are used, allowing compact storage of f16 values - // as integers when the full type information is preserved externally. - // - F16: Passthrough - // - F32/F64: Numeric conversion with potential precision loss - // - Other types: Not supported - // - // Note: This bit-pattern interpretation means that integer value 0x3C00u16 - // would be interpreted as f16(1.0), not as f16(15360.0). - match value { - PValue::U8(u) => Ok(Self::from_bits(u as u16)), - PValue::U16(u) => Ok(Self::from_bits(u)), - PValue::U32(u) => { - vortex_ensure!( - u <= u16::MAX as u32, - "Cannot coerce U32 value to f16: value out of range" - ); - Ok(Self::from_bits(u as u16)) - } - PValue::U64(u) => { - vortex_ensure!( - u <= u16::MAX as u64, - "Cannot coerce U64 value to f16: value out of range" - ); - Ok(Self::from_bits(u as u16)) - } - PValue::F16(u) => Ok(u), - PValue::F32(f) => { - ::from(f).ok_or_else(|| vortex_err!("Cannot convert f32 to f16")) - } - PValue::F64(f) => { - ::from(f).ok_or_else(|| vortex_err!("Cannot convert f64 to f16")) - } - PValue::I8(_) | PValue::I16(_) | PValue::I32(_) | PValue::I64(_) => { - vortex_bail!("Cannot coerce {value:?} to f16: type not supported for coercion") - } - } - } -} - -impl CoercePValue for f32 { - #[expect( - clippy::cast_possible_truncation, - reason = "truncation is intentional and checked where needed" - )] - fn coerce(value: PValue) -> VortexResult { - // F32 coercion: U32 values are interpreted as bit patterns, not numeric conversions - match value { - PValue::U8(u) => Ok(Self::from_bits(u as u32)), - PValue::U16(u) => Ok(Self::from_bits(u as u32)), - PValue::U32(u) => Ok(Self::from_bits(u)), - PValue::U64(u) => { - vortex_ensure!( - u <= u32::MAX as u64, - "Cannot coerce U64 value to f32: value out of range" - ); - Ok(Self::from_bits(u as u32)) - } - PValue::F16(f) => { - ::from(f).ok_or_else(|| vortex_err!("Cannot convert f16 to f32")) - } - PValue::F32(f) => Ok(f), - PValue::F64(f) => { - ::from(f).ok_or_else(|| vortex_err!("Cannot convert f64 to f32")) - } - PValue::I8(_) | PValue::I16(_) | PValue::I32(_) | PValue::I64(_) => { - vortex_bail!("Unsupported PValue {value:?} type for f32") - } - } - } -} - -impl CoercePValue for f64 { - fn coerce(value: PValue) -> VortexResult { - // F64 coercion: U64 values are interpreted as bit patterns, not numeric conversions - match value { - PValue::U8(u) => Ok(Self::from_bits(u as u64)), - PValue::U16(u) => Ok(Self::from_bits(u as u64)), - PValue::U32(u) => Ok(Self::from_bits(u as u64)), - PValue::U64(u) => Ok(Self::from_bits(u)), - PValue::F16(f) => { - ::from(f).ok_or_else(|| vortex_err!("Cannot convert f16 to f64")) - } - PValue::F32(f) => { - ::from(f).ok_or_else(|| vortex_err!("Cannot convert f32 to f64")) - } - PValue::F64(f) => Ok(f), - PValue::I8(_) | PValue::I16(_) | PValue::I32(_) | PValue::I64(_) => { - vortex_bail!("Unsupported PValue {value:?} type for f64") - } - } - } -} diff --git a/vortex-array/src/scalar/typed_view/primitive/scalar.rs b/vortex-array/src/scalar/typed_view/primitive/scalar.rs index e3981de8dd3..d86071e92ac 100644 --- a/vortex-array/src/scalar/typed_view/primitive/scalar.rs +++ b/vortex-array/src/scalar/typed_view/primitive/scalar.rs @@ -21,7 +21,6 @@ use vortex_error::VortexResult; use vortex_error::vortex_ensure; use vortex_error::vortex_panic; -use super::pvalue::CoercePValue; use crate::dtype::DType; use crate::dtype::FromPrimitiveOrF16; use crate::dtype::NativePType; @@ -83,15 +82,7 @@ impl<'a> PrimitiveScalar<'a> { pub fn try_new(dtype: &'a DType, value: Option<&ScalarValue>) -> VortexResult { let ptype = PType::try_from(dtype)?; - // Read the serialized value into the correct PValue. - // The serialized form may come back over the wire as e.g. any integer type. - let pvalue = match value { - None => None, - Some(v) => { - let pv = v.as_primitive(); - match_each_native_ptype!(ptype, |T| { Some(PValue::from(::coerce(*pv)?)) }) - } - }; + let pvalue = value.map(|v| v.as_primitive()).cloned(); Ok(Self { dtype, diff --git a/vortex-array/src/scalar/typed_view/primitive/tests.rs b/vortex-array/src/scalar/typed_view/primitive/tests.rs index b0c0063a3df..9eb189e6b34 100644 --- a/vortex-array/src/scalar/typed_view/primitive/tests.rs +++ b/vortex-array/src/scalar/typed_view/primitive/tests.rs @@ -8,7 +8,6 @@ use rstest::rstest; use vortex_error::VortexExpect; use vortex_utils::aliases::hash_set::HashSet; -use super::pvalue::CoercePValue; use super::*; use crate::dtype::DType; use crate::dtype::FromPrimitiveOrF16; @@ -653,80 +652,6 @@ fn test_f16_special_values() { ); } -#[test] -fn test_coerce_pvalue() { - // Test integer coercion - assert_eq!(u32::coerce(PValue::U16(42)).unwrap(), 42u32); - assert_eq!(i64::coerce(PValue::I32(-42)).unwrap(), -42i64); - - // Test float coercion from bits - assert_eq!(f32::coerce(PValue::U32(0x3f800000)).unwrap(), 1.0f32); - assert_eq!( - f64::coerce(PValue::U64(0x3ff0000000000000)).unwrap(), - 1.0f64 - ); -} - -#[test] -fn test_coerce_f16_beyond_u16_max() { - // Test U32 to f16 coercion within valid range - assert!(f16::coerce(PValue::U32(u16::MAX as u32)).is_ok()); - assert_eq!( - f16::coerce(PValue::U32(0x3C00)).unwrap(), - f16::from_bits(0x3C00) // 1.0 in f16 - ); - - // Test U32 to f16 coercion beyond u16::MAX - should fail - assert!(f16::coerce(PValue::U32((u16::MAX as u32) + 1)).is_err()); - assert!(f16::coerce(PValue::U32(u32::MAX)).is_err()); - - // Test U64 to f16 coercion within valid range - assert!(f16::coerce(PValue::U64(u16::MAX as u64)).is_ok()); - assert_eq!( - f16::coerce(PValue::U64(0x3C00)).unwrap(), - f16::from_bits(0x3C00) // 1.0 in f16 - ); - - // Test U64 to f16 coercion beyond u16::MAX - should fail - assert!(f16::coerce(PValue::U64((u16::MAX as u64) + 1)).is_err()); - assert!(f16::coerce(PValue::U64(u32::MAX as u64)).is_err()); - assert!(f16::coerce(PValue::U64(u64::MAX)).is_err()); -} - -#[test] -fn test_coerce_f32_beyond_u32_max() { - // Test U64 to f32 coercion within valid range - assert!(f32::coerce(PValue::U64(u32::MAX as u64)).is_ok()); - assert_eq!( - f32::coerce(PValue::U64(0x3f800000)).unwrap(), - 1.0f32 // 0x3f800000 is 1.0 in f32 - ); - - // Test U64 to f32 coercion beyond u32::MAX - should fail - assert!(f32::coerce(PValue::U64((u32::MAX as u64) + 1)).is_err()); - assert!(f32::coerce(PValue::U64(u64::MAX)).is_err()); - - // Test smaller types still work - assert!(f32::coerce(PValue::U8(255)).is_ok()); - assert!(f32::coerce(PValue::U16(u16::MAX)).is_ok()); - assert!(f32::coerce(PValue::U32(u32::MAX)).is_ok()); -} - -#[test] -fn test_coerce_f64_all_unsigned() { - // Test f64 can accept all unsigned integer values as bit patterns - assert!(f64::coerce(PValue::U8(u8::MAX)).is_ok()); - assert!(f64::coerce(PValue::U16(u16::MAX)).is_ok()); - assert!(f64::coerce(PValue::U32(u32::MAX)).is_ok()); - assert!(f64::coerce(PValue::U64(u64::MAX)).is_ok()); - - // Verify specific bit patterns - assert_eq!( - f64::coerce(PValue::U64(0x3ff0000000000000)).unwrap(), - 1.0f64 // 0x3ff0000000000000 is 1.0 in f64 - ); -} - #[test] fn test_f16_nans_equal() { let nan1 = f16::from_le_bytes([154, 253]);