From 941249322abb29565996729df3e0134e0400f7e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Franz=20P=C3=B6schel?= <franz.poeschel@gmail.com>
Date: Wed, 18 Dec 2024 14:15:26 +0100
Subject: [PATCH] Fix flushStep() logic

---
 include/openPMD/Iteration.hpp    |  2 +-
 src/IO/AbstractIOHandlerImpl.cpp | 24 +++++++++++++++++++++++-
 src/Iteration.cpp                | 20 +-------------------
 src/Series.cpp                   | 14 +++++++++++---
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/include/openPMD/Iteration.hpp b/include/openPMD/Iteration.hpp
index e5d3242dbc..e33c0964b5 100644
--- a/include/openPMD/Iteration.hpp
+++ b/include/openPMD/Iteration.hpp
@@ -285,7 +285,7 @@ class Iteration : public Attributable
     void flushFileBased(
         std::string const &, IterationIndex_t, internal::FlushParams const &);
     void flushGroupBased(IterationIndex_t, internal::FlushParams const &);
-    void flushVariableBased(IterationIndex_t, internal::FlushParams const &);
+    void flushVariableBased(internal::FlushParams const &);
     void flush(internal::FlushParams const &);
     void deferParseAccess(internal::DeferredParseAccess);
     /*
diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp
index 7675cc3e07..dbb8839f76 100644
--- a/src/IO/AbstractIOHandlerImpl.cpp
+++ b/src/IO/AbstractIOHandlerImpl.cpp
@@ -22,6 +22,7 @@
 #include "openPMD/IO/AbstractIOHandlerImpl.hpp"
 
 #include "openPMD/auxiliary/Environment.hpp"
+#include "openPMD/auxiliary/TypeTraits.hpp"
 #include "openPMD/backend/Writable.hpp"
 
 #include <iostream>
@@ -301,7 +302,28 @@ std::future<void> AbstractIOHandlerImpl::flush()
                     "] WRITE_ATT: (",
                     parameter.dtype,
                     ") ",
-                    parameter.name);
+                    parameter.name,
+                    "=",
+                    [&]() {
+                        return std::visit(
+                            [&](auto const &val) {
+                                using dtype = std::remove_cv_t<
+                                    std::remove_reference_t<decltype(val)>>;
+                                if constexpr (
+                                    auxiliary::IsArray_v<dtype> ||
+                                    auxiliary::IsVector_v<dtype>)
+                                {
+                                    return vec_as_string(val);
+                                }
+                                else
+                                {
+                                    std::stringstream res;
+                                    res << val;
+                                    return res.str();
+                                }
+                            },
+                            parameter.resource);
+                    });
                 writeAttribute(i.writable, parameter);
                 break;
             }
diff --git a/src/Iteration.cpp b/src/Iteration.cpp
index b51fadd741..53abad548b 100644
--- a/src/Iteration.cpp
+++ b/src/Iteration.cpp
@@ -290,8 +290,7 @@ void Iteration::flushGroupBased(
     }
 }
 
-void Iteration::flushVariableBased(
-    IterationIndex_t i, internal::FlushParams const &flushParams)
+void Iteration::flushVariableBased(internal::FlushParams const &flushParams)
 {
     setDirty(true);
 
@@ -305,23 +304,6 @@ void Iteration::flushVariableBased(
         flush(flushParams);
         break;
     }
-
-    // @todo maybe dont repeat this upon each invocation
-    {
-        /*
-         * In v-based encoding, the snapshot attribute must always be written.
-         * Reason: Even in backends that don't support changing attributes,
-         * variable-based iteration encoding can be used to write one single
-         * iteration. Then, this attribute determines which iteration it is.
-         */
-        Parameter<Operation::WRITE_ATT> wAttr;
-        wAttr.changesOverSteps =
-            Parameter<Operation::WRITE_ATT>::ChangesOverSteps::IfPossible;
-        wAttr.name = "snapshot";
-        wAttr.resource = (unsigned long long)i;
-        wAttr.dtype = Datatype::ULONGLONG;
-        IOHandler()->enqueue(IOTask(this, wAttr));
-    }
 }
 
 void Iteration::flush(internal::FlushParams const &flushParams)
diff --git a/src/Series.cpp b/src/Series.cpp
index 92bd973021..a99c7f92ff 100644
--- a/src/Series.cpp
+++ b/src/Series.cpp
@@ -1521,8 +1521,8 @@ void Series::flushGorVBased(
                             "Iterations must be the same backend object as the "
                             "Iterations themselves.");
                     }
-                    series.m_currentlyActiveIterations.emplace(it->first);
                 }
+                series.m_currentlyActiveIterations.emplace(it->first);
                 switch (iterationEncoding())
                 {
                     using IE = IterationEncoding;
@@ -1530,7 +1530,7 @@ void Series::flushGorVBased(
                     it->second.flushGroupBased(it->first, flushParams);
                     break;
                 case IE::variableBased:
-                    it->second.flushVariableBased(it->first, flushParams);
+                    it->second.flushVariableBased(flushParams);
                     break;
                 default:
                     throw std::runtime_error(
@@ -2636,8 +2636,16 @@ void Series::flushStep(bool doFlush)
          * one IO step.
          */
         Parameter<Operation::WRITE_ATT> wAttr;
+        /*
+         * In v-based encoding, the snapshot attribute must always be written.
+         * Reason: Even in backends that don't support changing attributes,
+         * variable-based iteration encoding can be used to write one single
+         * iteration. Then, this attribute determines which iteration it is.
+         */
         wAttr.changesOverSteps =
-            Parameter<Operation::WRITE_ATT>::ChangesOverSteps::Yes;
+            iterationEncoding() == IterationEncoding::variableBased
+            ? Parameter<Operation::WRITE_ATT>::ChangesOverSteps::IfPossible
+            : Parameter<Operation::WRITE_ATT>::ChangesOverSteps::Yes;
         wAttr.name = "snapshot";
         wAttr.resource = std::vector<unsigned long long>{
             series.m_currentlyActiveIterations.begin(),