working around an unusual encounter when the joined_dim has actual value "max::size_t - 1" (#1740)

* working around an unusal encounter when the joined_dim has actual
value "max::size_t - 1"

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add test for redundant resetDataset()

* Merge check into above logic

* Better error messages in verifyDataset

* Add further safety guards to createDataset and extendDataset tasks

* Move joinedDim logic into middle-end for extendDataset

* Update include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>

Author: 3 people

include/openPMD/Dataset.hpp

@@ -86,5 +86,6 @@ class Dataset
     bool empty() const;
 
     std::optional<size_t> joinedDimension() const;
+    static std::optional<size_t> joinedDimension(Extent const &);
 };
 } // namespace openPMD

include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp

@@ -34,6 +34,7 @@
 #include "openPMD/IterationEncoding.hpp"
 #include "openPMD/ThrowError.hpp"
 #include "openPMD/auxiliary/JSON_internal.hpp"
+#include "openPMD/auxiliary/StringManip.hpp"
 #include "openPMD/backend/Writable.hpp"
 #include "openPMD/config.hpp"
 #include <stdexcept>
@@ -489,20 +490,44 @@ class ADIOS2IOHandlerImpl
             }
         }
         auto joinedDim = joinedDimension(shape);
-        if (joinedDim.has_value())
+        auto make_runtime_error = [&](char const *message) {
+            std::stringstream s;
+            s << "[ADIOS2IOHandlerImpl::verifyDataset()] " << message;
+            s << "\nNote: Variable '" << varName << "' has shape ";
+            auxiliary::write_vec_to_stream(s, shape)
+                << ", is accessed from offset ";
+            auxiliary::write_vec_to_stream(s, offset) << " with extent ";
+            auxiliary::write_vec_to_stream(s, extent);
+            if (joinedDim.has_value())
+            {
+                s << " (joined dimension on index " << *joinedDim << ").";
+            }
+            else
+            {
+                s << " (no joined dimension).";
+            }
+            return std::runtime_error(s.str());
+        };
+        if (joinedDim.has_value() ||
+            var.ShapeID() == adios2::ShapeID::JoinedArray)
         {
             if (!offset.empty())
             {
-                throw std::runtime_error(
-                    "[ADIOS2] Offset must be an empty vector in case of joined "
-                    "array.");
+                throw make_runtime_error(
+                    "Offset must be an empty vector in case of joined array.");
+            }
+            if (!joinedDim.has_value())
+            {
+                throw make_runtime_error(
+                    "Trying to access a dataset as a non-joined array, but it "
+                    "has previously been array.");
             }
             for (unsigned int i = 0; i < actualDim; i++)
             {
                 if (*joinedDim != i && extent[i] != shape[i])
                 {
-                    throw std::runtime_error(
-                        "[ADIOS2] store_chunk extent of non-joined dimensions "
+                    throw make_runtime_error(
+                        "store_chunk extent of non-joined dimensions "
                         "must be equivalent to the total extent.");
                 }
             }
@@ -514,8 +539,7 @@ class ADIOS2IOHandlerImpl
                 if (!(joinedDim.has_value() && *joinedDim == i) &&
                     offset[i] + extent[i] > shape[i])
                 {
-                    throw std::runtime_error(
-                        "[ADIOS2] Dataset access out of bounds.");
+                    throw make_runtime_error("Dataset access out of bounds.");
                 }
             }
         }

include/openPMD/IO/IOTask.hpp

@@ -116,6 +116,13 @@ struct OPENPMDAPI_EXPORT AbstractParameter
         std::string const &currentBackendName,
         std::string const &warningMessage);
 
+    // Used as a tag in some constructors to make callsites explicitly
+    // acknowledge that joined dimensions will not be automatically resolved
+    struct I_dont_want_to_use_joined_dimensions_t
+    {};
+    constexpr static I_dont_want_to_use_joined_dimensions_t
+        I_dont_want_to_use_joined_dimensions{};
+
 protected:
     // avoid object slicing
     // by allow only child classes to use these things for defining their own
