Merge pull request #514 from E3SM-Project/dqwu/hdf5_type_refactoring

This PR completes the first phase of support for HDF5
IO type by adding error handling to all HDF5 API calls.

It also utilizes some internal helper functions to improve code
organization and readability.

Additionally, minor fixes for HDF5 and ADIOS IO types have been
implemented.

The nightly test for HDF5 IO type has also been updated to enable
it only when direct HDF5 support is available.

Author: dqwu

examples/c/CMakeLists.txt

@@ -71,7 +71,9 @@ add_spio_executable(example1 TRUE "" example1.c)
 add_spio_executable(darray_no_async TRUE "" darray_no_async.c)
 add_spio_executable(darray_async TRUE "" darray_async.c)
 add_spio_executable(put_var TRUE "" put_var.c)
-add_spio_executable(test_hdf5 TRUE "" test_hdf5.c)
+if (WITH_HDF5)
+  add_spio_executable(test_hdf5 TRUE "" test_hdf5.c)
+endif ()
 
 if (PIO_ENABLE_FORTRAN)
   if (MPE_C_FOUND)
@@ -92,5 +94,7 @@ else ()
   add_mpi_test(put_var EXECUTABLE ${CMAKE_CURRENT_BINARY_DIR}/put_var NUMPROCS 4 TIMEOUT 60)
   #add_mpi_test(darray_async EXECUTABLE ${CMAKE_CURRENT_BINARY_DIR}/darray_async NUMPROCS 5 TIMEOUT 60)
   add_mpi_test(darray_no_async EXECUTABLE ${CMAKE_CURRENT_BINARY_DIR}/darray_no_async NUMPROCS 4 TIMEOUT 60)
-  add_mpi_test(test_hdf5 EXECUTABLE ${CMAKE_CURRENT_BINARY_DIR}/test_hdf5 NUMPROCS 4 TIMEOUT 60)
+  if (WITH_HDF5)
+    add_mpi_test(test_hdf5 EXECUTABLE ${CMAKE_CURRENT_BINARY_DIR}/test_hdf5 NUMPROCS 4 TIMEOUT 60)
+  endif ()
 endif ()

examples/c/test_hdf5.c

@@ -168,7 +168,8 @@ int main(int argc, char* argv[])
         ret = PIOc_put_att(ncid, varid_lat, "long_name", PIO_CHAR, strlen("latitude"), "latitude"); ERR
         ret = PIOc_put_att(ncid, varid_lat, "units", PIO_CHAR, strlen("degrees_north"), "degrees_north"); ERR
         ret = PIOc_put_att(ncid, varid_P0, "long_name", PIO_CHAR, strlen("reference pressure"), "reference pressure"); ERR
-        ret = PIOc_put_att(ncid, varid_P0, "units", PIO_CHAR, strlen("Pa"), "Pa"); ERR
+        ret = PIOc_put_att(ncid, varid_P0, "units", PIO_CHAR, strlen("pA"), "pA"); ERR /* Wrong attribute value */
+        ret = PIOc_put_att(ncid, varid_P0, "units", PIO_CHAR, strlen("Pa"), "Pa"); ERR /* Correction: overwrite an existing attribute */
         const int mdims_val = 1;
         ret = PIOc_put_att(ncid, varid_U, "mdims", PIO_INT, 1, &mdims_val); ERR
         ret = PIOc_put_att(ncid, varid_U, "units", PIO_CHAR, strlen("m/s"), "m/s"); ERR
@@ -569,6 +570,9 @@ int main(int argc, char* argv[])
 
 #else
     printf("SCORPIO is not configured with HDF5 support.\n");
+
+    /* Make the test fail if HDF5 support is not enabled/available. */
+    return -1;
 #endif
 
     return 0;

src/clib/pio.h

