From 58308e7e8300b237769d52b1bc7295f6b7402fd6 Mon Sep 17 00:00:00 2001 From: Aleksandar Jelenak Date: Tue, 9 Jun 2026 22:18:13 -0400 Subject: [PATCH 1/5] =?UTF-8?q?BitGroom/BitRound:=20skip=20=C2=B10.0=20and?= =?UTF-8?q?=20NaN;=20use=20HDF5=20fill-value=20terminology?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port netcdf-c PR #3093 -- all quantization loops in ccr_bgr() and ccr_gbr() now skip ±0.0 and NaN via (val != 0.0 && !isnan(val)), superseding the bit-pattern u*_ptr[idx] != 0U checks that only caught +0.0. The has_mss_val == 0 fallback now uses NaN as a no-op sentinel (NC_FILL_FLOAT/DOUBLE macros dropped). --- BITGROOM/src/H5Zbitgroom.c | 38 ++++++++++++++++++------------------ BITROUND/src/H5Zgranularbr.c | 26 ++++++++++++------------ 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/BITGROOM/src/H5Zbitgroom.c b/BITGROOM/src/H5Zbitgroom.c index 59859e9ca..2c81fa990 100644 --- a/BITGROOM/src/H5Zbitgroom.c +++ b/BITGROOM/src/H5Zbitgroom.c @@ -78,7 +78,7 @@ 2 /**< Ordinal position of missing value flag in parameter list (cd_params array) */ #define CCR_FLT_PRM_PSN_MSS_VAL \ 3 /**< Ordinal position of missing value in parameter list (cd_params array) \ - NB: Missing value (_FillValue) uses two cd_params slots so it can be single or double-precision. \ + NB: HDF5 dataset fill value uses two cd_params slots so it can be single or double-precision. \ Single-precision values are read as first 4-bytes starting at cd_params[4] \ (and cd_params[5] is ignored), while double-precision values are read as first 8-bytes starting \ at cd_params[4] and ending with cd_params[5]. */ @@ -91,12 +91,6 @@ #ifndef NC_DOUBLE #define NC_DOUBLE 6 #endif /* !NC_DOUBLE */ -#ifndef NC_FILL_FLOAT -#define NC_FILL_FLOAT (9.9692099683868690e+36f) /* near 15 * 2^119 */ -#endif /* !NC_FILL_FLOAT */ -#ifndef NC_FILL_DOUBLE -#define NC_FILL_DOUBLE (9.9692099683868690e+36) -#endif /* !NC_FILL_DOUBLE */ /* Minimum number of explicit significant bits to preserve when zeroing/bit-masking floating point values Codes will preserve at least two explicit bits, IEEE significant representation contains one implicit bit @@ -485,8 +479,10 @@ ccr_bgr(const int nsd, const int type, const size_t sz, const int has_mss_val, p double prc_bnr_xct; /* [nbr] Binary digits of precision, exact */ double mss_val_cmp_dbl; /* Missing value for comparison to double precision values */ + double val_dbl; /* [frc] Copy of input value to avoid indirection */ float mss_val_cmp_flt; /* Missing value for comparison to single precision values */ + float val_flt; /* [frc] Copy of input value to avoid indirection */ int bit_xpl_nbr_sgn = -1; /* [nbr] Number of explicit bits in significand */ int bit_xpl_nbr_zro; /* [nbr] Number of explicit bits to zero */ @@ -527,11 +523,13 @@ ccr_bgr(const int nsd, const int type, const size_t sz, const int has_mss_val, p switch (type) { case NC_FLOAT: - /* Missing value for comparison is _FillValue (if any) otherwise default NC_FILL_FLOAT/DOUBLE */ + /* Comparison value is the dataset's HDF5 fill value (when user-defined), + * otherwise NaN -- which compares unequal to every finite value, so the + * fill-value test is an inert no-op when none was set on the dataset. */ if (has_mss_val) mss_val_cmp_flt = *mss_val.fp; else - mss_val_cmp_flt = NC_FILL_FLOAT; + mss_val_cmp_flt = NAN; bit_xpl_nbr_sgn = bit_xpl_nbr_sgn_flt; bit_xpl_nbr_zro = bit_xpl_nbr_sgn - prc_bnr_xpl_rqr; if (bit_xpl_nbr_zro > bit_xpl_nbr_sgn - NCO_PPC_BIT_XPL_NBR_MIN) @@ -549,21 +547,23 @@ ccr_bgr(const int nsd, const int type, const size_t sz, const int has_mss_val, p msk_f32_u32_one = ~msk_f32_u32_zro; // msk_f32_u32_hshv=msk_f32_u32_one & (msk_f32_u32_zro >> 1); /* Set one bit: the MSB of LSBs */ - /* Bit-Groom: alternately shave and set LSBs */ + /* Bit-Groom: alternately shave and set LSBs. + * Do not quantize the dataset fill value, +/- zero, or NaN. */ for (idx = 0L; idx < sz; idx += 2L) - if (op1.fp[idx] != mss_val_cmp_flt) + if ((val_flt = op1.fp[idx]) != mss_val_cmp_flt && val_flt != 0.0f && !isnan(val_flt)) u32_ptr[idx] &= msk_f32_u32_zro; for (idx = 1L; idx < sz; idx += 2L) - if (op1.fp[idx] != mss_val_cmp_flt && - u32_ptr[idx] != 0U) /* Never quantize upwards floating point values of zero */ + if ((val_flt = op1.fp[idx]) != mss_val_cmp_flt && val_flt != 0.0f && !isnan(val_flt)) u32_ptr[idx] |= msk_f32_u32_one; break; case NC_DOUBLE: - /* Missing value for comparison is _FillValue (if any) otherwise default NC_FILL_FLOAT/DOUBLE */ + /* Comparison value is the dataset's HDF5 fill value (when user-defined), + * otherwise NaN -- which compares unequal to every finite value, so the + * fill-value test is an inert no-op when none was set on the dataset. */ if (has_mss_val) mss_val_cmp_dbl = *mss_val.dp; else - mss_val_cmp_dbl = NC_FILL_DOUBLE; + mss_val_cmp_dbl = NAN; bit_xpl_nbr_sgn = bit_xpl_nbr_sgn_dbl; bit_xpl_nbr_zro = bit_xpl_nbr_sgn - prc_bnr_xpl_rqr; if (bit_xpl_nbr_zro > bit_xpl_nbr_sgn - NCO_PPC_BIT_XPL_NBR_MIN) @@ -580,13 +580,13 @@ ccr_bgr(const int nsd, const int type, const size_t sz, const int has_mss_val, p /* Bit Set mask for OR: Put ones into bits to be set, zeros in untouched bits */ msk_f64_u64_one = ~msk_f64_u64_zro; // msk_f64_u64_hshv=msk_f64_u64_one & (msk_f64_u64_zro >> 1); /* Set one bit: the MSB of LSBs */ - /* Bit-Groom: alternately shave and set LSBs */ + /* Bit-Groom: alternately shave and set LSBs. + * Do not quantize the dataset fill value, +/- zero, or NaN. */ for (idx = 0L; idx < sz; idx += 2L) - if (op1.dp[idx] != mss_val_cmp_dbl) + if ((val_dbl = op1.dp[idx]) != mss_val_cmp_dbl && val_dbl != 0.0 && !isnan(val_dbl)) u64_ptr[idx] &= msk_f64_u64_zro; for (idx = 1L; idx < sz; idx += 2L) - if (op1.dp[idx] != mss_val_cmp_dbl && - u64_ptr[idx] != 0ULL) /* Never quantize upwards floating point values of zero */ + if ((val_dbl = op1.dp[idx]) != mss_val_cmp_dbl && val_dbl != 0.0 && !isnan(val_dbl)) u64_ptr[idx] |= msk_f64_u64_one; break; default: diff --git a/BITROUND/src/H5Zgranularbr.c b/BITROUND/src/H5Zgranularbr.c index 8e468da81..fd08b539e 100644 --- a/BITROUND/src/H5Zgranularbr.c +++ b/BITROUND/src/H5Zgranularbr.c @@ -75,7 +75,7 @@ 2 /**< Ordinal position of missing value flag in parameter list (cd_params array) */ #define CCR_FLT_PRM_PSN_MSS_VAL \ 3 /**< Ordinal position of missing value in parameter list (cd_params array) \ - NB: Missing value (_FillValue) uses two cd_params slots so it can be single or double-precision. \ + NB: HDF5 dataset fill value uses two cd_params slots so it can be single or double-precision. \ Single-precision values are read as first 4-bytes starting at cd_params[4] \ (and cd_params[5] is ignored), while double-precision values are read as first 8-bytes starting \ at cd_params[4] and ending with cd_params[5]. */ @@ -88,12 +88,6 @@ #ifndef NC_DOUBLE #define NC_DOUBLE 6 #endif /* !NC_DOUBLE */ -#ifndef NC_FILL_FLOAT -#define NC_FILL_FLOAT (9.9692099683868690e+36f) /* near 15 * 2^119 */ -#endif /* !NC_FILL_FLOAT */ -#ifndef NC_FILL_DOUBLE -#define NC_FILL_DOUBLE (9.9692099683868690e+36) -#endif /* !NC_FILL_DOUBLE */ /* Minimum number of explicit significant bits to preserve when zeroing/bit-masking floating point values Codes will preserve at least two explicit bits, IEEE significant representation contains one implicit bit @@ -636,15 +630,18 @@ ccr_gbr(const int nsd, const int type, const size_t sz, const int has_mss_val, p switch (type) { case NC_FLOAT: - /* Missing value for comparison is _FillValue (if any) otherwise default NC_FILL_FLOAT/DOUBLE */ + /* Comparison value is the dataset's HDF5 fill value (when user-defined), + * otherwise NaN -- which compares unequal to every finite value, so the + * fill-value test is an inert no-op when none was set on the dataset. */ if (has_mss_val) mss_val_cmp_flt = *mss_val.fp; else - mss_val_cmp_flt = NC_FILL_FLOAT; + mss_val_cmp_flt = NAN; bit_xpl_nbr_sgn = bit_xpl_nbr_sgn_flt; u32_ptr = op1.ui32p; + /* Do not quantize the dataset fill value, +/- zero, or NaN. */ for (idx = 0L; idx < sz; idx++) { - if ((val = op1.fp[idx]) != mss_val_cmp_flt && u32_ptr[idx] != 0U) { + if ((val = op1.fp[idx]) != mss_val_cmp_flt && val != 0.0 && !isnan(val)) { mnt = frexp(val, &xpn_bs2); /* DGG19 p. 4102 (8) */ mnt_fabs = fabs(mnt); /* dgt_nbr = floor(log10(|val|)) + 1, computed via deterministic @@ -681,16 +678,19 @@ ccr_gbr(const int nsd, const int type, const size_t sz, const int has_mss_val, p } break; case NC_DOUBLE: - /* Missing value for comparison is _FillValue (if any) otherwise default NC_FILL_FLOAT/DOUBLE */ + /* Comparison value is the dataset's HDF5 fill value (when user-defined), + * otherwise NaN -- which compares unequal to every finite value, so the + * fill-value test is an inert no-op when none was set on the dataset. */ if (has_mss_val) mss_val_cmp_dbl = *mss_val.dp; else - mss_val_cmp_dbl = NC_FILL_DOUBLE; + mss_val_cmp_dbl = NAN; bit_xpl_nbr_sgn = bit_xpl_nbr_sgn_dbl; u64_ptr = op1.ui64p; + /* Do not quantize the dataset fill value, +/- zero, or NaN. */ for (idx = 0L; idx < sz; idx++) { - if ((val = op1.dp[idx]) != mss_val_cmp_dbl && u64_ptr[idx] != 0U) { + if ((val = op1.dp[idx]) != mss_val_cmp_dbl && val != 0.0 && !isnan(val)) { mnt = frexp(val, &xpn_bs2); /* DGG19 p. 4102 (8) */ mnt_fabs = fabs(mnt); /* dgt_nbr = floor(log10(|val|)) + 1, computed via deterministic From 66e54054fc088ae06b333b2c60405fd5dda7ab76 Mon Sep 17 00:00:00 2001 From: Aleksandar Jelenak Date: Tue, 9 Jun 2026 23:09:34 -0400 Subject: [PATCH 2/5] =?UTF-8?q?BitGroom/BitRound:=20block=20=C2=B1Inf=20an?= =?UTF-8?q?d=20add=20special-value=20round-trip=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop bodies have well-defined behavior only for finite inputs: BitRound's per-value bit-shift becomes an out-of-range shift on ±Inf (undefined behavior), and BitGroom's loop turns ±Inf bit patterns into NaN by ORing low mantissa bits. Both guards now use isfinite(val) instead of !isnan(val) to block ±Inf in addition to NaN. New h5filter_specials_{bitround,bitgroom} round-trip tests write NaN, ±Inf, ±0.0, and normal values via H5Pset_filter, read back, and assert in C that the special values pass through unchanged while at least one normal value was quantized. --- BITGROOM/example/CMakeLists.txt | 47 ++++ BITGROOM/example/h5filter_specials_bitgroom.c | 228 ++++++++++++++++++ BITGROOM/src/H5Zbitgroom.c | 17 +- BITROUND/example/CMakeLists.txt | 46 ++++ BITROUND/example/h5filter_specials_bitround.c | 224 +++++++++++++++++ BITROUND/src/H5Zgranularbr.c | 25 +- 6 files changed, 568 insertions(+), 19 deletions(-) create mode 100644 BITGROOM/example/h5filter_specials_bitgroom.c create mode 100644 BITROUND/example/h5filter_specials_bitround.c diff --git a/BITGROOM/example/CMakeLists.txt b/BITGROOM/example/CMakeLists.txt index 2062fe2ff..ad4297f6d 100755 --- a/BITGROOM/example/CMakeLists.txt +++ b/BITGROOM/example/CMakeLists.txt @@ -43,6 +43,15 @@ if (H5PL_BUILD_TESTING) target_link_libraries (h5repack_floats_bitgroom PRIVATE dl) endif () + # Round-trip test for special floating-point values (NaN, +/- Inf, +/- zero). + # See the test definition lower in this file for the full description. + add_executable (h5filter_specials_bitgroom ${PROJECT_SOURCE_DIR}/h5filter_specials_bitgroom.c) + TARGET_C_PROPERTIES (h5filter_specials_bitgroom ${LIB_TYPE}) + target_link_libraries (h5filter_specials_bitgroom PRIVATE ${H5PL_HDF5_LINK_LIBS}) + if (NOT WIN32) + target_link_libraries (h5filter_specials_bitgroom PRIVATE dl) + endif () + macro (ADD_H5_TEST testname) add_test ( NAME ${testname}-clearall @@ -261,6 +270,44 @@ if (H5PL_BUILD_TESTING) if (NOT DISABLE_H5BITGROOM_ENCODER) #UD BITGROOM ADD_H5_UD_TEST (ud_convert 0 h5repack_floats.h5 --enable-error-stack -v -f UD=32022,0,5,3,4,0,0,0 -l CHUNK=4x8) + + # Special-value round-trip test: writes a small chunked dataset containing + # NaN, +/- Inf, +/- zero, plus normal values; applies the BitGroom filter + # via H5Pset_filter; reads back; and asserts in C that the special values + # are preserved bit-exact while at least one normal value was quantized. + # BitGroom alternates shave (AND) on even indices and set (OR) on odd + # indices; the special values straddle both. The verdict comes from the + # program's exit code; no h5dump comparison is involved, so cross-platform + # NaN/Inf printf formatting cannot affect the result. TESTLIBDIR is + # already set by ADD_H5_UD_TEST above. + add_test ( + NAME H5BITGROOM_UD-specials-roundtrip-clearall + COMMAND ${CMAKE_COMMAND} -E remove + h5filter_specials_bitgroom.h5 + h5filter_specials_bitgroom.out + h5filter_specials_bitgroom.out.err + ) + if (NOT "${last_test}" STREQUAL "") + set_tests_properties (H5BITGROOM_UD-specials-roundtrip-clearall PROPERTIES DEPENDS ${last_test}) + endif () + set (last_test "H5BITGROOM_UD-specials-roundtrip-clearall") + add_test ( + NAME H5BITGROOM_UD-specials-roundtrip + COMMAND "${CMAKE_COMMAND}" + -D "TEST_PROGRAM=$" + -D "TEST_FOLDER=${PROJECT_BINARY_DIR}" + -D "TEST_EXPECT=0" + -D "TEST_OUTPUT=h5filter_specials_bitgroom.out" + -D "TEST_SKIP_COMPARE=1" + -D "TEST_LIBRARY_DIRECTORY=${TESTLIBDIR}" + -D "TEST_ENV_VAR=HDF5_PLUGIN_PATH" + -D "TEST_ENV_VALUE=${CMAKE_BINARY_DIR}/plugins" + -P "${H5BITGROOM_RESOURCES_DIR}/runTest.cmake" + ) + set_tests_properties (H5BITGROOM_UD-specials-roundtrip PROPERTIES + WORKING_DIRECTORY "${PROJECT_BINARY_DIR}" + DEPENDS H5BITGROOM_UD-specials-roundtrip-clearall) + set (last_test "H5BITGROOM_UD-specials-roundtrip") endif () endif () diff --git a/BITGROOM/example/h5filter_specials_bitgroom.c b/BITGROOM/example/h5filter_specials_bitgroom.c new file mode 100644 index 000000000..c5628cecc --- /dev/null +++ b/BITGROOM/example/h5filter_specials_bitgroom.c @@ -0,0 +1,228 @@ +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * Copyright by The HDF Group. * + * All rights reserved. * + * * + * This file is part of the HDF5 BitGroom filter plugin source. * + * The full copyright notice, including terms governing use, modification, * + * and redistribution, is contained in the file COPYING, which can be found * + * at the root of the source code distribution tree. If you do not have * + * access to this file, you may request a copy from help@hdfgroup.org. * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + +/* + * Round-trip test for special floating-point values through the BitGroom + * filter. Verifies that NaN, +Inf, -Inf, -0.0, and +0.0 are passed through + * the filter unchanged (bit-identical) on both the float and double code + * paths, and that at least one normal value is quantized (proving the + * filter actually ran). The filter is loaded dynamically via HDF5's plugin + * mechanism (HDF5_PLUGIN_PATH); no h5dump comparison is involved so cross- + * platform NaN/Inf printf formatting cannot affect the result. + * + * BitGroom alternates shave (AND-mask) on even indices and set (OR-mask) + * on odd indices; the special values are placed at both even and odd + * positions to exercise both loops. + */ + +#include "hdf5.h" +#include +#include +#include + +#define FILENAME "h5filter_specials_bitgroom.h5" +#define DSET_F32 "f32" +#define DSET_F64 "f64" +#define N 16 +#define N_SPECIALS 5 +#define H5Z_FILTER_BITGROOM 32022 + +/* Build exact bit patterns via memcpy so the compiler cannot constant-fold + * away -0.0f or normalize a quiet NaN to a different payload. */ +static void +init_input_f32(float *buf, uint32_t *bits_out) +{ + const uint32_t specials[N_SPECIALS] = { + 0x7FC00000U, /* quiet NaN */ + 0x7F800000U, /* +Inf */ + 0xFF800000U, /* -Inf */ + 0x80000000U, /* -0.0 */ + 0x00000000U, /* +0.0 */ + }; + for (int i = 0; i < N_SPECIALS; i++) + memcpy(&buf[i], &specials[i], sizeof(float)); + for (int i = N_SPECIALS; i < N; i++) + buf[i] = ((float)i - 3.7f) * 0.137f; + memcpy(bits_out, buf, N * sizeof(uint32_t)); +} + +static void +init_input_f64(double *buf, uint64_t *bits_out) +{ + const uint64_t specials[N_SPECIALS] = { + 0x7FF8000000000000ULL, /* quiet NaN */ + 0x7FF0000000000000ULL, /* +Inf */ + 0xFFF0000000000000ULL, /* -Inf */ + 0x8000000000000000ULL, /* -0.0 */ + 0x0000000000000000ULL, /* +0.0 */ + }; + for (int i = 0; i < N_SPECIALS; i++) + memcpy(&buf[i], &specials[i], sizeof(double)); + for (int i = N_SPECIALS; i < N; i++) + buf[i] = ((double)i - 3.7) * 0.137; + memcpy(bits_out, buf, N * sizeof(uint64_t)); +} + +static int +roundtrip_f32(hid_t file_id) +{ + hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; + int ret = 1; + hsize_t dims[1] = {N}, chunk[1] = {N}; + /* NSD=3, sizeof(data)=4, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ + const unsigned int cd_values[5] = {3, 4, 0, 0, 0}; + float input[N], output[N]; + uint32_t input_bits[N], output_bits[N]; + int any_quantized = 0; + + init_input_f32(input, input_bits); + + if ((space_id = H5Screate_simple(1, dims, NULL)) < 0) + goto done; + if ((dcpl_id = H5Pcreate(H5P_DATASET_CREATE)) < 0) + goto done; + if (H5Pset_chunk(dcpl_id, 1, chunk) < 0) + goto done; + if (H5Pset_filter(dcpl_id, H5Z_FILTER_BITGROOM, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) + goto done; + if ((dset_id = H5Dcreate(file_id, DSET_F32, H5T_IEEE_F32LE, space_id, H5P_DEFAULT, dcpl_id, + H5P_DEFAULT)) < 0) + goto done; + if (H5Dwrite(dset_id, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) + goto done; + H5Dclose(dset_id); + dset_id = H5I_INVALID_HID; + + if ((dset_id = H5Dopen(file_id, DSET_F32, H5P_DEFAULT)) < 0) + goto done; + if (H5Dread(dset_id, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, output) < 0) + goto done; + memcpy(output_bits, output, sizeof(output_bits)); + + for (int i = 0; i < N_SPECIALS; i++) { + if (input_bits[i] != output_bits[i]) { + fprintf(stderr, "FAIL [f32, idx %d]: special value mutated: in=0x%08x out=0x%08x\n", i, + input_bits[i], output_bits[i]); + goto done; + } + } + for (int i = N_SPECIALS; i < N; i++) { + if (input_bits[i] != output_bits[i]) { + any_quantized = 1; + break; + } + } + if (!any_quantized) { + fprintf(stderr, "FAIL [f32]: no normal value was quantized; filter did not run\n"); + goto done; + } + + ret = 0; + +done: + if (dset_id >= 0) + H5Dclose(dset_id); + if (dcpl_id >= 0) + H5Pclose(dcpl_id); + if (space_id >= 0) + H5Sclose(space_id); + return ret; +} + +static int +roundtrip_f64(hid_t file_id) +{ + hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; + int ret = 1; + hsize_t dims[1] = {N}, chunk[1] = {N}; + /* NSD=3, sizeof(data)=8, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ + const unsigned int cd_values[5] = {3, 8, 0, 0, 0}; + double input[N], output[N]; + uint64_t input_bits[N], output_bits[N]; + int any_quantized = 0; + + init_input_f64(input, input_bits); + + if ((space_id = H5Screate_simple(1, dims, NULL)) < 0) + goto done; + if ((dcpl_id = H5Pcreate(H5P_DATASET_CREATE)) < 0) + goto done; + if (H5Pset_chunk(dcpl_id, 1, chunk) < 0) + goto done; + if (H5Pset_filter(dcpl_id, H5Z_FILTER_BITGROOM, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) + goto done; + if ((dset_id = H5Dcreate(file_id, DSET_F64, H5T_IEEE_F64LE, space_id, H5P_DEFAULT, dcpl_id, + H5P_DEFAULT)) < 0) + goto done; + if (H5Dwrite(dset_id, H5T_NATIVE_DOUBLE, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) + goto done; + H5Dclose(dset_id); + dset_id = H5I_INVALID_HID; + + if ((dset_id = H5Dopen(file_id, DSET_F64, H5P_DEFAULT)) < 0) + goto done; + if (H5Dread(dset_id, H5T_NATIVE_DOUBLE, H5S_ALL, H5S_ALL, H5P_DEFAULT, output) < 0) + goto done; + memcpy(output_bits, output, sizeof(output_bits)); + + for (int i = 0; i < N_SPECIALS; i++) { + if (input_bits[i] != output_bits[i]) { + fprintf(stderr, "FAIL [f64, idx %d]: special value mutated: in=0x%016llx out=0x%016llx\n", i, + (unsigned long long)input_bits[i], (unsigned long long)output_bits[i]); + goto done; + } + } + for (int i = N_SPECIALS; i < N; i++) { + if (input_bits[i] != output_bits[i]) { + any_quantized = 1; + break; + } + } + if (!any_quantized) { + fprintf(stderr, "FAIL [f64]: no normal value was quantized; filter did not run\n"); + goto done; + } + + ret = 0; + +done: + if (dset_id >= 0) + H5Dclose(dset_id); + if (dcpl_id >= 0) + H5Pclose(dcpl_id); + if (space_id >= 0) + H5Sclose(space_id); + return ret; +} + +int +main(void) +{ + hid_t file_id; + int r_f32 = 1, r_f64 = 1; + + file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + if (file_id < 0) { + fprintf(stderr, "FAIL: H5Fcreate(%s) failed\n", FILENAME); + return 1; + } + + r_f32 = roundtrip_f32(file_id); + r_f64 = roundtrip_f64(file_id); + + H5Fclose(file_id); + + if (r_f32 == 0 && r_f64 == 0) { + printf("PASS: BitGroom special-value round-trip (f32 + f64)\n"); + return 0; + } + return 1; +} diff --git a/BITGROOM/src/H5Zbitgroom.c b/BITGROOM/src/H5Zbitgroom.c index 2c81fa990..597a0e137 100644 --- a/BITGROOM/src/H5Zbitgroom.c +++ b/BITGROOM/src/H5Zbitgroom.c @@ -548,12 +548,16 @@ ccr_bgr(const int nsd, const int type, const size_t sz, const int has_mss_val, p // msk_f32_u32_hshv=msk_f32_u32_one & (msk_f32_u32_zro >> 1); /* Set one bit: the MSB of LSBs */ /* Bit-Groom: alternately shave and set LSBs. - * Do not quantize the dataset fill value, +/- zero, or NaN. */ + * Do not quantize the dataset fill value, +/- zero, NaN, or +/- Inf. + * isfinite(val_flt) covers !isnan(val_flt) and additionally blocks + * +/- Inf: the shave (AND) loop preserves Inf bit patterns, but the + * set (OR) loop turns +/- Inf (exponent all 1, mantissa 0) into a + * NaN (exponent all 1, mantissa non-zero) by ORing low mantissa bits. */ for (idx = 0L; idx < sz; idx += 2L) - if ((val_flt = op1.fp[idx]) != mss_val_cmp_flt && val_flt != 0.0f && !isnan(val_flt)) + if ((val_flt = op1.fp[idx]) != mss_val_cmp_flt && val_flt != 0.0f && isfinite(val_flt)) u32_ptr[idx] &= msk_f32_u32_zro; for (idx = 1L; idx < sz; idx += 2L) - if ((val_flt = op1.fp[idx]) != mss_val_cmp_flt && val_flt != 0.0f && !isnan(val_flt)) + if ((val_flt = op1.fp[idx]) != mss_val_cmp_flt && val_flt != 0.0f && isfinite(val_flt)) u32_ptr[idx] |= msk_f32_u32_one; break; case NC_DOUBLE: @@ -580,13 +584,12 @@ ccr_bgr(const int nsd, const int type, const size_t sz, const int has_mss_val, p /* Bit Set mask for OR: Put ones into bits to be set, zeros in untouched bits */ msk_f64_u64_one = ~msk_f64_u64_zro; // msk_f64_u64_hshv=msk_f64_u64_one & (msk_f64_u64_zro >> 1); /* Set one bit: the MSB of LSBs */ - /* Bit-Groom: alternately shave and set LSBs. - * Do not quantize the dataset fill value, +/- zero, or NaN. */ + /* See float branch above for why isfinite(val_dbl) is used instead of !isnan(val_dbl). */ for (idx = 0L; idx < sz; idx += 2L) - if ((val_dbl = op1.dp[idx]) != mss_val_cmp_dbl && val_dbl != 0.0 && !isnan(val_dbl)) + if ((val_dbl = op1.dp[idx]) != mss_val_cmp_dbl && val_dbl != 0.0 && isfinite(val_dbl)) u64_ptr[idx] &= msk_f64_u64_zro; for (idx = 1L; idx < sz; idx += 2L) - if ((val_dbl = op1.dp[idx]) != mss_val_cmp_dbl && val_dbl != 0.0 && !isnan(val_dbl)) + if ((val_dbl = op1.dp[idx]) != mss_val_cmp_dbl && val_dbl != 0.0 && isfinite(val_dbl)) u64_ptr[idx] |= msk_f64_u64_one; break; default: diff --git a/BITROUND/example/CMakeLists.txt b/BITROUND/example/CMakeLists.txt index 9be01cc55..c4e19c07f 100644 --- a/BITROUND/example/CMakeLists.txt +++ b/BITROUND/example/CMakeLists.txt @@ -43,6 +43,15 @@ if (H5PL_BUILD_TESTING) target_link_libraries (h5repack_floats_bitround PRIVATE dl) endif () + # Round-trip test for special floating-point values (NaN, +/- Inf, +/- zero). + # See the test definition lower in this file for the full description. + add_executable (h5filter_specials_bitround ${PROJECT_SOURCE_DIR}/h5filter_specials_bitround.c) + TARGET_C_PROPERTIES (h5filter_specials_bitround ${LIB_TYPE}) + target_link_libraries (h5filter_specials_bitround PRIVATE ${H5PL_HDF5_LINK_LIBS}) + if (NOT WIN32) + target_link_libraries (h5filter_specials_bitround PRIVATE dl) + endif () + macro (ADD_H5_TEST testname) add_test ( NAME ${testname}-clearall @@ -266,6 +275,43 @@ if (H5PL_BUILD_TESTING) if (NOT DISABLE_H5BITROUND_ENCODER) #UD BITROUND ADD_H5_UD_TEST (ud_convert 0 h5repack_floats.h5 --enable-error-stack -v -f UD=32023,0,5,3,4,0,0,0 -l CHUNK=4x8) + + # Special-value round-trip test: writes a small chunked dataset containing + # NaN, +/- Inf, +/- zero, plus normal values; applies the Granular BitRound + # filter via H5Pset_filter; reads back; and asserts in C that the special + # values are preserved bit-exact while at least one normal value was + # quantized. The verdict comes from the program's exit code; no h5dump + # comparison is involved, so cross-platform NaN/Inf printf formatting + # cannot affect the result. TESTLIBDIR is already set by ADD_H5_UD_TEST + # above. + add_test ( + NAME H5BITROUND_UD-specials-roundtrip-clearall + COMMAND ${CMAKE_COMMAND} -E remove + h5filter_specials_bitround.h5 + h5filter_specials_bitround.out + h5filter_specials_bitround.out.err + ) + if (NOT "${last_test}" STREQUAL "") + set_tests_properties (H5BITROUND_UD-specials-roundtrip-clearall PROPERTIES DEPENDS ${last_test}) + endif () + set (last_test "H5BITROUND_UD-specials-roundtrip-clearall") + add_test ( + NAME H5BITROUND_UD-specials-roundtrip + COMMAND "${CMAKE_COMMAND}" + -D "TEST_PROGRAM=$" + -D "TEST_FOLDER=${PROJECT_BINARY_DIR}" + -D "TEST_EXPECT=0" + -D "TEST_OUTPUT=h5filter_specials_bitround.out" + -D "TEST_SKIP_COMPARE=1" + -D "TEST_LIBRARY_DIRECTORY=${TESTLIBDIR}" + -D "TEST_ENV_VAR=HDF5_PLUGIN_PATH" + -D "TEST_ENV_VALUE=${CMAKE_BINARY_DIR}/plugins" + -P "${H5BITROUND_RESOURCES_DIR}/runTest.cmake" + ) + set_tests_properties (H5BITROUND_UD-specials-roundtrip PROPERTIES + WORKING_DIRECTORY "${PROJECT_BINARY_DIR}" + DEPENDS H5BITROUND_UD-specials-roundtrip-clearall) + set (last_test "H5BITROUND_UD-specials-roundtrip") endif () endif () diff --git a/BITROUND/example/h5filter_specials_bitround.c b/BITROUND/example/h5filter_specials_bitround.c new file mode 100644 index 000000000..4ebc9a76c --- /dev/null +++ b/BITROUND/example/h5filter_specials_bitround.c @@ -0,0 +1,224 @@ +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * Copyright by The HDF Group. * + * All rights reserved. * + * * + * This file is part of the HDF5 Granular BitRound filter plugin source. * + * The full copyright notice, including terms governing use, modification, * + * and redistribution, is contained in the file COPYING, which can be found * + * at the root of the source code distribution tree. If you do not have * + * access to this file, you may request a copy from help@hdfgroup.org. * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + +/* + * Round-trip test for special floating-point values through the Granular + * BitRound filter. Verifies that NaN, +Inf, -Inf, -0.0, and +0.0 are passed + * through the filter unchanged (bit-identical) on both the float and double + * code paths, and that at least one normal value is quantized (proving the + * filter actually ran). The filter is loaded dynamically via HDF5's plugin + * mechanism (HDF5_PLUGIN_PATH); no h5dump comparison is involved so cross- + * platform NaN/Inf printf formatting cannot affect the result. + */ + +#include "hdf5.h" +#include +#include +#include + +#define FILENAME "h5filter_specials_bitround.h5" +#define DSET_F32 "f32" +#define DSET_F64 "f64" +#define N 16 +#define N_SPECIALS 5 +#define H5Z_FILTER_GRANULARBR 32023 + +/* Build exact bit patterns via memcpy so the compiler cannot constant-fold + * away -0.0f or normalize a quiet NaN to a different payload. */ +static void +init_input_f32(float *buf, uint32_t *bits_out) +{ + const uint32_t specials[N_SPECIALS] = { + 0x7FC00000U, /* quiet NaN */ + 0x7F800000U, /* +Inf */ + 0xFF800000U, /* -Inf */ + 0x80000000U, /* -0.0 */ + 0x00000000U, /* +0.0 */ + }; + for (int i = 0; i < N_SPECIALS; i++) + memcpy(&buf[i], &specials[i], sizeof(float)); + for (int i = N_SPECIALS; i < N; i++) + buf[i] = ((float)i - 3.7f) * 0.137f; + memcpy(bits_out, buf, N * sizeof(uint32_t)); +} + +static void +init_input_f64(double *buf, uint64_t *bits_out) +{ + const uint64_t specials[N_SPECIALS] = { + 0x7FF8000000000000ULL, /* quiet NaN */ + 0x7FF0000000000000ULL, /* +Inf */ + 0xFFF0000000000000ULL, /* -Inf */ + 0x8000000000000000ULL, /* -0.0 */ + 0x0000000000000000ULL, /* +0.0 */ + }; + for (int i = 0; i < N_SPECIALS; i++) + memcpy(&buf[i], &specials[i], sizeof(double)); + for (int i = N_SPECIALS; i < N; i++) + buf[i] = ((double)i - 3.7) * 0.137; + memcpy(bits_out, buf, N * sizeof(uint64_t)); +} + +static int +roundtrip_f32(hid_t file_id) +{ + hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; + int ret = 1; + hsize_t dims[1] = {N}, chunk[1] = {N}; + /* NSD=3, sizeof(data)=4, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ + const unsigned int cd_values[5] = {3, 4, 0, 0, 0}; + float input[N], output[N]; + uint32_t input_bits[N], output_bits[N]; + int any_quantized = 0; + + init_input_f32(input, input_bits); + + if ((space_id = H5Screate_simple(1, dims, NULL)) < 0) + goto done; + if ((dcpl_id = H5Pcreate(H5P_DATASET_CREATE)) < 0) + goto done; + if (H5Pset_chunk(dcpl_id, 1, chunk) < 0) + goto done; + if (H5Pset_filter(dcpl_id, H5Z_FILTER_GRANULARBR, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) + goto done; + if ((dset_id = H5Dcreate(file_id, DSET_F32, H5T_IEEE_F32LE, space_id, H5P_DEFAULT, dcpl_id, + H5P_DEFAULT)) < 0) + goto done; + if (H5Dwrite(dset_id, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) + goto done; + H5Dclose(dset_id); + dset_id = H5I_INVALID_HID; + + if ((dset_id = H5Dopen(file_id, DSET_F32, H5P_DEFAULT)) < 0) + goto done; + if (H5Dread(dset_id, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, output) < 0) + goto done; + memcpy(output_bits, output, sizeof(output_bits)); + + for (int i = 0; i < N_SPECIALS; i++) { + if (input_bits[i] != output_bits[i]) { + fprintf(stderr, "FAIL [f32, idx %d]: special value mutated: in=0x%08x out=0x%08x\n", i, + input_bits[i], output_bits[i]); + goto done; + } + } + for (int i = N_SPECIALS; i < N; i++) { + if (input_bits[i] != output_bits[i]) { + any_quantized = 1; + break; + } + } + if (!any_quantized) { + fprintf(stderr, "FAIL [f32]: no normal value was quantized; filter did not run\n"); + goto done; + } + + ret = 0; + +done: + if (dset_id >= 0) + H5Dclose(dset_id); + if (dcpl_id >= 0) + H5Pclose(dcpl_id); + if (space_id >= 0) + H5Sclose(space_id); + return ret; +} + +static int +roundtrip_f64(hid_t file_id) +{ + hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; + int ret = 1; + hsize_t dims[1] = {N}, chunk[1] = {N}; + /* NSD=3, sizeof(data)=8, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ + const unsigned int cd_values[5] = {3, 8, 0, 0, 0}; + double input[N], output[N]; + uint64_t input_bits[N], output_bits[N]; + int any_quantized = 0; + + init_input_f64(input, input_bits); + + if ((space_id = H5Screate_simple(1, dims, NULL)) < 0) + goto done; + if ((dcpl_id = H5Pcreate(H5P_DATASET_CREATE)) < 0) + goto done; + if (H5Pset_chunk(dcpl_id, 1, chunk) < 0) + goto done; + if (H5Pset_filter(dcpl_id, H5Z_FILTER_GRANULARBR, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) + goto done; + if ((dset_id = H5Dcreate(file_id, DSET_F64, H5T_IEEE_F64LE, space_id, H5P_DEFAULT, dcpl_id, + H5P_DEFAULT)) < 0) + goto done; + if (H5Dwrite(dset_id, H5T_NATIVE_DOUBLE, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) + goto done; + H5Dclose(dset_id); + dset_id = H5I_INVALID_HID; + + if ((dset_id = H5Dopen(file_id, DSET_F64, H5P_DEFAULT)) < 0) + goto done; + if (H5Dread(dset_id, H5T_NATIVE_DOUBLE, H5S_ALL, H5S_ALL, H5P_DEFAULT, output) < 0) + goto done; + memcpy(output_bits, output, sizeof(output_bits)); + + for (int i = 0; i < N_SPECIALS; i++) { + if (input_bits[i] != output_bits[i]) { + fprintf(stderr, "FAIL [f64, idx %d]: special value mutated: in=0x%016llx out=0x%016llx\n", i, + (unsigned long long)input_bits[i], (unsigned long long)output_bits[i]); + goto done; + } + } + for (int i = N_SPECIALS; i < N; i++) { + if (input_bits[i] != output_bits[i]) { + any_quantized = 1; + break; + } + } + if (!any_quantized) { + fprintf(stderr, "FAIL [f64]: no normal value was quantized; filter did not run\n"); + goto done; + } + + ret = 0; + +done: + if (dset_id >= 0) + H5Dclose(dset_id); + if (dcpl_id >= 0) + H5Pclose(dcpl_id); + if (space_id >= 0) + H5Sclose(space_id); + return ret; +} + +int +main(void) +{ + hid_t file_id; + int r_f32 = 1, r_f64 = 1; + + file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + if (file_id < 0) { + fprintf(stderr, "FAIL: H5Fcreate(%s) failed\n", FILENAME); + return 1; + } + + r_f32 = roundtrip_f32(file_id); + r_f64 = roundtrip_f64(file_id); + + H5Fclose(file_id); + + if (r_f32 == 0 && r_f64 == 0) { + printf("PASS: BitRound special-value round-trip (f32 + f64)\n"); + return 0; + } + return 1; +} diff --git a/BITROUND/src/H5Zgranularbr.c b/BITROUND/src/H5Zgranularbr.c index fd08b539e..37de1def7 100644 --- a/BITROUND/src/H5Zgranularbr.c +++ b/BITROUND/src/H5Zgranularbr.c @@ -639,9 +639,14 @@ ccr_gbr(const int nsd, const int type, const size_t sz, const int has_mss_val, p mss_val_cmp_flt = NAN; bit_xpl_nbr_sgn = bit_xpl_nbr_sgn_flt; u32_ptr = op1.ui32p; - /* Do not quantize the dataset fill value, +/- zero, or NaN. */ + /* Do not quantize the dataset fill value, +/- zero, NaN, or +/- Inf. + * isfinite(val) covers !isnan(val) and additionally blocks +/- Inf, + * which would otherwise reach the bit-shift below with an out-of-range + * shift count (undefined behavior). netcdf-c upstream does not gate Inf; + * we diverge here because the standalone filter's loop body computes + * per-value bit masks that Inf inputs corrupt. */ for (idx = 0L; idx < sz; idx++) { - if ((val = op1.fp[idx]) != mss_val_cmp_flt && val != 0.0 && !isnan(val)) { + if ((val = op1.fp[idx]) != mss_val_cmp_flt && val != 0.0 && isfinite(val)) { mnt = frexp(val, &xpn_bs2); /* DGG19 p. 4102 (8) */ mnt_fabs = fabs(mnt); /* dgt_nbr = floor(log10(|val|)) + 1, computed via deterministic @@ -654,10 +659,8 @@ ccr_gbr(const int nsd, const int type, const size_t sz, const int has_mss_val, p * exactly 0.5 (val is an exact power of 2). This integer-only * formulation is bit-deterministic across platforms; the * original libm-based form is not. */ - prc_bnr_xpl_rqr = (mnt_fabs == 0.0) - ? 0 - : (unsigned short)abs(((mnt_fabs == 0.5) ? xpn_bs2 + 1 : xpn_bs2) - - qnt_pwr); /* Protect against mnt = -0.0 */ + prc_bnr_xpl_rqr = + (unsigned short)abs(((mnt_fabs == 0.5) ? xpn_bs2 + 1 : xpn_bs2) - qnt_pwr); prc_bnr_xpl_rqr--; /* 20211003 Reduce formula result by 1 bit: Passes all tests, improves CR by ~10% */ @@ -688,9 +691,9 @@ ccr_gbr(const int nsd, const int type, const size_t sz, const int has_mss_val, p bit_xpl_nbr_sgn = bit_xpl_nbr_sgn_dbl; u64_ptr = op1.ui64p; - /* Do not quantize the dataset fill value, +/- zero, or NaN. */ + /* See float branch above for why isfinite(val) is used instead of !isnan(val). */ for (idx = 0L; idx < sz; idx++) { - if ((val = op1.dp[idx]) != mss_val_cmp_dbl && val != 0.0 && !isnan(val)) { + if ((val = op1.dp[idx]) != mss_val_cmp_dbl && val != 0.0 && isfinite(val)) { mnt = frexp(val, &xpn_bs2); /* DGG19 p. 4102 (8) */ mnt_fabs = fabs(mnt); /* dgt_nbr = floor(log10(|val|)) + 1, computed via deterministic @@ -703,10 +706,8 @@ ccr_gbr(const int nsd, const int type, const size_t sz, const int has_mss_val, p * exactly 0.5 (val is an exact power of 2). This integer-only * formulation is bit-deterministic across platforms; the * original libm-based form is not. */ - prc_bnr_xpl_rqr = (mnt_fabs == 0.0) - ? 0 - : (unsigned short)abs(((mnt_fabs == 0.5) ? xpn_bs2 + 1 : xpn_bs2) - - qnt_pwr); /* Protect against mnt = -0.0 */ + prc_bnr_xpl_rqr = + (unsigned short)abs(((mnt_fabs == 0.5) ? xpn_bs2 + 1 : xpn_bs2) - qnt_pwr); prc_bnr_xpl_rqr--; /* 20211003 Reduce formula result by 1 bit: Passes all tests, improves CR by ~10% */ From 9b9871365c864878be79958bc2ac25cdf38f0f80 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 10 Jun 2026 15:51:55 +0000 Subject: [PATCH 3/5] Committing clang-format changes --- BITGROOM/example/h5filter_specials_bitgroom.c | 12 ++++++------ BITGROOM/src/H5Zbitgroom.c | 2 +- BITROUND/example/h5filter_specials_bitround.c | 12 ++++++------ BITROUND/src/H5Zgranularbr.c | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/BITGROOM/example/h5filter_specials_bitgroom.c b/BITGROOM/example/h5filter_specials_bitgroom.c index c5628cecc..ca87770cb 100644 --- a/BITGROOM/example/h5filter_specials_bitgroom.c +++ b/BITGROOM/example/h5filter_specials_bitgroom.c @@ -75,7 +75,7 @@ static int roundtrip_f32(hid_t file_id) { hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; - int ret = 1; + int ret = 1; hsize_t dims[1] = {N}, chunk[1] = {N}; /* NSD=3, sizeof(data)=4, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ const unsigned int cd_values[5] = {3, 4, 0, 0, 0}; @@ -93,8 +93,8 @@ roundtrip_f32(hid_t file_id) goto done; if (H5Pset_filter(dcpl_id, H5Z_FILTER_BITGROOM, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) goto done; - if ((dset_id = H5Dcreate(file_id, DSET_F32, H5T_IEEE_F32LE, space_id, H5P_DEFAULT, dcpl_id, - H5P_DEFAULT)) < 0) + if ((dset_id = + H5Dcreate(file_id, DSET_F32, H5T_IEEE_F32LE, space_id, H5P_DEFAULT, dcpl_id, H5P_DEFAULT)) < 0) goto done; if (H5Dwrite(dset_id, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) goto done; @@ -141,7 +141,7 @@ static int roundtrip_f64(hid_t file_id) { hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; - int ret = 1; + int ret = 1; hsize_t dims[1] = {N}, chunk[1] = {N}; /* NSD=3, sizeof(data)=8, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ const unsigned int cd_values[5] = {3, 8, 0, 0, 0}; @@ -159,8 +159,8 @@ roundtrip_f64(hid_t file_id) goto done; if (H5Pset_filter(dcpl_id, H5Z_FILTER_BITGROOM, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) goto done; - if ((dset_id = H5Dcreate(file_id, DSET_F64, H5T_IEEE_F64LE, space_id, H5P_DEFAULT, dcpl_id, - H5P_DEFAULT)) < 0) + if ((dset_id = + H5Dcreate(file_id, DSET_F64, H5T_IEEE_F64LE, space_id, H5P_DEFAULT, dcpl_id, H5P_DEFAULT)) < 0) goto done; if (H5Dwrite(dset_id, H5T_NATIVE_DOUBLE, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) goto done; diff --git a/BITGROOM/src/H5Zbitgroom.c b/BITGROOM/src/H5Zbitgroom.c index 597a0e137..a4f773c0f 100644 --- a/BITGROOM/src/H5Zbitgroom.c +++ b/BITGROOM/src/H5Zbitgroom.c @@ -78,7 +78,7 @@ 2 /**< Ordinal position of missing value flag in parameter list (cd_params array) */ #define CCR_FLT_PRM_PSN_MSS_VAL \ 3 /**< Ordinal position of missing value in parameter list (cd_params array) \ - NB: HDF5 dataset fill value uses two cd_params slots so it can be single or double-precision. \ + NB: HDF5 dataset fill value uses two cd_params slots so it can be single or double-precision. \ Single-precision values are read as first 4-bytes starting at cd_params[4] \ (and cd_params[5] is ignored), while double-precision values are read as first 8-bytes starting \ at cd_params[4] and ending with cd_params[5]. */ diff --git a/BITROUND/example/h5filter_specials_bitround.c b/BITROUND/example/h5filter_specials_bitround.c index 4ebc9a76c..2e0d8384e 100644 --- a/BITROUND/example/h5filter_specials_bitround.c +++ b/BITROUND/example/h5filter_specials_bitround.c @@ -71,7 +71,7 @@ static int roundtrip_f32(hid_t file_id) { hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; - int ret = 1; + int ret = 1; hsize_t dims[1] = {N}, chunk[1] = {N}; /* NSD=3, sizeof(data)=4, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ const unsigned int cd_values[5] = {3, 4, 0, 0, 0}; @@ -89,8 +89,8 @@ roundtrip_f32(hid_t file_id) goto done; if (H5Pset_filter(dcpl_id, H5Z_FILTER_GRANULARBR, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) goto done; - if ((dset_id = H5Dcreate(file_id, DSET_F32, H5T_IEEE_F32LE, space_id, H5P_DEFAULT, dcpl_id, - H5P_DEFAULT)) < 0) + if ((dset_id = + H5Dcreate(file_id, DSET_F32, H5T_IEEE_F32LE, space_id, H5P_DEFAULT, dcpl_id, H5P_DEFAULT)) < 0) goto done; if (H5Dwrite(dset_id, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) goto done; @@ -137,7 +137,7 @@ static int roundtrip_f64(hid_t file_id) { hid_t space_id = H5I_INVALID_HID, dcpl_id = H5I_INVALID_HID, dset_id = H5I_INVALID_HID; - int ret = 1; + int ret = 1; hsize_t dims[1] = {N}, chunk[1] = {N}; /* NSD=3, sizeof(data)=8, has_mss_val=0, mss_val_byt_1to4=0, mss_val_byt_5to8=0 */ const unsigned int cd_values[5] = {3, 8, 0, 0, 0}; @@ -155,8 +155,8 @@ roundtrip_f64(hid_t file_id) goto done; if (H5Pset_filter(dcpl_id, H5Z_FILTER_GRANULARBR, H5Z_FLAG_MANDATORY, 5, cd_values) < 0) goto done; - if ((dset_id = H5Dcreate(file_id, DSET_F64, H5T_IEEE_F64LE, space_id, H5P_DEFAULT, dcpl_id, - H5P_DEFAULT)) < 0) + if ((dset_id = + H5Dcreate(file_id, DSET_F64, H5T_IEEE_F64LE, space_id, H5P_DEFAULT, dcpl_id, H5P_DEFAULT)) < 0) goto done; if (H5Dwrite(dset_id, H5T_NATIVE_DOUBLE, H5S_ALL, H5S_ALL, H5P_DEFAULT, input) < 0) goto done; diff --git a/BITROUND/src/H5Zgranularbr.c b/BITROUND/src/H5Zgranularbr.c index 37de1def7..fa950760d 100644 --- a/BITROUND/src/H5Zgranularbr.c +++ b/BITROUND/src/H5Zgranularbr.c @@ -75,7 +75,7 @@ 2 /**< Ordinal position of missing value flag in parameter list (cd_params array) */ #define CCR_FLT_PRM_PSN_MSS_VAL \ 3 /**< Ordinal position of missing value in parameter list (cd_params array) \ - NB: HDF5 dataset fill value uses two cd_params slots so it can be single or double-precision. \ + NB: HDF5 dataset fill value uses two cd_params slots so it can be single or double-precision. \ Single-precision values are read as first 4-bytes starting at cd_params[4] \ (and cd_params[5] is ignored), while double-precision values are read as first 8-bytes starting \ at cd_params[4] and ending with cd_params[5]. */ From 872ba1ba9b7b9730ea625358a872f9f57e6095ab Mon Sep 17 00:00:00 2001 From: Aleksandar Jelenak Date: Tue, 23 Jun 2026 17:05:08 -0500 Subject: [PATCH 4/5] Fixes after review --- BITGROOM/example/CMakeLists.txt | 16 ++++++------- BITGROOM/example/h5filter_specials_bitgroom.c | 24 +++++++++---------- BITROUND/example/h5filter_specials_bitround.c | 24 +++++++++---------- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/BITGROOM/example/CMakeLists.txt b/BITGROOM/example/CMakeLists.txt index ad4297f6d..ebe945713 100755 --- a/BITGROOM/example/CMakeLists.txt +++ b/BITGROOM/example/CMakeLists.txt @@ -44,7 +44,7 @@ if (H5PL_BUILD_TESTING) endif () # Round-trip test for special floating-point values (NaN, +/- Inf, +/- zero). - # See the test definition lower in this file for the full description. + # See the test definition below for the full description. add_executable (h5filter_specials_bitgroom ${PROJECT_SOURCE_DIR}/h5filter_specials_bitgroom.c) TARGET_C_PROPERTIES (h5filter_specials_bitgroom ${LIB_TYPE}) target_link_libraries (h5filter_specials_bitgroom PRIVATE ${H5PL_HDF5_LINK_LIBS}) @@ -272,14 +272,12 @@ if (H5PL_BUILD_TESTING) ADD_H5_UD_TEST (ud_convert 0 h5repack_floats.h5 --enable-error-stack -v -f UD=32022,0,5,3,4,0,0,0 -l CHUNK=4x8) # Special-value round-trip test: writes a small chunked dataset containing - # NaN, +/- Inf, +/- zero, plus normal values; applies the BitGroom filter - # via H5Pset_filter; reads back; and asserts in C that the special values - # are preserved bit-exact while at least one normal value was quantized. - # BitGroom alternates shave (AND) on even indices and set (OR) on odd - # indices; the special values straddle both. The verdict comes from the - # program's exit code; no h5dump comparison is involved, so cross-platform - # NaN/Inf printf formatting cannot affect the result. TESTLIBDIR is - # already set by ADD_H5_UD_TEST above. + # NaN, +/- Inf, +/- zero, plus normal values, then applies the BitGroom + # filter and reads the data back asserting that the special values are + # preserved bit-exact while at least one normal value was quantized. The + # verdict comes from the program's exit code; no h5dump comparison is + # involved, so cross-platform NaN/Inf printf formatting cannot affect the + # result. TESTLIBDIR is already set by ADD_H5_UD_TEST above. add_test ( NAME H5BITGROOM_UD-specials-roundtrip-clearall COMMAND ${CMAKE_COMMAND} -E remove diff --git a/BITGROOM/example/h5filter_specials_bitgroom.c b/BITGROOM/example/h5filter_specials_bitgroom.c index ca87770cb..47dbb0e54 100644 --- a/BITGROOM/example/h5filter_specials_bitgroom.c +++ b/BITGROOM/example/h5filter_specials_bitgroom.c @@ -9,6 +9,11 @@ * access to this file, you may request a copy from help@hdfgroup.org. * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ +#include "hdf5.h" +#include +#include +#include + /* * Round-trip test for special floating-point values through the BitGroom * filter. Verifies that NaN, +Inf, -Inf, -0.0, and +0.0 are passed through @@ -23,11 +28,6 @@ * positions to exercise both loops. */ -#include "hdf5.h" -#include -#include -#include - #define FILENAME "h5filter_specials_bitgroom.h5" #define DSET_F32 "f32" #define DSET_F64 "f64" @@ -128,11 +128,11 @@ roundtrip_f32(hid_t file_id) ret = 0; done: - if (dset_id >= 0) + if (dset_id > 0) H5Dclose(dset_id); - if (dcpl_id >= 0) + if (dcpl_id > 0) H5Pclose(dcpl_id); - if (space_id >= 0) + if (space_id > 0) H5Sclose(space_id); return ret; } @@ -194,11 +194,11 @@ roundtrip_f64(hid_t file_id) ret = 0; done: - if (dset_id >= 0) + if (dset_id > 0) H5Dclose(dset_id); - if (dcpl_id >= 0) + if (dcpl_id > 0) H5Pclose(dcpl_id); - if (space_id >= 0) + if (space_id > 0) H5Sclose(space_id); return ret; } @@ -210,7 +210,7 @@ main(void) int r_f32 = 1, r_f64 = 1; file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); - if (file_id < 0) { + if (file_id == H5I_INVALID_HID) { fprintf(stderr, "FAIL: H5Fcreate(%s) failed\n", FILENAME); return 1; } diff --git a/BITROUND/example/h5filter_specials_bitround.c b/BITROUND/example/h5filter_specials_bitround.c index 2e0d8384e..0bb569ef9 100644 --- a/BITROUND/example/h5filter_specials_bitround.c +++ b/BITROUND/example/h5filter_specials_bitround.c @@ -9,6 +9,11 @@ * access to this file, you may request a copy from help@hdfgroup.org. * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ +#include "hdf5.h" +#include +#include +#include + /* * Round-trip test for special floating-point values through the Granular * BitRound filter. Verifies that NaN, +Inf, -Inf, -0.0, and +0.0 are passed @@ -19,11 +24,6 @@ * platform NaN/Inf printf formatting cannot affect the result. */ -#include "hdf5.h" -#include -#include -#include - #define FILENAME "h5filter_specials_bitround.h5" #define DSET_F32 "f32" #define DSET_F64 "f64" @@ -124,11 +124,11 @@ roundtrip_f32(hid_t file_id) ret = 0; done: - if (dset_id >= 0) + if (dset_id > 0) H5Dclose(dset_id); - if (dcpl_id >= 0) + if (dcpl_id > 0) H5Pclose(dcpl_id); - if (space_id >= 0) + if (space_id > 0) H5Sclose(space_id); return ret; } @@ -190,11 +190,11 @@ roundtrip_f64(hid_t file_id) ret = 0; done: - if (dset_id >= 0) + if (dset_id > 0) H5Dclose(dset_id); - if (dcpl_id >= 0) + if (dcpl_id > 0) H5Pclose(dcpl_id); - if (space_id >= 0) + if (space_id > 0) H5Sclose(space_id); return ret; } @@ -206,7 +206,7 @@ main(void) int r_f32 = 1, r_f64 = 1; file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); - if (file_id < 0) { + if (file_id == H5I_INVALID_HID) { fprintf(stderr, "FAIL: H5Fcreate(%s) failed\n", FILENAME); return 1; } From 54bc8cbd3b506d9b2073f4cac8f45b82159edcd4 Mon Sep 17 00:00:00 2001 From: Aleksandar Jelenak Date: Wed, 24 Jun 2026 20:41:32 -0500 Subject: [PATCH 5/5] H5I_INVALID_HID throughout in bit{round,groom} test code --- BITGROOM/example/h5filter_specials_bitgroom.c | 22 +++++++++---------- BITROUND/example/h5filter_specials_bitround.c | 22 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/BITGROOM/example/h5filter_specials_bitgroom.c b/BITGROOM/example/h5filter_specials_bitgroom.c index 47dbb0e54..d4f897284 100644 --- a/BITGROOM/example/h5filter_specials_bitgroom.c +++ b/BITGROOM/example/h5filter_specials_bitgroom.c @@ -9,11 +9,6 @@ * access to this file, you may request a copy from help@hdfgroup.org. * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ -#include "hdf5.h" -#include -#include -#include - /* * Round-trip test for special floating-point values through the BitGroom * filter. Verifies that NaN, +Inf, -Inf, -0.0, and +0.0 are passed through @@ -28,6 +23,11 @@ * positions to exercise both loops. */ +#include +#include +#include +#include "hdf5.h" + #define FILENAME "h5filter_specials_bitgroom.h5" #define DSET_F32 "f32" #define DSET_F64 "f64" @@ -128,11 +128,11 @@ roundtrip_f32(hid_t file_id) ret = 0; done: - if (dset_id > 0) + if (dset_id != H5I_INVALID_HID) H5Dclose(dset_id); - if (dcpl_id > 0) + if (dcpl_id != H5I_INVALID_HID) H5Pclose(dcpl_id); - if (space_id > 0) + if (space_id != H5I_INVALID_HID) H5Sclose(space_id); return ret; } @@ -194,11 +194,11 @@ roundtrip_f64(hid_t file_id) ret = 0; done: - if (dset_id > 0) + if (dset_id != H5I_INVALID_HID) H5Dclose(dset_id); - if (dcpl_id > 0) + if (dcpl_id != H5I_INVALID_HID) H5Pclose(dcpl_id); - if (space_id > 0) + if (space_id != H5I_INVALID_HID) H5Sclose(space_id); return ret; } diff --git a/BITROUND/example/h5filter_specials_bitround.c b/BITROUND/example/h5filter_specials_bitround.c index 0bb569ef9..fc71a4fa4 100644 --- a/BITROUND/example/h5filter_specials_bitround.c +++ b/BITROUND/example/h5filter_specials_bitround.c @@ -9,11 +9,6 @@ * access to this file, you may request a copy from help@hdfgroup.org. * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ -#include "hdf5.h" -#include -#include -#include - /* * Round-trip test for special floating-point values through the Granular * BitRound filter. Verifies that NaN, +Inf, -Inf, -0.0, and +0.0 are passed @@ -24,6 +19,11 @@ * platform NaN/Inf printf formatting cannot affect the result. */ +#include +#include +#include +#include "hdf5.h" + #define FILENAME "h5filter_specials_bitround.h5" #define DSET_F32 "f32" #define DSET_F64 "f64" @@ -124,11 +124,11 @@ roundtrip_f32(hid_t file_id) ret = 0; done: - if (dset_id > 0) + if (dset_id != H5I_INVALID_HID) H5Dclose(dset_id); - if (dcpl_id > 0) + if (dcpl_id != H5I_INVALID_HID) H5Pclose(dcpl_id); - if (space_id > 0) + if (space_id != H5I_INVALID_HID) H5Sclose(space_id); return ret; } @@ -190,11 +190,11 @@ roundtrip_f64(hid_t file_id) ret = 0; done: - if (dset_id > 0) + if (dset_id != H5I_INVALID_HID) H5Dclose(dset_id); - if (dcpl_id > 0) + if (dcpl_id != H5I_INVALID_HID) H5Pclose(dcpl_id); - if (space_id > 0) + if (space_id != H5I_INVALID_HID) H5Sclose(space_id); return ret; }