Skip to content

Commit

Permalink
Fix some bugs in the blosc filter wrapper
Browse files Browse the repository at this point in the history
re: Issue Unidata#2458

The above Github Issue revealed some bugs in the file netcdf-c/plugins/H5Zblosc.c. Fixed and added a testcase. Also discovered that the Blosc LZ sub-compressors do not work well with small datasets.

Misc. Other Change(s): I noticed that the file "dap4_test/baselinethredds/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds" is still causing tar errors during "make distcheck", so I made some changes to do rename at test-time.
  • Loading branch information
DennisHeimbigner committed Jul 12, 2022
1 parent 64033e3 commit 3623e17
Show file tree
Hide file tree
Showing 14 changed files with 222 additions and 113 deletions.
1 change: 1 addition & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release

## 4.9.1 - T.B.D.

* [Bug Fix] Fix blosc plugin errors. See [Github #????](https://github.com/Unidata/netcdf-c/pull/????).
* [Bug Fix] Fix quantize with CLASSIC_MODEL files. See [Github #2405](https://github.com/Unidata/netcdf-c/pull/2445).
* [Enhancement] Add `--disable-quantize` option to `configure`.
* [Bug Fix] Fix CMakeLists.txt to handle all acceptable boolean values for -DPLUGIN_INSTALL_DIR. See [Github #2430](https://github.com/Unidata/netcdf-c/pull/2430).
Expand Down
1 change: 0 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ AC_CONFIG_LINKS([nc_test4/ref_hdf5_compat3.nc:nc_test4/ref_hdf5_compat3.nc])

AC_CONFIG_LINKS([hdf4_test/ref_chunked.hdf4:hdf4_test/ref_chunked.hdf4])
AC_CONFIG_LINKS([hdf4_test/ref_contiguous.hdf4:hdf4_test/ref_contiguous.hdf4])
AC_CONFIG_LINKS([dap4_test/baselinethredds/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds:dap4_test/baselinethredds/GOES16_TEST1.nc4.thredds])
AM_INIT_AUTOMAKE([foreign dist-zip subdir-objects])

# Check for the existence of this file before proceeding.
Expand Down
3 changes: 2 additions & 1 deletion dap4_test/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ EXTRA_DIST = test_parse.sh test_meta.sh test_data.sh \
CLEANFILES = *.exe
# This should only be left behind if using parallel io
CLEANFILES += tmp_*

CLEANFILES += ${execdir}/baselinethredds/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds

DISTCLEANFILES = findtestserver4.c pingurl4.c

# One last thing
Expand Down
16 changes: 9 additions & 7 deletions dap4_test/baselinethredds/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@

# See netcdf-c/COPYRIGHT file for more info.


FILE(REMOVE ${CMAKE_CURRENT_SOURCE_DIR}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds)
FILE(GLOB COPY_FILES ${CMAKE_CURRENT_SOURCE_DIR}/*)
FILE(COPY ${COPY_FILES} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/ FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE)
#FILE(REMOVE ${CMAKE_CURRENT_SOURCE_DIR}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds)
#FILE(GLOB COPY_FILES ${CMAKE_CURRENT_SOURCE_DIR}/*)
#FILE(COPY ${COPY_FILES} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/ FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE)

##
# Rename file in support of https://github.com/Unidata/netcdf-c/issues/2077
##
IF(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds)
FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/GOES16_TEST1.nc4.thredds DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds)
ENDIF()
#IF(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds)
#FILE(COPY ${CMAKE_CURRENT_SOURCE_DIR}/GOES16_TEST1.nc4.thredds DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds)
#ENDIF()

FILE(GLOB COPY_FILES ${CMAKE_CURRENT_SOURCE_DIR}/*)
FILE(COPY ${COPY_FILES} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/ FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE)

FILE(GLOB CUR_EXTRA_DIST RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/*)
SET(CUR_EXTRA_DIST ${CUR_EXTRA_DIST} CMakeLists.txt)
Expand Down
10 changes: 10 additions & 0 deletions dap4_test/test_thredds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ echo "test_thredds.sh:"

FRAG="#checksummode=ignore"

# The name GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4 is too long during distcheck,
# so rename on the fly
GOESSHORT=${srcdir}/baselinethredds/GOES16_TEST1.nc4.thredds
GOESLONG=${BASELINETH}/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4.thredds
if ! test -f "${GOESLONG}" ; then
cp "${GOESSHORT}" "${GOESLONG}"
fi

F="\
harvey/goes16/CONUS/Channel01/20170821/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4?dap4.ce=y \
"
Expand Down Expand Up @@ -44,6 +52,8 @@ for f in $F ; do
fi
done

#rm -fr "${GOESLONG}"

echo "*** Pass"
exit 0

7 changes: 2 additions & 5 deletions libnczarr/zfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,10 @@ ncz_sync_netcdf4_file(NC_FILE_INFO_T* file, int isclose)
LOG((3, "%s", __func__));
ZTRACE(2,"file=%s",file->hdr.name);

/* If we're in define mode, that's an error, for strict nc3 rules,
* otherwise, end define mode. */
/* End depend mode if needed. (Error checking for classic mode has
* already happened). */
if (file->flags & NC_INDEF)
{
if (file->cmode & NC_CLASSIC_MODEL)
return NC_EINDEFINE;

/* Turn define mode off. */
file->flags ^= NC_INDEF;

Expand Down
4 changes: 1 addition & 3 deletions nc_test4/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ IF(USE_HDF5 AND ENABLE_FILTER_TESTING)
ADD_SH_TEST(nc_test4 tst_filter)
ADD_SH_TEST(nc_test4 tst_unknown)
ADD_SH_TEST(nc_test4 tst_specific_filters)
IF(ENABLE_CLIENTSIDE_FILTERS)
add_bin_test(nc_test4 test_filter_reg)
ENDIF(ENABLE_CLIENTSIDE_FILTERS)
ADD_SH_TEST(nc_test4 tst_bloscfail)
ENDIF(USE_HDF5 AND ENABLE_FILTER_TESTING)

ENDIF(BUILD_UTILITIES)
Expand Down
3 changes: 2 additions & 1 deletion nc_test4/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ check_PROGRAMS += tst_multifilter tst_filter_avail
TESTS += tst_filter.sh
TESTS += tst_unknown.sh
TESTS += tst_specific_filters.sh
TESTS += tst_bloscfail.sh
endif
endif # USE_HDF5
endif # BUILD_UTILITIES
Expand Down Expand Up @@ -112,7 +113,7 @@ ref_filter_repeat.txt ref_fillonly.cdl test_fillonly.sh \
ref_filter_order_create.txt ref_filter_order_read.txt \
ref_any.cdl tst_specific_filters.sh tst_unknown.sh \
tst_virtual_datasets.c noop1.cdl unknown.cdl \
tst_broken_files.c
tst_broken_files.c ref_bloscx.cdl tst_bloscfail.sh

# The tst_filterinstall test can only be run after an install
# occurred with --with-plugin-dir enabled. So there is no point
Expand Down
35 changes: 35 additions & 0 deletions nc_test4/ref_bloscx.cdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
netcdf x3 {
dimensions:
time = 10 ;
lat = 2 ;
lon = 4 ;
variables:
float three_dmn_rec_var(time, lat, lon) ;
three_dmn_rec_var:long_name = "three dimensional record variable" ;
three_dmn_rec_var:units = "watt meter-2" ;
three_dmn_rec_var:_FillValue = -99.f ;
data:

three_dmn_rec_var =
1, 2, 3, 4,
5, 6, 7, 8,
9, 10, 11, 12,
13, 14, 15, 16,
17, 18, 19, 20,
21, 22, 23, 24,
25, 26, 27, 28,
29, 30, 31, 32,
33, 34, 35, 36,
37, 38, 39, 40,
41, 42, 43, 44,
45, 46, 47, 48,
49, 50, 51, 52,
53, 54, 55, 56,
57, 58, 59, 60,
61, 62, 63, 64,
65, 66, 67, 68,
69, 70, 71, 72,
73, 74, 75, 76,
77, 78, 79, 80 ;

}
70 changes: 70 additions & 0 deletions nc_test4/tst_bloscfail.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#!/bin/bash

if test "x$srcdir" = x ; then srcdir=`pwd`; fi
. ../test_common.sh

set -e

# Load the findplugins function
. ${builddir}/findplugin.sh
echo "findplugin.sh loaded"

# Function to remove selected -s attributes from file;
# These attributes might be platform dependent
sclean() {
cat $1 \
| sed -e '/:_IsNetcdf4/d' \
| sed -e '/:_Endianness/d' \
| sed -e '/_NCProperties/d' \
| sed -e '/_SuperblockVersion/d' \
| cat > $2
}

# Function to extract _Filter attribute from a file
# These attributes might be platform dependent
getfilterattr() {
sed -e '/var.*:_Filter/p' -ed <$1 >$2
}

trimleft() {
sed -e 's/[ ]*\([^ ].*\)/\1/' <$1 >$2
}

if test "x$TESTNCZARR" = x1 ; then
. "$srcdir/test_nczarr.sh"
fi

if ! avail blosc; then echo "Blosc compressor not found"; exit 0; fi

# Locate the plugin dir and the library names; argument order is critical
# Find bzip2 and capture
findplugin h5blosc
BLOSCLIB="${HDF5_PLUGIN_LIB}"
BLOSCDIR="${HDF5_PLUGIN_DIR}/${BZIP2LIB}"

echo "final HDF5_PLUGIN_DIR=${HDF5_PLUGIN_DIR}"
export HDF5_PLUGIN_DIR
export HDF5_PLUGIN_PATH="$HDF5_PLUGIN_DIR"

localclean() {
rm -f ./tmp_bloscx3.nc ./tmp_bloscx4.nc ./tmp_bloscx4_fail.nc
}

# Execute the specified tests

echo "*** Testing dynamic filters using API"
localclean
${NCGEN} -3 -o tmp_bloscx3.nc ${srcdir}/ref_bloscx.cdl
# This should pass
${NCCOPY} -4 -V three_dmn_rec_var -F *,32001,0,0,0,0,1,1,0 ./tmp_bloscx3.nc ./tmp_bloscx4.nc
# This should fail because shuffle is off
if ${NCCOPY} -4 -V three_dmn_rec_var -F *,32001,0,0,0,0,1,0,0 ./tmp_bloscx3.nc ./tmp_bloscx4_fail.nc ; then
echo "*** not xfail: nccopy "
exit 1;
else
echo "*** xfail: nccopy "
fi

localclean

exit 0
8 changes: 2 additions & 6 deletions nc_test4/tst_specific_filters.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@
if test "x$srcdir" = x ; then srcdir=`pwd`; fi
. ../test_common.sh

set -x
set -e

if test "x$TESTNCZARR" = x1 ; then
. "$srcdir/test_nczarr.sh"
BLOSCARGS="32001,0,0,0,256,5,1,1"
BLOSCCODEC='[{\"id\": \"blosc\",\"clevel\": 5,\"blocksize\": 256,\"cname\": \"lz4\",\"shuffle\": 1}]'
else
BLOSCARGS="32001,2,2,4,256,5,1,1"
BLOSCCODEC='[{\"id\": \"blosc\",\"clevel\": 5,\"blocksize\": 256,\"cname\": \"lz4\",\"shuffle\": 1}]'
fi
BLOSCARGS="32001,0,0,4,256,5,1,1"
BLOSCCODEC='[{\"id\": \"blosc\",\"clevel\": 5,\"blocksize\": 256,\"cname\": \"lz4\",\"shuffle\": 1}]'

# Load the findplugins function
. ${builddir}/findplugin.sh
Expand Down
8 changes: 4 additions & 4 deletions nc_test4/tst_vars3.c
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ main(int argc, char **argv)
if (nc_close(ncid)) ERR;
}
SUMMARIZE_ERR;
#ifdef HAVE_SZ
#ifdef HAVE_H5Z_SZIP
printf("**** testing simple szip filter setup...");
{
int ncid;
Expand Down Expand Up @@ -748,9 +748,9 @@ main(int argc, char **argv)
params[1] = NC_SZIP_EC_BPP_IN; /* pixels_per_block */
if (nc_def_var_chunking(ncid, varid, NC_CHUNKED, NULL)) ERR;
{ int stat;
if ((stat = nc_def_var_filter(ncid, varid, H5_FILTER_SZIP, NUM_PARAMS_IN,
params)) != NC_ENOFILTER)
ERR;
stat = nc_def_var_filter(ncid, varid, H5_FILTER_SZIP, NUM_PARAMS_IN, params);
if(stat != NC_ENOFILTER)
ERR;
}
if (nc_def_var_szip(ncid, varid, NC_SZIP_NN,
NC_SZIP_EC_BPP_IN) != NC_ENOFILTER) ERR;
Expand Down
Loading

0 comments on commit 3623e17

Please sign in to comment.