Merge pull request #234 from Flamefire/master

Make filename creation consistent

Author: ax3l

src/HandleMgr.cpp

@@ -1,5 +1,5 @@
 /**
- * Copyright 2013 Felix Schmitt
+ * Copyright 2013-2016 Felix Schmitt, Alexander Grund
  *
  * This file is part of libSplash.
  *
@@ -27,17 +27,17 @@
 #include <hdf5.h>
 
 #include "splash/core/HandleMgr.hpp"
+#include "splash/core/logging.hpp"
+#include "splash/core/DCHelper.hpp"
 
 namespace splash
 {
 
     HandleMgr::HandleMgr(uint32_t maxHandles, FileNameScheme fileNameScheme) :
     maxHandles(maxHandles),
-    numHandles(0),
     mpiSize(1, 1, 1),
     fileNameScheme(fileNameScheme),
     fileFlags(0),
-    singleFile(true),
     fileCreateCallback(NULL),
     fileCreateUserData(NULL),
     fileOpenCallback(NULL),
@@ -69,26 +69,53 @@ namespace splash
         return full_msg.str();
     }
 
+    void HandleMgr::setFileNameScheme(FileNameScheme fileNameScheme) throw (DCException)
+    {
+        if (this->fileNameScheme == fileNameScheme)
+            return;
+        if (!filename.empty())
+            throw DCException(getExceptionString("setFileNameScheme",
+                                    "Tried to change scheme while file(s) were still open", ""));
+        this->fileNameScheme = fileNameScheme;
+    }
+
     void HandleMgr::open(Dimensions mpiSize, const std::string baseFilename,
             hid_t fileAccProperties, unsigned flags)
+    throw (DCException)
     {
-        this->numHandles = mpiSize.getScalarSize();
         this->mpiSize.set(mpiSize);
         this->filename = baseFilename;
         this->fileAccProperties = fileAccProperties;
         this->fileFlags = flags;
-        this->singleFile = false;
+        // Validation: For parallel files we normally append MPI rank or iteration number.
+        //             This is disabled by using FNS_FULLNAME
+        //             or when the filename already contains an h5-extension.
+        if (fileNameScheme != FNS_FULLNAME && baseFilename.find(".h5") == baseFilename.length() - 3)
+        {
+            if (mpiSize.getScalarSize() > 1)
+            {
+                throw DCException(getExceptionString("open",
+                        "Passed full filename for parallel file operations",
+                        baseFilename.c_str()));
+            } else
+            {
+                log_msg(1, "\n"
+                        "\tWarning: Passed full filename for parallel file operations: %s\n"
+                        "It is recommended to pass only the base name (no extension)"
+                        "and let the implementation choose a filename.\n", filename.c_str());
+            }
+        }
     }
 
     void HandleMgr::open(const std::string fullFilename,
             hid_t fileAccProperties, unsigned flags)
+    throw (DCException)
     {
-        this->numHandles = 1;
+        setFileNameScheme(FNS_FULLNAME);
         this->mpiSize.set(1, 1, 1);
         this->filename = fullFilename;
         this->fileAccProperties = fileAccProperties;
         this->fileFlags = flags;
-        this->singleFile = true;
     }
 
     uint32_t HandleMgr::indexFromPos(Dimensions& mpiPos)
@@ -125,7 +152,7 @@ namespace splash
     throw (DCException)
     {
         uint32_t index = 0;
-        if (!singleFile)
+        if (fileNameScheme != FNS_FULLNAME)
             index = indexFromPos(mpiPos);
 
         HandleMap::iterator iter = handles.find(index);
@@ -150,28 +177,34 @@ namespace splash
                 leastAccIndex.ctr = 0;
             }
 
-            std::stringstream filenameStream;
-            filenameStream << filename;
-            if (!singleFile)
+            // Append prefix and extension if we don't have a full filename (extension)
+            std::string fullFilename;
+            if (fileNameScheme != FNS_FULLNAME && filename.find(".h5") != filename.length() - 3)
             {
+                std::stringstream filenameStream;
+                filenameStream << filename;
                 if (fileNameScheme == FNS_MPI)
                 {
                     filenameStream << "_" << mpiPos[0] << "_" << mpiPos[1] <<
                             "_" << mpiPos[2] << ".h5";
-                } else
+                } else if(fileNameScheme == FNS_ITERATIONS)
                     filenameStream << "_" << mpiPos[0] << ".h5";
-            }
+                fullFilename = filenameStream.str();
+            }else
+                fullFilename = filename;
 
             H5Handle newHandle = 0;
 
             // serve requests to create files once as create and as read/write afterwards
             if ((fileFlags & H5F_ACC_TRUNC) && (createdFiles.find(index) == createdFiles.end()))
             {
-                newHandle = H5Fcreate(filenameStream.str().c_str(), fileFlags,
+                DCHelper::testFilename(fullFilename);
+
+                newHandle = H5Fcreate(fullFilename.c_str(), fileFlags,
                         H5P_FILE_CREATE_DEFAULT, fileAccProperties);
                 if (newHandle < 0)
                     throw DCException(getExceptionString("get", "Failed to create file",
-                        filenameStream.str().c_str()));
+                            fullFilename.c_str()));
 
                 createdFiles.insert(index);
 
@@ -184,10 +217,10 @@ namespace splash
                 if (fileFlags & H5F_ACC_TRUNC)
                     tmp_flags = H5F_ACC_RDWR;
 
-                newHandle = H5Fopen(filenameStream.str().c_str(), tmp_flags, fileAccProperties);
+                newHandle = H5Fopen(fullFilename.c_str(), tmp_flags, fileAccProperties);
                 if (newHandle < 0)
                     throw DCException(getExceptionString("get", "Failed to open file",
-                        filenameStream.str().c_str()));
+                            fullFilename.c_str()));
 
                 if (fileOpenCallback)
                     fileOpenCallback(newHandle, index, fileOpenUserData);
@@ -224,8 +257,6 @@ namespace splash
         leastAccIndex.ctr = 0;
         leastAccIndex.index = 0;
         mpiSize.set(1, 1, 1);
-        numHandles = 0;
-        singleFile = true;
 
         // close all remaining handles
         HandleMap::const_iterator iter = handles.begin();

src/ParallelDataCollector.cpp

@@ -1,5 +1,5 @@
 /**
- * Copyright 2013-2015 Felix Schmitt, Axel Huebl
+ * Copyright 2013-2016 Felix Schmitt, Axel Huebl, Alexander Grund
  *
  * This file is part of libSplash.
  *
@@ -67,14 +67,6 @@ namespace splash
         log_msg(3, "Raw Data Cache (File) = %llu KiB", (long long unsigned) (rawCacheSize / 1024));
     }
 
-    std::string ParallelDataCollector::getFullFilename(uint32_t id, std::string baseFilename)
-    {
-        std::stringstream serial_filename;
-        serial_filename << baseFilename << "_" << id << ".h5";
-
-        return serial_filename.str();
-    }
-
     std::string ParallelDataCollector::getExceptionString(std::string func, std::string msg,
             const char *info)
     {
@@ -201,15 +193,16 @@ namespace splash
 
     ParallelDataCollector::~ParallelDataCollector()
     {
+        close();
         H5Pclose(fileAccProperties);
+        finalize();
     }
 
     void ParallelDataCollector::finalize()
     {
-        log_msg(1, "finalizing data collector");
-
         if (options.mpiComm != MPI_COMM_NULL)
         {
+            log_msg(1, "finalizing data collector");
             MPI_Comm_free(&options.mpiComm);
             options.mpiComm = MPI_COMM_NULL;
         }
@@ -245,6 +238,9 @@ namespace splash
 
     void ParallelDataCollector::close()
     {
+        if (fileStatus == FST_CLOSED)
+            return;
+
         log_msg(1, "closing parallel data collector");
 
         // close opened hdf5 file handles

src/SerialDataCollector.cpp

@@ -1,5 +1,5 @@
 /**
- * Copyright 2013-2015 Felix Schmitt, Axel Huebl
+ * Copyright 2013-2016 Felix Schmitt, Axel Huebl, Alexander Grund
  *
  * This file is part of libSplash.
  *
@@ -35,7 +35,6 @@
 #include "splash/core/DCAttribute.hpp"
 #include "splash/core/DCDataSet.hpp"
 #include "splash/core/DCGroup.hpp"
-#include "splash/core/DCHelper.hpp"
 #include "splash/core/SDCHelper.hpp"
 #include "splash/core/logging.hpp"
 #include "splash/basetypes/basetypes.hpp"
@@ -70,8 +69,18 @@ namespace splash
         return (stat(filename.c_str(), &fileInfo) == 0);
     }
 
-    std::string SerialDataCollector::getFullFilename(const Dimensions mpiPos, std::string baseFilename) const
+    std::string SerialDataCollector::getFullFilename(const Dimensions mpiPos, std::string baseFilename,
+            bool isFullNameAllowed) const throw (DCException)
     {
+        // Check for existing extension
+        if (baseFilename.find(".h5") == baseFilename.length() - 3)
+        {
+            if (isFullNameAllowed)
+                return baseFilename;
+            else
+                throw DCException("Full filename is not allowed!");
+        }
+
         std::stringstream serial_filename;
         serial_filename << baseFilename << "_" << mpiPos[0] << "_" << mpiPos[1] <<
                 "_" << mpiPos[2] << ".h5";
@@ -97,7 +106,7 @@ namespace splash
      *******************************************************************************/
 
     SerialDataCollector::SerialDataCollector(uint32_t maxFileHandles) :
