Merge pull request #1035 from pnorbert/fix_attributes

Serialize new attributes in non-first steps in BP3Serializer.
ornladios · Dec 4, 2018 · bae9c99 · bae9c99
2 parents 97a0ee0 + 550e560
commit bae9c99
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 18 deletions.
diff --git a/source/adios2/engine/insitumpi/InSituMPIReader.cpp b/source/adios2/engine/insitumpi/InSituMPIReader.cpp
@@ -182,7 +182,6 @@ StepStatus InSituMPIReader::BeginStep(const StepMode mode,
 
         // Parse metadata into Variables and Attributes maps
         m_IO.RemoveAllVariables();
-        m_IO.RemoveAllAttributes();
         m_BP3Deserializer.ParseMetadata(m_BP3Deserializer.m_Metadata, m_IO);
 
         if (m_Verbosity == 5)

diff --git a/source/adios2/toolkit/format/bp3/BP3Base.h b/source/adios2/toolkit/format/bp3/BP3Base.h
@@ -17,6 +17,7 @@
 #include <set>
 #include <string>
 #include <unordered_map>
+#include <unordered_set>
 #include <vector>
 /// \endcond
 
@@ -87,8 +88,6 @@ class BP3Base
         /** @brief key: attribute name, value: bp metadata attribute index */
         std::unordered_map<std::string, SerialElementIndex> AttributesIndices;
 
-        bool AreAttributesWritten = false;
-
         /** Fixed size for mini footer, adding 28 bytes for ADIOS version */
         const unsigned int MiniFooterSize = 28 + 28;
 
@@ -176,6 +175,11 @@ class BP3Base
     /** tracks the overall size of deferred variables */
     size_t m_DeferredVariablesDataSize = 0;
 
+    /** attributes are serialized only once, this set contains the names of ones
+     * already serialized.
+     */
+    std::unordered_set<std::string> m_SerializedAttributes;
+
     /**
      * Unique constructor
      * @param mpiComm for m_BP1Aggregator

diff --git a/source/adios2/toolkit/format/bp3/BP3Serializer.cpp b/source/adios2/toolkit/format/bp3/BP3Serializer.cpp
@@ -482,6 +482,14 @@ void BP3Serializer::PutAttributes(core::IO &io)
         const std::string name(attributePair.first);
         const std::string type(attributePair.second.first);
 
+        // each attribute is only written to output once
+        // so filter out the ones already written
+        auto it = m_SerializedAttributes.find(name);
+        if (it != m_SerializedAttributes.end())
+        {
+            continue;
+        }
+
         if (type == "unknown")
         {
         }
@@ -635,20 +643,21 @@ void BP3Serializer::SerializeDataBuffer(core::IO &io) noexcept
     helper::CopyToBuffer(buffer, m_MetadataSet.DataPGVarsCountPosition,
                          &varsLength);
 
-    // attributes are only written once
-
-    if (!m_MetadataSet.AreAttributesWritten)
+    // each attribute is only written to output once
+    size_t attributesSizeInData = GetAttributesSizeInData(io);
+    if (attributesSizeInData)
     {
-        const size_t attributesSizeInData = GetAttributesSizeInData(io);
+        attributesSizeInData += 12; // count + length
         m_Data.Resize(position + attributesSizeInData,
                       "when writing Attributes in rank=0\n");
 
         PutAttributes(io);
-        m_MetadataSet.AreAttributesWritten = true;
     }
     else
     {
         m_Data.Resize(position + 12, "for empty Attributes\n");
+        // Attribute index header for zero attributes: 0, 0LL
+        // Resize() already takes care of this
         position += 12;
         absolutePosition += 12;
     }
@@ -1614,14 +1623,22 @@ uint32_t BP3Serializer::GetFileIndex() const noexcept
 
 size_t BP3Serializer::GetAttributesSizeInData(core::IO &io) const noexcept
 {
-    size_t attributesSizeInData = 12; // count + length
+    size_t attributesSizeInData = 0;
 
     auto &attributes = io.GetAttributesDataMap();
 
     for (const auto &attribute : attributes)
     {
         const std::string type = attribute.second.first;
 
+        // each attribute is only written to output once
+        // so filter out the ones already written
+        auto it = m_SerializedAttributes.find(attribute.first);
+        if (it != m_SerializedAttributes.end())
+        {
+            continue;
+        }
+
         if (type == "compound")
         {
         }

diff --git a/source/adios2/toolkit/format/bp3/BP3Serializer.tcc b/source/adios2/toolkit/format/bp3/BP3Serializer.tcc
@@ -334,8 +334,9 @@ void BP3Serializer::PutAttributeInIndex(const core::Attribute<T> &attribute,
     helper::CopyToBuffer(buffer, backPosition,
                          &characteristicsLength); // length
 
-    // Finish characteristic count length
+    // Remember this attribute and its serialized piece
     m_MetadataSet.AttributesIndices.emplace(attribute.m_Name, index);
+    m_SerializedAttributes.emplace(attribute.m_Name);
 }
 
 template <>

diff --git a/testing/adios2/engine/common/CMakeLists.txt b/testing/adios2/engine/common/CMakeLists.txt
@@ -19,9 +19,9 @@ endif()
 #   2nd arg: 1 for serialized execution, 0 for concurrent execution of Writer/Reader
 #   3rd arg: engine parameters
 
-#gtest_add_tests(TARGET TestEngineCommon ${extra_test_args} 
-#                EXTRA_ARGS "BPFile" "1"
-#                TEST_SUFFIX _BPFile)
+gtest_add_tests(TARGET TestEngineCommon ${extra_test_args} 
+                EXTRA_ARGS "BPFile" "1"
+                TEST_SUFFIX _BPFile)
 
 if(ADIOS2_HAVE_HDF5)
 gtest_add_tests(TARGET TestEngineCommon ${extra_test_args} 
@@ -38,11 +38,11 @@ gtest_add_tests(TARGET TestEngineCommon ${extra_test_args}
                 TEST_SUFFIX _SST_BP)
 endif()
 
-#if(ADIOS2_HAVE_MPI)
-#gtest_add_tests(TARGET TestEngineCommon ${extra_test_args} 
-#                EXTRA_ARGS "InSituMPI" "0"
-#                TEST_SUFFIX _InSituMPI)
-#endif()
+if(ADIOS2_HAVE_MPI)
+gtest_add_tests(TARGET TestEngineCommon ${extra_test_args} 
+                EXTRA_ARGS "InSituMPI" "0"
+                TEST_SUFFIX _InSituMPI)
+endif()
 
 #if(ADIOS2_HAVE_DataMan)
 #gtest_add_tests(TARGET TestEngineCommon ${extra_test_args}