diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 045ad9b..d432f5e 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -19,7 +19,7 @@ use crate::metadata_index::{DatasetMetadata, MetadataBlock, MetadataIndex}; use crate::object_header_writer::ObjectHeaderWriter; use crate::superblock::Superblock; use crate::type_builders::{ - DatasetBuilder, FillTime, FinishedGroup, GroupBuilder, build_attr_message, + DatasetBuilder, FinishedGroup, GroupBuilder, build_attr_message, fill_value_message, }; // Re-export public types that moved to type_builders for API compatibility. @@ -80,12 +80,12 @@ pub(crate) fn build_chunked_dataset_oh( pipeline_message: Option<&[u8]>, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, + fill_message: &[u8], ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); w.add_message(MessageType::DataLayout, layout_message.to_vec()); if let Some(pm) = pipeline_message { w.add_message(MessageType::FilterPipeline, pm.to_vec()); @@ -107,12 +107,12 @@ pub(crate) fn build_dataset_oh( data_size: u64, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, + fill_message: &[u8], ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); let mut dl = Vec::new(); dl.push(4); // version dl.push(1); // class = contiguous @@ -142,12 +142,12 @@ pub(crate) fn build_compact_dataset_oh( data: &[u8], attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, + fill_message: &[u8], ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); // Compact layout message: version=4, class=0, u16 size, inline data let mut dl = Vec::new(); dl.push(4); // version @@ -932,12 +932,12 @@ pub(crate) fn build_vds_dataset_oh( global_heap_addr: u64, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, - fill_time: FillTime, + fill_message: &[u8], ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); - w.add_message_with_flags(MessageType::FillValue, vec![3, fill_time.to_byte()], 0x01); + w.add_message_with_flags(MessageType::FillValue, fill_message.to_vec(), 0x01); // VDS layout message: version=4, class=3, global_heap_address(8), global_heap_index=1(4) let mut dl = Vec::new(); dl.push(4u8); // version @@ -1070,7 +1070,8 @@ impl FileWriter { attrs: Vec, chunk_options: ChunkOptions, maxshape: Option>, - fill_time: FillTime, + /// Serialized Fill Value message. + fill_message: Vec, compact: bool, alignment: usize, /// VDS source mappings (set for Virtual datasets). @@ -1120,6 +1121,7 @@ impl FileWriter { }; attrs.extend(p.build_attrs(&raw)); } + let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?; Ok(DsFlat { name: db.name, dt, @@ -1128,7 +1130,7 @@ impl FileWriter { attrs, chunk_options: db.chunk_options, maxshape: db.maxshape, - fill_time: db.fill_time, + fill_message, compact: db.compact, alignment: db.alignment, virtual_sources: db.virtual_sources, @@ -1274,7 +1276,7 @@ impl FileWriter { 0, // dummy address &d.attrs, dense_blob.as_ref(), - d.fill_time, + &d.fill_message, )?; // Global heap blob size is address-independent; compute it now // so pass 2 can place it correctly. @@ -1318,7 +1320,7 @@ impl FileWriter { result.pipeline_message.as_deref(), &d.attrs, dense_blob.as_ref(), - d.fill_time, + &d.fill_message, )?; dummy_blobs.push(DataBlob { data: result.data_bytes, @@ -1337,7 +1339,7 @@ impl FileWriter { &d.raw, &d.attrs, dense_blob.as_ref(), - d.fill_time, + &d.fill_message, )?; dummy_blobs.push(DataBlob { data: vec![], @@ -1357,7 +1359,7 @@ impl FileWriter { d.raw.len() as u64, &d.attrs, dense_blob.as_ref(), - d.fill_time, + &d.fill_message, )?; dummy_blobs.push(DataBlob { data: d.raw.clone(), @@ -1466,7 +1468,7 @@ impl FileWriter { heap_addr, &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, + &d.fill_message, )?; ds_blobs2.push(DataBlob { data: gcol_bytes.clone(), @@ -1493,7 +1495,7 @@ impl FileWriter { result.pipeline_message.as_deref(), &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, + &d.fill_message, )?; ds_blobs2.push(DataBlob { data: result.data_bytes, @@ -1508,7 +1510,7 @@ impl FileWriter { &d.raw, &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, + &d.fill_message, )?; ds_blobs2.push(DataBlob { data: vec![], @@ -1533,7 +1535,7 @@ impl FileWriter { d.raw.len() as u64, &d.attrs, ds_dense_blobs[i].as_ref(), - d.fill_time, + &d.fill_message, )?; let mut data = vec![0u8; padding]; data.extend_from_slice(&d.raw); diff --git a/crates/clawhdf5-format/src/property_list.rs b/crates/clawhdf5-format/src/property_list.rs index 15ea6b9..adf21d4 100644 --- a/crates/clawhdf5-format/src/property_list.rs +++ b/crates/clawhdf5-format/src/property_list.rs @@ -43,7 +43,7 @@ impl Default for DatasetCreateProps { fletcher32: false, lz4: false, zstd_level: None, - fill_time: FillTime::Alloc, + fill_time: FillTime::IfSet, compact: false, alignment: 0, } @@ -335,7 +335,7 @@ mod tests { fn dcpl_defaults() { let dcpl = DatasetCreateProps::new(); assert!(dcpl.chunk_dims.is_none()); - assert_eq!(dcpl.fill_time, FillTime::Alloc); + assert_eq!(dcpl.fill_time, FillTime::IfSet); assert!(!dcpl.compact); } diff --git a/crates/clawhdf5-format/src/type_builders.rs b/crates/clawhdf5-format/src/type_builders.rs index 9ee2520..b47cdba 100644 --- a/crates/clawhdf5-format/src/type_builders.rs +++ b/crates/clawhdf5-format/src/type_builders.rs @@ -15,29 +15,81 @@ use crate::datatype::{ /// Controls when fill values are written to dataset storage. /// -/// Corresponds to the HDF5 fill value message's "fill time" field. +/// Corresponds to the HDF5 fill value message's "fill time" field +/// (`H5D_fill_time_t`). #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum FillTime { - /// Never write fill values (0x02). Avoids initialization overhead - /// for datasets that will be fully written before any read. + /// Never write fill values (`H5D_FILL_TIME_NEVER`). Avoids + /// initialization overhead for datasets that will be fully written + /// before any read. Never, - /// Write fill values at allocation time (0x0a). This is the default - /// and matches the HDF5 C library's behavior. - #[default] + /// Write fill values when storage is allocated (`H5D_FILL_TIME_ALLOC`). Alloc, - /// Write fill values only when the fill value has been explicitly set (0x06). + /// Write fill values at allocation only if one was set explicitly + /// (`H5D_FILL_TIME_IFSET`). The default, as in the HDF5 C library. + #[default] IfSet, } +/// Space allocation time written with every fill value message: late +/// (`H5D_ALLOC_TIME_LATE`), bits 0-1 of the flags byte. +const ALLOC_TIME_LATE: u8 = 2; + impl FillTime { - /// Serialize to the byte used in the fill value message (version 3). + /// Serialize to the flags byte of a version 3 fill value message: the + /// space allocation time (late) in bits 0-1 and the fill time in bits + /// 2-3 (`H5D_FILL_TIME_ALLOC` = 0, `NEVER` = 1, `IFSET` = 2). + /// + /// This used to put `Never` in the ALLOC slot, `Alloc` in IFSET and + /// `IfSet` in NEVER, so libhdf5 saw every choice as a different one. pub fn to_byte(self) -> u8 { - match self { - FillTime::Never => 0x02, - FillTime::Alloc => 0x0a, - FillTime::IfSet => 0x06, + ALLOC_TIME_LATE | (self.code() << 2) + } + + /// Decode the fill time from a version 3 fill value message's flags. + pub fn from_byte(flags: u8) -> Option { + match (flags >> 2) & 0x03 { + 0 => Some(FillTime::Alloc), + 1 => Some(FillTime::Never), + 2 => Some(FillTime::IfSet), + _ => None, } } + + fn code(self) -> u8 { + match self { + FillTime::Alloc => 0, + FillTime::Never => 1, + FillTime::IfSet => 2, + } + } +} + +/// Serialize a version 3 Fill Value message for a dataset of `dt`: the fill +/// time, and the user-defined fill value if there is one (bit 5). +pub(crate) fn fill_value_message( + fill_time: FillTime, + value: Option<&[u8]>, + dt: &Datatype, +) -> Result, crate::error::FormatError> { + let mut msg = vec![3, fill_time.to_byte()]; + if let Some(value) = value { + if matches!(dt, Datatype::VariableLength { .. }) { + return Err(crate::error::FormatError::SerializationError( + "a fill value for a variable-length datatype is not supported".into(), + )); + } + if value.len() != dt.type_size() as usize { + return Err(crate::error::FormatError::DataSizeMismatch { + expected: dt.type_size() as usize, + actual: value.len(), + }); + } + msg[1] |= 0x20; // fill value defined + msg.extend_from_slice(&(value.len() as u32).to_le_bytes()); + msg.extend_from_slice(value); + } + Ok(msg) } // ---- Datatype constructors ---- @@ -431,8 +483,10 @@ pub struct DatasetBuilder { pub(crate) data: Option>, pub(crate) attrs: Vec<(String, AttrValue)>, pub(crate) chunk_options: ChunkOptions, - /// Controls when fill values are written. Default is `FillTime::Alloc`. + /// Controls when fill values are written. Default is `FillTime::IfSet`. pub(crate) fill_time: FillTime, + /// User-defined fill value: one element's bytes, as stored. + pub(crate) fill_value: Option>, /// Use compact (inline) storage: data is stored in the object header. /// Only valid when raw data is <= 65536 bytes and dataset is not chunked. pub(crate) compact: bool, @@ -459,6 +513,7 @@ impl DatasetBuilder { attrs: Vec::new(), chunk_options: ChunkOptions::default(), fill_time: FillTime::default(), + fill_value: None, compact: false, alignment: 0, virtual_sources: None, @@ -715,6 +770,16 @@ impl DatasetBuilder { self } + /// Set the dataset's fill value: what readers return for storage that + /// was never written (e.g. after the dataset is extended). `value` is one + /// element's bytes as stored — the dataset datatype's size and byte order + /// (`(-1i32).to_le_bytes()` for an `i32` dataset). A size mismatch, or a + /// variable-length datatype, makes `finish` fail. + pub fn with_fill_value(&mut self, value: &[u8]) -> &mut Self { + self.fill_value = Some(value.to_vec()); + self + } + /// Use compact (inline) storage for this dataset. /// /// The raw data is stored directly in the dataset's object header rather diff --git a/crates/clawhdf5-format/tests/writer_meta_tests.rs b/crates/clawhdf5-format/tests/writer_meta_tests.rs index 48f5c12..77da856 100644 --- a/crates/clawhdf5-format/tests/writer_meta_tests.rs +++ b/crates/clawhdf5-format/tests/writer_meta_tests.rs @@ -13,7 +13,7 @@ use clawhdf5_format::message_type::MessageType; use clawhdf5_format::object_header::ObjectHeader; use clawhdf5_format::signature; use clawhdf5_format::superblock::Superblock; -use clawhdf5_format::type_builders::make_u8_type; +use clawhdf5_format::type_builders::{FillTime, make_u8_type}; // ---- helpers ---- @@ -373,3 +373,94 @@ fn h5py_opens_paged_files() { h5dump_ok(&path); } } + +// ---- 4. fill time and fill value ---- + +fn fill_message(bytes: &[u8], path: &str) -> clawhdf5_format::object_header::HeaderMessage { + let (_, oh) = header_at(bytes, path); + oh.messages + .into_iter() + .find(|m| m.msg_type == MessageType::FillValue) + .unwrap() +} + +fn fill_file() -> Vec { + let mut fw = FileWriter::new(); + fw.create_dataset("never") + .with_f64_data(&[1.0, 2.0]) + .fill_time(FillTime::Never); + fw.create_dataset("alloc") + .with_f64_data(&[1.0, 2.0]) + .fill_time(FillTime::Alloc); + fw.create_dataset("ifset") + .with_f64_data(&[1.0, 2.0]) + .fill_time(FillTime::IfSet); + fw.create_dataset("default").with_f64_data(&[1.0, 2.0]); + fw.create_dataset("filled") + .with_i32_data(&[1, 2, 3, 4]) + .with_chunks(&[2]) + .with_maxshape(&[u64::MAX]) + .with_fill_value(&(-1i32).to_le_bytes()); + fw.finish().unwrap() +} + +#[test] +fn fill_time_uses_libhdf5_codes() { + // H5D_FILL_TIME_ALLOC = 0, NEVER = 1, IFSET = 2, in bits 2-3. Measured: + // h5py saw our Never as ALLOC, Alloc as IFSET and IfSet as NEVER. + let bytes = fill_file(); + for (path, code) in [("never", 1), ("alloc", 0), ("ifset", 2), ("default", 2)] { + let msg = fill_message(&bytes, path); + assert_eq!((msg.data[1] >> 2) & 3, code, "{path}"); + assert_eq!(msg.data[1] & 3, 2, "{path}: allocation time stays late"); + } + for ft in [FillTime::Never, FillTime::Alloc, FillTime::IfSet] { + assert_eq!(FillTime::from_byte(ft.to_byte()), Some(ft)); + } + assert_eq!(FillTime::default(), FillTime::IfSet); +} + +#[test] +fn fill_value_is_written_and_read_back() { + let bytes = fill_file(); + let msg = fill_message(&bytes, "filled"); + assert_eq!( + clawhdf5_format::fill_value::parse_fill_value(&msg).unwrap(), + Some((-1i32).to_le_bytes().to_vec()) + ); + assert_eq!( + clawhdf5_format::fill_value::parse_fill_value(&fill_message(&bytes, "ifset")).unwrap(), + None + ); + + // One element's bytes, no more, no less. + let mut fw = FileWriter::new(); + fw.create_dataset("d") + .with_f64_data(&[1.0]) + .with_fill_value(&[0; 4]); + assert!(fw.finish().is_err()); +} + +#[test] +#[ignore = "requires Python h5py module and h5dump"] +fn h5py_sees_our_fill_time_and_fill_value() { + let path = write_tmp("fill", &fill_file()); + let out = h5py( + &path, + "from h5py import h5d\n\ + f = h5py.File(path, 'r')\n\ + names = {h5d.FILL_TIME_NEVER: 'never', h5d.FILL_TIME_ALLOC: 'alloc', h5d.FILL_TIME_IFSET: 'ifset'}\n\ + t = [names[f[n].id.get_create_plist().get_fill_time()] for n in ('never', 'alloc', 'ifset', 'default')]\n\ + print(json.dumps([t, int(f['filled'].fillvalue), f['filled'][()].tolist()]))\n\ + f.close()\n\ + f = h5py.File(path, 'r+')\n\ + f['filled'].resize((7,))\n\ + f.close()\n\ + print(json.dumps(h5py.File(path, 'r')['filled'][()].tolist()))", + ); + assert_eq!( + out, + "[[\"never\", \"alloc\", \"ifset\", \"ifset\"], -1, [1, 2, 3, 4]]\n[1, 2, 3, 4, -1, -1, -1]" + ); + h5dump_ok(&path); +}