Fix Iteration::open(), needed for correct use of Span API (#1794)

* Shortcut zero-size storeChunk calls without breaking collectivity

* Fix the documentation

* Fix: Iteration::open() should access files

* Revert "Fix the documentation"

This reverts commit dfc8cea.

Author: franzpoeschel

include/openPMD/RecordComponent.hpp

@@ -81,16 +81,6 @@ namespace internal
          * Ignored otherwise.
          */
         Attribute m_constantValue{-1};
-        /**
-         * The same std::string that the parent class would pass as parameter to
-         * RecordComponent::flush().
-         * This is stored only upon RecordComponent::flush() if
-         * AbstractIOHandler::flushLevel is set to FlushLevel::SkeletonOnly
-         * (for use by the Span<T>-based overload of
-         * RecordComponent::storeChunk()).
-         * @todo Merge functionality with ownKeyInParent?
-         */
-        std::string m_name;
         /**
          * True if this component is an empty dataset, i.e. its extent is zero
          * in at least one dimension.
@@ -109,7 +99,6 @@ namespace internal
             BaseRecordComponentData::reset();
             m_chunks = std::queue<IOTask>();
             m_constantValue = -1;
-            m_name = std::string();
             m_isEmpty = false;
             m_hasBeenExtended = false;
         }

include/openPMD/RecordComponent.tpp

@@ -29,6 +29,7 @@
 #include "openPMD/auxiliary/ShareRawInternal.hpp"
 #include "openPMD/auxiliary/TypeTraits.hpp"
 #include "openPMD/auxiliary/UniquePtr.hpp"
+#include "openPMD/backend/Attributable.hpp"
 
 #include <memory>
 #include <type_traits>
@@ -111,18 +112,10 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer)
                 "using storeChunk() (see RecordComponent::resetDataset()).");
         }
         Parameter<Operation::CREATE_DATASET> dCreate(rc.m_dataset.value());
-        dCreate.name = rc.m_name;
+        dCreate.name = Attributable::get().m_writable.ownKeyWithinParent;
         IOHandler()->enqueue(IOTask(this, dCreate));
     }
 
-    if (size == 0)
-    {
-        // Don't forward this operation to the backend as it might create ugly
-        // zero-blocks in ADIOS2
-        setDirtyRecursive(true);
-        return DynamicMemoryView<T>();
-    }
-
     Parameter<Operation::GET_BUFFER_VIEW> getBufferView;
     getBufferView.offset = o;
     getBufferView.extent = e;
@@ -136,7 +129,10 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer)
         // type shared_ptr<T> or shared_ptr<T[]>
         auto data = std::forward<F>(createBuffer)(size);
         out.ptr = static_cast<void *>(data.get());
-        storeChunk(std::move(data), std::move(o), std::move(e));
+        if (size > 0)
+        {
+            storeChunk(std::move(data), std::move(o), std::move(e));
+        }
     }
     setDirtyRecursive(true);
     return DynamicMemoryView<T>{std::move(getBufferView), size, *this};

include/openPMD/backend/Writable.hpp

@@ -89,6 +89,7 @@ class Writable final
     friend class ParticleSpecies;
     friend class Series;
     friend class Record;
+    friend class RecordComponent;
     friend class AbstractIOHandlerImpl;
     friend class ADIOS2IOHandlerImpl;
     friend class detail::ADIOS2File;

src/IO/ADIOS/ADIOS2IOHandler.cpp

@@ -1299,6 +1299,17 @@ void ADIOS2IOHandlerImpl::getBufferView(
     auto file = refreshFileFromParent(writable, /* preferParentFile = */ false);
     detail::ADIOS2File &ba = getFileData(file, IfFileNotOpen::ThrowError);
 
+    if (std::any_of(
+            parameters.extent.begin(), parameters.extent.end(), [](auto val) {
+                return val == 0;
+            }))
+    {
+        // Refuse empty operations, ADIOS2 creates ugly zero blocks for them,
+        // tell the frontend to do sth about it instead
+        parameters.out->backendManagedBuffer = false;
+        return;
+    }
+
     std::string name = nameOfVariable(writable);
     switch (m_useSpanBasedPutByDefault)
     {

src/Iteration.cpp

@@ -161,12 +161,21 @@ Iteration &Iteration::open()
         it.m_closed = CloseStatus::Open;
         runDeferredParseAccess();
     }
-    if (getStepStatus() == StepStatus::OutOfStep)
+    switch (getStepStatus())
     {
+    case StepStatus::OutOfStep:
         beginStep(/* reread = */ false);
         setStepStatus(StepStatus::DuringStep);
+        break;
+    case StepStatus::DuringStep:
+    case StepStatus::NoStep: {
+        auto end = begin;
+        ++end;
+        s.flush_impl(begin, end, {FlushLevel::CreateOrOpenFiles});
     }
-    IOHandler()->flush(internal::defaultFlushParams);
+    break;
+    }
+    // IOHandler()->flush(internal::defaultFlushParams);
     return *this;
 }
 

src/RecordComponent.cpp

@@ -262,7 +262,6 @@ void RecordComponent::flush(
     auto &rc = get();
     if (flushParams.flushLevel == FlushLevel::SkeletonOnly)
     {
-        rc.m_name = name;
         return;
     }
     if (access::readOnly(IOHandler()->m_frontendAccess))