From 10b62203a35c9107f8b62e3026b4b31895554b7d Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Wed, 11 Apr 2012 15:57:10 -0500 Subject: [svn-r22277] Purpose: Fix for HDFFV-7993 - h5repack fails with error "chunk size must be <= maximum dimension size for fixed-sized dimensions" Description: Fixed a failure when change the chunk size of a specified chunked dataset with unlimited max dims. Also took care of converting to contiguous and compact from the dataset. Test cases were added and tagged with jira#. Tested: jam (linux32-LE), koala (linux64-LE), ostrich (linuxppc64-BE), tejeda (mac32-LE), linew (solaris-BE), Windows (32-LE cmake), Cmake (jam) --- MANIFEST | 1 + release_docs/RELEASE.txt | 2 + tools/h5repack/CMakeLists.txt | 31 ++++++-- tools/h5repack/h5repack.c | 9 ++- tools/h5repack/h5repack.h | 1 + tools/h5repack/h5repack.sh.in | 51 +++++++++++--- tools/h5repack/h5repack_copy.c | 46 ++++++++++-- tools/h5repack/h5repack_main.c | 2 + tools/h5repack/h5repack_opttable.c | 13 +++- tools/h5repack/h5repacktst.c | 102 +++++++++++++++++++++++++++ tools/h5repack/testfiles/h5repack_layout3.h5 | Bin 0 -> 966904 bytes 11 files changed, 235 insertions(+), 23 deletions(-) create mode 100644 tools/h5repack/testfiles/h5repack_layout3.h5 diff --git a/MANIFEST b/MANIFEST index a163532..48ba3d3 100644 --- a/MANIFEST +++ b/MANIFEST @@ -1944,6 +1944,7 @@ ./tools/h5repack/testfiles/h5repack_layout.h5 ./tools/h5repack/testfiles/h5repack_layouto.h5 ./tools/h5repack/testfiles/h5repack_layout2.h5 +./tools/h5repack/testfiles/h5repack_layout3.h5 ./tools/h5repack/testfiles/h5repack_early.h5 ./tools/h5repack/testfiles/h5repack_szip.h5 ./tools/h5repack/testfiles/h5repack_deflate.h5 diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 8761fb9..836d033 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -696,6 +696,8 @@ Bug Fixes since HDF5-1.8.0 release Tools ----- + - h5repack: Fixed a failure when change the chunk size of a specified + chunked dataset with unlimited max dims. HDFFV-7993 (JKM 2012/04/11) - h5diff: Fixed failure for comparing same named object with different object types in comparing groups. Prior to the fix, h5diff resulted in error. After the fix, h5diff detects such case as non-comparable diff --git a/tools/h5repack/CMakeLists.txt b/tools/h5repack/CMakeLists.txt index ae7a658..98d5c5a 100644 --- a/tools/h5repack/CMakeLists.txt +++ b/tools/h5repack/CMakeLists.txt @@ -96,6 +96,7 @@ IF (BUILD_TESTING) ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layout.h5 ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layouto.h5 ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layout2.h5 + ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_layout3.h5 ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_named_dtypes.h5 ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_nbit.h5 ${HDF5_TOOLS_H5REPACK_SOURCE_DIR}/testfiles/h5repack_objs.h5 @@ -322,10 +323,12 @@ IF (BUILD_TESTING) -E remove ./testfiles/h5repack_filters.h5.out ./testfiles/h5repack_filters.h5.out.err - ./testfiles/h5repack_layout2.h5-v.out - ./testfiles/h5repack_layout2.h5-v.out.err ./testfiles/h5repack_layout.h5-v.out ./testfiles/h5repack_layout.h5-v.out.err + ./testfiles/h5repack_layout2.h5-v.out + ./testfiles/h5repack_layout2.h5-v.out.err + ./testfiles/h5repack_layout3.h5-v.out + ./testfiles/h5repack_layout3.h5-v.out.err ./testfiles/out.tfamily%05d.h5 ./testfiles/out.h5diff_attr1.h5 ./testfiles/out.h5repack_attr.h5 @@ -336,9 +339,10 @@ IF (BUILD_TESTING) ./testfiles/out.h5repack_filters.h5 ./testfiles/out.h5repack_fletcher.h5 ./testfiles/out.h5repack_hlink.h5 - ./testfiles/out.h5repack_layout2.h5 ./testfiles/out.h5repack_layout.h5 ./testfiles/out.h5repack_layouto.h5 + ./testfiles/out.h5repack_layout2.h5 + ./testfiles/out.h5repack_layout3.h5 ./testfiles/out.h5repack_named_dtypes.h5 ./testfiles/out.h5repack_nbit.h5 ./testfiles/out.h5repack_objs.h5 @@ -369,9 +373,10 @@ IF (BUILD_TESTING) h5repack_fletcher_out.h5 h5repack_hlink.h5 h5repack_hlink_out.h5 - h5repack_layout2.h5 h5repack_layout.h5 h5repack_layout_out.h5 + h5repack_layout2.h5 + h5repack_layout3.h5 h5repack_named_dtypes.h5 h5repack_named_dtypes_out.h5 h5repack_nbit.h5 @@ -777,6 +782,24 @@ IF (BUILD_TESTING) ADD_H5_TEST (contig_small_fixed_compa "TEST" ${FILE18} -l chunked_small_fixed:COMPA) ADD_H5_VERIFY_TEST (contig_small_fixed_compa "TEST" 0 ${FILE18} chunked_small_fixed COMPACT) +#--------------------------------------------------------------------------- +# Test file contains chunked datasets (need multiple dsets) with +# unlimited max dims. (HDFFV-7933) +# Use first dset to test. +#--------------------------------------------------------------------------- +# chunk to chunk - specify chunk dim bigger than any current dim +ADD_H5_TEST (chunk2chunk "TEST" h5repack_layout3.h5 -l chunk_unlimit1:CHUNK=100x300) +ADD_H5_VERIFY_TEST (chunk2chunk "TEST" 0 h5repack_layout3.h5 chunk_unlimit1 CHUNK) + +# chunk to contiguous +ADD_H5_TEST (chunk2conti "TEST" h5repack_layout3.h5 -l chunk_unlimit1:CONTI) +ADD_H5_VERIFY_TEST (chunk2conti "TEST" 0 h5repack_layout3.h5 chunk_unlimit1 CONTI) + +# chunk to compact - convert big dataset (should be > 64k) for this purpose, +# should remain as original layout (chunk) +ADD_H5_TEST (chunk2compa "TEST" h5repack_layout3.h5 -l chunk_unlimit1:COMPA) +ADD_H5_VERIFY_TEST (chunk2compa "TEST" 0 h5repack_layout3.h5 chunk_unlimit1 CHUNK) + # Native option # Do not use FILE1, as the named dtype will be converted to native, and h5diff will # report a difference. diff --git a/tools/h5repack/h5repack.c b/tools/h5repack/h5repack.c index 9778fdd..292f4ec 100644 --- a/tools/h5repack/h5repack.c +++ b/tools/h5repack/h5repack.c @@ -206,10 +206,12 @@ int h5repack_addlayout(const char* str, if (obj_list==NULL) return -1; - /* set global layout option */ + /* set layout option */ + options->layout_g = pack.layout; + + /* no individual dataset specified */ if (options->all_layout==1 ) { - options->layout_g=pack.layout; if (pack.layout==H5D_CHUNKED) { /* -2 means the NONE option, remove chunking @@ -228,6 +230,7 @@ int h5repack_addlayout(const char* str, } } + /* individual dataset specified */ if (options->all_layout==0) options_add_layout(obj_list, n_objs, @@ -504,7 +507,7 @@ int copy_attr(hid_t loc_in, if (type_class == H5T_COMPOUND) { int nmembers = H5Tget_nmembers(wtype_id) ; for (j=0; j $layoutfile $GREP $expectlayout $layoutfile > /dev/null if [ $? -eq 0 ]; then @@ -260,7 +261,7 @@ VERIFY_LAYOUT_ALL() #--------------------------------- # check the layout from a dataset # check if the other layouts still exsit - VERIFY "Layout " + VERIFY "layouts" # if CONTIGUOUS if [ $expectlayout = "CONTIGUOUS" ]; then $RUNSERIAL $H5DUMP_BIN -pH $outfile > $layoutfile @@ -360,7 +361,8 @@ TOOLTEST1() rm -f $outfile } -# Call h5repack and compare output to a text file for -v option +# This is same as TOOLTEST() with comparing display output +# from -v option # TOOLTESTV() { @@ -374,10 +376,20 @@ TOOLTESTV() outfile=$TESTDIR/out.$1 shift $RUNSERIAL $H5REPACK_BIN "$@" $infile $outfile >$actual 2>$actual_err - cp $actual $actual_sav + RET=$? + if [ $RET != 0 ] ; then + echo "*FAILED*" + nerrors="`expr $nerrors + 1`" + else + echo " PASSED" + DIFFTEST $infile $outfile + fi + + # display output compare STDOUT_FILTER $actual cat $actual_err >> $actual + VERIFY output from $H5REPACK $@ if cmp -s $expect $actual; then echo " PASSED" else @@ -387,7 +399,8 @@ TOOLTESTV() test yes = "$verbose" && diff -c $expect $actual |sed 's/^/ /' fi - rm -f $actual $actual_err $actual_sav + rm -f $actual $actual_err + rm -f $outfile } # TOOLTEST_META: @@ -559,7 +572,8 @@ arg="h5repack_filters.h5 -v -f /dset_deflate:GZIP=9" if test $USE_FILTER_DEFLATE != "yes" ; then SKIP $arg else - TOOLTEST $arg + # compare output + TOOLTESTV $arg fi ########################################################### @@ -778,6 +792,24 @@ VERIFY_LAYOUT_DSET h5repack_layout2.h5 contig_small COMPACT TOOLTEST_MAIN h5repack_layout2.h5 -l chunked_small_fixed:COMPA VERIFY_LAYOUT_DSET h5repack_layout2.h5 chunked_small_fixed COMPACT +#--------------------------------------------------------------------------- +# Test file contains chunked datasets (need multiple dsets) with +# unlimited max dims. (HDFFV-7933) +# Use first dset to test. +#--------------------------------------------------------------------------- +# chunk to chunk - specify chunk dim bigger than any current dim +TOOLTEST_MAIN h5repack_layout3.h5 -l chunk_unlimit1:CHUNK=100x300 +VERIFY_LAYOUT_DSET h5repack_layout3.h5 chunk_unlimit1 CHUNK + +# chunk to contiguous +TOOLTEST_MAIN h5repack_layout3.h5 -l chunk_unlimit1:CONTI +VERIFY_LAYOUT_DSET h5repack_layout3.h5 chunk_unlimit1 CONTI + +# chunk to compact - convert big dataset (should be > 64k) for this purpose, +# should remain as original layout (chunk) +TOOLTEST_MAIN h5repack_layout3.h5 -l chunk_unlimit1:COMPA +VERIFY_LAYOUT_DSET h5repack_layout3.h5 chunk_unlimit1 CHUNK + # Native option # Do not use FILE1, as the named dtype will be converted to native, and h5diff will # report a difference. @@ -856,8 +888,7 @@ TOOLTEST h5repack_attr_refs.h5 # Note: this test is experimental for sharing test file among tools TOOLTEST h5diff_attr1.h5 -# tests for metadata block size option ('-M') -TOOLTEST_META h5repack_layout.h5 -M 8192 +# tests for metadata block size option TOOLTEST_META h5repack_layout.h5 --metadata_block_size=8192 if test $nerrors -eq 0 ; then diff --git a/tools/h5repack/h5repack_copy.c b/tools/h5repack/h5repack_copy.c index d38a15c..957d3c8 100644 --- a/tools/h5repack/h5repack_copy.c +++ b/tools/h5repack/h5repack_copy.c @@ -778,6 +778,8 @@ int do_copy_objects(hid_t fidin, unsigned u; int is_ref=0; htri_t is_named; + hbool_t limit_maxdims; + hsize_t size_dset; /*------------------------------------------------------------------------- @@ -794,8 +796,10 @@ int do_copy_objects(hid_t fidin, for ( i = 0; i < travt->nobjs; i++) { - + /* init variables per obj */ buf = NULL; + limit_maxdims = FALSE; + switch ( travt->objs[i].type ) { @@ -970,6 +974,9 @@ int do_copy_objects(hid_t fidin, if((msize = H5Tget_size(wtype_id)) == 0) goto error; + /* size of current dset */ + size_dset = nelmts * msize; + /*------------------------------------------------------------------------- * check if the dataset creation property list has filters that * are not registered in the current configuration @@ -999,7 +1006,7 @@ int do_copy_objects(hid_t fidin, */ if (options->layout_g != H5D_COMPACT) { - if ( nelmts*msize < options->min_comp ) + if ( size_dset < options->min_comp ) apply_s=0; } @@ -1016,9 +1023,38 @@ int do_copy_objects(hid_t fidin, goto error; } - /* unset the unlimimted dimensions, which cannot be applied to layout other than chunked. */ - if (options->layout_g != H5D_CHUNKED) { - H5Sset_extent_simple( f_space_id, rank, dims, NULL ); + /*------------------------------------------------- + * Unset the unlimited max dims if convert to other + * than chunk layouts, because unlimited max dims + * only can be applied to chunk layout. + * Also perform only for targeted dataset + * Also check for size limit to convert to compact + *-------------------------------------------------*/ + if (options->layout_g != H5D_CHUNKED) + { + /* any dataset is specified */ + if (options->op_tbl->nelems > 0) + { + /* if current obj match specified obj */ + if (options_get_object (travt->objs[i].name, options->op_tbl)) + limit_maxdims = TRUE; + } + else /* no dataset is specified */ + { + limit_maxdims = TRUE; + } + + /* if convert to COMPACT */ + if (options->layout_g == H5D_COMPACT) + { + /* should be smaller than 64K */ + if ( size_dset > MAX_COMPACT_DSIZE ) + limit_maxdims = FALSE; + } + + /* unset unlimited max dims */ + if (limit_maxdims) + H5Sset_extent_simple( f_space_id, rank, dims, NULL ); } /*------------------------------------------------------------------------- diff --git a/tools/h5repack/h5repack_main.c b/tools/h5repack/h5repack_main.c index 6004084..f61c961 100644 --- a/tools/h5repack/h5repack_main.c +++ b/tools/h5repack/h5repack_main.c @@ -466,6 +466,8 @@ void parse_command_line(int argc, const char **argv, pack_opt_t* options) options->fs_threshold = (hsize_t)HDatol( opt_arg ); break; + default: + break; } /* switch */ diff --git a/tools/h5repack/h5repack_opttable.c b/tools/h5repack/h5repack_opttable.c index d0fcff3..4ec7fcf 100644 --- a/tools/h5repack/h5repack_opttable.c +++ b/tools/h5repack/h5repack_opttable.c @@ -388,11 +388,22 @@ pack_info_t* options_get_object( const char *path, pack_opttbl_t *table ) { unsigned int i; + const char tbl_path[MAX_NC_NAME]; + for ( i = 0; i < table->nelems; i++) { + /* make full path (start with "/") to compare correctly */ + if (HDstrncmp(table->objs[i].path, "/", 1)) + { + HDstrcpy(tbl_path, "/"); + HDstrcat(tbl_path, table->objs[i].path); + } + else + HDstrcpy(tbl_path, table->objs[i].path); + /* found it */ - if (HDstrcmp(table->objs[i].path,path)==0) + if (HDstrcmp(tbl_path, path)==0) { return (&table->objs[i]); } diff --git a/tools/h5repack/h5repacktst.c b/tools/h5repack/h5repacktst.c index 6b15f0a..bc94eed 100644 --- a/tools/h5repack/h5repacktst.c +++ b/tools/h5repack/h5repacktst.c @@ -124,6 +124,7 @@ static int make_hlinks(hid_t loc_id); static int make_early(void); static int make_layout(hid_t loc_id); static int make_layout2(hid_t loc_id); +static int make_layout3(hid_t loc_id); #ifdef H5_HAVE_FILTER_SZIP static int make_szip(hid_t loc_id); #endif /* H5_HAVE_FILTER_SZIP */ @@ -1712,6 +1713,19 @@ int make_testfiles(void) return -1; /*------------------------------------------------------------------------- + * for test layout conversions form chunk with unlimited max dims + *------------------------------------------------------------------------- + */ + if((fid = H5Fcreate("h5repack_layout3.h5", H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0) + return -1; + + if(make_layout3(fid) < 0) + goto out; + + if(H5Fclose(fid) < 0) + return -1; + + /*------------------------------------------------------------------------- * create a file for the H5D_ALLOC_TIME_EARLY test *------------------------------------------------------------------------- */ @@ -3075,6 +3089,94 @@ out: } /* make_layout2() */ /*------------------------------------------------------------------------- +* Function: make_layout3 +* +* Purpose: make chunked datasets with unlimited max dim and chunk dim is +* bigger than current dim. (HDFFV-7933) +* Test for converting chunk to chunk , chunk to conti and chunk +* to compact. +* - The chunk to chunk changes layout bigger than any current dim +* again. +* - The chunk to compact test dataset bigger than 64K, should +* remain original layout.* +* +*------------------------------------------------------------------------- +*/ +#define DIM1_L3 300 +#define DIM2_L3 200 +static +int make_layout3(hid_t loc_id) +{ + hid_t dcpl=-1; /* dataset creation property list */ + hid_t sid=-1; /* dataspace ID */ + hsize_t dims[RANK]={DIM1_L3,DIM2_L3}; + hsize_t maxdims[RANK]={H5S_UNLIMITED, H5S_UNLIMITED}; + hsize_t chunk_dims[RANK]={DIM1_L3*2,5}; + int buf[DIM1_L3][DIM2_L3]; + int i, j, n; + + for (i=n=0; i