Merge pull request #185 from aldanor/feature/blosc-lzf-fix

Fix multiple filter problems and add tests

Author: aldanor

CHANGELOG.md

@@ -6,7 +6,18 @@
 
 - `Error` now implements `From<Infallible>`, which allows passing convertible
   extents (like tuples of integers) where `impl TryInto<Extents>` is required.
-- Support for HDF5 versions 1.12.1 and 1.10.8.
+- Support for HDF5 versions 1.12.1 and 1.10.8
+
+### Changed
+
+- Renamed `filters::gzip_available()` to `deflate_available()` (the old name is
+  present but marked as deprecated).
+
+### Fixed
+
+- Fixed a bug where all blosc filter settings were discarded / zeroed out.
+- Fixed errors when recovering filter pipelines from stored datasets.
+- Fixed a bug where filter availability was computed incorrectly.
 
 ## 0.8.0
 

src/hl/dataset.rs

@@ -1045,32 +1045,71 @@ impl<'d, T2: H5Type, D2: ndarray::Dimension> DatasetBuilderData<'d, T2, D2> {
     impl_builder_methods!();
 }
 
-#[test]
-fn test_compute_chunk_shape() {
-    let e = SimpleExtents::new(&[1, 1]);
-    assert_eq!(compute_chunk_shape(&e, 1), vec![1, 1]);
-    let e = SimpleExtents::new(&[1, 10]);
-    assert_eq!(compute_chunk_shape(&e, 3), vec![1, 3]);
-    let e = SimpleExtents::new(&[1, 10]);
-    assert_eq!(compute_chunk_shape(&e, 11), vec![1, 10]);
-
-    let e = SimpleExtents::new(&[Extent::from(1), Extent::from(10..)]);
-    assert_eq!(compute_chunk_shape(&e, 11), vec![1, 11]);
-
-    let e = SimpleExtents::new(&[Extent::from(1), Extent::from(10..)]);
-    assert_eq!(compute_chunk_shape(&e, 9), vec![1, 9]);
-
-    let e = SimpleExtents::new(&[4, 4, 4]);
-    // chunk shape should be greedy here, a minimal
-    // chunk shape would be (1, 3, 4) + (1, 1, 4)
-    assert_eq!(compute_chunk_shape(&e, 12), vec![1, 4, 4]);
-
-    let e = SimpleExtents::new(&[4, 4, 4]);
-    assert_eq!(compute_chunk_shape(&e, 100), vec![4, 4, 4]);
-
-    let e = SimpleExtents::new(&[4, 4, 4]);
-    assert_eq!(compute_chunk_shape(&e, 9), vec![1, 2, 4]);
-
-    let e = SimpleExtents::new(&[1, 1, 100]);
-    assert_eq!(compute_chunk_shape(&e, 51), vec![1, 1, 100]);
+#[cfg(test)]
+mod tests {
+    use super::{compute_chunk_shape, DatasetBuilder};
+    use crate::filters::Filter;
+    use crate::test::with_tmp_file;
+    use crate::{Extent, Result, SimpleExtents};
+
+    #[cfg(feature = "blosc")]
+    use crate::filters::{Blosc, BloscShuffle};
+
+    use ndarray::Array2;
+
+    #[allow(dead_code)]
+    fn check_filter(func: impl Fn(DatasetBuilder) -> DatasetBuilder, flt: Filter) {
+        let filters = vec![flt];
+        with_tmp_file::<Result<_>, _>(|file| {
+            let arr = Array2::<i64>::ones((1000, 20));
+            func(file.new_dataset_builder()).with_data(&arr).create("foo")?;
+            let ds = file.dataset("foo")?;
+            assert_eq!(ds.filters(), filters);
+            assert_eq!(ds.read_2d::<i64>()?, &arr);
+            Ok(())
+        })
+        .unwrap()
+    }
+
+    #[test]
+    #[cfg(feature = "blosc")]
+    fn test_blosc() {
+        check_filter(|d| d.blosc_zstd(9, true), Filter::Blosc(Blosc::ZStd, 9, BloscShuffle::Byte));
+    }
+
+    #[test]
+    #[cfg(feature = "lzf")]
+    fn test_lzf() {
+        check_filter(|d| d.lzf(), Filter::LZF);
+    }
+
+    #[test]
+    fn test_compute_chunk_shape() {
+        let e = SimpleExtents::new(&[1, 1]);
+        assert_eq!(compute_chunk_shape(&e, 1), vec![1, 1]);
+        let e = SimpleExtents::new(&[1, 10]);
+        assert_eq!(compute_chunk_shape(&e, 3), vec![1, 3]);
+        let e = SimpleExtents::new(&[1, 10]);
+        assert_eq!(compute_chunk_shape(&e, 11), vec![1, 10]);
+
+        let e = SimpleExtents::new(&[Extent::from(1), Extent::from(10..)]);
+        assert_eq!(compute_chunk_shape(&e, 11), vec![1, 11]);
+
+        let e = SimpleExtents::new(&[Extent::from(1), Extent::from(10..)]);
+        assert_eq!(compute_chunk_shape(&e, 9), vec![1, 9]);
+
+        let e = SimpleExtents::new(&[4, 4, 4]);
+        // chunk shape should be greedy here, a minimal
+        // chunk shape would be (1, 3, 4) + (1, 1, 4)
+        assert_eq!(compute_chunk_shape(&e, 12), vec![1, 4, 4]);
+
+        let e = SimpleExtents::new(&[4, 4, 4]);
+        assert_eq!(compute_chunk_shape(&e, 100), vec![4, 4, 4]);
+
+        let e = SimpleExtents::new(&[4, 4, 4]);
+        assert_eq!(compute_chunk_shape(&e, 9), vec![1, 2, 4]);
+
+        let e = SimpleExtents::new(&[1, 1, 100]);
+        assert_eq!(compute_chunk_shape(&e, 51), vec![1, 1, 100]);
+    }
 }

src/hl/filters.rs

@@ -7,13 +7,15 @@ use hdf5_sys::h5p::{
 };
 use hdf5_sys::h5t::H5T_class_t;
 use hdf5_sys::h5z::{
-    H5Z_filter_t, H5Zfilter_avail, H5Zget_filter_info, H5Z_FILTER_CONFIG_DECODE_ENABLED,
+    H5Zfilter_avail, H5Zget_filter_info, H5Z_FILTER_CONFIG_DECODE_ENABLED,
     H5Z_FILTER_CONFIG_ENCODE_ENABLED, H5Z_FILTER_DEFLATE, H5Z_FILTER_FLETCHER32, H5Z_FILTER_NBIT,
     H5Z_FILTER_SCALEOFFSET, H5Z_FILTER_SHUFFLE, H5Z_FILTER_SZIP, H5Z_FLAG_OPTIONAL,
     H5Z_SO_FLOAT_DSCALE, H5Z_SO_INT, H5_SZIP_EC_OPTION_MASK, H5_SZIP_MAX_PIXELS_PER_BLOCK,
     H5_SZIP_NN_OPTION_MASK,
 };
 
+pub use hdf5_sys::h5z::H5Z_filter_t;
+
 use crate::internal_prelude::*;
 
 #[cfg(feature = "blosc")]
@@ -121,19 +123,26 @@ pub struct FilterInfo {
 pub(crate) fn register_filters() {
     #[cfg(feature = "lzf")]
     if let Err(e) = lzf::register_lzf() {
-        eprintln!("{}", e);
+        eprintln!("Error while registering LZF filter: {}", e);
     }
     #[cfg(feature = "blosc")]
     if let Err(e) = blosc::register_blosc() {
-        eprintln!("{}", e);
+        eprintln!("Error while registering Blosc filter: {}", e);
     }
 }
 
-/// Returns `true` if gzip filter is available.
-pub fn gzip_available() -> bool {
+/// Returns `true` if deflate filter is available.
+pub fn deflate_available() -> bool {
     h5lock!(H5Zfilter_avail(H5Z_FILTER_DEFLATE) == 1)
 }
 
+/// Returns `true` if deflate filter is available.
+#[doc(hidden)]
+#[deprecated(note = "deprecated; use deflate_available()")]
+pub fn gzip_available() -> bool {
+    deflate_available()
+}
+
 /// Returns `true` if szip filter is available.
 pub fn szip_available() -> bool {
     h5lock!(H5Zfilter_avail(H5Z_FILTER_SZIP) == 1)
@@ -167,7 +176,7 @@ impl Filter {
     }
 
     pub fn get_info(filter_id: H5Z_filter_t) -> FilterInfo {
-        if h5call!(H5Zfilter_avail(filter_id)).map(|x| x > 0).unwrap_or_default() {
+        if !h5call!(H5Zfilter_avail(filter_id)).map(|x| x > 0).unwrap_or_default() {
             return FilterInfo::default();
         }
         let mut flags: c_uint = 0;
@@ -281,28 +290,25 @@ impl Filter {
     }
 
     fn parse_deflate(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.len() == 1, "expected length 1 cdata for deflate filter");
+        ensure!(!cdata.is_empty(), "expected cdata.len() >= 1 for deflate filter");
         ensure!(cdata[0] <= 9, "invalid deflate level: {}", cdata[0]);
         Ok(Self::deflate(cdata[0] as _))
     }
 
-    fn parse_shuffle(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.is_empty(), "expected length 0 cdata for shuffle filter");
+    fn parse_shuffle(_cdata: &[c_uint]) -> Result<Self> {
         Ok(Self::shuffle())
     }
 
-    fn parse_fletcher32(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.is_empty(), "expected length 0 cdata for fletcher32 filter");
+    fn parse_fletcher32(_cdata: &[c_uint]) -> Result<Self> {
         Ok(Self::fletcher32())
     }
 
-    fn parse_nbit(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.is_empty(), "expected length 0 cdata for nbit filter");
+    fn parse_nbit(_cdata: &[c_uint]) -> Result<Self> {
         Ok(Self::nbit())
     }
 
     fn parse_szip(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.len() == 2, "expected length 2 cdata for szip filter");
+        ensure!(cdata.len() >= 2, "expected cdata.len() >= 2 for szip filter");
         let m = cdata[0];
         ensure!(
             (m & H5_SZIP_EC_OPTION_MASK != 0) != (m & H5_SZIP_NN_OPTION_MASK != 0),
@@ -321,7 +327,7 @@ impl Filter {
     }
 
     fn parse_scaleoffset(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.len() == 2, "expected length 2 cdata for scaleoffset filter");
+        ensure!(cdata.len() >= 2, "expected cdata.len() >= 2 for scaleoffset filter");
         let scale_type = cdata[0];
         let mode = if scale_type == (H5Z_SO_INT as c_uint) {
             ensure!(
@@ -344,8 +350,7 @@ impl Filter {
     }
 
     #[cfg(feature = "lzf")]
-    fn parse_lzf(cdata: &[c_uint]) -> Result<Self> {
-        ensure!(cdata.is_empty(), "expected length 0 cdata for lzf filter");
+    fn parse_lzf(_cdata: &[c_uint]) -> Result<Self> {
         Ok(Self::lzf())
     }
 
@@ -506,9 +511,7 @@ impl Filter {
                     ptr::null_mut(),
                 ));
                 let cdata = &cd_values[..(cd_nelmts as _)];
-                let flt = Self::from_raw(filter_id, cdata)
-                    .ok()
-                    .unwrap_or_else(|| Self::user(filter_id, cdata));
+                let flt = Self::from_raw(filter_id, cdata)?;
                 filters.push(flt);
             }
             Ok(filters)
@@ -523,6 +526,8 @@ pub(crate) fn validate_filters(filters: &[Filter], type_class: H5T_class_t) -> R
     let mut comp_filter: Option<&Filter> = None;
 
     for filter in filters {
+        ensure!(filter.is_available(), "Filter not available: {:?}", filter);
+
         let id = filter.id();
 
         if let Some(f) = map.get(&id) {
@@ -563,3 +568,91 @@ pub(crate) fn validate_filters(filters: &[Filter], type_class: H5T_class_t) -> R
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use hdf5_sys::h5t::H5T_class_t;
+
+    use super::{
+        blosc_available, deflate_available, lzf_available, szip_available, validate_filters,
+        Filter, FilterInfo, SZip, ScaleOffset,
+    };
+    use crate::test::with_tmp_file;
+    use crate::{plist::DatasetCreate, Result};
+
+    #[test]
+    fn test_filter_pipeline() -> Result<()> {
+        let mut comp_filters = vec![];
+        if deflate_available() {
+            comp_filters.push(Filter::deflate(3));
+        }
+        if szip_available() {
+            comp_filters.push(Filter::szip(SZip::Entropy, 8));
+        }
+        assert_eq!(cfg!(feature = "lzf"), lzf_available());
+        #[cfg(feature = "lzf")]
+        {
+            comp_filters.push(Filter::lzf());
+        }
+        assert_eq!(cfg!(feature = "blosc"), blosc_available());
+        #[cfg(feature = "blosc")]
+        {
+            use super::BloscShuffle;
+            comp_filters.push(Filter::blosc_blosclz(1, false));
+            comp_filters.push(Filter::blosc_lz4(3, true));
+            comp_filters.push(Filter::blosc_lz4hc(5, BloscShuffle::Bit));
+            comp_filters.push(Filter::blosc_zlib(7, BloscShuffle::None));
+            comp_filters.push(Filter::blosc_zstd(9, BloscShuffle::Byte));
+            comp_filters.push(Filter::blosc_snappy(0, BloscShuffle::Bit));
+        }
+        for c in &comp_filters {
+            assert!(c.is_available());
+            assert!(c.encode_enabled());
+            assert!(c.decode_enabled());
+
+            let pipeline = vec![
+                Filter::nbit(),
+                Filter::shuffle(),
+                c.clone(),
+                Filter::fletcher32(),
+                Filter::scale_offset(ScaleOffset::Integer(3)),
+            ];
+            validate_filters(&pipeline, H5T_class_t::H5T_INTEGER)?;
+
+            let plist = DatasetCreate::try_new()?;
+            for flt in &pipeline {
+                flt.apply_to_plist(plist.id())?;
+            }
+            assert_eq!(Filter::extract_pipeline(plist.id())?, pipeline);
+
+            let mut b = DatasetCreate::build();
+            b.set_filters(&pipeline);
+            let plist = b.finish()?;
+            assert_eq!(Filter::extract_pipeline(plist.id())?, pipeline);
+
+            let res = with_tmp_file(|file| {
+                file.new_dataset_builder()
+                    .empty::<i32>()
+                    .shape((10_000, 20))
+                    .with_dcpl(|p| p.set_filters(&pipeline))
+                    .create("foo")
+                    .unwrap();
+                let plist = file.dataset("foo").unwrap().dcpl().unwrap();
+                Filter::extract_pipeline(plist.id()).unwrap()
+            });
+            assert_eq!(res, pipeline);
+        }
+
+        let bad_filter = Filter::user(12_345, &[1, 2, 3, 4, 5]);
+        assert_eq!(Filter::get_info(bad_filter.id()), FilterInfo::default());
+        assert!(!bad_filter.is_available());
+        assert!(!bad_filter.encode_enabled());
+        assert!(!bad_filter.decode_enabled());
+        assert_err!(
+            validate_filters(&[bad_filter], H5T_class_t::H5T_INTEGER),
+            "Filter not available"
+        );
+
+        Ok(())
+    }
+}

src/hl/filters/blosc.rs

@@ -56,7 +56,7 @@ pub fn register_blosc() -> Result<(), &'static str> {
 extern "C" fn set_local_blosc(dcpl_id: hid_t, type_id: hid_t, _space_id: hid_t) -> herr_t {
     const MAX_NDIMS: usize = 32;
     let mut flags: c_uint = 0;
-    let mut nelmts: size_t = 0;
+    let mut nelmts: size_t = 8;
     let mut values: Vec<c_uint> = vec![0; 8];
     let ret = unsafe {
         H5Pget_filter_by_id2(

src/test.rs

@@ -4,19 +4,19 @@ use tempfile::tempdir;
 
 use crate::internal_prelude::*;
 
-pub fn with_tmp_dir<F: Fn(PathBuf)>(func: F) {
+pub fn with_tmp_dir<T, F: Fn(PathBuf) -> T>(func: F) -> T {
     let dir = tempdir().unwrap();
     let path = dir.path().to_path_buf();
-    func(path);
+    func(path)
 }
 
-pub fn with_tmp_path<F: Fn(PathBuf)>(func: F) {
+pub fn with_tmp_path<T, F: Fn(PathBuf) -> T>(func: F) -> T {
     with_tmp_dir(|dir| func(dir.join("foo.h5")))
 }
 
-pub fn with_tmp_file<F: Fn(File)>(func: F) {
+pub fn with_tmp_file<T, F: Fn(File) -> T>(func: F) -> T {
     with_tmp_path(|path| {
         let file = File::create(&path).unwrap();
-        func(file);
+        func(file)
     })
 }