@@ -78,6 +78,7 @@ extern "C" {
 #ifdef _HDF5
 #include <hdf5.h>
 #include <hdf5_hl.h>
+#include <unistd.h>
 #endif
 
 /* PIO_OFFSET_C_TYPENAME is defined in pio_config.h */
@@ -1652,12 +1653,6 @@ extern "C" {
     int end_adios2_step(file_desc_t *file, iosystem_desc_t *ios);
 #endif
 
-    /* FIXME: Shouldn't these HDF5 functions be moved to pio_internal.h ? */
-#ifdef _HDF5
-    hid_t nc_type_to_hdf5_type(nc_type xtype);
-    PIO_Offset hdf5_get_nc_type_size(nc_type xtype);
-#endif
-
 #if defined(__cplusplus)
 }
 #endif

src/clib/pio_darray_int.c

@@ -441,13 +441,17 @@ int write_darray_multi_par(file_desc_t *file, int nvars, int fndims, const int *
                     if (!(startlist[rrcnt] = calloc(fndims, sizeof(PIO_Offset))))
                     {
                         ierr = pio_err(ios, file, PIO_ENOMEM, __FILE__, __LINE__,
-                                          "Writing variables (number of variables = %d) to file (%s, ncid=%d) using PIO_IOTYPE_HDF5 iotype failed. Out of memory allocating buffer (%lld bytes) for array to store starts of I/O regions written out to file", nvars, pio_get_fname_from_file(file), file->pio_ncid, (long long int) (fndims * sizeof(PIO_Offset)));
+                                       "Writing variables (number of variables = %d) to file (%s, ncid=%d) using HDF5 iotype failed. "
+                                       "Out of memory allocating buffer (%lld bytes) for array to store starts of I/O regions written out to file",
+                                       nvars, pio_get_fname_from_file(file), file->pio_ncid, (long long int) (fndims * sizeof(PIO_Offset)));
                         break;
                     }
                     if (!(countlist[rrcnt] = calloc(fndims, sizeof(PIO_Offset))))
                     {
                         ierr = pio_err(ios, file, PIO_ENOMEM, __FILE__, __LINE__,
-                                          "Writing variables (number of variables = %d) to file (%s, ncid=%d) using PIO_IOTYPE_HDF5 iotype failed. Out of memory allocating buffer (%lld bytes) for array to store counts of I/O regions written out to file", nvars, pio_get_fname_from_file(file), file->pio_ncid, (long long int) (fndims * sizeof(PIO_Offset)));
+                                       "Writing variables (number of variables = %d) to file (%s, ncid=%d) using HDF5 iotype failed. "
+                                       "Out of memory allocating buffer (%lld bytes) for array to store counts of I/O regions written out to file",
+                                       nvars, pio_get_fname_from_file(file), file->pio_ncid, (long long int) (fndims * sizeof(PIO_Offset)));
                         break;
                     }
 
