Add reviewer's suggestions

include/openPMD/Iteration.hpp

@@ -116,7 +116,8 @@ class Iteration : public Attributable
      *        Background: Upon calling Iteration::close(), the openPMD API
      *        will add metadata to the iteration in form of an attribute,
      *        indicating that the iteration has indeed been closed.
-     *        Useful mainly in streaming context.
+     *        Useful mainly in streaming context when a reader inquires from
+     *        a writer that it is done writing.
      *
      * @return Whether the iteration has been explicitly closed (yet) by the
      *         writer.
@@ -136,11 +137,17 @@ class Iteration : public Attributable
     void flush();
     void read();
 
+    /**
+     * @brief Whether an iteration has been closed yet.
+     *
+     */
     enum class CloseStatus
     {
-        Open,
-        ClosedInFrontend,
-        ClosedInBackend
+        Open,             //!< Iteration has not been closed
+        ClosedInFrontend, /*!< Iteration has been closed, but task has not yet
+                               been propagated to the backend */
+        ClosedInBackend   /*!< Iteration has been closed and task has been
+                               propagated to the backend */
     };
 
     /*
@@ -152,14 +159,16 @@ class Iteration : public Attributable
     std::shared_ptr< CloseStatus > m_closed =
         std::make_shared< CloseStatus >( CloseStatus::Open );
 
-    /**
-     * @brief Verify that a closed iteration has not been wrongly accessed.
+    /*
+     * @brief Check recursively whether this Iteration is dirty.
+     *        It is dirty if any attribute or dataset is read from or written to
+     *        the backend.
      *
-     * @return true If closed iteration had no wrong accesses.
+     * @return true If dirty.
      * @return false Otherwise.
      */
     bool
-    verifyClosed() const;
+    dirtyRecursive() const;
 
     virtual void linkHierarchy(std::shared_ptr< Writable > const& w);
 };  // Iteration

include/openPMD/Mesh.hpp

@@ -182,14 +182,6 @@ class Mesh : public BaseRecord< MeshRecordComponent >
 
     void flush_impl(std::string const&) override;
     void read() override;
-
-    /**
-     * @brief Verify that a mesh in a closed iteration has not
-     *        been wrongly accessed.
-     *
-     * @return true If closed iteration had no wrong accesses.
-     * @return false Otherwise.
-     */
 }; // Mesh
 
 template< typename T >

include/openPMD/ParticleSpecies.hpp

@@ -47,14 +47,15 @@ class ParticleSpecies : public Container< Record >
     void flush(std::string const &) override;
 
     /**
-     * @brief Verify that a particle species in a closed iteration has not
-     *        been wrongly accessed.
+     * @brief Check recursively whether this ParticleSpecies is dirty.
+     *        It is dirty if any attribute or dataset is read from or written to
+     *        the backend.
      *
-     * @return true If closed iteration had no wrong accesses.
+     * @return true If dirty.
      * @return false Otherwise.
      */
     bool
-    verifyClosed() const;
+    dirtyRecursive() const;
 };
 
 namespace traits

include/openPMD/RecordComponent.hpp

@@ -200,14 +200,15 @@ class RecordComponent : public BaseRecordComponent
     RecordComponent& makeEmpty( Dataset d );
 
     /**
-     * @brief Verify that a record component in a closed iteration has not
-     *        been wrongly accessed.
+     * @brief Check recursively whether this RecordComponent is dirty.
+     *        It is dirty if any attribute or dataset is read from or written to
+     *        the backend.
      *
-     * @return true If closed iteration had no wrong accesses.
+     * @return true If dirty.
      * @return false Otherwise.
      */
     bool
-    verifyClosed() const;
+    dirtyRecursive() const;
 }; // RecordComponent
 
 

include/openPMD/backend/BaseRecord.hpp

@@ -99,14 +99,15 @@ class BaseRecord : public Container< T_elem >
     virtual void read() = 0;
 
     /**
-     * @brief Verify that a base record in a closed iteration has not
-     *        been wrongly accessed.
+     * @brief Check recursively whether this BaseRecord is dirty.
+     *        It is dirty if any attribute or dataset is read from or written to
+     *        the backend.
      *
-     * @return true If closed iteration had no wrong accesses.
+     * @return true If dirty.
      * @return false Otherwise.
      */
     bool
-    verifyClosed() const;
+    dirtyRecursive() const;
 }; // BaseRecord
 
 
@@ -308,15 +309,15 @@ BaseRecord< T_elem >::flush(std::string const& name)
 
 template< typename T_elem >
 inline bool
