vortex-data / vortex / 16758385233

88%

Build Type

push

github

Committed by web-flow

Commit Message

fix: error-prone f16 Scalar serde (#4113)

⏺ Summary of Changes

The diff shows comprehensive changes to fix scalar casting bugs and
ensure backwards compatibility for serialized scalar values. Here are
the
  key changes:

1. Core Bug Fix: Floating-Point Conversion (vortex-scalar/src/pvalue.rs)

- Fixed critical bug where u64 to f64 conversion was using bit
reinterpretation (f64::from_bits()) instead of numeric conversion
  - Changed to use NumCast::from() for proper numeric conversions

  2. Type Coercion System (vortex-scalar/src/lib.rs)

- Added coerce_value() method in Scalar::new() to handle backwards
compatibility
- Specifically coerces f16 values stored as u64 (for backwards
compatibility with serialized data)
- Includes recursive coercion for structs and lists containing f16
fields
- Protected against overflow (only coerces u64 to f16 if value ≤
u16::MAX)

  3. Protobuf Support for f16 (vortex-proto/proto/scalar.proto)

  - Added dedicated f16_value field to protobuf schema (field 10)
- Updated serialization to use this dedicated field instead of storing
f16 as u64

  4. Consistent Scalar Construction

- Updated all direct Scalar { dtype, value } constructions to use
Scalar::new()
  - Ensures coercion logic is always applied
  - Changed in:
    - bool.rs: into_scalar(), bool(), From<bool>
    - primitive.rs: primitive_value(), From<$T> for Scalar
    - struct_.rs: field_by_idx(), cast(), struct_()
    - list.rs: element(), list(), list_empty()

  5. Test Coverage (vortex-scalar/src/lib.rs and proto.rs)

  - Fixed test_canonicalize_scalar_values to create f16 values properly
  - Added comprehensive coercion tests:
    - f16 coercion from u64
    - No coercion for other type combinations
    - Struct field coercion
    - List element coercion
    - Overflow protection
  - Added ScalarValue protobuf roundtrip tests:
    - Direct f16 roundtripping
    - Value preservation for all primitive types
- Handling of type consolidation (u8/u16/u32 → u64, ... (continued)

Run Details

378 of 419 new or added lines in 8 files covered. (90.21%)

1 existing line in 1 file now uncovered.

47052 of 56319 relevant lines covered (83.55%)

460105.08 hits per line