From d074385944c8c49f1325a39de6208227e90e581b Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:08:36 -0500 Subject: [PATCH] fix(format): read partial edge chunks stored unfiltered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layout message v4 flag bit 0 (H5D_CHUNK_DONT_FILTER_PARTIAL_CHUNKS, set with H5Pset_chunk_opts) makes libhdf5 store every chunk that extends past the dataset's extent without the filter pipeline, while its filter mask still reads 0. The parser ignored the flag, so readers tried to inflate raw bytes: libhdf5's own h5fc_edge_v3.h5 failed with "deflate: ... unknown compression method". DataLayout::Chunked gains dont_filter_partial_edge_chunks (always false for v3), and list_chunks — the one place every read path gets its chunk list from — marks such partial chunks as having skipped every filter, so the full, cached, indexed, parallel and selection readers all copy them as-is. chunked_write.rs gets `..` in one exhaustive test pattern for the new field. Regression: libhdf5_edge_chunk_fixture_reads (h5fc_edge_v3.h5 from the HDF5 tools test files, committed as a 2.5 KB fixture), and h5py_unfiltered_partial_edge_chunks_read (the flag set through h5py's bundled libhdf5 via ctypes, as h5py has no binding for it: fixed array, extensible array and B-tree v2 indexes, 1-D and 2-D, plus a hyperslab of the last chunk), and v4_chunked_dont_filter_partial_edge_chunks_flag. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/chunked_read.rs | 27 ++++- crates/clawhdf5-format/src/chunked_write.rs | 1 + crates/clawhdf5-format/src/data_layout.rs | 34 ++++++ .../clawhdf5/tests/fixtures/h5fc_edge_v3.h5 | Bin 0 -> 2478 bytes .../clawhdf5/tests/h5py_chunked_read_tests.rs | 106 ++++++++++++++++++ 5 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 crates/clawhdf5/tests/fixtures/h5fc_edge_v3.h5 diff --git a/crates/clawhdf5-format/src/chunked_read.rs b/crates/clawhdf5-format/src/chunked_read.rs index 2dab147..bc006df 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -519,6 +519,7 @@ pub fn list_chunks( addr_opt, single_filtered_size, single_filter_mask, + unfiltered_edges, ) = match layout { DataLayout::Chunked { chunk_dimensions, @@ -527,6 +528,7 @@ pub fn list_chunks( chunk_index_type, single_chunk_filtered_size, single_chunk_filter_mask, + dont_filter_partial_edge_chunks, } => ( chunk_dimensions, *version, @@ -534,6 +536,7 @@ pub fn list_chunks( *btree_address, *single_chunk_filtered_size, *single_chunk_filter_mask, + *dont_filter_partial_edge_chunks, ), _ => { return Err(FormatError::ChunkedReadError( @@ -566,7 +569,7 @@ pub fn list_chunks( } // Collect chunks based on version and index type - let chunks = match (version, chunk_index_type) { + let mut chunks = match (version, chunk_index_type) { (3, _) => { let ndims = chunk_dimensions.len(); // rank+1 collect_chunk_info(file_data, addr, ndims, offset_size, length_size)? @@ -645,6 +648,23 @@ pub fn list_chunks( } }; + // With "don't filter partial edge chunks", a chunk that extends past the + // dataset's extent is stored raw while its filter mask still reads 0. + // Mark every filter skipped so all read paths copy it as-is. + if unfiltered_edges { + for chunk in &mut chunks { + let partial = chunk + .offsets + .iter() + .zip(&chunk_dims) + .zip(&ds_dims) + .any(|((&off, &cd), &dd)| off.saturating_add(cd as u64) > dd as u64); + if partial { + chunk.filter_mask = u32::MAX; + } + } + } + Ok((chunks, chunk_dims)) } @@ -1829,6 +1849,7 @@ mod tests { chunk_index_type: None, single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, }; let dataspace = Dataspace { @@ -1852,6 +1873,7 @@ mod tests { chunk_index_type: None, single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, }; let dataspace = Dataspace { space_type: DataspaceType::Simple, @@ -2009,6 +2031,7 @@ mod tests { chunk_index_type: None, single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, }; let dataspace = Dataspace { space_type: DataspaceType::Simple, @@ -2091,6 +2114,7 @@ mod tests { chunk_index_type: None, single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, }; let dataspace = Dataspace { space_type: DataspaceType::Simple, @@ -2253,6 +2277,7 @@ mod tests { chunk_index_type: Some(1), single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, }; let dataspace = Dataspace { space_type: DataspaceType::Simple, diff --git a/crates/clawhdf5-format/src/chunked_write.rs b/crates/clawhdf5-format/src/chunked_write.rs index 6816546..04d2b3e 100644 --- a/crates/clawhdf5-format/src/chunked_write.rs +++ b/crates/clawhdf5-format/src/chunked_write.rs @@ -1314,6 +1314,7 @@ mod tests { chunk_index_type, single_chunk_filtered_size, single_chunk_filter_mask, + .. } => { assert_eq!(version, 4); assert_eq!(chunk_index_type, Some(1)); diff --git a/crates/clawhdf5-format/src/data_layout.rs b/crates/clawhdf5-format/src/data_layout.rs index dd931ff..8429c5d 100644 --- a/crates/clawhdf5-format/src/data_layout.rs +++ b/crates/clawhdf5-format/src/data_layout.rs @@ -53,6 +53,11 @@ pub enum DataLayout { single_chunk_filtered_size: Option, /// Filter mask for v4 single chunk with filters. single_chunk_filter_mask: Option, + /// Layout v4 flag bit 0 (`H5D_CHUNK_DONT_FILTER_PARTIAL_CHUNKS`): + /// partial edge chunks — those extending past the dataset's current + /// extent in some dimension — are stored without the filter pipeline, + /// even though their filter mask is 0. Always `false` for v3. + dont_filter_partial_edge_chunks: bool, }, /// Virtual dataset layout (v4 only). Virtual { @@ -322,6 +327,7 @@ impl DataLayout { chunk_index_type: None, single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, }) } _ => Err(FormatError::InvalidLayoutClass(layout_class)), @@ -505,6 +511,7 @@ impl DataLayout { chunk_index_type: Some(chunk_index_type), single_chunk_filtered_size, single_chunk_filter_mask, + dont_filter_partial_edge_chunks: flags & 0x01 != 0, }) } 3 => { @@ -602,6 +609,7 @@ mod tests { chunk_index_type: None, single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, } ); } @@ -679,10 +687,35 @@ mod tests { chunk_index_type: Some(1), single_chunk_filtered_size: None, single_chunk_filter_mask: None, + dont_filter_partial_edge_chunks: false, } ); } + #[test] + fn v4_chunked_dont_filter_partial_edge_chunks_flag() { + let mut buf = vec![4u8, 2]; // version=4, class=2 + buf.push(0x01); // flags bit 0 = don't filter partial edge chunks + buf.push(2); // dimensionality=2 + buf.push(4); // dim_size_encoded_length=4 + buf.extend_from_slice(&5u32.to_le_bytes()); + buf.extend_from_slice(&4u32.to_le_bytes()); + buf.push(3); // Fixed Array + buf.push(10); // max_dblk_page_nelmts_bits + buf.extend_from_slice(&0x3000u64.to_le_bytes()); + match DataLayout::parse(&buf, 8, 8).unwrap() { + DataLayout::Chunked { + dont_filter_partial_edge_chunks, + btree_address, + .. + } => { + assert!(dont_filter_partial_edge_chunks); + assert_eq!(btree_address, Some(0x3000)); + } + other => panic!("expected Chunked, got {other:?}"), + } + } + #[test] fn v4_chunked_single_chunk_with_filters() { let mut buf = vec![4u8, 2]; // version=4, class=2 @@ -705,6 +738,7 @@ mod tests { chunk_index_type: Some(1), single_chunk_filtered_size: Some(1024), single_chunk_filter_mask: Some(0), + dont_filter_partial_edge_chunks: false, } ); } diff --git a/crates/clawhdf5/tests/fixtures/h5fc_edge_v3.h5 b/crates/clawhdf5/tests/fixtures/h5fc_edge_v3.h5 new file mode 100644 index 0000000000000000000000000000000000000000..6f92057cf653c4dc47cad6133e2ecdc425e861ae GIT binary patch literal 2478 zcmeD5aB<`1lHy|G;9!7(|4^`w6Cz>&mAE-AN6_EHC5TC3cZXdBGOl0}0;z#%gwb3~ z42%p6Y$6Omiqj?7H6-5E#ohHV)HntQ7=wKJL%-5Nu#*&#odaSsFfytzFfcJOGV(x; zXM@t{d}a{_MnMI61{Scq0)sdQgLplV%?flqGdDL610&E7pbvn40E%(|eagbb$jr#f z%EHWbALu_Ym;j{*5q0yk(ib;J4;KbTJ}#Kwe?#4S_Jl%}o1=>p10&1_DbV=mhxh=> zO9AtWQRJFXiz2}1{Kt`Q18UmvsFd71*A%GkLC!COZ5H@wiDsCkw pSUf!9kj%rtD=7IJjmyA5UE6S*2eO9{#taMMMTqqW@g}0H1ONf9YWx5I literal 0 HcmV?d00001 diff --git a/crates/clawhdf5/tests/h5py_chunked_read_tests.rs b/crates/clawhdf5/tests/h5py_chunked_read_tests.rs index deda24f..2b9e902 100644 --- a/crates/clawhdf5/tests/h5py_chunked_read_tests.rs +++ b/crates/clawhdf5/tests/h5py_chunked_read_tests.rs @@ -237,3 +237,109 @@ with h5py.File("{p}", "w") as f: grid ); } + +// --------------------------------------------------------------------------- +// "Don't filter partial edge chunks" +// --------------------------------------------------------------------------- + +/// libhdf5's own test file for `H5Pset_chunk_opts(H5D_CHUNK_DONT_FILTER_PARTIAL_CHUNKS)`: +/// a 12x6 f32 dataset in 5x5 gzip chunks whose edge chunks are stored raw +/// with a filter mask of 0. Reading it tried to inflate the raw edge chunks +/// ("deflate: ... unknown compression method"). +#[test] +fn libhdf5_edge_chunk_fixture_reads() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/h5fc_edge_v3.h5" + ); + let file = File::open(path).unwrap(); + let ds = file.dataset("DSET_EDGE").unwrap(); + assert_eq!(ds.shape().unwrap(), vec![12, 6]); + assert_eq!(ds.read_f32().unwrap(), vec![100.0f32; 72]); +} + +/// The same layout flag on datasets with varied contents, set through the +/// libhdf5 that h5py ships (h5py has no binding for `H5Pset_chunk_opts`). +#[test] +fn h5py_unfiltered_partial_edge_chunks_read() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("edge.h5"); + let p = path.display().to_string(); + let script = format!( + r#" +import ctypes, glob, os, sys +import h5py, numpy as np +here = os.path.dirname(h5py.__file__) +libs = glob.glob(os.path.join(here, "..", "h5py.libs", "libhdf5-*.so*")) +libs += glob.glob(os.path.join(here, ".dylibs", "libhdf5*.dylib")) +if not libs: + print("NO_LIBHDF5") + sys.exit(0) +lib = ctypes.CDLL(libs[0]) +lib.H5Pset_chunk_opts.argtypes = [ctypes.c_int64, ctypes.c_uint] +def make(f, name, data, chunk, maxshape, shuffle): + dcpl = h5py.h5p.create(h5py.h5p.DATASET_CREATE) + dcpl.set_chunk(chunk) + if shuffle: + dcpl.set_shuffle() + dcpl.set_deflate(6) + assert lib.H5Pset_chunk_opts(dcpl.id, 0x0002) >= 0 + space = h5py.h5s.create_simple(data.shape, maxshape) + d = h5py.h5d.create(f.id, name.encode(), h5py.h5t.py_create(data.dtype), space, dcpl=dcpl) + d.write(h5py.h5s.ALL, h5py.h5s.ALL, np.ascontiguousarray(data)) +with h5py.File("{p}", "w") as f: + line = np.sin(np.arange(1000) / 7.0) + grid = np.arange(37 * 53, dtype=" = (0..1000).map(|i| (f64::from(i) / 7.0).sin()).collect(); + for name in ["fixed_1d", "ea_1d"] { + let values = file.dataset(name).unwrap().read_f64().unwrap(); + assert_eq!(values.len(), line.len(), "{name}"); + for (i, (v, w)) in values.iter().zip(&line).enumerate() { + assert!((v - w).abs() < 1e-12, "{name}[{i}]: {v} vs {w}"); + } + } + let grid: Vec = (0..37 * 53).map(|i| f64::from(i) * 0.5).collect(); + for name in ["bt2_2d", "fixed_2d"] { + assert_eq!( + file.dataset(name).unwrap().read_f64().unwrap(), + grid, + "{name}" + ); + } + // A selection touching only the last (partial, unfiltered) chunk. + let tail = file + .dataset("fixed_1d") + .unwrap() + .read_selection(&Selection::Hyperslab { + start: vec![990], + stride: vec![1], + count: vec![1], + block: vec![10], + }) + .unwrap(); + let want: Vec = line[990..].iter().flat_map(|v| v.to_le_bytes()).collect(); + assert_eq!(tail, want); +}