@@ -469,7 +473,15 @@ int write_darray_multi_par(file_desc_t *file, int nvars, int fndims, const int *
                     for (int nv = 0; nv < nvars; nv++)
                     {
                         hid_t file_space_id = H5Dget_space(file->hdf5_vars[varids[nv]].hdf5_dataset_id);
-                        hid_t mem_space_id;
+                        if (file_space_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. "
+                                           "The low level (HDF5) I/O library call failed to make a copy of the dataspace of the dataset associated with 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 mem_space_id = H5I_INVALID_HID;
 
                         if (dsize_all > 0)
                         {
@@ -487,31 +499,86 @@ int write_darray_multi_par(file_desc_t *file, int nvars, int fndims, const int *
                             for (int i = 0; i < rrcnt; i++)
                             {
                                 /* Union hyperslabs of all regions */
-                                H5Sselect_hyperslab(file_space_id, op, (hsize_t*)startlist[i], NULL, (hsize_t*)countlist[i], NULL);
+                                if (H5Sselect_hyperslab(file_space_id, op, (hsize_t*)startlist[i], NULL, (hsize_t*)countlist[i], NULL) < 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. "
+                                                   "The low level (HDF5) I/O library call failed to select a hyperslab region for a dataspace copied from the dataset associated with variable (%s, varid=%d)",
+                                                   nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                                }
+
                                 op = H5S_SELECT_OR;
                             }
 
                             mem_space_id = H5Screate_simple(1, &dsize_all, NULL);
+                            if (mem_space_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. "
+                                               "The low level (HDF5) I/O library call failed to create a new simple dataspace for variable (%s, varid=%d)",
+                                               nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                            }
 
                             /* Get a pointer to the data. */
                             bufptr = (void *)((char *)iobuf + nv * iodesc->mpitype_size * llen);
                         }
                         else
                         {
                             /* No data to write on this IO task. */
-                            H5Sselect_none(file_space_id);
+                            if (H5Sselect_none(file_space_id) < 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. "
+                                               "The low level (HDF5) I/O library call failed to reset the selection region for a dataspace copied from the dataset associated with variable (%s, varid=%d)",
+                                               nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                            }
 
                             mem_space_id = H5Screate(H5S_NULL);
+                            if (mem_space_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. "
+                                               "The low level (HDF5) I/O library call failed to create a new NULL dataspace for variable (%s, varid=%d)",
+                                               nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                            }
 
                             bufptr = NULL;
                         }
 
                         /* Collective write */
-                        hid_t mem_type_id = nc_type_to_hdf5_type(iodesc->piotype);
-                        H5Dwrite(file->hdf5_vars[varids[nv]].hdf5_dataset_id, mem_type_id, mem_space_id, file_space_id, file->dxplid_coll, bufptr);
+                        hid_t mem_type_id = spio_nc_type_to_hdf5_type(iodesc->piotype);
+                        if (mem_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. "
+                                           "Unsupported memory type (type=%x) for variable (%s, varid=%d)",
+                                           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)
+                        {
+                            return pio_err(ios, file, PIO_EHDF5ERR, __FILE__, __LINE__,
+                                           "Writing variables (number of variables = %d) to file (%s, ncid=%d) using HDF5 iotype failed. "
+                                           "The low level (HDF5) I/O library call failed to write the dataset associated with variable (%s, varid=%d)",
+                                           nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                        }
 
-                        H5Sclose(file_space_id);
-                        H5Sclose(mem_space_id);
+                        if (H5Sclose(file_space_id) < 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. "
+                                           "The low level (HDF5) I/O library call failed to release a dataspace copied from the dataset associated with variable (%s, varid=%d)",
+                                           nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                        }
+
+                        if (H5Sclose(mem_space_id) < 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. "
+                                           "The low level (HDF5) I/O library call failed to release a simple (or NULL) dataspace for variable (%s, varid=%d)",
+                                           nvars, pio_get_fname_from_file(file), file->pio_ncid, pio_get_vname_from_file(file, varids[nv]), varids[nv]);
+                        }
                     }
 
                     /* Free resources. */

src/clib/pio_file.c

@@ -943,27 +943,7 @@ int PIOc_closefile(int ncid)
 #endif
 #ifdef _HDF5
         case PIO_IOTYPE_HDF5:
-            ierr = PIO_NOERR;
-
-            H5Pclose(file->dxplid_coll);
-            H5Pclose(file->dxplid_indep);
-
-            for (int i = 0; i < file->hdf5_num_dims; i++)
-            {
-                if (!file->hdf5_dims[i].has_coord_var)
-                    H5Dclose(file->hdf5_dims[i].hdf5_dataset_id);
-            }
-
-            for (int i = 0; i < file->hdf5_num_vars; i++)
-            {
-                H5Dclose(file->hdf5_vars[i].hdf5_dataset_id);
-
-                if (file->hdf5_vars[i].nc_type == NC_CHAR)
-                    H5Tclose(file->hdf5_vars[i].hdf5_type);
-            }
-
-            H5Fclose(file->hdf5_file_id);
-
+            ierr = spio_hdf5_close(ios, file);
             break;
 #endif
         default: