Skip to content

Remove primitive scalar coercion logic #7224

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.

Sign up for GitHub

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

Closed
robert3005 wants to merge 2 commits into from
Closed

Remove primitive scalar coercion logic #7224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d43e528
Remove primitive scalar coercion logic
robert3005 Mar 31, 2026
6ec6d0c
delete
robert3005 Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 0 additions & 133 deletions vortex-array/src/scalar/typed_view/primitive/pvalue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ 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;
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;
Expand Down Expand Up @@ -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<Self>;
}

/// Implements [`CoercePValue`] for an integer type.
macro_rules! int_coerce {
($T:ty) => {
impl CoercePValue for $T {
#[inline]
fn coerce(value: PValue) -> VortexResult<Self> {
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<Self> {
// 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) => {
<Self as NumCast>::from(f).ok_or_else(|| vortex_err!("Cannot convert f32 to f16"))
}
PValue::F64(f) => {
<Self as NumCast>::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<Self> {
// 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) => {
<Self as NumCast>::from(f).ok_or_else(|| vortex_err!("Cannot convert f16 to f32"))
}
PValue::F32(f) => Ok(f),
PValue::F64(f) => {
<Self as NumCast>::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<Self> {
// 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) => {
<Self as NumCast>::from(f).ok_or_else(|| vortex_err!("Cannot convert f16 to f64"))
}
PValue::F32(f) => {
<Self as NumCast>::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")
}
}
}
}
11 changes: 1 addition & 10 deletions vortex-array/src/scalar/typed_view/primitive/scalar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,15 +82,7 @@ impl<'a> PrimitiveScalar<'a> {
pub fn try_new(dtype: &'a DType, value: Option<&ScalarValue>) -> VortexResult<Self> {
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(<T>::coerce(*pv)?)) })
}
};
let pvalue = value.map(|v| v.as_primitive()).cloned();

Ok(Self {
dtype,
Expand Down
75 changes: 0 additions & 75 deletions vortex-array/src/scalar/typed_view/primitive/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
Expand Down
Loading