Merge pull request #1119 from NetCDF-World-Domination-Council/ejh_loo…

…p_cleanup_2

Fix checking for HDF5 max dims, no longer re-create atts if not needed, confirm behavior for HDF5 cyclical files, allow user to set mpiexec

CMakeLists.txt

@@ -315,6 +315,7 @@ SET(CHUNK_CACHE_PREEMPTION 0.75 CACHE STRING "Default file chunk cache preemptio
 SET(MAX_DEFAULT_CACHE_SIZE 67108864 CACHE STRING "Default maximum cache size.")
 SET(NETCDF_LIB_NAME "" CACHE STRING "Default name of the netcdf library.")
 SET(TEMP_LARGE "." CACHE STRING "Where to put large temp files if large file tests are run.")
+SET(MPIEXEC "mpiexec" CACHE STRING "Command to run MPI programs if parallel tests are run.")
 
 IF(NOT NETCDF_LIB_NAME STREQUAL "")
   SET(MOD_NETCDF_NAME ON)
@@ -987,7 +988,7 @@ IF(NOT ENABLE_NETCDF_4 AND ENABLE_EXAMPLE_TESTS)
   SET(ENABLE_EXAMPLE_TESTS OFF)
 ENDIF()
 
-# Enable Parallel (different than pnetcdf).
+# Enable Parallel IO with netCDF-4/HDF5 files using HDF5 parallel I/O.
 SET(STATUS_PARALLEL "OFF")
 OPTION(ENABLE_PARALLEL4 "Build netCDF-4 with parallel IO" "${HDF5_PARALLEL}")
 IF(ENABLE_PARALLEL4 AND ENABLE_NETCDF_4)
@@ -999,10 +1000,16 @@ IF(ENABLE_PARALLEL4 AND ENABLE_NETCDF_4)
     SET(USE_PARALLEL ON CACHE BOOL "")
     SET(USE_PARALLEL4 ON CACHE BOOL "")
     SET(STATUS_PARALLEL "ON")
+    configure_file("${netCDF_SOURCE_DIR}/nc_test4/run_par_test.sh.in"
+      "${netCDF_BINARY_DIR}/tmp/run_par_test.sh" @ONLY NEWLINE_STYLE LF)
+    FILE(COPY "${netCDF_BINARY_DIR}/tmp/run_par_test.sh"
+      DESTINATION ${netCDF_BINARY_DIR}/nc_test4
+      FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE)
+
   ENDIF()
 ENDIF()
 
-# Options to enable parallel IO, tests.
+# Options to enable parallel IO for classic formats with parallel-netcdf library.
 SET(STATUS_PNETCDF "OFF")
 OPTION(ENABLE_PNETCDF "Build with parallel I/O for classic and 64-bit offset files using parallel-netcdf." OFF)
 

configure.ac

@@ -1,16 +1,13 @@
 #                                               -*- Autoconf -*-
 ## Process this file with autoconf to produce a configure script.
 
-# This is part of Unidata's netCDF package. Copyright 2005-2012, see
+# This is part of Unidata's netCDF package. Copyright 2005-2018, see
 # the COPYRIGHT file for more information.
 # Ed Hartnett, Ward Fisher, Dennis Heimbigner
 
 # Recall that ${VAR-exp} expands to $VAR if var is set (even to null),
 # and to exp otherwise.
 
-## This puts the cvs ID tag in the output configure script.
-AC_REVISION([$Id: configure.ac,v 1.450 2010/05/28 19:42:47 dmh Exp $])
-
 # Running autoconf on this file will trigger a warning if
 # autoconf is not at least the specified version.
 AC_PREREQ([2.59])
@@ -259,6 +256,15 @@ AC_ARG_ENABLE([parallel-tests],
 test "x$enable_parallel_tests" = xyes || enable_parallel_tests=no
 AC_MSG_RESULT($enable_parallel_tests)
 
+# Did the user specify an MPI launcher other than mpiexec?
+AC_MSG_CHECKING([whether a user specified program to run mpi programs])
+AC_ARG_WITH([mpiexec],
+              [AS_HELP_STRING([--with-mpiexec=<command>],
+                              [Specify command to launch MPI parallel tests.])],
+            [MPIEXEC=$with_mpiexec], [MPIEXEC=mpiexec])
+AC_MSG_RESULT([$MPIEXEC])
+AC_SUBST([MPIEXEC], [$MPIEXEC])
+
 # Did the user specify a default chunk size?
 AC_MSG_CHECKING([whether a default chunk size in bytes was specified])
 AC_ARG_WITH([default-chunk-size],
@@ -1518,8 +1524,9 @@ AC_CONFIG_FILES(dap4_test/findtestserver4.c:ncdap_test/findtestserver.c.in)
 #####
 
 AC_MSG_NOTICE([generating header files and makefiles])
+AC_CONFIG_FILES([nc_test4/run_par_test.sh], [chmod ugo+x nc_test4/run_par_test.sh])
+AC_CONFIG_FILES([nc-config], [chmod 755 nc-config])
 AC_CONFIG_FILES([Makefile
-                 nc-config
                  netcdf.pc
                  libnetcdf.settings
                  postinstall.sh
@@ -1556,11 +1563,7 @@ AC_CONFIG_FILES([Makefile
                  ncdap_test/expectremote3/Makefile
                  dap4_test/Makefile
                  plugins/Makefile
-                 ],
-                 [test -f  nc-config && chmod 755 nc-config])
-
+                 ])
 AC_OUTPUT()
 
-    #mv -f ${abs_top_srcdir}/test_common.sh ${abs_top_builddir}/test_common.sh
-
 cat libnetcdf.settings

include/netcdf.h

@@ -66,7 +66,7 @@ extern "C" {
 #define NC_FILL_BYTE    ((signed char)-127)
 #define NC_FILL_CHAR    ((char)0)
 #define NC_FILL_SHORT   ((short)-32767)
-#define NC_FILL_INT     (-2147483647L)
+#define NC_FILL_INT     (-2147483647)
 #define NC_FILL_FLOAT   (9.9692099683868690e+36f) /* near 15 * 2^119 */
 #define NC_FILL_DOUBLE  (9.9692099683868690e+36)
 #define NC_FILL_UBYTE   (255)

libdispatch/dparallel.c

@@ -1,10 +1,9 @@
-/** \file
-
-This file has the parallel I/O functions which correspond to the
-serial I/O functions.
-
-Copyright 2010 University Corporation for Atmospheric
-Research/Unidata. See COPYRIGHT file for more info.
+/* Copyright 2010 University Corporation for Atmospheric
+   Research/Unidata. See COPYRIGHT file for more info. */
+/**
+ * @file
+ * @internal This file has the parallel I/O functions which correspond
+ * to the serial I/O functions.
 */
 #include "config.h"
 #include "ncdispatch.h"

libdispatch/dvar.c

@@ -149,7 +149,8 @@ nc_inq_ncid().
 @param ndims Number of dimensions for the variable. For example, 2
 specifies a matrix, 1 specifies a vector, and 0 means the variable is
 a scalar with no dimensions. Must not be negative or greater than the
-predefined constant ::NC_MAX_VAR_DIMS.
+predefined constant ::NC_MAX_VAR_DIMS. In netCDF-4/HDF5 files, may not
+exceed the HDF5 maximum number of dimensions (32).
 
 @param dimidsp Vector of ndims dimension IDs corresponding to the
 variable dimensions. For classic model netCDF files, if the ID of the

libhdf5/hdf5file.c

@@ -202,7 +202,7 @@ nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio* memio)
 
    /* Close hdf file. It may not be open, since this function is also
     * called by NC_create() when a file opening is aborted. */
-   if (hdf5_info->hdfid && H5Fclose(hdf5_info->hdfid) < 0)
+   if (hdf5_info->hdfid > 0 && H5Fclose(hdf5_info->hdfid) < 0)
    {
       dumpopenobjects(h5);
       BAIL(NC_EHDFERR);
@@ -286,7 +286,6 @@ size_t nc4_chunk_cache_size = CHUNK_CACHE_SIZE;            /**< Default chunk ca
 size_t nc4_chunk_cache_nelems = CHUNK_CACHE_NELEMS;        /**< Default chunk cache number of elements. */
 float nc4_chunk_cache_preemption = CHUNK_CACHE_PREEMPTION; /**< Default chunk cache preemption. */
 
-
 /**
  * Set chunk cache size. Only affects files opened/created *after* it
  * is called.

libhdf5/hdf5open.c

@@ -230,8 +230,8 @@ get_type_info2(NC_FILE_INFO_T *h5, hid_t datasetid,
 }
 
 /**
- * @internal This function reads the hacked in coordinates attribute I
- * use for multi-dimensional coordinates.
+ * @internal This function reads the coordinates attribute used for
+ * multi-dimensional coordinates.
  *
  * @param grp Group info pointer.
  * @param var Var info pointer.
@@ -244,37 +244,46 @@ read_coord_dimids(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var)
 {
    hid_t coord_att_typeid = -1, coord_attid = -1, spaceid = -1;
    hssize_t npoints;
-   int ret = 0;
+   int retval = NC_NOERR;
    int d;
 
    /* There is a hidden attribute telling us the ids of the
     * dimensions that apply to this multi-dimensional coordinate
     * variable. Read it. */
-   if ((coord_attid = H5Aopen_name(var->hdf_datasetid, COORDINATES)) < 0) ret++;
-   if (!ret && (coord_att_typeid = H5Aget_type(coord_attid)) < 0) ret++;
+   if ((coord_attid = H5Aopen_name(var->hdf_datasetid, COORDINATES)) < 0)
+      BAIL(NC_EATTMETA);
+
+   if ((coord_att_typeid = H5Aget_type(coord_attid)) < 0)
+      BAIL(NC_EATTMETA);
 
    /* How many dimensions are there? */
-   if (!ret && (spaceid = H5Aget_space(coord_attid)) < 0) ret++;
-   if (!ret && (npoints = H5Sget_simple_extent_npoints(spaceid)) < 0) ret++;
+   if ((spaceid = H5Aget_space(coord_attid)) < 0)
+      BAIL(NC_EATTMETA);
+   if ((npoints = H5Sget_simple_extent_npoints(spaceid)) < 0)
+      BAIL(NC_EATTMETA);
 
-   /* Check that the number of points is the same as the number of dimensions
-    *   for the variable */
-   if (!ret && npoints != var->ndims) ret++;
+   /* Check that the number of points is the same as the number of
+    * dimensions for the variable. */
+   if (npoints != var->ndims)
+      BAIL(NC_EATTMETA);
 
-   if (!ret && H5Aread(coord_attid, coord_att_typeid, var->dimids) < 0) ret++;
+   if (H5Aread(coord_attid, coord_att_typeid, var->dimids) < 0)
+      BAIL(NC_EATTMETA);
    LOG((4, "dimscale %s is multidimensional and has coords", var->hdr.name));
 
-   /* Update var->dim field based on the var->dimids */
-   for (d = 0; d < var->ndims; d++) {
-      /* Ok if does not find a dim at this time, but if found set it */
+   /* Update var->dim field based on the var->dimids. Ok if does not
+    * find a dim at this time, but if found set it. */
+   for (d = 0; d < var->ndims; d++)
       nc4_find_dim(grp, var->dimids[d], &var->dim[d], NULL);
-   }
 
-   /* Set my HDF5 IDs free! */
-   if (spaceid >= 0 && H5Sclose(spaceid) < 0) ret++;
-   if (coord_att_typeid >= 0 && H5Tclose(coord_att_typeid) < 0) ret++;
-   if (coord_attid >= 0 && H5Aclose(coord_attid) < 0) ret++;
-   return ret ? NC_EATTMETA : NC_NOERR;
+exit:
+   if (spaceid >= 0 && H5Sclose(spaceid) < 0)
+      BAIL2(NC_EHDFERR);
+   if (coord_att_typeid >= 0 && H5Tclose(coord_att_typeid) < 0)
+      BAIL2(NC_EHDFERR);
+   if (coord_attid >= 0 && H5Aclose(coord_attid) < 0)
+      BAIL2(NC_EHDFERR);
+   return retval;
 }
 
 /**
@@ -391,7 +400,7 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc)
    if ((fapl_id = H5Pcreate(H5P_FILE_ACCESS)) < 0)
       BAIL(NC_EHDFERR);
 
-   if (H5Pset_fclose_degree(fapl_id, H5F_CLOSE_SEMI))
+   if (H5Pset_fclose_degree(fapl_id, H5F_CLOSE_SEMI) < 0)
       BAIL(NC_EHDFERR);
 
 #ifdef USE_PARALLEL4
@@ -414,10 +423,9 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc)
             BAIL(NC_EPARINIT);
       }
 #else /* USE_PARALLEL_POSIX */
-      /* Should not happen! Code in NC4_create/NC4_open should alias the
-       *        NC_MPIPOSIX flag to NC_MPIIO, if the MPI-POSIX VFD is not
-       *        available in HDF5. -QAK
-       */
+      /* Should not happen! Code in NC4_create/NC4_open should alias
+       * the NC_MPIPOSIX flag to NC_MPIIO, if the MPI-POSIX VFD is not
+       * available in HDF5. */
       else /* MPI/POSIX */
          BAIL(NC_EPARINIT);
 #endif /* USE_PARALLEL_POSIX */
@@ -438,6 +446,12 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc)
          nc4_info->info = mpiinfo->info;
       }
    }
