diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index c998fad..01060a6 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -991,21 +991,24 @@ enum Scalar { } impl Scalar { - /// Float to integer conversions follow libhdf5's hard conversions: - /// truncate toward zero, and saturate a value outside the target range to - /// its minimum or maximum. NaN converts to 0 (libhdf5 leaves that case to - /// the C cast, whose result is platform-dependent). + // Every conversion follows libhdf5's default (hard) conversions: a value + // outside the target type's range saturates to its minimum or maximum — + // including a negative value read as unsigned, which reads as 0 — rather + // than being truncated to its low bits. Floats truncate toward zero; NaN + // converts to 0 (libhdf5 leaves that case to the C cast, whose result is + // platform-dependent). + fn to_i64(self) -> i64 { match self { Scalar::Signed(v) => v, - Scalar::Unsigned(v) => v as i64, + Scalar::Unsigned(v) => i64::try_from(v).unwrap_or(i64::MAX), Scalar::Float(v) => v as i64, } } fn to_u64(self) -> u64 { match self { - Scalar::Signed(v) => v as u64, + Scalar::Signed(v) => u64::try_from(v).unwrap_or(0), Scalar::Unsigned(v) => v, Scalar::Float(v) => v as u64, } @@ -1013,8 +1016,8 @@ impl Scalar { fn to_i32(self) -> i32 { match self { - Scalar::Signed(v) => v as i32, - Scalar::Unsigned(v) => v as i32, + Scalar::Signed(v) => v.clamp(i32::MIN.into(), i32::MAX.into()) as i32, + Scalar::Unsigned(v) => i32::try_from(v).unwrap_or(i32::MAX), Scalar::Float(v) => v as i32, } } @@ -1048,8 +1051,10 @@ fn decode_scalar( /// Convert raw bytes to `i64` values. /// -/// Floating-point data is converted the way libhdf5 converts it: truncated -/// toward zero, saturating at the target type's range, with NaN read as 0. +/// Values are converted the way libhdf5 converts them: integers outside the +/// target range saturate at its minimum or maximum (a negative value read as +/// unsigned is 0), and floating-point data is truncated toward zero and +/// saturated, with NaN read as 0. pub fn read_as_i64(raw: &[u8], datatype: &Datatype) -> Result, FormatError> { if let Datatype::Array { base_type, .. } = datatype { return read_as_i64(raw, base_type); @@ -1091,8 +1096,10 @@ pub fn read_as_i64(raw: &[u8], datatype: &Datatype) -> Result, FormatEr /// Convert raw bytes to `u64` values. /// -/// Floating-point data is converted the way libhdf5 converts it: truncated -/// toward zero, saturating at the target type's range, with NaN read as 0. +/// Values are converted the way libhdf5 converts them: integers outside the +/// target range saturate at its minimum or maximum (a negative value read as +/// unsigned is 0), and floating-point data is truncated toward zero and +/// saturated, with NaN read as 0. pub fn read_as_u64(raw: &[u8], datatype: &Datatype) -> Result, FormatError> { if let Datatype::Array { base_type, .. } = datatype { return read_as_u64(raw, base_type); @@ -1207,8 +1214,10 @@ pub fn read_as_f32(raw: &[u8], datatype: &Datatype) -> Result, FormatEr /// Convert raw bytes to `i32` values. /// -/// Floating-point data is converted the way libhdf5 converts it: truncated -/// toward zero, saturating at the target type's range, with NaN read as 0. +/// Values are converted the way libhdf5 converts them: integers outside the +/// target range saturate at its minimum or maximum (a negative value read as +/// unsigned is 0), and floating-point data is truncated toward zero and +/// saturated, with NaN read as 0. pub fn read_as_i32(raw: &[u8], datatype: &Datatype) -> Result, FormatError> { if let Datatype::Array { base_type, .. } = datatype { return read_as_i32(raw, base_type); @@ -1231,6 +1240,7 @@ pub fn read_as_i32(raw: &[u8], datatype: &Datatype) -> Result, FormatEr datatype, Datatype::FixedPoint { byte_order: DatatypeByteOrder::LittleEndian, + signed: true, .. } ) diff --git a/crates/clawhdf5/tests/numeric_conversion_interop.rs b/crates/clawhdf5/tests/numeric_conversion_interop.rs index 5c0fc3c..e75700a 100644 --- a/crates/clawhdf5/tests/numeric_conversion_interop.rs +++ b/crates/clawhdf5/tests/numeric_conversion_interop.rs @@ -127,3 +127,64 @@ with h5py.File("{path}", "r") as f: let f2 = file.dataset("f2").unwrap(); assert_eq!(f2.read_u64().unwrap(), vec![1, 0, 0, 0, 7, 65504, 0]); } + +#[test] +fn integer_reads_saturate_out_of_range_values_like_libhdf5() { + // Narrowing reads used to keep the low bits (i64 2^40+5 read as i32 was + // 5, u64::MAX read as i64 was -1) and signed-to-unsigned reads wrapped + // (-1 read as u64 was 4294967295). + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("int_narrowing.h5"); + let script = format!( + r#"{PRELUDE} +data = {{ + "i8": np.array([2**40 + 5, -(2**35), 7, -1, 2**63 - 1, -(2**63)], "i8"), + "u8": np.array([2**64 - 1, 2**63, 5, 0], "i2"), + "u1": np.array([255, 0, 128], "u1"), +}} +with h5py.File("{path}", "w") as f: + for name, arr in data.items(): + f.create_dataset(name, data=arr) +with h5py.File("{path}", "r") as f: + for name in data: + d = f[name] + emit(name + ":i32", d.astype("(&expected[&format!("{name}:i32")]), + "{name} as i32" + ); + assert_eq!( + ds.read_i64().unwrap(), + parse::(&expected[&format!("{name}:i64")]), + "{name} as i64" + ); + // libhdf5 wraps a negative big-endian i64 read as little-endian u64 + // (it only byte-swaps when the sizes match and the order differs); + // every other signed-to-unsigned read saturates at 0, so do that. + if name != "i8be" { + assert_eq!( + ds.read_u64().unwrap(), + parse::(&expected[&format!("{name}:u64")]), + "{name} as u64" + ); + } + } + let i8be = file.dataset("i8be").unwrap(); + assert_eq!(i8be.read_u64().unwrap(), vec![(1 << 40) + 5, 0, 7, 0]); +}