@@ -359,7 +366,17 @@ template <>
 struct OPENPMDAPI_EXPORT Parameter<Operation::CREATE_DATASET>
     : public AbstractParameter
 {
-    Parameter() = default;
+    Parameter(Dataset const &ds)
+        : extent(ds.extent)
+        , dtype(ds.dtype)
+        , options(ds.options)
+        , joinedDimension(ds.joinedDimension())
+    {}
+
+    // default constructor, but callsites need to explicitly acknowledge that
+    // joined dimensions will not be automatically configured when using it
+    Parameter(I_dont_want_to_use_joined_dimensions_t)
+    {}
     Parameter(Parameter &&) = default;
     Parameter(Parameter const &) = default;
     Parameter &operator=(Parameter &&) = default;
@@ -388,7 +405,15 @@ template <>
 struct OPENPMDAPI_EXPORT Parameter<Operation::EXTEND_DATASET>
     : public AbstractParameter
 {
-    Parameter() = default;
+    Parameter(Extent e) : joinedDimension(Dataset::joinedDimension(e))
+    {
+        this->extent = std::move(e);
+    }
+
+    // default constructor, but callsites need to explicitly acknowledge that
+    // joined dimensions will not be automatically configured when using it
+    Parameter(I_dont_want_to_use_joined_dimensions_t)
+    {}
     Parameter(Parameter &&) = default;
     Parameter(Parameter const &) = default;
     Parameter &operator=(Parameter &&) = default;
@@ -401,6 +426,7 @@ struct OPENPMDAPI_EXPORT Parameter<Operation::EXTEND_DATASET>
     }
 
     Extent extent = {};
+    std::optional<size_t> joinedDimension;
 };
 
 template <>

include/openPMD/RecordComponent.tpp

@@ -356,18 +356,14 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer)
     if (!written())
     {
         auto &rc = get();
-        Parameter<Operation::CREATE_DATASET> dCreate;
-        dCreate.name = rc.m_name;
-        dCreate.extent = getExtent();
-        dCreate.dtype = getDatatype();
-        dCreate.joinedDimension = joinedDimension();
         if (!rc.m_dataset.has_value())
         {
             throw error::WrongAPIUsage(
                 "[RecordComponent] Must specify dataset type and extent before "
                 "using storeChunk() (see RecordComponent::resetDataset()).");
         }
-        dCreate.options = rc.m_dataset.value().options;
+        Parameter<Operation::CREATE_DATASET> dCreate(rc.m_dataset.value());
+        dCreate.name = rc.m_name;
         IOHandler()->enqueue(IOTask(this, dCreate));
     }
     Parameter<Operation::GET_BUFFER_VIEW> getBufferView;

include/openPMD/auxiliary/StringManip.hpp

@@ -242,5 +242,40 @@ namespace auxiliary
         });
         return std::forward<S>(s);
     }
+
+    template <typename Stream, typename Vec>
+    auto write_vec_to_stream(Stream &&s, Vec const &vec) -> Stream &&
+    {
+        if (vec.empty())
+        {
+            s << "[]";
+        }
+        else
+        {
+            s << '[';
+            auto it = vec.begin();
+            s << *it++;
+            auto end = vec.end();
+            for (; it != end; ++it)
+            {
+                s << ", " << *it;
+            }
+            s << ']';
+        }
+        return std::forward<Stream>(s);
+    }
+
+    template <typename Vec>
+    auto vec_as_string(Vec const &vec) -> std::string
+    {
+        if (vec.empty())
+        {
+            return "[]";
+        }
+        else
+        {
+            return write_vec_to_stream(std::stringstream(), vec).str();
+        }
+    }
 } // namespace auxiliary
 } // namespace openPMD

src/Dataset.cpp

@@ -68,6 +68,11 @@ bool Dataset::empty() const
 }
 
 std::optional<size_t> Dataset::joinedDimension() const