+
+#ifdef HDF5_HAS_COLL_METADATA_OPS
+   if (H5Pset_all_coll_metadata_ops(fapl_id, 1) < 0)
+      BAIL(NC_EPARINIT);
+#endif
+
 #else /* only set cache for non-parallel. */
    if (H5Pset_cache(fapl_id, 0, nc4_chunk_cache_nelems, nc4_chunk_cache_size,
                     nc4_chunk_cache_preemption) < 0)
@@ -447,13 +461,6 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc)
         nc4_chunk_cache_preemption));
 #endif /* USE_PARALLEL4 */
 
-   /* The NetCDF-3.x prototype contains an mode option NC_SHARE for
-      multiple processes accessing the dataset concurrently.  As there
-      is no HDF5 equivalent, NC_SHARE is treated as NC_NOWRITE. */
-#ifdef HDF5_HAS_COLL_METADATA_OPS
-   H5Pset_all_coll_metadata_ops(fapl_id, 1 );
-#endif
-
    /* Does the mode specify that this file is read-only? */
    if ((mode & NC_WRITE) == 0)
       nc4_info->no_write = NC_TRUE;
@@ -535,9 +542,11 @@ nc4_open_file(const char *path, int mode, void* parameters, NC *nc)
    if (comm_duped) MPI_Comm_free(&nc4_info->comm);
    if (info_duped) MPI_Info_free(&nc4_info->info);
 #endif
