Files open in h5py again; float16 embedding storage #4
@@ -640,7 +640,8 @@ impl Datatype {
|
|||||||
mantissa_size,
|
mantissa_size,
|
||||||
exponent_bias,
|
exponent_bias,
|
||||||
} => {
|
} => {
|
||||||
let mut bf0 = 0x20u8; // bit 5: sign location bit (standard IEEE 754)
|
// Bits 4-5: mantissa normalization = 2 (implied leading 1, IEEE 754).
|
||||||
|
let mut bf0 = 0x20u8;
|
||||||
match byte_order {
|
match byte_order {
|
||||||
DatatypeByteOrder::BigEndian => {
|
DatatypeByteOrder::BigEndian => {
|
||||||
bf0 |= 0x01;
|
bf0 |= 0x01;
|
||||||
@@ -650,9 +651,14 @@ impl Datatype {
|
|||||||
}
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
}
|
}
|
||||||
// bf[1] bits 0-1: mantissa normalization = 2 (MSB not stored, IEEE 754)
|
// Bits 8-15: the sign bit's position, the top bit of the value.
|
||||||
let bf1 = 0x3fu8; // matching what h5py generates
|
// This was hard-coded to 63, which is right only for f64: the
|
||||||
let mut buf = Self::build_header(1, 1, [bf0, bf1, 0], *size);
|
// HDF5 library rejects any other float with "sign bit position
|
||||||
|
// out of bounds", so every f32 dataset and attribute we wrote
|
||||||
|
// was unreadable by h5py and libhdf5.
|
||||||
|
let sign_location =
|
||||||
|
(u32::from(*bit_offset) + u32::from(*bit_precision)).saturating_sub(1) as u8;
|
||||||
|
let mut buf = Self::build_header(1, 1, [bf0, sign_location, 0], *size);
|
||||||
buf.extend_from_slice(&bit_offset.to_le_bytes());
|
buf.extend_from_slice(&bit_offset.to_le_bytes());
|
||||||
buf.extend_from_slice(&bit_precision.to_le_bytes());
|
buf.extend_from_slice(&bit_precision.to_le_bytes());
|
||||||
buf.push(*exponent_location);
|
buf.push(*exponent_location);
|
||||||
@@ -818,6 +824,20 @@ fn build_dt_header(class: u8, version: u8, bf: [u8; 3], size: u32) -> Vec<u8> {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn float_sign_location_is_the_top_bit_of_the_value() {
|
||||||
|
// The HDF5 library rejects a float whose sign position is not inside
|
||||||
|
// its precision; this was hard-coded to 63, so every f32 we wrote was
|
||||||
|
// unreadable by h5py. Byte 2 of the message is the sign position.
|
||||||
|
use crate::type_builders::{make_f32_type, make_f64_type};
|
||||||
|
for (dt, sign) in [(make_f32_type(), 31), (make_f64_type(), 63)] {
|
||||||
|
let bytes = dt.serialize();
|
||||||
|
assert_eq!(bytes[2], sign, "{dt:?}");
|
||||||
|
let (parsed, _) = Datatype::parse(&bytes).unwrap();
|
||||||
|
assert_eq!(parsed, dt);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Helper to build a fixed-point datatype message
|
// Helper to build a fixed-point datatype message
|
||||||
fn build_fixed_point(
|
fn build_fixed_point(
|
||||||
size: u32,
|
size: u32,
|
||||||
|
|||||||
@@ -86,6 +86,12 @@ pub(crate) fn build_dataset_oh(
|
|||||||
let mut dl = Vec::new();
|
let mut dl = Vec::new();
|
||||||
dl.push(4); // version
|
dl.push(4); // version
|
||||||
dl.push(1); // class = contiguous
|
dl.push(1); // class = contiguous
|
||||||
|
// An empty dataset has no storage: its address must be the undefined
|
||||||
|
// address, as libhdf5 writes it. A real address with size 0 trips
|
||||||
|
// libhdf5's `addr + size <= addr` overflow check, and it refuses the
|
||||||
|
// dataset as "invalid dataset size, likely file corruption" — which made
|
||||||
|
// every store with no sessions or knowledge graph unreadable by h5py.
|
||||||
|
let data_addr = if data_size == 0 { u64::MAX } else { data_addr };
|
||||||
dl.extend_from_slice(&data_addr.to_le_bytes());
|
dl.extend_from_slice(&data_addr.to_le_bytes());
|
||||||
dl.extend_from_slice(&data_size.to_le_bytes());
|
dl.extend_from_slice(&data_size.to_le_bytes());
|
||||||
w.add_message(MessageType::DataLayout, dl);
|
w.add_message(MessageType::DataLayout, dl);
|
||||||
|
|||||||
@@ -1387,3 +1387,42 @@ with h5py.File("{path_str}", "w", libver="latest") as f:
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn clawhdf5_writes_f32_h5py_reads() {
|
||||||
|
// Every f32 dataset used to be unreadable by h5py ("sign bit position out
|
||||||
|
// of bounds"): the float datatype's sign position was hard-coded for f64.
|
||||||
|
skip_if_no_python!();
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let path = dir.path().join("ours_f32.h5");
|
||||||
|
let path_str = path.display().to_string();
|
||||||
|
let values: Vec<f32> = vec![1.5, -2.25, 3.0e-7, 65536.5, f32::MAX, -0.0];
|
||||||
|
|
||||||
|
let mut fb = FileBuilder::new();
|
||||||
|
fb.create_dataset("plain").with_f32_data(&values);
|
||||||
|
fb.create_dataset("chunked")
|
||||||
|
.with_f32_data(&values)
|
||||||
|
.with_shape(&[values.len() as u64])
|
||||||
|
.with_chunks(&[4])
|
||||||
|
.with_deflate(6);
|
||||||
|
fb.write(&path).unwrap();
|
||||||
|
|
||||||
|
let bits = values
|
||||||
|
.iter()
|
||||||
|
.map(|v| v.to_bits().to_string())
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(",");
|
||||||
|
let script = format!(
|
||||||
|
r#"
|
||||||
|
import h5py, numpy as np
|
||||||
|
expected = np.array([{bits}], dtype=np.uint32)
|
||||||
|
with h5py.File("{path_str}", "r") as f:
|
||||||
|
for name in ("plain", "chunked"):
|
||||||
|
d = f[name]
|
||||||
|
assert d.dtype == np.float32, (name, d.dtype)
|
||||||
|
assert (d[:].view(np.uint32) == expected).all(), (name, d[:])
|
||||||
|
print("ok")
|
||||||
|
"#
|
||||||
|
);
|
||||||
|
assert_eq!(run_python_output(&script), "ok");
|
||||||
|
}
|
||||||
|
|||||||
@@ -227,3 +227,55 @@ wrong structure. All four are fixed and covered by interop tests against
|
|||||||
HDF5 2.0 at sizes that cross each boundary, including paged data blocks.
|
HDF5 2.0 at sizes that cross each boundary, including paged data blocks.
|
||||||
|
|
||||||
Files written by this crate are unaffected — this was purely a read-path bug.
|
Files written by this crate are unaffected — this was purely a read-path bug.
|
||||||
|
|
||||||
|
## Every `f32` dataset we wrote was unreadable by h5py / libhdf5
|
||||||
|
|
||||||
|
**Status:** fixed 2026-09-23, after v2.7.0. **Every
|
||||||
|
release up to and including v2.7.0 is affected** — the encoder was already
|
||||||
|
wrong in v2.1.0.
|
||||||
|
|
||||||
|
The floating-point datatype message carries the position of the sign bit
|
||||||
|
(bits 8–15 of its class bit field). `clawhdf5-format` wrote 63 for every
|
||||||
|
float, which is correct only for `f64`. libhdf5 validates the field, so opening
|
||||||
|
any `f32` dataset written by this crate failed:
|
||||||
|
|
||||||
|
```
|
||||||
|
KeyError: 'Unable to synchronously open object (sign bit position out of bounds)'
|
||||||
|
```
|
||||||
|
|
||||||
|
That covers every agent store (`/memory/embeddings`, `norms` and
|
||||||
|
`activation_weights` are `f32`). `clawhdf5` itself ignores the field on read,
|
||||||
|
and the interop suites only ever wrote `f64` from our side, so nothing here
|
||||||
|
noticed.
|
||||||
|
|
||||||
|
**Fix:** the sign position is computed from the type (`bit_offset +
|
||||||
|
bit_precision - 1`: 15, 31, 63 for half, single, double). Regression tests:
|
||||||
|
`float_sign_location_is_the_top_bit_of_the_value` (byte level),
|
||||||
|
`clawhdf5_writes_f32_h5py_reads` and the agent's
|
||||||
|
`h5py_reads_every_dataset_of_an_agent_store`.
|
||||||
|
|
||||||
|
**Existing files:** an agent store is rewritten in full at every checkpoint, so
|
||||||
|
it becomes readable by h5py at its next checkpoint with a fixed build. Other
|
||||||
|
files with `f32` datasets need to be rewritten.
|
||||||
|
|
||||||
|
## Empty datasets we wrote were unreadable by h5py / libhdf5
|
||||||
|
|
||||||
|
**Status:** fixed 2026-09-23, after v2.7.0. Every
|
||||||
|
release up to and including v2.7.0 is affected.
|
||||||
|
|
||||||
|
A dataset with no elements was written with a real file address and a storage
|
||||||
|
size of 0. libhdf5 guards contiguous storage with an overflow check
|
||||||
|
(`addr + size <= addr`) that is always true when the size is 0, so it rejected
|
||||||
|
the dataset:
|
||||||
|
|
||||||
|
```
|
||||||
|
KeyError: 'Unable to synchronously open object (invalid dataset size, likely file corruption)'
|
||||||
|
```
|
||||||
|
|
||||||
|
In practice: every agent store without sessions or a knowledge graph — the
|
||||||
|
`/sessions` and `/knowledge_graph` datasets are empty until something is added
|
||||||
|
— could not be read by h5py even once the `f32` bug above was fixed. Found by
|
||||||
|
the same agent-store interop test.
|
||||||
|
|
||||||
|
**Fix:** an empty contiguous dataset gets the undefined address (all `0xff`),
|
||||||
|
which is what libhdf5 itself writes.
|
||||||
|
|||||||
Reference in New Issue
Block a user