From 1e0dd173852799accad0f21cff53d26510c0dafb Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Tue, 2 Dec 2025 17:08:52 -0600 Subject: [PATCH 1/9] Fix hdf5 redef issue Ensure that dimensions and attributes for coordinate variables are only created once. When users call enddef multiple times (redef + enddef) ensure that only newly created dimensions & variables are processed. --- src/clib/pio_nc.cpp | 2 + src/clib/pioc_support.cpp | 190 ++++++++++++++++++-------------------- 2 files changed, 91 insertions(+), 101 deletions(-) diff --git a/src/clib/pio_nc.cpp b/src/clib/pio_nc.cpp index d3683671af..a218276ce3 100644 --- a/src/clib/pio_nc.cpp +++ b/src/clib/pio_nc.cpp @@ -3204,6 +3204,7 @@ int PIOc_def_dim_impl(int ncid, const char *name, PIO_Offset len, int *idp) file->hdf5_dims[file->hdf5_num_dims].len = len; file->hdf5_dims[file->hdf5_num_dims].has_coord_var = false; + file->hdf5_dims[file->hdf5_num_dims].hdf5_dataset_id = H5I_INVALID_HID; *idp = file->hdf5_num_dims; file->hdf5_num_dims++; } @@ -3548,6 +3549,7 @@ int PIOc_def_var_impl(int ncid, const char *name, nc_type xtype, int ndims, file->hdf5_vars[file->hdf5_num_vars].nc_type = xtype; file->hdf5_vars[file->hdf5_num_vars].ndims = ndims; file->hdf5_vars[file->hdf5_num_vars].is_coord_var = false; + file->hdf5_vars[file->hdf5_num_vars].hdf5_dataset_id = H5I_INVALID_HID; file->hdf5_vars[file->hdf5_num_vars].hdf5_dimids = (int*)malloc(ndims * sizeof(int)); if (file->hdf5_vars[file->hdf5_num_vars].hdf5_dimids == NULL) diff --git a/src/clib/pioc_support.cpp b/src/clib/pioc_support.cpp index f0cdd3e40d..a2b6f9c0d2 100644 --- a/src/clib/pioc_support.cpp +++ b/src/clib/pioc_support.cpp @@ -7098,7 +7098,10 @@ int spio_hdf5_enddef(iosystem_desc_t *ios, file_desc_t *file) for (i = 0; i < file->hdf5_num_dims; i++) { - if (!file->hdf5_dims[i].has_coord_var) + /* For dimensions without an associated coordinate var, define them here. However since the + * the user can call redef() multiple times define it only its not already defined/valid + */ + if (!file->hdf5_dims[i].has_coord_var && (file->hdf5_dims[i].hdf5_dataset_id == H5I_INVALID_HID)) { hid_t space_id, dcpl_id = H5I_INVALID_HID, dimscale_id; hsize_t dims[1], max_dims[1], chunk_dims[1] = {1}; @@ -7261,119 +7264,104 @@ int spio_hdf5_enddef(iosystem_desc_t *ios, file_desc_t *file) } } - for (i = 0; i < file->hdf5_num_vars; i++) - { - /* Upgrade the dataset of a coordinate variable to a dimension scale */ - if (file->hdf5_vars[i].is_coord_var) - { - if (H5DSset_scale(file->hdf5_vars[i].hdf5_dataset_id, file->hdf5_vars[i].name) < 0) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to convert a dataset (for coordinate variable %s) to a dimension scale", - pio_get_fname_from_file(file), file->pio_ncid, file->hdf5_vars[i].name); - } + for(i = 0; i < file->hdf5_num_vars; i++){ + /* Upgrade the dataset of a coordinate variable to a dimension scale */ + if(file->hdf5_vars[i].is_coord_var){ + /* Write a special attribute (_Netcdf4Dimid) for the netCDF-4 dimension ID. */ + hid_t dimscale_id = file->hdf5_vars[i].hdf5_dataset_id; + hid_t dimid_att_id; + htri_t attr_exists; - assert(file->hdf5_vars[i].ndims > 0); - int dimid = file->hdf5_vars[i].hdf5_dimids[0]; - file->hdf5_dims[dimid].hdf5_dataset_id = file->hdf5_vars[i].hdf5_dataset_id; + /* Writing _Netcdf4Dimid attribute */ + const char* attr_name = "_Netcdf4Dimid"; - /* Write a special attribute (_Netcdf4Dimid) for the netCDF-4 dimension ID. */ - hid_t dimscale_id = file->hdf5_vars[i].hdf5_dataset_id; - hid_t dimid_att_id; - htri_t attr_exists; + attr_exists = H5Aexists(dimscale_id, attr_name); + if(attr_exists < 0){ + /* Error determining whether an attribute with a given name exists on an object */ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to determine whether an attribute (%s) exists on a dimension scale (%s)", + pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); + } - hid_t dimid_space_id = H5Screate(H5S_SCALAR); - if (dimid_space_id == H5I_INVALID_HID) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to create a new scalar dataspace", - pio_get_fname_from_file(file), file->pio_ncid); - } + if(attr_exists > 0){ + /* If redef/enddef is called, potentially multiple times, + * this attribute might already have been created + */ + continue; + } - /* Writing _Netcdf4Dimid attribute */ - const char* attr_name = "_Netcdf4Dimid"; + if(H5DSset_scale(file->hdf5_vars[i].hdf5_dataset_id, file->hdf5_vars[i].name) < 0){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to convert a dataset (for coordinate variable %s) to a dimension scale", + pio_get_fname_from_file(file), file->pio_ncid, file->hdf5_vars[i].name); + } - attr_exists = H5Aexists(dimscale_id, attr_name); - if (attr_exists > 0) - { - assert(0); - } - else if (attr_exists == 0) - { - dimid_att_id = H5Acreate2(dimscale_id, attr_name, - H5T_NATIVE_INT, dimid_space_id, H5P_DEFAULT, H5P_DEFAULT); - if (dimid_att_id == H5I_INVALID_HID) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to create a new attribute (%s) attached to a dimension scale (%s)", - pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); - } - } - else - { - /* Error determining whether an attribute with a given name exists on an object */ - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to determine whether an attribute (%s) exists on a dimension scale (%s)", - pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); - } + assert(file->hdf5_vars[i].ndims > 0); + int dimid = file->hdf5_vars[i].hdf5_dimids[0]; + file->hdf5_dims[dimid].hdf5_dataset_id = file->hdf5_vars[i].hdf5_dataset_id; - if (H5Awrite(dimid_att_id, H5T_NATIVE_INT, &dimid) < 0) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to write an attribute (%s) attached to a dimension scale (%s)", - pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); - } + hid_t dimid_space_id = H5Screate(H5S_SCALAR); + if(dimid_space_id == H5I_INVALID_HID){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to create a new scalar dataspace", + pio_get_fname_from_file(file), file->pio_ncid); + } - if (H5Sclose(dimid_space_id) < 0) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to release a scalar dataspace", - pio_get_fname_from_file(file), file->pio_ncid); - } + dimid_att_id = H5Acreate2(dimscale_id, attr_name, + H5T_NATIVE_INT, dimid_space_id, H5P_DEFAULT, H5P_DEFAULT); + if(dimid_att_id == H5I_INVALID_HID){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to create a new attribute (%s) attached to a dimension scale (%s)", + pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); + } - if (H5Aclose(dimid_att_id) < 0) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to close an attribute (%s) attached to a dimension scale (%s)", - pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); - } + if(H5Awrite(dimid_att_id, H5T_NATIVE_INT, &dimid) < 0){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to write an attribute (%s) attached to a dimension scale (%s)", + pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); } - } - for (i = 0; i < file->hdf5_num_vars; i++) - { - if (!file->hdf5_vars[i].is_coord_var) - { - int ndims = file->hdf5_vars[i].ndims; - if (ndims > 0) - { - int* dimids = file->hdf5_vars[i].hdf5_dimids; - for (int j = 0; j < ndims; j++) - { - if (H5DSattach_scale(file->hdf5_vars[i].hdf5_dataset_id, file->hdf5_dims[dimids[j]].hdf5_dataset_id, j) < 0) - { - return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, - "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " - "The low level (HDF5) I/O library call failed to attach a dimension scale (for dimension %s) to %dth dimension of a dataset (for variable %s)", - pio_get_fname_from_file(file), file->pio_ncid, file->hdf5_dims[dimids[j]].name, j, file->hdf5_vars[i].name); - } + if(H5Aclose(dimid_att_id) < 0){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to close an attribute (%s) attached to a dimension scale (%s)", + pio_get_fname_from_file(file), file->pio_ncid, attr_name, file->hdf5_vars[i].name); + } - /* According to HDF5 developers, the H5DS routines are not parallel, so all the ranks are going to be - * doing the same operations. At some point, with enough iterations of the loop, HDF5 might get out of - * step between the ranks. - * Workaround: place a barrier to sync H5DSattach_scale calls. - */ - MPI_Barrier(ios->io_comm); - } + if(H5Sclose(dimid_space_id) < 0){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to release a scalar dataspace", + pio_get_fname_from_file(file), file->pio_ncid); + } + } + else{ + /* Not a coordinate var */ + int ndims = file->hdf5_vars[i].ndims; + if(ndims > 0){ + int* dimids = file->hdf5_vars[i].hdf5_dimids; + for(int j = 0; j < ndims; j++){ + if(H5DSattach_scale(file->hdf5_vars[i].hdf5_dataset_id, file->hdf5_dims[dimids[j]].hdf5_dataset_id, j) < 0){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Ending the define mode for file (%s, ncid=%d) using HDF5 iotype failed. " + "The low level (HDF5) I/O library call failed to attach a dimension scale (for dimension %s) to %dth dimension of a dataset (for variable %s)", + pio_get_fname_from_file(file), file->pio_ncid, file->hdf5_dims[dimids[j]].name, j, file->hdf5_vars[i].name); } + + /* According to HDF5 developers, the H5DS routines are not parallel, so all the ranks are going to be + * doing the same operations. At some point, with enough iterations of the loop, HDF5 might get out of + * step between the ranks. + * Workaround: place a barrier to sync H5DSattach_scale calls. + */ + MPI_Barrier(ios->io_comm); + } } + } } return PIO_NOERR; From aa9f3895cf75e7a454383815066039f3b2e17697 Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Tue, 2 Dec 2025 17:10:41 -0600 Subject: [PATCH 2/9] Adding a test for multiple redefs Adding a test that includes multiple redef/enddefs to add dimensions and variables --- tests/general/ncdf_simple_tests.F90.in | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/general/ncdf_simple_tests.F90.in b/tests/general/ncdf_simple_tests.F90.in index 176ad9a66e..aa6a920b9a 100644 --- a/tests/general/ncdf_simple_tests.F90.in +++ b/tests/general/ncdf_simple_tests.F90.in @@ -73,6 +73,51 @@ PIO_TF_AUTO_TEST_SUB_BEGIN test_redef_enddef PIO_TF_AUTO_TEST_SUB_END test_redef_enddef +PIO_TF_AUTO_TEST_SUB_BEGIN test_redef_twice + use ncdf_simple_tests_tgv + Implicit none + character(len=PIO_TF_MAX_STR_LEN), parameter :: tmp_fname = "pio_test_create_redef_twice.nc" + type(file_desc_t) :: pio_file + type(var_desc_t) :: pio_var1, pio_var2 + integer :: pio_dim1, pio_dim2 + integer :: ret + + ret = PIO_createfile(pio_tf_iosystem_, pio_file, tgv_iotype, tmp_fname, PIO_CLOBBER) + PIO_TF_CHECK_ERR(ret, "Failed to create:" // trim(tgv_fname)) + + ret = PIO_enddef(pio_file) + PIO_TF_CHECK_ERR(ret, "Failed to enddef:" // trim(tgv_fname)) + + ! A simple redef and then enddef + ret = PIO_redef(pio_file) + PIO_TF_CHECK_ERR(ret, "Failed to redef:" // trim(tgv_fname)) + + ret = PIO_def_dim(pio_file, 'dummy_dim_def_var_redef1', 100, pio_dim1) + PIO_TF_CHECK_ERR(ret, "Failed to define dim (1st redef):" // trim(tgv_fname)) + + ret = PIO_def_var(pio_file, 'dummy_var_def_var_redef1', PIO_int, (/pio_dim1/), pio_var1) + PIO_TF_CHECK_ERR(ret, "Failed to define var (1st redef):" // trim(tgv_fname)) + + ret = PIO_enddef(pio_file) + PIO_TF_CHECK_ERR(ret, "Failed to enddef:" // trim(tgv_fname)) + + ! A second redef and then enddef + ret = PIO_redef(pio_file) + PIO_TF_CHECK_ERR(ret, "Failed to redef:" // trim(tgv_fname)) + + ret = PIO_def_dim(pio_file, 'dummy_dim_def_var_redef2', 100, pio_dim2) + PIO_TF_CHECK_ERR(ret, "Failed to define dim (2nd redef):" // trim(tgv_fname)) + + ret = PIO_def_var(pio_file, 'dummy_var_def_var_redef2', PIO_int, (/pio_dim2/), pio_var2) + PIO_TF_CHECK_ERR(ret, "Failed to define var (2nd redef):" // trim(tgv_fname)) + + ret = PIO_enddef(pio_file) + PIO_TF_CHECK_ERR(ret, "Failed to enddef (2nd redef):" // trim(tgv_fname)) + + call PIO_closefile(pio_file) + +PIO_TF_AUTO_TEST_SUB_END test_redef_twice + PIO_TF_AUTO_TEST_SUB_BEGIN test_def_dim use ncdf_simple_tests_tgv Implicit none From 545b40476d36d68cb79daa6261b8f73dccf867f1 Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Wed, 3 Dec 2025 01:10:44 -0600 Subject: [PATCH 3/9] Adding a datatype converter util Adding a util class for datatype conversion of buffers. The util class also owns the temporary buffers created during the datatype conversion and includes functions to free it as needed --- src/clib/CMakeLists.txt | 2 +- src/clib/spio_dt_converter.cpp | 49 +++++ src/clib/spio_dt_converter.hpp | 97 +++++++++ tests/cunit/CMakeLists.txt | 5 +- tests/cunit/test_spio_dt_converter.cpp | 282 +++++++++++++++++++++++++ 5 files changed, 433 insertions(+), 2 deletions(-) create mode 100644 src/clib/spio_dt_converter.cpp create mode 100644 src/clib/spio_dt_converter.hpp create mode 100644 tests/cunit/test_spio_dt_converter.cpp diff --git a/src/clib/CMakeLists.txt b/src/clib/CMakeLists.txt index 76c92f1937..5c0bf65239 100644 --- a/src/clib/CMakeLists.txt +++ b/src/clib/CMakeLists.txt @@ -44,7 +44,7 @@ add_library (pioc ${pio_api_src} pio_darray.cpp pio_darray_int.cpp spio_hash.cpp pio_sdecomps_regex.cpp spio_io_summary.cpp spio_ltimer.cpp spio_serializer.cpp spio_file_mvcache.cpp spio_tracer.cpp spio_tracer_mdata.cpp spio_tracer_decomp.cpp - spio_rearrange_any.cpp) + spio_rearrange_any.cpp spio_dt_converter.cpp) #============================================================================== # FIND EXTERNAL LIBRARIES/DEPENDENCIES diff --git a/src/clib/spio_dt_converter.cpp b/src/clib/spio_dt_converter.cpp new file mode 100644 index 0000000000..2ee65713cd --- /dev/null +++ b/src/clib/spio_dt_converter.cpp @@ -0,0 +1,49 @@ +#include "spio_dt_converter.hpp" + +void *SPIO_Util::File_Util::DTConverter::convert(int ncid, void *buf, std::size_t sz, int from_pio_type, int to_pio_type) +{ + if(from_pio_type == to_pio_type){ + /* No conversion required, return the buffer as is */ + return buf; + } + + std::size_t nelems = sz / size_of(from_pio_type); + + Cachebuf cbuf = { bget(nelems * size_of(to_pio_type)), to_pio_type }; + + copy_to(buf, from_pio_type, cbuf.buf, to_pio_type, nelems); + + std::map >::iterator cbufs_iter = cbufs_.find(ncid); + if(cbufs_iter != cbufs_.end()){ + cbufs_iter->second.push_back(cbuf); + } + else{ + cbufs_.insert(std::make_pair(ncid, std::vector({cbuf}))); + } + + return cbuf.buf; +} + +void SPIO_Util::File_Util::DTConverter::free(int ncid) +{ + std::map >::iterator cbufs_iter = cbufs_.find(ncid); + if(cbufs_iter != cbufs_.end()){ + for(std::vector::iterator iter = cbufs_iter->second.begin(); + iter != cbufs_iter->second.end(); ++iter){ + brel(iter->buf); + } + cbufs_.erase(cbufs_iter); + } +} + +void SPIO_Util::File_Util::DTConverter::clear(void) +{ + for(std::map >::iterator cbufs_iter = cbufs_.begin(); + cbufs_iter != cbufs_.end(); ++cbufs_iter){ + for(std::vector::iterator iter = cbufs_iter->second.begin(); + iter != cbufs_iter->second.end(); ++iter){ + brel(iter->buf); + } + } + cbufs_.clear(); +} diff --git a/src/clib/spio_dt_converter.hpp b/src/clib/spio_dt_converter.hpp new file mode 100644 index 0000000000..69d2fbd793 --- /dev/null +++ b/src/clib/spio_dt_converter.hpp @@ -0,0 +1,97 @@ +#ifndef __SPIO_DT_CONVERTER_HPP__ +#define __SPIO_DT_CONVERTER_HPP__ + +#include +#include +#include + +#include "pio_config.h" +#include "pio.h" +#include "pio_internal.h" +#include "spio_file_mvcache.h" + +namespace SPIO_Util{ + namespace File_Util{ + /* Datatype converter class. + * Caches temp buffers (as a result of conversion) and these + * buffers can be freed using free() + */ + class DTConverter{ + public: + /* Convert buffer to requested type, buffer size, sz, is in bytes */ + void *convert(int ncid, void *buf, std::size_t sz, int from_pio_type, int to_pio_type); + /* Free the scratch/temp buffers associated with ncid/file */ + void free(int ncid); + /* Clear all scratch/temp buffers */ + void clear(void ); + + static inline std::size_t size_of(int pio_type){ + switch(pio_type){ + case PIO_DOUBLE : return sizeof(double); + case PIO_FLOAT : return sizeof(float); + case PIO_INT : return sizeof(int); + case PIO_UINT : return sizeof(unsigned int); + case PIO_SHORT : return sizeof(short int); + case PIO_USHORT : return sizeof(unsigned short int); + case PIO_INT64 : return sizeof(int64_t); + case PIO_UINT64 : return sizeof(uint64_t); + case PIO_CHAR : return sizeof(char); + case PIO_BYTE : return sizeof(char); + case PIO_UBYTE : return sizeof(unsigned char); + default : assert(0); + } + } + + private: + struct Cachebuf{ + void *buf; + int piotype; + }; + /* Internal map to store scrach/temp bufs for each file/ncid */ + std::map > cbufs_; + + template + static inline void copy_to(F *from_buf, T *to_buf, std::size_t nelems){ + std::transform(from_buf, from_buf + nelems, to_buf, + [](F val){ return static_cast(val); }); + } + + template + static inline void copy_to(F *from_buf, void *to_buf, int to_pio_type, std::size_t nelems){ + switch(to_pio_type){ + case PIO_DOUBLE : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_FLOAT : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_INT : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_UINT : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_SHORT : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_USHORT : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_INT64 : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_UINT64 : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_CHAR : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_BYTE : copy_to(from_buf, static_cast(to_buf), nelems); break; + case PIO_UBYTE : copy_to(from_buf, static_cast(to_buf), nelems); break; + default : assert(0); + } + } + + static inline void copy_to(void *from_buf, int from_pio_type, void *to_buf, int to_pio_type, std::size_t nelems){ + switch(from_pio_type){ + case PIO_DOUBLE : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_FLOAT : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_INT : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_UINT : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_SHORT : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_USHORT : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_INT64 : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_UINT64 : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_CHAR : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_BYTE : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + case PIO_UBYTE : copy_to(static_cast(from_buf), to_buf, to_pio_type, nelems); break; + default : assert(0); + } + } + }; + } // namespace File_Util +} // namespace SPIO_Util + +#endif // __SPIO_DT_CONVERTER_HPP__ diff --git a/tests/cunit/CMakeLists.txt b/tests/cunit/CMakeLists.txt index ff6c39d6b5..25c11fdb31 100644 --- a/tests/cunit/CMakeLists.txt +++ b/tests/cunit/CMakeLists.txt @@ -106,8 +106,10 @@ add_spio_executable (test_spio_tree TRUE "" test_spio_tree.cpp) add_spio_executable (test_spio_file_mvcache TRUE "" test_spio_file_mvcache.cpp) add_spio_executable (test_sdecomp_regex TRUE "" test_sdecomp_regex.cpp test_common.c) add_spio_executable(test_req_block_wait TRUE "" test_req_block_wait.c test_common.c) +add_spio_executable(test_spio_dt_converter TRUE "" test_spio_dt_converter.cpp) add_dependencies (tests test_spmd test_spio_ltimer test_spio_serializer test_spio_tree - test_spio_file_mvcache test_sdecomp_regex test_req_block_wait) + test_spio_file_mvcache test_sdecomp_regex test_req_block_wait + test_spio_dt_converter) # Test Timeout in seconds. if (PIO_VALGRIND_CHECK) @@ -127,6 +129,7 @@ add_test(NAME test_spio_ltimer COMMAND test_spio_ltimer) add_test(NAME test_spio_serializer COMMAND test_spio_serializer) add_test(NAME test_spio_tree COMMAND test_spio_tree) add_test(NAME test_spio_file_mvcache COMMAND test_spio_file_mvcache) +add_test(NAME test_spio_dt_converter COMMAND test_spio_dt_converter) add_test(NAME test_sdecomp_regex COMMAND test_sdecomp_regex) if (PIO_USE_MPISERIAL) diff --git a/tests/cunit/test_spio_dt_converter.cpp b/tests/cunit/test_spio_dt_converter.cpp new file mode 100644 index 0000000000..5996f30ea8 --- /dev/null +++ b/tests/cunit/test_spio_dt_converter.cpp @@ -0,0 +1,282 @@ +#include +#include +#include +#include +#include +#include + +#include "pio_config.h" +#include "pio.h" +#include "pio_tests.h" +#include "spio_dt_converter.hpp" + +#ifdef SPIO_ENABLE_GPTL_TIMING +#ifndef SPIO_ENABLE_GPTL_TIMING_INTERNAL +#include "gptl.h" +#endif +#endif + +#define LOG_RANK0(rank, ...) \ + do{ \ + if(rank == 0) \ + { \ + fprintf(stderr, __VA_ARGS__); \ + } \ + }while(0); + +static const int FAIL = -1; + +int test_double_to_float(int wrank) +{ + const int DUMMY_NCID = 1; + std::vector dval = {1.0, 2.0, 3.0, 4.0, 5.0}; + + SPIO_Util::File_Util::DTConverter dt; + + void *val = dt.convert(DUMMY_NCID, static_cast(dval.data()), dval.size() * sizeof(double), + PIO_DOUBLE, PIO_FLOAT); + float *fval = static_cast(val); + if(fval == NULL){ + LOG_RANK0(wrank, "Data type converter failed : Returned null buffer\n"); + return PIO_EINTERNAL; + } + + for(std::size_t i = 9; i < dval.size(); i++){ + if(static_cast(dval[i]) != fval[i]){ + LOG_RANK0(wrank, "Data type converter returned wrong value, val[%zu] = %f (expected %f)\n", + i, fval[i], dval[i]); + dt.clear(); + return PIO_EINTERNAL; + } + } + + dt.clear(); + return PIO_NOERR; +} + +int test_float_to_double(int wrank) +{ + const int DUMMY_NCID = 1; + std::vector fval = {1.0, 2.0, 3.0, 4.0, 5.0}; + + SPIO_Util::File_Util::DTConverter dt; + + void *val = dt.convert(DUMMY_NCID, static_cast(fval.data()), fval.size() * sizeof(float), + PIO_FLOAT, PIO_DOUBLE); + double *dval = static_cast(val); + if(dval == NULL){ + LOG_RANK0(wrank, "Data type converter failed : Returned null buffer\n"); + return PIO_EINTERNAL; + } + + for(std::size_t i = 9; i < fval.size(); i++){ + if(static_cast(fval[i]) != dval[i]){ + LOG_RANK0(wrank, "Data type converter returned wrong value, val[%zu] = %f (expected %f)\n", + i, dval[i], fval[i]); + dt.clear(); + return PIO_EINTERNAL; + } + } + + dt.clear(); + return PIO_NOERR; +} + +int test_int_to_double(int wrank) +{ + const int DUMMY_NCID = 1; + std::vector ival = {1, 2, 3, 4, 5}; + + SPIO_Util::File_Util::DTConverter dt; + + void *val = dt.convert(DUMMY_NCID, static_cast(ival.data()), ival.size() * sizeof(int), + PIO_INT, PIO_DOUBLE); + double *dval = static_cast(val); + if(dval == NULL){ + LOG_RANK0(wrank, "Data type converter failed : Returned null buffer\n"); + return PIO_EINTERNAL; + } + + for(std::size_t i = 9; i < ival.size(); i++){ + if(static_cast(ival[i]) != dval[i]){ + LOG_RANK0(wrank, "Data type converter returned wrong value, val[%zu] = %f (expected %f)\n", + i, dval[i], static_cast(ival[i])); + dt.clear(); + return PIO_EINTERNAL; + } + } + + dt.clear(); + return PIO_NOERR; +} + +int test_multi_convert(int wrank) +{ + const int DUMMY_NCID = 1; + std::vector fval = {1.1, 2.12, 3.123, 4.1234, 5.12345}; + + SPIO_Util::File_Util::DTConverter dt; + + void *val = dt.convert(DUMMY_NCID, static_cast(fval.data()), fval.size() * sizeof(float), + PIO_FLOAT, PIO_DOUBLE); + double *dval = static_cast(val); + if(dval == NULL){ + LOG_RANK0(wrank, "Data type converter failed : Returned null buffer\n"); + return PIO_EINTERNAL; + } + + for(std::size_t i = 9; i < fval.size(); i++){ + if(static_cast(fval[i]) != dval[i]){ + LOG_RANK0(wrank, "Data type converter returned wrong value, val[%zu] = %f (expected %f)\n", + i, dval[i], static_cast(fval[i])); + dt.clear(); + return PIO_EINTERNAL; + } + } + + val = dt.convert(DUMMY_NCID, static_cast(fval.data()), fval.size() * sizeof(float), + PIO_FLOAT, PIO_INT); + int *ival = static_cast(val); + if(ival == NULL){ + LOG_RANK0(wrank, "Data type converter failed : Returned null buffer\n"); + return PIO_EINTERNAL; + } + + for(std::size_t i = 9; i < fval.size(); i++){ + if(static_cast(fval[i]) != ival[i]){ + LOG_RANK0(wrank, "Data type converter returned wrong value, val[%zu] = %d (expected %d)\n", + i, ival[i], static_cast(fval[i])); + dt.clear(); + return PIO_EINTERNAL; + } + } + + dt.clear(); + return PIO_NOERR; +} + +int test_driver(MPI_Comm comm, int wrank, int wsz, int *num_errors) +{ + int nerrs = 0, ret = PIO_NOERR; + assert((comm != MPI_COMM_NULL) && (wrank >= 0) && (wsz > 0) && num_errors); + + try{ + ret = test_double_to_float(wrank); + } + catch(...){ + ret = PIO_EINTERNAL; + } + if(ret != PIO_NOERR){ + LOG_RANK0(wrank, "test_double_to_float() FAILED, ret = %d\n", ret); + nerrs++; + } + else{ + LOG_RANK0(wrank, "test_double_to_float() PASSED\n"); + } + + try{ + ret = test_float_to_double(wrank); + } + catch(...){ + ret = PIO_EINTERNAL; + } + if(ret != PIO_NOERR){ + LOG_RANK0(wrank, "test_float_to_double() FAILED, ret = %d\n", ret); + nerrs++; + } + else{ + LOG_RANK0(wrank, "test_float_to_double() PASSED\n"); + } + + try{ + ret = test_int_to_double(wrank); + } + catch(...){ + ret = PIO_EINTERNAL; + } + if(ret != PIO_NOERR){ + LOG_RANK0(wrank, "test_int_to_double() FAILED, ret = %d\n", ret); + nerrs++; + } + else{ + LOG_RANK0(wrank, "test_int_to_double() PASSED\n"); + } + + try{ + ret = test_multi_convert(wrank); + } + catch(...){ + ret = PIO_EINTERNAL; + } + if(ret != PIO_NOERR){ + LOG_RANK0(wrank, "test_multi_convert() FAILED, ret = %d\n", ret); + nerrs++; + } + else{ + LOG_RANK0(wrank, "test_multi_convert() PASSED\n"); + } + + *num_errors += nerrs; + return nerrs; +} + +int main(int argc, char *argv[]) +{ + int ret; + int wrank, wsz; + int num_errors; +#ifdef SPIO_ENABLE_GPTL_TIMING +#ifndef SPIO_ENABLE_GPTL_TIMING_INTERNAL + ret = GPTLinitialize(); + if(ret != 0){ + LOG_RANK0(wrank, "GPTLinitialize() FAILED, ret = %d\n", ret); + return ret; + } +#endif /* TIMING_INTERNAL */ +#endif /* TIMING */ + + ret = MPI_Init(&argc, &argv); + if(ret != MPI_SUCCESS){ + LOG_RANK0(wrank, "MPI_Init() FAILED, ret = %d\n", ret); + return ret; + } + + ret = MPI_Comm_rank(MPI_COMM_WORLD, &wrank); + if(ret != MPI_SUCCESS){ + LOG_RANK0(wrank, "MPI_Comm_rank() FAILED, ret = %d\n", ret); + return ret; + } + ret = MPI_Comm_size(MPI_COMM_WORLD, &wsz); + if(ret != MPI_SUCCESS){ + LOG_RANK0(wrank, "MPI_Comm_rank() FAILED, ret = %d\n", ret); + return ret; + } + + num_errors = 0; + ret = test_driver(MPI_COMM_WORLD, wrank, wsz, &num_errors); + if(ret != 0){ + LOG_RANK0(wrank, "Test driver FAILED\n"); + return FAIL; + } + else{ + LOG_RANK0(wrank, "All tests PASSED\n"); + } + + MPI_Finalize(); + +#ifdef SPIO_ENABLE_GPTL_TIMING +#ifndef SPIO_ENABLE_GPTL_TIMING_INTERNAL + ret = GPTLfinalize(); + if(ret != 0){ + LOG_RANK0(wrank, "GPTLinitialize() FAILED, ret = %d\n", ret); + return ret; + } +#endif /* TIMING_INTERNAL */ +#endif /* TIMING */ + + if(num_errors != 0){ + LOG_RANK0(wrank, "Total errors = %d\n", num_errors); + return FAIL; + } + return 0; +} From c89fea16ee0a869d01708db2ce6f6426fad07274 Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Wed, 3 Dec 2025 10:36:40 -0600 Subject: [PATCH 4/9] Use std::min instead of the custom C version Using std::min instead of the custom min() macro in minmax hdr --- src/clib/pio_darray_int.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/clib/pio_darray_int.cpp b/src/clib/pio_darray_int.cpp index fd57f4be68..131d7d2f8c 100644 --- a/src/clib/pio_darray_int.cpp +++ b/src/clib/pio_darray_int.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #ifdef PIO_MICRO_TIMING #include "pio_timer.h" #endif @@ -2508,7 +2507,7 @@ int compute_maxaggregate_bytes(iosystem_desc_t *ios, io_desc_t *iodesc) maxbytesoncomputetask = pio_cnbuffer_limit / iodesc->ndof; /* Take the min of the max IO and max comp bytes. */ - maxbytes = min(maxbytesoniotask, maxbytesoncomputetask); + maxbytes = std::min(maxbytesoniotask, maxbytesoncomputetask); LOG((2, "compute_maxaggregate_bytes maxbytesoniotask = %d maxbytesoncomputetask = %d", maxbytesoniotask, maxbytesoncomputetask)); From d357457779d93bdbafb2e2d76948a124bc5e681a Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Wed, 3 Dec 2025 10:39:22 -0600 Subject: [PATCH 5/9] Adding local data conversion for hdf5 output Using the data converter to explicitly convert user buffers to the type of the variable in file for HDF5 output. (Without this fix HDF5 filters, e.g. data compression filters, fail when data conversion is required) --- src/clib/pio.h | 3 +++ src/clib/pio_darray.cpp | 19 ++++++++++------- src/clib/pio_darray_int.cpp | 42 +++++++++++++++++++++++++++++++++++-- src/clib/pio_internal.h | 1 + src/clib/pio_lists.cpp | 8 +++++++ src/clib/pioc_support.cpp | 34 ++++++++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 10 deletions(-) diff --git a/src/clib/pio.h b/src/clib/pio.h index 7376373334..a708d8c166 100644 --- a/src/clib/pio.h +++ b/src/clib/pio.h @@ -1141,6 +1141,9 @@ typedef struct file_desc_t /** Number of global attrs defined */ int hdf5_num_gattrs; + + /** A datatype converter for user buffers */ + void *dt_converter; #endif /* _HDF5 */ /* File name - cached */ diff --git a/src/clib/pio_darray.cpp b/src/clib/pio_darray.cpp index 0e84dfcb48..13a400fc25 100644 --- a/src/clib/pio_darray.cpp +++ b/src/clib/pio_darray.cpp @@ -18,6 +18,7 @@ #include "spio_io_summary.h" #include "spio_file_mvcache.h" #include "spio_hash.h" +#include "spio_dt_converter.hpp" /* uint64_t definition */ #ifdef _ADIOS2 @@ -551,14 +552,16 @@ int PIOc_write_darray_multi_impl(int ncid, const int *varids, int ioid, int nvar } /* For PNETCDF the iobuf is freed in flush_output_buffer() */ - if (file->iotype != PIO_IOTYPE_PNETCDF) - { - /* Release resources. */ - if (mv_iobuf) - { - LOG((3,"freeing variable buffer in pio_darray")); - spio_file_mvcache_free(file, ioid); - } + if(file->iotype != PIO_IOTYPE_PNETCDF){ + /* Release resources. */ + if(mv_iobuf){ + LOG((3,"freeing variable buffer in pio_darray")); + spio_file_mvcache_free(file, ioid); + } + if((file->iotype == PIO_IOTYPE_HDF5) || (file->iotype == PIO_IOTYPE_HDF5C)){ + assert(file->dt_converter); + static_cast(file->dt_converter)->free(ioid); + } } /* The box rearranger will always have data (it could be fill diff --git a/src/clib/pio_darray_int.cpp b/src/clib/pio_darray_int.cpp index 131d7d2f8c..5059e1a910 100644 --- a/src/clib/pio_darray_int.cpp +++ b/src/clib/pio_darray_int.cpp @@ -13,11 +13,13 @@ #include #include #include +//#include #ifdef PIO_MICRO_TIMING #include "pio_timer.h" #endif #include "spio_io_summary.h" #include "spio_file_mvcache.h" +#include "spio_dt_converter.hpp" /* 10MB default limit. */ extern PIO_Offset pio_buffer_size_limit; @@ -575,8 +577,38 @@ int write_darray_multi_par(file_desc_t *file, int nvars, int fndims, const int * nvars, pio_get_fname_from_file(file), file->pio_ncid, iodesc->piotype, pio_get_vname_from_file(file, varids[nv]), varids[nv]); } - if (H5Dwrite(file->hdf5_vars[varids[nv]].hdf5_dataset_id, mem_type_id, mem_space_id, file_space_id, - file->dxplid_coll, bufptr) < 0) + hid_t file_var_type_id = H5Dget_type(file->hdf5_vars[varids[nv]].hdf5_dataset_id); + if(file_var_type_id == H5I_INVALID_HID){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Writing variables (number of variables = %d) to file (%s, ncid=%d) using HDF5 iotype failed. " + "Unable to query the type of variable (%s, varid=%d)", + nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]); + } + + hid_t file_var_ntype_id = H5Tget_native_type(file_var_type_id, H5T_DIR_DEFAULT); + assert(file_var_ntype_id != H5I_INVALID_HID); + + /* When HDF5 filters (e.g. data compression) are enabled collective writes fail when datatype conversion is required for writing user data. + * So we manually perform the data conversion here before passing it to HDF5. When filters are not enabled the write might succeed but HDF5 + * might be switching off collective writes (hurts performance) when datatype conversion is required + * FIXME: Disable datatype conversion when filters are not enabled on the dataset + */ + void *wbuf = bufptr; + if((dsize_all > 0) && !H5Tequal(mem_type_id, file_var_ntype_id)){ + assert(file->dt_converter); + wbuf = static_cast(file->dt_converter)->convert(iodesc->ioid, bufptr, iodesc->mpitype_size * dsize_all, + iodesc->piotype, spio_hdf5_type_to_pio_type(file_var_ntype_id)); + if(wbuf == NULL){ + return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, + "Writing variables (number of variables = %d) to file (%s, ncid=%d) using HDF5 iotype failed. " + "Unable to convert the type (from %d to %d) of variable (%s, varid=%d)", + nvars, pio_get_fname_from_file(file), file->pio_ncid, iodesc->piotype, + spio_hdf5_type_to_pio_type(file_var_ntype_id), pio_get_vname_from_file(file, varids[nv]), varids[nv]); + } + } + + if (H5Dwrite(file->hdf5_vars[varids[nv]].hdf5_dataset_id, file_var_ntype_id, mem_space_id, file_space_id, + file->dxplid_coll, wbuf) < 0) { return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, "Writing variables (number of variables = %d) to file (%s, ncid=%d) using HDF5 iotype failed. " @@ -2289,6 +2321,12 @@ int flush_output_buffer(file_desc_t *file, bool force, PIO_Offset addsize) /* Release resources. */ spio_file_mvcache_clear(file); + +#ifdef _HDF5 + assert(file->dt_converter); + static_cast(file->dt_converter)->clear(); +#endif + for (int i = 0; i < PIO_MAX_VARS; i++) { vdesc = file->varlist + i; diff --git a/src/clib/pio_internal.h b/src/clib/pio_internal.h index 78d3c2b276..524b3829ac 100644 --- a/src/clib/pio_internal.h +++ b/src/clib/pio_internal.h @@ -407,6 +407,7 @@ extern "C" { #ifdef _HDF5 hid_t nc_type_to_hdf5_type(nc_type xtype); + int spio_hdf5_type_to_pio_type(hid_t xtype); PIO_Offset hdf5_get_nc_type_size(nc_type xtype); #endif diff --git a/src/clib/pio_lists.cpp b/src/clib/pio_lists.cpp index 0e9b5f37eb..9b1416c838 100644 --- a/src/clib/pio_lists.cpp +++ b/src/clib/pio_lists.cpp @@ -13,6 +13,7 @@ #include "spio_file_mvcache.h" #include "spio_io_summary.h" #include "spio_hash.h" +#include "spio_dt_converter.hpp" static io_desc_t *pio_iodesc_list = NULL; static io_desc_t *current_iodesc = NULL; @@ -165,6 +166,13 @@ int pio_delete_file_from_list(int ncid) free(cfile->unlim_dimids); free(cfile->io_fstats); spio_file_mvcache_finalize(cfile); + +#ifdef _HDF5 + if(cfile->dt_converter != NULL){ + delete(static_cast(cfile->dt_converter)); + } +#endif + /* Free the memory used for this file. */ #ifdef _ADIOS2 if (cfile->cache_data_blocks != NULL) diff --git a/src/clib/pioc_support.cpp b/src/clib/pioc_support.cpp index a2b6f9c0d2..b8b590fd19 100644 --- a/src/clib/pioc_support.cpp +++ b/src/clib/pioc_support.cpp @@ -23,6 +23,7 @@ #include #include "spio_io_summary.h" #include "spio_file_mvcache.h" +#include "spio_dt_converter.hpp" #include "spio_hash.h" /* Include headers for HDF5 compression filters */ @@ -3850,6 +3851,11 @@ int spio_createfile_int(int iosysid, int *ncidp, const int *iotype, const char * */ spio_file_mvcache_init(file); +#ifdef _HDF5 + /* A datatype converter for converting user buffer to a "file type" buffer */ + file->dt_converter = static_cast(new SPIO_Util::File_Util::DTConverter()); +#endif + *ncidp = pio_add_to_file_list(file, comm); LOG((2, "Created file %s file->fh = %d file->pio_ncid = %d", filename, @@ -5532,6 +5538,11 @@ int PIOc_openfile_retry_impl(int iosysid, int *ncidp, int *iotype, const char *f */ spio_file_mvcache_init(file); +#ifdef _HDF5 + /* A datatype converter for converting user buffer to a "file type" buffer */ + file->dt_converter = static_cast(new SPIO_Util::File_Util::DTConverter()); +#endif + *ncidp = pio_add_to_file_list(file, comm); LOG((2, "Opened file %s file->pio_ncid = %d file->fh = %d ierr = %d", @@ -6494,6 +6505,29 @@ hid_t spio_nc_type_to_hdf5_type(nc_type xtype) return H5I_INVALID_HID; } +int spio_hdf5_type_to_pio_type(hid_t ntype) +{ +// hid_t ntype = H5Tget_native_type(xtype, H5T_DIR_DEFAULT); + + /* switch() does not work with HDF5 "types" */ + if(H5Tequal(ntype, H5T_NATIVE_UINT8)) { return PIO_BYTE; } + else if(H5Tequal(ntype, H5T_NATIVE_UCHAR)) { return PIO_UBYTE; } + else if(H5Tequal(ntype, H5T_NATIVE_CHAR)) { return PIO_CHAR; } + else if(H5Tequal(ntype, H5T_NATIVE_SHORT)) { return PIO_SHORT; } + else if(H5Tequal(ntype, H5T_NATIVE_USHORT)) { return PIO_USHORT; } + else if(H5Tequal(ntype, H5T_NATIVE_INT)) { return PIO_INT; } + else if(H5Tequal(ntype, H5T_NATIVE_UINT)) { return PIO_UINT; } + else if(H5Tequal(ntype, H5T_NATIVE_FLOAT)) { return PIO_FLOAT; } + else if(H5Tequal(ntype, H5T_NATIVE_DOUBLE)) { return PIO_DOUBLE; } + else if(H5Tequal(ntype, H5T_NATIVE_INT64)) { return PIO_INT64; } + else if(H5Tequal(ntype, H5T_NATIVE_UINT64)) { return PIO_UINT64; } + else{ + assert(0); + } + + return PIO_NAT; +} + int spio_hdf5_create(iosystem_desc_t *ios, file_desc_t *file, const char *filename) { int mpierr = MPI_SUCCESS; From c6206bd1ac019841ee2de09de8c0371fa8c3240e Mon Sep 17 00:00:00 2001 From: Jayesh Krishna Date: Fri, 5 Dec 2025 14:58:40 -0500 Subject: [PATCH 6/9] Fix ADIOS zfp comp rate typo Fix ADIOS zfp compression rate to match the name in config.h --- src/clib/pioc_support.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clib/pioc_support.cpp b/src/clib/pioc_support.cpp index b8b590fd19..8556dd7824 100644 --- a/src/clib/pioc_support.cpp +++ b/src/clib/pioc_support.cpp @@ -794,11 +794,11 @@ adios2_variable* spio_define_adios2_variable(iosystem_desc_t *ios, file_desc_t * } else if(SPIO_ADIOS2_ZFP_COMPRESSION_MODE == "ADIOS2_ZFP_MODE_RATE"){ /* Fixed Rate Mode : User specifies the number of bits to use for each value of the compressed data */ - adiosErr = adios2_add_operation(&operation_index, variable, ios->lossy_compression_operator, "rate", SPIO_ADIOS2_ZFP_RATE); + adiosErr = adios2_add_operation(&operation_index, variable, ios->lossy_compression_operator, "rate", SPIO_ADIOS2_ZFP_COMPRESSION_RATE); if(adiosErr != adios2_error_none){ pio_err(ios, file, PIO_EADIOS2ERR, __FILE__, __LINE__, "Failed to add ZFP compression operation (rate=%s bits) to variable %s (adios2_error=%s)", - SPIO_ADIOS2_ZFP_RATE, name, convert_adios2_error_to_string(adiosErr)); + SPIO_ADIOS2_ZFP_COMPRESSION_RATE, name, convert_adios2_error_to_string(adiosErr)); } } else if(SPIO_ADIOS2_ZFP_COMPRESSION_MODE == "ADIOS2_ZFP_MODE_REVERSIBLE"){ @@ -813,7 +813,7 @@ adios2_variable* spio_define_adios2_variable(iosystem_desc_t *ios, file_desc_t * else{ pio_err(ios, file, PIO_EADIOS2ERR, __FILE__, __LINE__, "Failed to add ZFP compression operation to variable %s (adios2_error=%s). Invalid compression mode (%s)", - SPIO_ADIOS2_ZFP_RATE, name, convert_adios2_error_to_string(adiosErr), SPIO_ADIOS2_ZFP_COMPRESSION_MODE); + SPIO_ADIOS2_ZFP_COMPRESSION_RATE, name, convert_adios2_error_to_string(adiosErr), SPIO_ADIOS2_ZFP_COMPRESSION_MODE); } } } From e0e6381ae9dcf92cb1fd23eccbc05dcc672bea61 Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Mon, 8 Dec 2025 13:37:20 -0600 Subject: [PATCH 7/9] Disable ZFP for HDF5 vars with only unlimited dims When using HDF5 for lossy data compression skip variables with only unlimited dimensions (HDF5 filters fail when chunking these variables and these variables tend to be really small 1D variables for E3SM runs) --- src/clib/pioc_support.cpp | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/clib/pioc_support.cpp b/src/clib/pioc_support.cpp index 8556dd7824..0e90c095df 100644 --- a/src/clib/pioc_support.cpp +++ b/src/clib/pioc_support.cpp @@ -6816,19 +6816,36 @@ int spio_hdf5_create(iosystem_desc_t *ios, file_desc_t *file, const char *filena } /* Create HDF5 dataset property ID */ -static hid_t spio_create_hdf5_dataset_pid(iosystem_desc_t *ios, file_desc_t *file, const char *var_name, int var_ndims, nc_type var_type) +static hid_t spio_create_hdf5_dataset_pid(iosystem_desc_t *ios, file_desc_t *file, const char *var_name, const std::vector &max_dim_sz, nc_type var_type) { herr_t ret; hid_t dpid = H5I_INVALID_HID; + std::size_t var_ndims = max_dim_sz.size(); + assert((ios != NULL) && (file != NULL) && (var_ndims >= 0)); + bool var_has_only_unlimited_dims = true; + for(std::vector::const_iterator citer = max_dim_sz.cbegin(); citer != max_dim_sz.cend(); ++citer){ + if(*citer != H5S_UNLIMITED){ + var_has_only_unlimited_dims = false; + break; + } + } + /* Initialize the compression filter property list */ dpid = H5Pcreate(H5P_DATASET_CREATE); assert(dpid != H5I_INVALID_HID); /* We currently support compression for non-scalar data */ - if((var_ndims < 1) || (var_type == NC_CHAR) || (file->iotype != PIO_IOTYPE_HDF5C)) return dpid; + if(file->iotype != PIO_IOTYPE_HDF5C) return dpid; + if((var_ndims < 1) || (var_type == NC_CHAR)){ + std::string msg("Disabling HDF5 compression for variable"); + msg += std::string("(name=") + std::string(var_name) + std::string(", file=") + std::string(pio_get_fname_from_file(file)) + std::string(")"); + msg += (var_ndims < 1) ? std::string(" since its a scalar variable") : std::string(" since its a string/char variable"); + PIOc_warn(ios->iosysid, file->fh, __FILE__, __LINE__, msg.c_str()); + return dpid; + } /* Check if any variables have compression disabled by the user */ /* FIXME: Variables written out in a chunk size different from the one defined can cause hangs @@ -6852,6 +6869,15 @@ static hid_t spio_create_hdf5_dataset_pid(iosystem_desc_t *ios, file_desc_t *fil #ifdef _SPIO_HDF5_USE_LOSSY_COMPRESSION #ifdef _SPIO_HAS_H5Z_ZFP + /* Avoid ZFP compression for vars with only unlimited dims */ + if(var_has_only_unlimited_dims){ + std::string msg("Disabling HDF5 compression for variable"); + msg += std::string("(name=") + std::string(var_name) + std::string(", file=") + std::string(pio_get_fname_from_file(file)) + std::string(")"); + msg += std::string(" since it only has unlimited dims"); + PIOc_warn(ios->iosysid, file->fh, __FILE__, __LINE__, msg.c_str()); + return dpid; + } + if(SPIO_HDF5_ZFP_COMPRESSION_MODE == "H5Z_ZFP_MODE_RATE"){ /* Lossy compression : Fixed bit rate : Number of bits used for compressed values is fixed, e.g. 16 */ ret = H5Pset_zfp_rate(dpid, SPIO_HDF5_ZFP_COMPRESSION_RATE); @@ -7056,7 +7082,7 @@ int spio_hdf5_def_var(iosystem_desc_t *ios, file_desc_t *file, const char *name, } /* Create HDF5 dataset (and optionally add filters as needed) */ - hid_t dcpl_id = spio_create_hdf5_dataset_pid(ios, file, name, ndims, xtype); + hid_t dcpl_id = spio_create_hdf5_dataset_pid(ios, file, name, max_dim_sz, xtype); if(dcpl_id == H5I_INVALID_HID){ return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__, "Defining variable (%s, varid = %d) in file (%s, ncid=%d) using HDF5 iotype failed. " From 0a59de16f95ed4b2d56f7f93abcb7047c59ecfa3 Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Mon, 8 Dec 2025 13:39:45 -0600 Subject: [PATCH 8/9] Adding a 1D put/get test for var with unlim dims Adding a put/get test for a 1D variable with only unlimited dimensions --- tests/general/pio_decomp_frame_tests.F90.in | 93 +++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/tests/general/pio_decomp_frame_tests.F90.in b/tests/general/pio_decomp_frame_tests.F90.in index 6db7964a27..090fecb122 100644 --- a/tests/general/pio_decomp_frame_tests.F90.in +++ b/tests/general/pio_decomp_frame_tests.F90.in @@ -1,3 +1,96 @@ +! Write multiple frames of a 1d variable with an unlimited dimension +PIO_TF_TEMPLATE +PIO_TF_AUTO_TEST_SUB_BEGIN nc_wr_rd_1d_unlim_dim + implicit none + integer, parameter :: NDIMS = 1 + integer, parameter :: NFRAMES = 6 + integer, parameter :: VEC_LOCAL_SZ = 7 + type(var_desc_t) :: pio_var + type(file_desc_t) :: pio_file + character(len=PIO_TF_MAX_STR_LEN) :: filename + character(len=*), parameter :: PIO_VAR_NAME = 'PIO_TF_test_var_1d_unlim' + PIO_TF_FC_DATA_TYPE, dimension(:), allocatable :: rbuf, wbuf, exp_val + integer, dimension(NDIMS) :: start, count + integer, dimension(NDIMS) :: pio_dims + integer :: i, ierr + integer(kind=pio_offset_kind) :: f + ! iotypes = valid io types + integer, dimension(:), allocatable :: iotypes + character(len=PIO_TF_MAX_STR_LEN), dimension(:), allocatable :: iotype_descs + integer :: num_iotypes + + start = 0 + count = 1 + + allocate(wbuf(1)) + allocate(rbuf(1)) + wbuf = 0 + rbuf = 0 + allocate(exp_val(NFRAMES)) + do f=1,NFRAMES + exp_val(f) = int(f) + end do + + num_iotypes = 0 + call PIO_TF_Get_nc_iotypes(iotypes, iotype_descs, num_iotypes) + filename = "test_pio_decomp_frame_tests.testfile" + do i=1,num_iotypes + PIO_TF_LOG(0,*) "Testing : PIO_TF_DATA_TYPE : ", iotype_descs(i) + ierr = PIO_createfile(pio_tf_iosystem_, pio_file, iotypes(i), filename, PIO_CLOBBER) + PIO_TF_CHECK_ERR(ierr, "Could not create file " // trim(filename)) + + ierr = PIO_def_dim(pio_file, 'PIO_TF_test_dim_time', pio_unlimited, pio_dims(1)) + PIO_TF_CHECK_ERR(ierr, "Failed to define a dim : " // trim(filename)) + + ierr = PIO_def_var(pio_file, PIO_VAR_NAME, PIO_TF_DATA_TYPE, pio_dims, pio_var) + PIO_TF_CHECK_ERR(ierr, "Failed to define a var : " // trim(filename)) + + ierr = PIO_enddef(pio_file) + PIO_TF_CHECK_ERR(ierr, "Failed to end redef mode : " // trim(filename)) + + do f=1,NFRAMES + start = int(f) + wbuf = int(f) + ! Write the current frame + ierr = PIO_put_var(pio_file, pio_var, start, count, wbuf) + PIO_TF_CHECK_ERR(ierr, "Failed to write darray : " // trim(filename)) + end do + +#ifdef PIO_TEST_CLOSE_OPEN_FOR_SYNC + call PIO_closefile(pio_file) + + ierr = PIO_openfile(pio_tf_iosystem_, pio_file, iotypes(i), filename, PIO_nowrite) + PIO_TF_CHECK_ERR(ierr, "Could not reopen file " // trim(filename)) + + ierr = PIO_inq_varid(pio_file, PIO_VAR_NAME, pio_var) + PIO_TF_CHECK_ERR(ierr, "Could not inq var : " // trim(filename)) +#else + call PIO_syncfile(pio_file) +#endif + + do f=1,NFRAMES + start = int(f) + rbuf = 0 + ierr = PIO_get_var(pio_file, pio_var, start, count, rbuf) + PIO_TF_CHECK_ERR(ierr, "Failed to read darray : " // trim(filename)) + PIO_TF_CHECK_VAL((rbuf, exp_val(f)), "Got wrong val, frame = ", f) + end do + + call PIO_closefile(pio_file) + + call PIO_deletefile(pio_tf_iosystem_, filename); + end do + + if(allocated(iotypes)) then + deallocate(iotypes) + deallocate(iotype_descs) + end if + + deallocate(exp_val) + deallocate(rbuf) + deallocate(wbuf) +PIO_TF_AUTO_TEST_SUB_END nc_wr_rd_1d_unlim_dim + ! Write multiple frames of a 2d variable with an unlimited dimension PIO_TF_TEMPLATE PIO_TF_AUTO_TEST_SUB_BEGIN nc_wr_rd_2d_unlim_dim From 0f3c4603c0f04ccbaa6b3f119f222cce49bff482 Mon Sep 17 00:00:00 2001 From: jayeshkrishna Date: Fri, 12 Dec 2025 11:32:15 -0600 Subject: [PATCH 9/9] Expose datatype converter for all I/O types Exposing the datatype converter for all I/O types. However the converter is currently only used by the HDF5 I/O type. --- src/clib/pio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clib/pio.h b/src/clib/pio.h index a708d8c166..fec6dbdd23 100644 --- a/src/clib/pio.h +++ b/src/clib/pio.h @@ -1142,9 +1142,9 @@ typedef struct file_desc_t /** Number of global attrs defined */ int hdf5_num_gattrs; +#endif /* _HDF5 */ /** A datatype converter for user buffers */ void *dt_converter; -#endif /* _HDF5 */ /* File name - cached */ char fname[PIO_MAX_NAME + 1];