-BaseRecord< T_elem >::verifyClosed() const
+BaseRecord< T_elem >::dirtyRecursive() const
 {
     if( Attributable::dirty )
     {
         return false;
     }
     for( auto const & pair : *this )
     {
-        if( !pair.second.verifyClosed() )
+        if( !pair.second.dirtyRecursive() )
         {
             return false;
         }

src/IO/HDF5/HDF5IOHandler.cpp

@@ -406,15 +406,17 @@ HDF5IOHandlerImpl::closeFile(
     auto fileID_it = m_fileIDs.find( writable );
     if( fileID_it == m_fileIDs.end() )
     {
+        throw std::runtime_error(
+            "[HDF5] Trying to close a file that is not "
+            "present in the backend" );
         return;
     }
     hid_t fileID = fileID_it->second;
     H5Fclose( fileID );
     m_openFileIDs.erase( fileID );
     m_fileIDs.erase( fileID_it );
 
-    /*
-     * std::unordered_map::erase:
+    /* std::unordered_map::erase:
      * References and iterators to the erased elements are invalidated. Other
      * iterators and references are not invalidated.
      */

src/Iteration.cpp

@@ -20,14 +20,15 @@
  */
 #include "openPMD/Iteration.hpp"
 
-#include <tuple>
-
 #include "openPMD/Dataset.hpp"
 #include "openPMD/Datatype.hpp"
 #include "openPMD/Series.hpp"
+#include "openPMD/auxiliary/DerefDynamicCast.hpp"
 #include "openPMD/auxiliary/StringManip.hpp"
 #include "openPMD/backend/Writable.hpp"
 
+#include <tuple>
+
 
 namespace openPMD
 {
@@ -113,7 +114,7 @@ Iteration::close( bool _flush )
     *m_closed = CloseStatus::ClosedInFrontend;
     if( _flush )
     {
-        Series * s = dynamic_cast< Series * >(
+        Series * s = &auxiliary::deref_dynamic_cast< Series >(
             parent->attributable->parent->attributable );
         // figure out my iteration number
         uint64_t index;
@@ -431,27 +432,27 @@ Iteration::read()
 }
 
 bool
-Iteration::verifyClosed() const
+Iteration::dirtyRecursive() const
 {
     if( dirty )
     {
-        return false;
+        return true;
     }
     for( auto const & pair : particles )
     {
-        if( !pair.second.verifyClosed() )
+        if( pair.second.dirtyRecursive() )
         {
-            return false;
+            return true;
         }
     }
     for( auto const & pair : meshes )
     {
-        if( !pair.second.verifyClosed() )
+        if( pair.second.dirtyRecursive() )
         {
-            return false;
+            return true;
         }
     }
-    return true;
+    return false;
 }
 
 void

src/ParticleSpecies.cpp

@@ -145,19 +145,19 @@ ParticleSpecies::flush(std::string const& path)
 }
 
 bool
-ParticleSpecies::verifyClosed() const
+ParticleSpecies::dirtyRecursive() const
 {
     if( dirty )
     {
-        return false;
+        return true;
     }
     for( auto const & pair : *this )
     {
-        if( !pair.second.verifyClosed() )
+        if( pair.second.dirtyRecursive() )
         {
-            return false;
+            return true;
         }
     }
-    return true;
+    return false;
 }
 } // namespace openPMD

src/RecordComponent.cpp

@@ -267,12 +267,12 @@ RecordComponent::readBase()
 }
 
 bool
-RecordComponent::verifyClosed() const
+RecordComponent::dirtyRecursive() const
 {
     if( Attributable::dirty )
     {
-        return false;
+        return true;
     }
-    return m_chunks->empty();
+    return !m_chunks->empty();
 }
 } // namespace openPMD

src/Series.cpp

@@ -566,7 +566,7 @@ Series::flushFileBased( IterationsContainer && iterationsToFlush )
                 // file corresponding with the iteration has previously been
                 // closed and fully flushed
                 // verify that there have been no further accesses
-                if( !i.second.verifyClosed() )
+                if( i.second.dirtyRecursive() )
                 {
                     throw std::runtime_error(
                         "[Series] Detected illegal access to iteration that "
@@ -599,7 +599,7 @@ Series::flushFileBased( IterationsContainer && iterationsToFlush )
                         "[Series] Closed iteration has not been written. This "
                         "is an internal error.");
                 }
-                if( !i.second.verifyClosed() )
+                if( i.second.dirtyRecursive() )
                 {
                     throw std::runtime_error(
                         "[Series] Detected illegal access to iteration that "
@@ -657,7 +657,7 @@ Series::flushGroupBased( IterationsContainer && iterationsToFlush )
                 // file corresponding with the iteration has previously been
                 // closed and fully flushed
                 // verify that there have been no further accesses
-                if( !i.second.verifyClosed() )
+                if( i.second.dirtyRecursive() )
                 {
                     throw std::runtime_error(
                         "[Series] Illegal access to iteration " +
@@ -692,7 +692,7 @@ Series::flushGroupBased( IterationsContainer && iterationsToFlush )
                         "[Series] Closed iteration has not been written. This "
                         "is an internal error.");
                 }
-                if( !i.second.verifyClosed() )
+                if( i.second.dirtyRecursive() )
                 {
                     throw std::runtime_error(
                         "[Series] Illegal access to iteration " +

test/SerialIOTest.cpp

@@ -94,19 +94,12 @@ TEST_CASE( "multi_series_test", "[serial]" )
 void
 close_iteration_test( std::string file_ending )
 {
-#if openPMD_HAVE_ADIOS1 && !openPMD_HAVE_ADIOS2
-    bool isAdios1 = file_ending == "bp";
-#elif openPMD_HAVE_ADIOS1 && openPMD_HAVE_ADIOS2
-    bool isAdios1 = file_ending == "bp" &&
-        auxiliary::getEnvString("OPENPMD_BP_BACKEND", "NOT_SET") == "ADIOS1";
-#else
-    bool isAdios1 = false;
-#endif
     std::string name = "../samples/close_iterations_%T." + file_ending;
 
     std::vector<int> data{2, 4, 6, 8};
     // { // we do *not* need these parentheses
     Series write(name, Access::CREATE);
+    bool isAdios1 = write.backend() == "ADIOS1";
     {
         Iteration it0 = write.iterations[ 0 ];
         auto E_x = it0.meshes[ "E" ][ "x" ];