+{
+    return joinedDimension(extent);
+}
+
+std::optional<size_t> Dataset::joinedDimension(Extent const &extent)
 {
     std::optional<size_t> res;
     for (size_t i = 0; i < extent.size(); ++i)

src/IO/ADIOS/ADIOS2IOHandler.cpp

@@ -873,7 +873,10 @@ namespace detail
     {
         template <typename T, typename... Args>
         static void call(
-            adios2::IO &IO, std::string const &variable, Extent const &newShape)
+            adios2::IO &IO,
+            std::string const &variable,
+            Extent const &newShape,
+            std::optional<size_t> const &newJoinedDim)
         {
             auto var = IO.InquireVariable<T>(variable);
             if (!var)
@@ -888,6 +891,63 @@ namespace detail
             {
                 dims.push_back(ext);
             }
+            auto oldJoinedDim = joinedDimension(var.Shape());
+            auto make_runtime_error = [&](char const *message) {
+                std::stringstream s;
+                s << "[ADIOS2IOHandlerImpl::extendDataset()] " << message
+                  << "\nNote: Variable '" << variable << "' has old shape ";
+                auxiliary::write_vec_to_stream(s, var.Shape());
+                if (oldJoinedDim.has_value())
+                {
+                    s << " (joined dimension on index " << *oldJoinedDim << ")";
+                }
+                else
+                {
+                    s << " (no joined dimension)";
+                }
+                s << " and is extended to new shape ";
+                auxiliary::write_vec_to_stream(s, newShape);
+                if (newJoinedDim.has_value())
+                {
+                    s << " (joined dimension on index " << *newJoinedDim
+                      << ").";
+                }
+                else
+                {
+                    s << " (no joined dimension).";
+                }
+                return std::runtime_error(s.str());
+            };
+            if (oldJoinedDim.has_value() ||
+                var.ShapeID() == adios2::ShapeID::JoinedArray)
+            {
+                if (!oldJoinedDim.has_value())
+                {
+                    throw make_runtime_error(
+                        "Inconsistent state of variable: Has shape ID "
+                        "JoinedArray, but its shape contains no value "
+                        "adios2::JoinedDim.");
+                }
+                if (newJoinedDim != oldJoinedDim)
+                {
+                    throw make_runtime_error(
+                        "Variable was previously configured with a joined "
+                        "dimension, so the new dataset extent must keep the "
+                        "joined dimension on that index.");
+                }
+                dims[*newJoinedDim] = adios2::JoinedDim;
+            }
+            else
+            {
+                if (newJoinedDim.has_value())
+                {
+                    throw make_runtime_error(
+                        "Variable was not previously configured with a "
+                        "joined dimension, but is now requested to change "
+                        "extent to a joined array.");
+                }
+            }
+
             var.SetShape(dims);
         }
 
@@ -907,7 +967,7 @@ void ADIOS2IOHandlerImpl::extendDataset(
     auto &filedata = getFileData(file, IfFileNotOpen::ThrowError);
     Datatype dt = detail::fromADIOS2Type(filedata.m_IO.VariableType(name));
     switchAdios2VariableType<detail::DatasetExtender>(
-        dt, filedata.m_IO, name, parameters.extent);
+        dt, filedata.m_IO, name, parameters.extent, parameters.joinedDimension);
 }
 
 void ADIOS2IOHandlerImpl::openFile(

src/IO/AbstractIOHandlerImpl.cpp

@@ -24,8 +24,8 @@
 #include "openPMD/IO/IOTask.hpp"
 #include "openPMD/Streaming.hpp"
 #include "openPMD/auxiliary/Environment.hpp"
+#include "openPMD/auxiliary/StringManip.hpp"
 #include "openPMD/auxiliary/Variant.hpp"
-#include "openPMD/backend/Attribute.hpp"
 #include "openPMD/backend/Writable.hpp"
 
 #include <iostream>
@@ -47,29 +47,6 @@ AbstractIOHandlerImpl::AbstractIOHandlerImpl(AbstractIOHandler *handler)
 
 namespace
 {
-    template <typename Vec>
-    auto vec_as_string(Vec const &vec) -> std::string
-    {
-        if (vec.empty())
-        {
-            return "[]";
-        }
-        else
-        {
-            std::stringstream res;
-            res << '[';
-            auto it = vec.begin();
-            res << *it++;
-            auto end = vec.end();
-            for (; it != end; ++it)
-            {
-                res << ", " << *it;
-            }
-            res << ']';
-            return res.str();
-        }
-    }
-
     template <typename T, typename SFINAE = void>
     struct self_or_invoked
     {

src/IO/HDF5/HDF5IOHandler.cpp

@@ -840,6 +840,12 @@ void HDF5IOHandlerImpl::extendDataset(
         throw std::runtime_error(
             "[HDF5] Extending an unwritten Dataset is not possible.");
 
+    if (parameters.joinedDimension.has_value())
+    {
+        error::throwOperationUnsupportedInBackend(
+            "HDF5", "Joined Arrays currently only supported in ADIOS2");
+    }
+
     auto res = getFile(writable);
     if (!res)
         res = getFile(writable->parent);

src/IO/JSON/JSONIOHandlerImpl.cpp

@@ -711,6 +711,13 @@ void JSONIOHandlerImpl::extendDataset(
     VERIFY_ALWAYS(
         access::write(m_handler->m_backendAccess),
         "[JSON] Cannot extend a dataset in read-only mode.")
+
+    if (parameters.joinedDimension.has_value())
+    {
+        error::throwOperationUnsupportedInBackend(
+            "JSON", "Joined Arrays currently only supported in ADIOS2");
+    }
+
     setAndGetFilePosition(writable);
     refreshFileFromParent(writable);
     auto &j = obtainJsonContents(writable);