Further in-code commenting

Author: franzpoeschel

include/openPMD/IO/AbstractIOHandlerImpl.hpp

@@ -338,6 +338,11 @@ class AbstractIOHandlerImpl
    * The attribute should be of datatype parameters.dtype.
    * Any existing attribute with the same name should be overwritten. If possible, only the value should be changed if the datatype stays the same.
    * The attribute should be written to physical storage after the operation completes successfully.
+   * If the parameter changesOverSteps is true, then the attribute must be able
+   * to hold different values across IO steps. If the backend does not support
+   * IO steps in such a way, the attribute should not be written.
+   * (IO steps are an optional backend feature and the frontend must implement
+   * fallback measures in such a case)
    * All datatypes of Datatype should be supported in a type-safe way.
    */
   virtual void writeAttribute(Writable*, Parameter< Operation::WRITE_ATT > const&) = 0;

include/openPMD/Iteration.hpp

@@ -256,6 +256,10 @@ class Iteration : public LegacyAttributable
             auxiliary::Option< std::deque< uint64_t > >;
 
         AdvanceStatus stepStatus{};
+        /*
+         * If the iteration attribute `snapshot` is present, the value of that
+         * attribute. Otherwise empty.
+         */
         AvailableIterations_t iterationsInOpenedStep;
 
         inline operator AdvanceStatus() const

src/Iteration.cpp

@@ -556,43 +556,43 @@ auto Iteration::beginStep() -> BeginStepStatus
     internal::AttributableData * file = nullptr;
     switch( series.iterationEncoding() )
     {
-        case IE::fileBased:
-            file = m_attributableData.get();
-            break;
-        case IE::groupBased:
-        case IE::variableBased:
-            file = &series;
-            break;
-        }
+    case IE::fileBased:
+        file = m_attributableData.get();
+        break;
+    case IE::groupBased:
+    case IE::variableBased:
+        file = &series;
+        break;
+    }
 
-        AdvanceStatus status = series.advance(
-            AdvanceMode::BEGINSTEP, *file, series.indexOf( *this ), *this );
-        switch( status )
-        {
-        case AdvanceStatus::OVER:
-            res.stepStatus = status;
-            return res;
-        case AdvanceStatus::OK:
-        case AdvanceStatus::RANDOMACCESS:
-            break;
-        }
+    AdvanceStatus status = series.advance(
+        AdvanceMode::BEGINSTEP, *file, series.indexOf( *this ), *this );
+    switch( status )
+    {
+    case AdvanceStatus::OVER:
+        res.stepStatus = status;
+        return res;
+    case AdvanceStatus::OK:
+    case AdvanceStatus::RANDOMACCESS:
+        break;
+    }
 
-        // re-read -> new datasets might be available
-        if( ( series.iterationEncoding() == IE::groupBased ||
-              series.iterationEncoding() == IE::variableBased ) &&
-            ( this->IOHandler()->m_frontendAccess == Access::READ_ONLY ||
-              this->IOHandler()->m_frontendAccess == Access::READ_WRITE ) )
-        {
-            bool previous = series.iterations.written();
-            series.iterations.written() = false;
-            auto oldType = this->IOHandler()->m_frontendAccess;
-            auto newType =
-                const_cast< Access * >( &this->IOHandler()->m_frontendAccess );
-            *newType = Access::READ_WRITE;
-            res.iterationsInOpenedStep = series.readGorVBased( false );
-            *newType = oldType;
-            series.iterations.written() = previous;
-        }
+    // re-read -> new datasets might be available
+    if( ( series.iterationEncoding() == IE::groupBased ||
+            series.iterationEncoding() == IE::variableBased ) &&
+        ( this->IOHandler()->m_frontendAccess == Access::READ_ONLY ||
+            this->IOHandler()->m_frontendAccess == Access::READ_WRITE ) )
+    {
+        bool previous = series.iterations.written();
+        series.iterations.written() = false;
+        auto oldType = this->IOHandler()->m_frontendAccess;
+        auto newType =
+            const_cast< Access * >( &this->IOHandler()->m_frontendAccess );
+        *newType = Access::READ_WRITE;
+        res.iterationsInOpenedStep = series.readGorVBased( false );
+        *newType = oldType;
+        series.iterations.written() = previous;
+    }
 
     res.stepStatus = status;
     return res;

src/ReadIterations.cpp

@@ -83,7 +83,14 @@ SeriesIterator::SeriesIterator( Series series )
             Iteration::BeginStepStatus::AvailableIterations_t
                 availableIterations;
             std::tie( status, availableIterations ) = it->second.beginStep();
-            if( availableIterations.has_value() )
+            /*
+             * In random-access mode, do not use the information read in the
+             * `snapshot` attribute, instead simply go through iterations
+             * one by one in ascending order (fallback implementation in the
+             * second if branch).
+             */
+            if( availableIterations.has_value() &&
+                status != AdvanceStatus::RANDOMACCESS )
             {
                 m_iterationsInCurrentStep = availableIterations.get();
                 if( !m_iterationsInCurrentStep.empty() )
@@ -173,9 +180,12 @@ SeriesIterator & SeriesIterator::operator++()
             series.iterations[ m_currentIteration ].beginStep();
             return *this;
         }
-        throw std::runtime_error( "Control flow error!" );
+        throw std::runtime_error( "Unreachable!" );
     }
 
+    // The currently active iterations have been exhausted.
+    // Now see if there are further iterations to be found.
+
     if( series.iterationEncoding() == IterationEncoding::fileBased )
     {
         // this one is handled above, stream is over once it proceeds to here
@@ -193,8 +203,9 @@ SeriesIterator & SeriesIterator::operator++()
     AdvanceStatus status;
     Iteration::BeginStepStatus::AvailableIterations_t availableIterations;
     std::tie( status, availableIterations ) = currentIteration.beginStep();
-    if( availableIterations.has_value()
-        && status != AdvanceStatus::RANDOMACCESS )
+
+    if( availableIterations.has_value() &&
+        status != AdvanceStatus::RANDOMACCESS )
     {
         m_iterationsInCurrentStep = availableIterations.get();
     }

src/Series.cpp

@@ -1084,6 +1084,13 @@ SeriesImpl::readGorVBased( bool do_init )
         [ &series ]() -> auxiliary::Option< std::vector< uint64_t > >
     {
         using vec_t = std::vector< uint64_t >;
+        /*
+         * In variable-based iteration encoding, iterations have no distinct
+         * group within `series.iterations`, meaning that the `snapshot`
+         * attribute is not found at `/data/0/snapshot`, but at
+         * `/data/snapshot`. This makes it possible to retrieve it from
+         * `series.iterations`.
+         */
         if( series.iterations.containsAttribute( "snapshot" ) )
         {
             auto const & attribute =