-   if (fapl_id != H5P_DEFAULT) H5Pclose(fapl_id);
-   if (!nc4_info) return retval;
-   nc4_close_netcdf4_file(nc4_info,1,NULL); /*  treat like abort*/
+
+   if (fapl_id > 0 && fapl_id != H5P_DEFAULT)
+      H5Pclose(fapl_id);
+   if (nc4_info)
+      nc4_close_netcdf4_file(nc4_info, 1, 0); /*  treat like abort*/
    return retval;
 }
 
@@ -1875,13 +1884,11 @@ static int
 nc4_rec_read_metadata_cb(hid_t grpid, const char *name, const H5L_info_t *info,
                          void *_op_data)
 {
-   NC4_rec_read_metadata_ud_t *udata = (NC4_rec_read_metadata_ud_t *)_op_data; /* Pointer to user data for callback */
+   /* Pointer to user data for callback */
+   NC4_rec_read_metadata_ud_t *udata = (NC4_rec_read_metadata_ud_t *)_op_data;
    NC4_rec_read_metadata_obj_info_t oinfo;    /* Pointer to info for object */
    int retval = H5_ITER_CONT;
 
-   /* Reset the memory for the object's info */
-   memset(&oinfo, 0, sizeof(oinfo));
-
    /* Open this critter. */
    if ((oinfo.oid = H5Oopen(grpid, name, H5P_DEFAULT)) < 0)
       BAIL(H5_ITER_ERROR);
@@ -1898,10 +1905,9 @@ nc4_rec_read_metadata_cb(hid_t grpid, const char *name, const H5L_info_t *info,
    case H5G_GROUP:
       LOG((3, "found group %s", oinfo.oname));
 
-      /* Defer descending into child group immediately, so that the types
-       *     in the current group can be processed and be ready for use by
-       *     vars in the child group(s).
-       */
+      /* Defer descending into child group immediately, so that the
+       * types in the current group can be processed and be ready for
+       * use by vars in the child group(s). */
       if (nc4_rec_read_metadata_cb_list_add(udata, &oinfo))
          BAIL(H5_ITER_ERROR);
       break;
@@ -1915,11 +1921,10 @@ nc4_rec_read_metadata_cb(hid_t grpid, const char *name, const H5L_info_t *info,
                                  &oinfo.statbuf)))
       {
          /* Allow NC_EBADTYPID to transparently skip over datasets
-          *  which have a datatype that netCDF-4 doesn't undertand
-          *  (currently), but break out of iteration for other
-          *  errors.
-          */
-         if(NC_EBADTYPID != retval)
+          * which have a datatype that netCDF-4 doesn't undertand
+          * (currently), but break out of iteration for other
+          * errors. */
+         if (retval != NC_EBADTYPID)
             BAIL(H5_ITER_ERROR);
          else
             retval = H5_ITER_CONT;

libhdf5/hdf5var.c

@@ -217,7 +217,7 @@ nc4_find_default_chunksizes2(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var)
  * @param ncid File ID.
  * @param name Name.
  * @param xtype Type.
- * @param ndims Number of dims.
+ * @param ndims Number of dims. HDF5 has maximim of 32.
  * @param dimidsp Array of dim IDs.
  * @param varidp Gets the var ID.
  *
@@ -259,6 +259,10 @@ NC4_def_var(int ncid, const char *name, nc_type xtype,
       BAIL(retval);
    assert(grp && h5);
 
+   /* HDF5 allows maximum of 32 dimensions. */
+   if (ndims > H5S_MAX_RANK)
+      BAIL(NC_EMAXDIMS);
+
    /* If it's not in define mode, strict nc3 files error out,
     * otherwise switch to define mode. This will also check that the
     * file is writable. */