diff --git a/include/openPMD/Dataset.hpp b/include/openPMD/Dataset.hpp index d79380105a..80513683f9 100644 --- a/include/openPMD/Dataset.hpp +++ b/include/openPMD/Dataset.hpp @@ -86,5 +86,6 @@ class Dataset bool empty() const; std::optional joinedDimension() const; + static std::optional joinedDimension(Extent const &); }; } // namespace openPMD diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index 9dee72e02e..45980776e4 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/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 @@ -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."); } } } diff --git a/include/openPMD/IO/IOTask.hpp b/include/openPMD/IO/IOTask.hpp index e637df2ede..5dd3891ad8 100644 --- a/include/openPMD/IO/IOTask.hpp +++ b/include/openPMD/IO/IOTask.hpp @@ -116,6 +116,13 @@ struct OPENPMDAPI_EXPORT AbstractParameter std::string const ¤tBackendName, 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 : 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 : 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 } Extent extent = {}; + std::optional joinedDimension; }; template <> diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 7beaae8b9d..18fe2d98b1 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -356,18 +356,14 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) if (!written()) { auto &rc = get(); - Parameter 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 dCreate(rc.m_dataset.value()); + dCreate.name = rc.m_name; IOHandler()->enqueue(IOTask(this, dCreate)); } Parameter getBufferView; diff --git a/include/openPMD/auxiliary/StringManip.hpp b/include/openPMD/auxiliary/StringManip.hpp index 36778f205f..eb3799d3be 100644 --- a/include/openPMD/auxiliary/StringManip.hpp +++ b/include/openPMD/auxiliary/StringManip.hpp @@ -242,5 +242,40 @@ namespace auxiliary }); return std::forward(s); } + + template + 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(s); + } + + template + 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 diff --git a/src/Dataset.cpp b/src/Dataset.cpp index c1546e9ef0..a56c566805 100644 --- a/src/Dataset.cpp +++ b/src/Dataset.cpp @@ -68,6 +68,11 @@ bool Dataset::empty() const } std::optional Dataset::joinedDimension() const +{ + return joinedDimension(extent); +} + +std::optional Dataset::joinedDimension(Extent const &extent) { std::optional res; for (size_t i = 0; i < extent.size(); ++i) diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index d90fe7c8c7..191729557a 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -873,7 +873,10 @@ namespace detail { template 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 const &newJoinedDim) { auto var = IO.InquireVariable(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( - dt, filedata.m_IO, name, parameters.extent); + dt, filedata.m_IO, name, parameters.extent, parameters.joinedDimension); } void ADIOS2IOHandlerImpl::openFile( diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index 95fb7dfa09..faad2f49a9 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/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 @@ -47,29 +47,6 @@ AbstractIOHandlerImpl::AbstractIOHandlerImpl(AbstractIOHandler *handler) namespace { - template - 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 struct self_or_invoked { diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index f852049463..a66cc42703 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/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); diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp index fb33d1cd29..9df7036156 100644 --- a/src/IO/JSON/JSONIOHandlerImpl.cpp +++ b/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); diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index fc17909fc6..d5420ea3b8 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -312,12 +312,9 @@ void RecordComponent::flush( } else { - Parameter dCreate; + Parameter dCreate( + rc.m_dataset.value()); dCreate.name = name; - dCreate.extent = getExtent(); - dCreate.dtype = getDatatype(); - dCreate.options = rc.m_dataset.value().options; - dCreate.joinedDimension = joinedDimension(); IOHandler()->enqueue(IOTask(this, dCreate)); } } @@ -342,8 +339,8 @@ void RecordComponent::flush( } else { - Parameter pExtend; - pExtend.extent = rc.m_dataset.value().extent; + Parameter pExtend( + rc.m_dataset.value().extent); IOHandler()->enqueue(IOTask(this, std::move(pExtend))); rc.m_hasBeenExtended = false; } diff --git a/src/Series.cpp b/src/Series.cpp index 47a642a88d..da443018e0 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -415,7 +415,8 @@ void Series::flushRankTable() { return; } - Parameter param; + Parameter param( + AbstractParameter::I_dont_want_to_use_joined_dimensions); param.name = "rankTable"; param.dtype = Datatype::CHAR; param.extent = {uint64_t(size), uint64_t(maxSize)}; diff --git a/src/backend/BaseRecordComponent.cpp b/src/backend/BaseRecordComponent.cpp index 3f0f1b35c0..2cd4551536 100644 --- a/src/backend/BaseRecordComponent.cpp +++ b/src/backend/BaseRecordComponent.cpp @@ -21,6 +21,7 @@ #include "openPMD/backend/BaseRecordComponent.hpp" #include "openPMD/Error.hpp" #include "openPMD/Iteration.hpp" +#include namespace openPMD { @@ -75,7 +76,7 @@ std::optional BaseRecordComponent::joinedDimension() const } else { - return false; + return std::nullopt; } } diff --git a/src/binding/python/Dataset.cpp b/src/binding/python/Dataset.cpp index 70d85721f2..313daee63e 100644 --- a/src/binding/python/Dataset.cpp +++ b/src/binding/python/Dataset.cpp @@ -79,7 +79,8 @@ void init_Dataset(py::module &m) }) .def_property_readonly( - "joined_dimension", &Dataset::joinedDimension) + "joined_dimension", + py::overload_cast<>(&Dataset::joinedDimension, py::const_)) .def_readonly("extent", &Dataset::extent) .def("extend", &Dataset::extend) .def_readonly("rank", &Dataset::rank) diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index e6b960913e..5427b4f797 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -1995,7 +1995,29 @@ void joined_dim(std::string const &ext) patchExtent.store(10); } writeFrom.clear(); + // There seems to be a bug making this flush call necessary, need to fix + it.seriesFlush(); it.close(); + + it = s.writeIterations()[200]; + + // Test issue fixed with + // https://github.com/openPMD/openPMD-api/pull/1740 + + auto bug_dataset = it.particles["flush_multiple_times"]["position"]; + + std::vector buffer(length_of_patch * 2); + std::iota(buffer.begin(), buffer.end(), length_of_patch * 2 * rank); + + bug_dataset.resetDataset({Datatype::INT, {Dataset::JOINED_DIMENSION}}); + bug_dataset.storeChunkRaw(buffer.data(), {}, {length_of_patch}); + it.seriesFlush(); + + bug_dataset.resetDataset({Datatype::INT, {Dataset::JOINED_DIMENSION}}); + bug_dataset.storeChunkRaw( + buffer.data() + length_of_patch, {}, {length_of_patch}); + it.seriesFlush(); + s.close(); }