fix(format): read unmapped VDS elements as the virtual dataset's fill value
Elements of a virtual dataset that no mapping supplies (unmapped regions, a missing source file, a missing source dataset) read as 0 instead of the fill value libhdf5 returns — silent wrong data for any VDS created with a non-zero fillvalue (read-matrix cases 0471/0472: -1 and 7 read as 0). A missing source dataset was an error; libhdf5 reads it as fill. Move VDS assembly into a new vds module following H5Dvirtual.c: vds::read_virtual_dataset takes the dataset's fill value and a VdsFileResolver that can refuse a name, and reports how many elements were unmapped. Sources are read with their own fill value, and a source whose datatype differs from the virtual dataset's is an error (libhdf5 converts). File passes the dataset's fill value, resolves source names against the virtual file's directory, and refuses names that leave it with an error instead of reading them as fill. read_selection on a VDS goes through the same fill-aware path. The raw-read API (read_raw_data_full*) has no fill value, so it now errors for a VDS with unmapped elements instead of guessing zeros. Tests: vds_interop::vds_unmapped_regions_read_as_fill_value (external, same-file, missing file/dataset, sparse source with its own fill, int fill; earliest and latest format) and vds_source_outside_directory_is_an_error_not_fill, both against h5py; integration_test::v4_virtual_dataset_raw_api_refuses_to_guess_the_fill_value. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -485,6 +485,7 @@ impl<'f> Dataset<'f> {
|
||||
self.file.length_size(),
|
||||
)?;
|
||||
let fill_matters = !clawhdf5_format::fill_value::has_storage(&dl)
|
||||
|| matches!(dl, DataLayout::Virtual { .. })
|
||||
|| (matches!(dl, DataLayout::Chunked { .. })
|
||||
&& !clawhdf5_format::fill_value::is_default(fill.as_deref()));
|
||||
if fill_matters {
|
||||
@@ -837,24 +838,9 @@ impl<'f> Dataset<'f> {
|
||||
let pipeline = self.filter_pipeline()?;
|
||||
|
||||
// Virtual datasets are assembled from source datasets; the per-file
|
||||
// chunk cache does not apply. Route them through the resolver path so
|
||||
// external sibling files resolve relative to this file's directory.
|
||||
// chunk cache does not apply.
|
||||
if matches!(dl, DataLayout::Virtual { .. }) {
|
||||
let base_dir = self.file.base_dir.clone();
|
||||
let resolver = move |name: &str| -> Option<Vec<u8>> {
|
||||
let dir = base_dir.as_ref()?;
|
||||
std::fs::read(dir.join(sibling_file_name(name)?)).ok()
|
||||
};
|
||||
return Ok(data_read::read_raw_data_full_with_resolver(
|
||||
self.file.data.as_bytes(),
|
||||
&dl,
|
||||
&ds,
|
||||
&dt,
|
||||
pipeline.as_ref(),
|
||||
self.file.offset_size(),
|
||||
self.file.length_size(),
|
||||
Some(&resolver),
|
||||
)?);
|
||||
return self.read_virtual(&dl, &ds, &dt);
|
||||
}
|
||||
|
||||
// Unallocated storage reads as the dataset's fill value.
|
||||
@@ -880,6 +866,62 @@ impl<'f> Dataset<'f> {
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// Resolver for external Virtual Dataset source files: names are
|
||||
/// resolved against the directory of the file that holds the virtual
|
||||
/// dataset, as libhdf5 does. A missing file is `Ok(None)` (its mappings
|
||||
/// read as the fill value); a name that would leave that directory is
|
||||
/// refused with an error rather than read as fill.
|
||||
fn vds_resolver(&self) -> impl Fn(&str) -> Result<Option<Vec<u8>>, FormatError> + use<> {
|
||||
let base_dir = self.file.base_dir.clone();
|
||||
move |name: &str| {
|
||||
let Some(dir) = base_dir.as_ref() else {
|
||||
return Err(FormatError::ChunkedReadError(format!(
|
||||
"virtual dataset source file {name:?} cannot be resolved for an in-memory file"
|
||||
)));
|
||||
};
|
||||
let rel = sibling_file_name(name).ok_or_else(|| {
|
||||
FormatError::ChunkedReadError(format!(
|
||||
"virtual dataset source file {name:?} is outside the virtual file's \
|
||||
directory and is not followed"
|
||||
))
|
||||
})?;
|
||||
match std::fs::read(dir.join(rel)) {
|
||||
Ok(bytes) => Ok(Some(bytes)),
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
|
||||
Err(e) => Err(FormatError::ChunkedReadError(format!(
|
||||
"cannot read virtual dataset source file {name:?}: {e}"
|
||||
))),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Read a whole virtual dataset; unmapped elements hold its fill value.
|
||||
fn read_virtual(
|
||||
&self,
|
||||
dl: &DataLayout,
|
||||
ds: &Dataspace,
|
||||
dt: &Datatype,
|
||||
) -> Result<Vec<u8>, Error> {
|
||||
let fill = clawhdf5_format::fill_value::dataset_fill_value_in(
|
||||
self.file.data.as_bytes(),
|
||||
&self.header.messages,
|
||||
self.file.offset_size(),
|
||||
self.file.length_size(),
|
||||
)?;
|
||||
let resolver = self.vds_resolver();
|
||||
let v = clawhdf5_format::vds::read_virtual_dataset(
|
||||
self.file.data.as_bytes(),
|
||||
dl,
|
||||
ds,
|
||||
dt,
|
||||
fill.as_deref(),
|
||||
self.file.offset_size(),
|
||||
self.file.length_size(),
|
||||
Some(&resolver),
|
||||
)?;
|
||||
Ok(v.data)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -190,3 +190,102 @@ expect("shared.h5", "v", "shared")
|
||||
);
|
||||
assert_matches_libhdf5(dir.path(), "shared.h5", "v", "shared");
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Fill value
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// Elements no mapping supplies read as the virtual dataset's fill value, not
|
||||
/// as 0: unmapped regions, a missing source file, a missing source dataset.
|
||||
/// A source's own unallocated chunks read as *its* fill value.
|
||||
#[test]
|
||||
fn vds_unmapped_regions_read_as_fill_value() {
|
||||
skip_if_no_python!();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
generate(
|
||||
dir.path(),
|
||||
r#"
|
||||
for i in range(3):
|
||||
with h5py.File(f"src_{i}.h5", "w") as s:
|
||||
s.create_dataset("data", data=np.arange(10.0) + i * 100)
|
||||
with h5py.File("sparse_src.h5", "w") as s:
|
||||
d = s.create_dataset("data", shape=(10,), chunks=(5,), dtype="f8", fillvalue=42.0)
|
||||
d[0:5] = np.arange(5.0) + 1000 # the second chunk is never written
|
||||
for libver in ["earliest", "latest"]:
|
||||
with h5py.File(f"fill_{libver}.h5", "w", libver=libver) as f:
|
||||
f.create_dataset("local", data=np.arange(10.0) * -1)
|
||||
lay = h5py.VirtualLayout(shape=(6, 10), dtype="f8")
|
||||
for i in range(3):
|
||||
lay[i] = h5py.VirtualSource(f"src_{i}.h5", "data", shape=(10,))
|
||||
lay[3] = h5py.VirtualSource("no_such_file.h5", "data", shape=(10,))
|
||||
lay[4] = h5py.VirtualSource("src_0.h5", "no_such_dataset", shape=(10,))
|
||||
# row 5 is not mapped at all
|
||||
f.create_virtual_dataset("files", lay, fillvalue=-1.0)
|
||||
lay = h5py.VirtualLayout(shape=(20,), dtype="f8")
|
||||
lay[0:10] = h5py.VirtualSource(".", "local", shape=(10,))
|
||||
f.create_virtual_dataset("same_file", lay, fillvalue=7.0)
|
||||
lay = h5py.VirtualLayout(shape=(12,), dtype="f8")
|
||||
lay[1:11] = h5py.VirtualSource("sparse_src.h5", "data", shape=(10,))
|
||||
f.create_virtual_dataset("sparse_source", lay, fillvalue=-3.5)
|
||||
lay = h5py.VirtualLayout(shape=(3, 4), dtype="i4")
|
||||
lay[1, :] = h5py.VirtualSource(".", "ints", shape=(4,))
|
||||
f.create_dataset("ints", data=np.arange(4, dtype="i4") + 1)
|
||||
f.create_virtual_dataset("int_fill", lay, fillvalue=-99)
|
||||
for name in ["files", "same_file", "sparse_source", "int_fill"]:
|
||||
expect(f"fill_{libver}.h5", name, f"{name}_{libver}")
|
||||
"#,
|
||||
);
|
||||
for libver in ["earliest", "latest"] {
|
||||
let file = format!("fill_{libver}.h5");
|
||||
for name in ["files", "same_file", "sparse_source", "int_fill"] {
|
||||
assert_matches_libhdf5(dir.path(), &file, name, &format!("{name}_{libver}"));
|
||||
}
|
||||
}
|
||||
|
||||
// A selection read goes through the same fill-aware assembly.
|
||||
let f = File::open(dir.path().join("fill_latest.h5")).unwrap();
|
||||
let sel = clawhdf5::Selection::slice(std::slice::from_ref(&(8..14)));
|
||||
let got = f
|
||||
.dataset("same_file")
|
||||
.unwrap()
|
||||
.read_f64_selection(&sel)
|
||||
.unwrap();
|
||||
assert_eq!(got, vec![-8.0, -9.0, 7.0, 7.0, 7.0, 7.0]);
|
||||
}
|
||||
|
||||
/// A source name that would leave the virtual file's directory is refused
|
||||
/// with an error; it used to be skipped and read silently as fill.
|
||||
#[test]
|
||||
fn vds_source_outside_directory_is_an_error_not_fill() {
|
||||
skip_if_no_python!();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::create_dir(dir.path().join("sub")).unwrap();
|
||||
generate(
|
||||
dir.path(),
|
||||
r#"
|
||||
with h5py.File("src.h5", "w") as s:
|
||||
s.create_dataset("data", data=np.arange(4.0))
|
||||
with h5py.File("sub/up.h5", "w", libver="latest") as f:
|
||||
lay = h5py.VirtualLayout(shape=(4,), dtype="f8")
|
||||
lay[:] = h5py.VirtualSource("../src.h5", "data", shape=(4,))
|
||||
f.create_virtual_dataset("v", lay, fillvalue=-1.0)
|
||||
with h5py.File("nested.h5", "w", libver="latest") as f:
|
||||
lay = h5py.VirtualLayout(shape=(4,), dtype="f8")
|
||||
lay[:] = h5py.VirtualSource("sub/inner.h5", "data", shape=(4,))
|
||||
f.create_virtual_dataset("v", lay, fillvalue=-1.0)
|
||||
with h5py.File("sub/inner.h5", "w") as s:
|
||||
s.create_dataset("data", data=np.arange(4.0) + 10)
|
||||
expect("nested.h5", "v", "nested")
|
||||
"#,
|
||||
);
|
||||
// libhdf5 resolves "../src.h5" (and would read [0, 1, 2, 3]); we refuse
|
||||
// to leave the directory, and say so.
|
||||
let f = File::open(dir.path().join("sub/up.h5")).unwrap();
|
||||
let err = f.dataset("v").unwrap().read_f64().unwrap_err();
|
||||
assert!(
|
||||
err.to_string().contains("not followed"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
// A relative name below the virtual file's directory resolves there.
|
||||
assert_matches_libhdf5(dir.path(), "nested.h5", "v", "nested");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user