-    handles(maxFileHandles, HandleMgr::FNS_MPI),
+    handles(maxFileHandles, HandleMgr::FNS_FULLNAME),
     fileStatus(FST_CLOSED),
     maxID(-1),
     mpiTopology(1, 1, 1)
@@ -113,7 +122,7 @@ namespace splash
                 "failed to initialize/open HDF5 library"));
 
 #ifndef SPLASH_VERBOSE_HDF5
-        // surpress automatic output of HDF5 exception messages
+        // Suppress automatic output of HDF5 exception messages
         if (H5Eset_auto2(H5E_DEFAULT, NULL, NULL) < 0)
             throw DCException(getExceptionString("SerialDataCollector",
                 "failed to disable error printing"));
@@ -125,6 +134,7 @@ namespace splash
 
     SerialDataCollector::~SerialDataCollector()
     {
+        close();
     }
 
     void SerialDataCollector::open(const char* filename, FileCreationAttr &attr)
@@ -157,16 +167,19 @@ namespace splash
 
     void SerialDataCollector::close()
     {
+        if (fileStatus == FST_CLOSED)
+            return;
+
         log_msg(1, "closing serial data collector");
 
-        if (fileStatus == FST_CREATING || fileStatus == FST_WRITING)
+        if ((fileStatus == FST_CREATING || fileStatus == FST_WRITING) &&
+            maxID >= 0)
         {
-            DCGroup group;
-            group.open(handles.get(0), SDC_GROUP_HEADER);
-
             // write number of iterations
             try
             {
+                DCGroup group;
+                group.open(handles.get(0), SDC_GROUP_HEADER);
                 ColTypeInt32 ctInt32;
                 DCAttribute::writeAttribute(SDC_ATTR_MAX_ID, ctInt32.getDataType(),
                         group.getHandle(), &maxID);
@@ -746,9 +759,8 @@ namespace splash
     {
         this->fileStatus = FST_CREATING;
 
-        // appends the mpiPosition to the filename (e.g. myfile_0_1_0.h5)
-        std::string full_filename = getFullFilename(attr.mpiPosition, filename);
-        DCHelper::testFilename(full_filename);
+        std::string full_filename = getFullFilename(attr.mpiPosition, filename,
+                attr.mpiSize.getScalarSize() == 1);
 
         this->enableCompression = attr.enableCompression;
 
@@ -779,11 +791,8 @@ namespace splash
     {
         fileStatus = FST_WRITING;
 
-        std::string full_filename = filename;
-        if (full_filename.find(".h5") == std::string::npos)
-            full_filename = getFullFilename(attr.mpiPosition, filename);
-
-        DCHelper::testFilename(full_filename);
+        std::string full_filename = getFullFilename(attr.mpiPosition, filename,
+                attr.mpiSize.getScalarSize() == 1);
 
         this->enableCompression = attr.enableCompression;
 
@@ -805,9 +814,7 @@ namespace splash
         this->fileStatus = FST_MERGING;
 
         // open reference file to get mpi information
-        std::string full_filename = getFullFilename(Dimensions(0, 0, 0), filename);
-
-        DCHelper::testFilename(full_filename);
+        std::string full_filename = getFullFilename(Dimensions(0, 0, 0), filename, true);
 
         if (!fileExists(full_filename))
         {
@@ -821,6 +828,7 @@ namespace splash
         // no compression for in-memory datasets
         this->enableCompression = false;
 
+        handles.setFileNameScheme(HandleMgr::FNS_MPI);
         handles.open(mpiTopology, filename, fileAccProperties, H5F_ACC_RDONLY);
     }
 
@@ -829,11 +837,7 @@ namespace splash
     {
         this->fileStatus = FST_READING;
 
-        std::string full_filename = filename;
-        if (full_filename.find(".h5") == std::string::npos)
-            full_filename = getFullFilename(attr.mpiPosition, filename);
-
-        DCHelper::testFilename(full_filename);
+        std::string full_filename = getFullFilename(attr.mpiPosition, filename, true);
 
         if (!fileExists(full_filename))
         {

src/include/splash/ParallelDataCollector.hpp

@@ -54,16 +54,6 @@ namespace splash
          */
         void setFileAccessParams(hid_t& fileAccProperties);
 
-        /**
-         * Constructs a filename from a base filename and the current id
-         * such as baseFilename+id+.h5
-         *
-         * @param id Iteration ID.
-         * @param baseFilename Base filename for the new file.
-         * @return newly Constructed filename including file extension.
-         */
-        static std::string getFullFilename(uint32_t id, std::string baseFilename);
-
         /**
          * Internal function for formatting exception messages.
          *

src/include/splash/SerialDataCollector.hpp

@@ -60,13 +60,17 @@ namespace splash
 
         /**
          * Constructs a filename from a base filename and the process' mpi position
-         * such as baseFilename+mpiPos+.h5
+         * such as myfile_0_1_0.h5
+         * Does nothing if the filename already ends with ".h5" (only allowed
+         * for Prod(mpiSize)==1 or reading).
          *
          * @param mpiPos MPI position of the process
          * @param baseFilename base filename for the new file
-         * @return newly constructed filename iucluding file exitension
+         * @param isFullNameAllowed If false, an exception is raised when a full name is passed
+         * @return newly constructed filename including file extension
          */
-        std::string getFullFilename(const Dimensions mpiPos, std::string baseFilename) const;
+        std::string getFullFilename(const Dimensions mpiPos, std::string baseFilename,
+                bool isFullNameAllowed) const throw (DCException);
 
         /**
          * Internal function for formatting exception messages.