Disallow using Fullname SerialDC with mutiple ranks in write-mode

Flamefire · Flamefire · commit f36f925dd797 · 2016-05-31T12:53:45.000+02:00
Add tests for that
diff --git a/src/SerialDataCollector.cpp b/src/SerialDataCollector.cpp
@@ -69,11 +69,17 @@ 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)
-            return baseFilename;
+        {
+            if (isFullNameAllowed)
+                return baseFilename;
+            else
+                throw DCException("Full filename is not allowed!");
+        }
 
         std::stringstream serial_filename;
         serial_filename << baseFilename << "_" << mpiPos[0] << "_" << mpiPos[1] <<
@@ -166,14 +172,14 @@ namespace splash
 
         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);
@@ -753,7 +759,8 @@ namespace splash
     {
         this->fileStatus = FST_CREATING;
 
-        std::string full_filename = getFullFilename(attr.mpiPosition, filename);
+        std::string full_filename = getFullFilename(attr.mpiPosition, filename,
+                attr.mpiSize.getScalarSize() == 1);
 
         this->enableCompression = attr.enableCompression;
 
@@ -784,7 +791,8 @@ namespace splash
     {
         fileStatus = FST_WRITING;
 
-        std::string full_filename = getFullFilename(attr.mpiPosition, filename);
+        std::string full_filename = getFullFilename(attr.mpiPosition, filename,
+                attr.mpiSize.getScalarSize() == 1);
 
         this->enableCompression = attr.enableCompression;
 
@@ -806,7 +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);
+        std::string full_filename = getFullFilename(Dimensions(0, 0, 0), filename, true);
 
         if (!fileExists(full_filename))
         {
@@ -829,7 +837,7 @@ namespace splash
     {
         this->fileStatus = FST_READING;
 
-        std::string full_filename = getFullFilename(attr.mpiPosition, filename);
+        std::string full_filename = getFullFilename(attr.mpiPosition, filename, true);
 
         if (!fileExists(full_filename))
         {
diff --git a/src/include/splash/SerialDataCollector.hpp b/src/include/splash/SerialDataCollector.hpp
@@ -62,13 +62,15 @@ namespace splash
          * Constructs a filename from a base filename and the process' mpi position
          * such as myfile_0_1_0.h5
          * Does nothing if the filename already ends with ".h5" (only allowed
-         * for Prod(mpiSize)==1).
+         * for Prod(mpiSize)==1 or reading).
          *
          * @param mpiPos MPI position of the process
          * @param baseFilename base filename for the new file
+         * @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.
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -67,15 +67,15 @@ IF(WITH_MPI)
     SET(TESTS ${TESTS} Benchmark Domains)
 ENDIF(WITH_MPI)
 
-OPTION(PARALLEL "enable tests for parallel libSplash" @HDF5_IS_PARALLEL@)
+OPTION(PARALLEL "enable tests for parallel libSplash" ${HDF5_IS_PARALLEL})
 IF(PARALLEL)
     IF(NOT WITH_MPI)
         # MPI is required package
         FIND_PACKAGE(MPI REQUIRED)
         INCLUDE_DIRECTORIES(${MPI_C_INCLUDE_PATH} ${MPI_CXX_INCLUDE_PATH})
     ENDIF(NOT WITH_MPI)
 
-    SET(TESTS ${TESTS} Parallel_Attributes Parallel_Filename Parallel_Domains Parallel_ListFiles Parallel_References Parallel_Remove Parallel_SimpleData Parallel_ZeroAccess)
+    SET(TESTS ${TESTS} Parallel_Attributes Parallel_Filename Parallel_Domains Parallel_ListFiles Parallel_References Parallel_Remove Parallel_SerialDC Parallel_SimpleData Parallel_ZeroAccess)
 ELSE(PARALLEL)
     SET(TESTS ${TESTS})
 ENDIF(PARALLEL)
diff --git a/tests/Parallel_SerialDCTest.cpp b/tests/Parallel_SerialDCTest.cpp
@@ -0,0 +1,253 @@
+/**
+ * Copyright 2016 Alexander Grund
+ *
+ * This file is part of libSplash.
+ *
+ * libSplash is free software: you can redistribute it and/or modify
+ * it under the terms of of either the GNU General Public License or
+ * the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * libSplash is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License and the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * and the GNU Lesser General Public License along with libSplash.
+ * If not, see <http://www.gnu.org/licenses/>.
+ */
+ 
+ #include <cppunit/extensions/HelperMacros.h>
+
+#include "splash/splash.h"
+#include <mpi.h>
+#include <unistd.h>
+#include <sys/stat.h>
+
+using namespace splash;
+
+class FilenameTest : public CPPUNIT_NS::TestFixture
+{
+    CPPUNIT_TEST_SUITE(FilenameTest);
+
+    CPPUNIT_TEST(testBaseName);
+    CPPUNIT_TEST(testFullName);
+
+    CPPUNIT_TEST_SUITE_END();
+
+public:
+
+    FilenameTest();
+    virtual ~FilenameTest();
+
+private:
+    /**
+     * Tests that file naming is correct when passing only base name
+     */
+    void testBaseName();
+
+    /**
+     * Tests that file naming is correct when passing full name
+     */
+    void testFullName();
+
+    bool fileExists(const std::string& filename);
+
+    DataCollector *dataCollector;
+    ColTypeInt ctInt;
+    int mpiRank;
+    static int initCt;
+};
+
+int FilenameTest::initCt = 0;
+
+CPPUNIT_TEST_SUITE_REGISTRATION(FilenameTest);
+
+#define HDF5_FILE "h5/testParallelSerialDC"
+#define HDF5_BASE_FILE HDF5_FILE "_0_0_0.h5"
+#define HDF5_FULL_FILE HDF5_FILE "Full.h5"
+
+#define MPI_SIZE_X 4
+#define MPI_SIZE_Y 2
+
+FilenameTest::FilenameTest()
+{
+    if(!initCt)
+    {
+        MPI_Init(NULL, NULL);
+        srand(time(NULL));
+        // Remove old files
+        unlink(HDF5_BASE_FILE);
+        unlink(HDF5_FULL_FILE);
+    }
+    initCt++;
+
+    int mpiSize;
+    MPI_Comm_rank(MPI_COMM_WORLD, &mpiRank);
+    MPI_Comm_size(MPI_COMM_WORLD, &mpiSize);
+
+    CPPUNIT_ASSERT(mpiSize == (MPI_SIZE_X * MPI_SIZE_Y));
+
+    dataCollector = new SerialDataCollector(10);
+}
+
+FilenameTest::~FilenameTest()
+{
+    if (dataCollector)
+    {
+        delete dataCollector;
+        dataCollector = NULL;
+    }
+    if(!--initCt)
+        MPI_Finalize();
+}
+
+bool FilenameTest::fileExists(const std::string& filename)
+{
+    struct stat fileInfo;
+    return (stat(filename.c_str(), &fileInfo) == 0);
+}
+
+void FilenameTest::testBaseName()
+{
+    // MPI rank should be appended
+    CPPUNIT_ASSERT(!fileExists(HDF5_BASE_FILE));
+
+    // Wait till all checked the above
+    MPI_Barrier(MPI_COMM_WORLD);
+
+    DataCollector::FileCreationAttr attr;
+    DataCollector::initFileCreationAttr(attr);
+    attr.fileAccType = DataCollector::FAT_CREATE;
+    attr.mpiSize = Dimensions(MPI_SIZE_X, MPI_SIZE_Y, 1);
+    attr.mpiPosition = Dimensions(mpiRank % MPI_SIZE_X, mpiRank / MPI_SIZE_X, 0);
+
+    // write first dataset to file (create file)
+    dataCollector->open(HDF5_FILE, attr);
+    int data1 = rand();
+
+    dataCollector->write(2, ctInt, 1, Selection(Dimensions(1, 1, 1)), "data1", &data1);
+    dataCollector->close();
+
+    // Now file must exist
+    CPPUNIT_ASSERT(fileExists(HDF5_BASE_FILE));
+
+    // write second dataset to file (write to existing file of same name
+    attr.fileAccType = DataCollector::FAT_WRITE;
+    dataCollector->open(HDF5_FILE, attr);
+    int data2 = rand();
+
+    dataCollector->write(2, ctInt, 1, Selection(Dimensions(1, 1, 1)), "data2", &data2);
+    dataCollector->close();
+
+
+    // read data from file
+    attr.fileAccType = DataCollector::FAT_READ;
+    Dimensions data_size;
+
+    int data = -1;
+    dataCollector->open(HDF5_FILE, attr);
+
+    dataCollector->read(2, "data1", data_size, &data);
+
+    CPPUNIT_ASSERT(data_size.getScalarSize() == 1);
+    CPPUNIT_ASSERT(data == data1);
+
+    dataCollector->read(2, "data2", data_size, &data);
+
+    CPPUNIT_ASSERT(data_size.getScalarSize() == 1);
+    CPPUNIT_ASSERT(data == data2);
+
+    dataCollector->close();
+
+    // erase file
+    attr.fileAccType = DataCollector::FAT_CREATE;
+    dataCollector->open(HDF5_FILE, attr);
+
+    CPPUNIT_ASSERT_THROW(dataCollector->read(2, "data1", data_size, &data), DCException);
+    int data3 = rand();
+    dataCollector->write(2, ctInt, 1, Selection(Dimensions(1, 1, 1)), "data1", &data3);
+    dataCollector->close();
+
+
+    data = -1;
+    // Read from created file
+    attr.fileAccType = DataCollector::FAT_READ;
+    dataCollector->open(HDF5_FILE, attr);
+
+    dataCollector->read(2, "data1", data_size, &data);
+
+    CPPUNIT_ASSERT(data_size.getScalarSize() == 1);
+    CPPUNIT_ASSERT(data == data3);
+    dataCollector->close();
+}
+
+void FilenameTest::testFullName()
+{
+    // MPI rank should be appended
+    CPPUNIT_ASSERT(!fileExists(HDF5_FULL_FILE));
+
+    // Wait till all checked the above
+    MPI_Barrier(MPI_COMM_WORLD);
+
+    DataCollector::FileCreationAttr attr;
+    DataCollector::initFileCreationAttr(attr);
+    attr.fileAccType = DataCollector::FAT_CREATE;
+    attr.mpiSize = Dimensions(MPI_SIZE_X, MPI_SIZE_Y, 1);
+    attr.mpiPosition = Dimensions(mpiRank % MPI_SIZE_X, mpiRank / MPI_SIZE_X, 0);
+
+    CPPUNIT_ASSERT_THROW(dataCollector->open(HDF5_FULL_FILE, attr), DCException);
+    dataCollector->close();
+    attr.fileAccType = DataCollector::FAT_WRITE;
+    CPPUNIT_ASSERT_THROW(dataCollector->open(HDF5_FULL_FILE, attr), DCException);
+    dataCollector->close();
+
+    int data1, data2;
+    if (mpiRank == 0)
+    {
+        attr.fileAccType = DataCollector::FAT_CREATE;
+        attr.mpiSize = Dimensions(1, 1, 1);
+        dataCollector->open(HDF5_FULL_FILE, attr);
+        data1 = rand();
+
+        dataCollector->write(2, ctInt, 1, Selection(Dimensions(1, 1, 1)), "data1", &data1);
+        dataCollector->close();
+
+        // Now file must exist
+        CPPUNIT_ASSERT(fileExists(HDF5_FULL_FILE));
+
+        // write second dataset to file (write to existing file of same name
+        attr.fileAccType = DataCollector::FAT_WRITE;
+        dataCollector->open(HDF5_FULL_FILE, attr);
+        data2 = rand();
+
+        dataCollector->write(2, ctInt, 1, Selection(Dimensions(1, 1, 1)), "data2", &data2);
+        dataCollector->close();
+        attr.mpiSize = Dimensions(MPI_SIZE_X, MPI_SIZE_Y, 1);
+    }
+    MPI_Bcast(&data1, 1, MPI_INT, 0, MPI_COMM_WORLD);
+    MPI_Bcast(&data2, 1, MPI_INT, 0, MPI_COMM_WORLD);
+
+    // read data from file
+    attr.fileAccType = DataCollector::FAT_READ;
+    Dimensions data_size;
+
+    int data = -1;
+    dataCollector->open(HDF5_FULL_FILE, attr);
+
+    dataCollector->read(2, "data1", data_size, &data);
+
+    CPPUNIT_ASSERT(data_size.getScalarSize() == 1);
+    CPPUNIT_ASSERT(data == data1);
+
+    dataCollector->read(2, "data2", data_size, &data);
+
+    CPPUNIT_ASSERT(data_size.getScalarSize() == 1);
+    CPPUNIT_ASSERT(data == data2);
+
+    dataCollector->close();
+}
+
diff --git a/tests/run_parallel_tests b/tests/run_parallel_tests
@@ -55,6 +55,8 @@ function testMPI()
 testMPI ./Parallel_SimpleDataTest.cpp.out 8 "Testing simple data read/write (parallel)..."
 testMPI ./Parallel_FilenameTest.cpp.out 1 "Testing file naming (parallel)..."
 
+testMPI ./Parallel_SerialDC.cpp.out 8 "Testing SerialDataCollector (parallel)..."
+
 testMPI ./Parallel_ListFilesTest.cpp.out 1 "Testing list files (parallel)..."
 
 testMPI ./Parallel_DomainsTest.cpp.out 8 "Testing domains (parallel)..."
