diff --git a/.appveyor.yml b/.appveyor.yml index 017d855253..9adee4737b 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -3,7 +3,7 @@ environment: - TARGET_ARCH: x64 CONDA_PY: 3.10 CONDA_INSTALL_LOCN: C:\\Miniconda37-x64 - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017 + APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019 platform: x64 SHARED: OFF @@ -101,12 +101,12 @@ before_build: # Compiler & Generator Selection - cmd: if "%APPVEYOR_BUILD_WORKER_IMAGE%"=="Visual Studio 2017" set OPENPMD_CMAKE_GENERATOR=Visual Studio 15 2017 - cmd: if "%APPVEYOR_BUILD_WORKER_IMAGE%"=="Visual Studio 2019" set OPENPMD_CMAKE_GENERATOR=Visual Studio 16 2019 - - cmd: if "%TARGET_ARCH%"=="x64" set OPENPMD_CMAKE_GENERATOR=%OPENPMD_CMAKE_GENERATOR% Win64 # - cmd: if "%TARGET_ARCH%"=="x86" "C:\Program Files (x86)\Microsoft Visual Studio 15.9\VC\vcvarsall.bat" x86 # - cmd: if "%TARGET_ARCH%"=="x64" "C:\Program Files (x86)\Microsoft Visual Studio 15.9\VC\vcvarsall.bat" amd64 # CMake configure - - cmd: cmake -G "%OPENPMD_CMAKE_GENERATOR%" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DBUILD_SHARED_LIBS=%SHARED% -DBUILD_TESTING=ON -DopenPMD_USE_PYTHON=ON -DPython_EXECUTABLE="%CONDA_INSTALL_LOCN%\python.exe" -DCMAKE_INSTALL_PREFIX="%CONDA_INSTALL_LOCN%" -DCMAKE_INSTALL_BINDIR="Library\bin" ".." + - cmd: if "%TARGET_ARCH%"=="x64" cmake -G "%OPENPMD_CMAKE_GENERATOR%" -A "%TARGET_ARCH%" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DBUILD_SHARED_LIBS=%SHARED% -DBUILD_TESTING=ON -DopenPMD_USE_PYTHON=ON -DPython_EXECUTABLE="%CONDA_INSTALL_LOCN%\python.exe" -DCMAKE_INSTALL_PREFIX="%CONDA_INSTALL_LOCN%" -DCMAKE_INSTALL_BINDIR="Library\bin" ".." + - cmd: if "%TARGET_ARCH%"=="x86" cmake -G "%OPENPMD_CMAKE_GENERATOR%" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DBUILD_SHARED_LIBS=%SHARED% -DBUILD_TESTING=ON -DopenPMD_USE_PYTHON=ON -DPython_EXECUTABLE="%CONDA_INSTALL_LOCN%\python.exe" -DCMAKE_INSTALL_PREFIX="%CONDA_INSTALL_LOCN%" -DCMAKE_INSTALL_BINDIR="Library\bin" ".." build_script: - cmd: cmake --build . --config %CONFIGURATION% -j 2 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4be899e900..088f8a8f74 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,7 +1,45 @@ .. _install-changelog: Changelog -========= +========== + +0.17.1 +------ +**Date:** YYYY-MM-DD + +Bug Fixes +""""""""" + +- ADIOS2: + + - Flush dirty ADIOS2 files in sorted order (#1868) + - Keep written flag consistent across MPI ranks for rankTable (#1869) + - Fix parallel deletion (#1858) + - Do not flush if the backend does not support Span API (#1863) +- HDF5: + + - Proper status check (#1870) + - Close operations also upon failure (#1866) +- API: + + - Cache Iteration indexes instead of computing them on the spot (#1860) + - Harmonize Datatype equality checks (#1854) + +Other +""""" + +- Python: + + - Fix keep_alive specifications, add tests for keep_alive (#1851) + - Remove duplicate Python test (#1867) +- Dependencies: + + - Bump toml11 to 4.4.0, nlohmann_json to 3.12.0 (#1842) + - Version bump pybind11 -> 3.0.2 (#1849) +- CI/Infrastructure: + + - create_directories: preserve sticky and setgid permissions (#1855) + - Fix AppVeyor 64bit build (#1832) 0.17.0 ------ diff --git a/CMakeLists.txt b/CMakeLists.txt index dba77d38be..cc71572c37 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -837,6 +837,7 @@ if(openPMD_BUILD_TESTING) elseif(${test_name} STREQUAL "Core") list(APPEND ${out_list} test/Files_Core/automatic_variable_encoding.cpp + test/Files_Core/read_nonexistent_attribute.cpp ) endif() endmacro() diff --git a/cmake/dependencies/json.cmake b/cmake/dependencies/json.cmake index 751ffc87f5..b35d094a84 100644 --- a/cmake/dependencies/json.cmake +++ b/cmake/dependencies/json.cmake @@ -56,10 +56,10 @@ set(openPMD_json_src "" "Local path to nlohmann_json source directory (preferred if set)") # tarball fetcher -set(openPMD_json_tar "https://github.com/nlohmann/json/archive/refs/tags/v3.11.3.tar.gz" +set(openPMD_json_tar "https://github.com/nlohmann/json/archive/refs/tags/v3.12.0.tar.gz" CACHE STRING "Remote tarball link to pull and build nlohmann_json from if(openPMD_USE_INTERNAL_JSON)") -set(openPMD_json_tar_hash "SHA256=0d8ef5af7f9794e3263480193c491549b2ba6cc74bb018906202ada498a79406" +set(openPMD_json_tar_hash "SHA256=4b92eb0c06d10683f7447ce9406cb97cd4b453be18d7279320f7b2f025c10187" CACHE STRING "Hash checksum of the tarball of nlohmann_json if(openPMD_USE_INTERNAL_JSON)") @@ -67,7 +67,7 @@ set(openPMD_json_tar_hash "SHA256=0d8ef5af7f9794e3263480193c491549b2ba6cc74bb018 set(openPMD_json_repo "https://github.com/nlohmann/json.git" CACHE STRING "Repository URI to pull and build nlohmann_json from if(openPMD_USE_INTERNAL_JSON)") -set(openPMD_json_branch "v3.11.3" +set(openPMD_json_branch "v3.12.0" CACHE STRING "Repository branch for openPMD_json_repo if(openPMD_USE_INTERNAL_JSON)") diff --git a/cmake/dependencies/pybind11.cmake b/cmake/dependencies/pybind11.cmake index 68c0986e4f..643421df56 100644 --- a/cmake/dependencies/pybind11.cmake +++ b/cmake/dependencies/pybind11.cmake @@ -51,9 +51,9 @@ function(find_pybind11) mark_as_advanced(FETCHCONTENT_UPDATES_DISCONNECTED_FETCHEDpybind11) elseif(NOT openPMD_USE_INTERNAL_PYBIND11) if(openPMD_USE_PYTHON STREQUAL AUTO) - find_package(pybind11 2.13.0 CONFIG) + find_package(pybind11 3.0.0 CONFIG) elseif(openPMD_USE_PYTHON) - find_package(pybind11 2.13.0 CONFIG REQUIRED) + find_package(pybind11 3.0.0 CONFIG REQUIRED) endif() if(TARGET pybind11::module) message(STATUS "pybind11: Found version '${pybind11_VERSION}'") @@ -67,10 +67,10 @@ set(openPMD_pybind11_src "" "Local path to pybind11 source directory (preferred if set)") # tarball fetcher -set(openPMD_pybind11_tar "https://github.com/pybind/pybind11/archive/refs/tags/v2.13.6.tar.gz" +set(openPMD_pybind11_tar "https://github.com/pybind/pybind11/archive/refs/tags/v3.0.2.tar.gz" CACHE STRING "Remote tarball link to pull and build pybind11 from if(openPMD_USE_INTERNAL_PYBIND11)") -set(openPMD_pybind11_tar_hash "SHA256=e08cb87f4773da97fa7b5f035de8763abc656d87d5773e62f6da0587d1f0ec20" +set(openPMD_pybind11_tar_hash "SHA256=2f20a0af0b921815e0e169ea7fec63909869323581b89d7de1553468553f6a2d" CACHE STRING "Hash checksum of the tarball of pybind11 if(openPMD_USE_INTERNAL_PYBIND11)") @@ -78,7 +78,7 @@ set(openPMD_pybind11_tar_hash "SHA256=e08cb87f4773da97fa7b5f035de8763abc656d87d5 set(openPMD_pybind11_repo "https://github.com/pybind/pybind11.git" CACHE STRING "Repository URI to pull and build pybind11 from if(openPMD_USE_INTERNAL_PYBIND11)") -set(openPMD_pybind11_branch "v2.13.6" +set(openPMD_pybind11_branch "v3.0.2" CACHE STRING "Repository branch for openPMD_pybind11_repo if(openPMD_USE_INTERNAL_PYBIND11)") diff --git a/cmake/dependencies/toml11.cmake b/cmake/dependencies/toml11.cmake index c1f8832a81..000e285a7f 100644 --- a/cmake/dependencies/toml11.cmake +++ b/cmake/dependencies/toml11.cmake @@ -64,10 +64,10 @@ set(openPMD_toml11_src "" "Local path to toml11 source directory (preferred if set)") # tarball fetcher -set(openPMD_toml11_tar "https://github.com/ToruNiina/toml11/archive/refs/tags/v4.2.0.tar.gz" +set(openPMD_toml11_tar "https://github.com/ToruNiina/toml11/archive/refs/tags/v4.4.0.tar.gz" CACHE STRING "Remote tarball link to pull and build toml11 from if(openPMD_USE_INTERNAL_TOML11)") -set(openPMD_toml11_tar_hash "SHA256=9287971cd4a1a3992ef37e7b95a3972d1ae56410e7f8e3f300727ab1d6c79c2c" +set(openPMD_toml11_tar_hash "SHA256=815bfe6792aa11a13a133b86e7f0f45edc5d71eb78f5fb6686c49c7f792b9049" CACHE STRING "Hash checksum of the tarball of toml11 if(openPMD_USE_INTERNAL_TOML11)") @@ -75,7 +75,7 @@ set(openPMD_toml11_tar_hash "SHA256=9287971cd4a1a3992ef37e7b95a3972d1ae56410e7f8 set(openPMD_toml11_repo "https://github.com/ToruNiina/toml11.git" CACHE STRING "Repository URI to pull and build toml11 from if(openPMD_USE_INTERNAL_TOML11)") -set(openPMD_toml11_branch "v3.7.1" +set(openPMD_toml11_branch "v4.4.0" CACHE STRING "Repository branch for openPMD_toml11_repo if(openPMD_USE_INTERNAL_TOML11)") diff --git a/include/openPMD/Datatype.hpp b/include/openPMD/Datatype.hpp index a11d3db75f..17cf6b67f4 100644 --- a/include/openPMD/Datatype.hpp +++ b/include/openPMD/Datatype.hpp @@ -294,7 +294,8 @@ template inline constexpr Datatype determineDatatype(T &&val) { (void)val; // don't need this, it only has a name for Doxygen - using T_stripped = std::remove_cv_t>; + using T_stripped = + std::remove_extent_t>>; if constexpr (auxiliary::IsPointer_v) { return determineDatatype>(); @@ -419,6 +420,8 @@ inline size_t toBits(Datatype d) return toBytes(d) * CHAR_BIT; } +constexpr bool isSigned(Datatype d); + /** Compare if a Datatype is a vector type * * @param d Datatype to test @@ -595,14 +598,19 @@ inline std::tuple isInteger() */ template inline bool isSameFloatingPoint(Datatype d) +{ + return isSameFloatingPoint(d, determineDatatype()); +} + +inline bool isSameFloatingPoint(Datatype d1, Datatype d2) { // template - bool tt_is_fp = isFloatingPoint(); + bool tt_is_fp = isFloatingPoint(d1); // Datatype - bool dt_is_fp = isFloatingPoint(d); + bool dt_is_fp = isFloatingPoint(d2); - if (tt_is_fp && dt_is_fp && toBits(d) == toBits(determineDatatype())) + if (tt_is_fp && dt_is_fp && toBits(d1) == toBits(d2)) return true; else return false; @@ -617,15 +625,19 @@ inline bool isSameFloatingPoint(Datatype d) */ template inline bool isSameComplexFloatingPoint(Datatype d) +{ + return isSameComplexFloatingPoint(d, determineDatatype()); +} + +inline bool isSameComplexFloatingPoint(Datatype d1, Datatype d2) { // template - bool tt_is_cfp = isComplexFloatingPoint(); + bool tt_is_cfp = isComplexFloatingPoint(d1); // Datatype - bool dt_is_cfp = isComplexFloatingPoint(d); + bool dt_is_cfp = isComplexFloatingPoint(d2); - if (tt_is_cfp && dt_is_cfp && - toBits(d) == toBits(determineDatatype())) + if (tt_is_cfp && dt_is_cfp && toBits(d1) == toBits(d2)) return true; else return false; @@ -640,17 +652,22 @@ inline bool isSameComplexFloatingPoint(Datatype d) */ template inline bool isSameInteger(Datatype d) +{ + return isSameInteger(d, determineDatatype()); +} + +inline bool isSameInteger(Datatype d1, Datatype d2) { // template bool tt_is_int, tt_is_sig; - std::tie(tt_is_int, tt_is_sig) = isInteger(); + std::tie(tt_is_int, tt_is_sig) = isInteger(d1); // Datatype bool dt_is_int, dt_is_sig; - std::tie(dt_is_int, dt_is_sig) = isInteger(d); + std::tie(dt_is_int, dt_is_sig) = isInteger(d2); if (tt_is_int && dt_is_int && tt_is_sig == dt_is_sig && - toBits(d) == toBits(determineDatatype())) + toBits(d1) == toBits(d2)) return true; else return false; @@ -691,46 +708,15 @@ constexpr bool isChar(Datatype d) template constexpr bool isSameChar(Datatype d); +constexpr bool isSameChar(Datatype d1, Datatype d2); + /** Comparison for two Datatypes * * Besides returning true for the same types, identical implementations on * some platforms, e.g. if long and long long are the same or double and * long double will also return true. */ -inline bool isSame(openPMD::Datatype const d, openPMD::Datatype const e) -{ - // exact same type - if (static_cast(d) == static_cast(e)) - return true; - - bool d_is_vec = isVector(d); - bool e_is_vec = isVector(e); - - // same int - bool d_is_int, d_is_sig; - std::tie(d_is_int, d_is_sig) = isInteger(d); - bool e_is_int, e_is_sig; - std::tie(e_is_int, e_is_sig) = isInteger(e); - if (d_is_int && e_is_int && d_is_vec == e_is_vec && d_is_sig == e_is_sig && - toBits(d) == toBits(e)) - return true; - - // same float - bool d_is_fp = isFloatingPoint(d); - bool e_is_fp = isFloatingPoint(e); - - if (d_is_fp && e_is_fp && d_is_vec == e_is_vec && toBits(d) == toBits(e)) - return true; - - // same complex floating point - bool d_is_cfp = isComplexFloatingPoint(d); - bool e_is_cfp = isComplexFloatingPoint(e); - - if (d_is_cfp && e_is_cfp && d_is_vec == e_is_vec && toBits(d) == toBits(e)) - return true; - - return false; -} +constexpr bool isSame(openPMD::Datatype d, openPMD::Datatype e); /** * @brief basicDatatype Strip openPMD Datatype of std::vector, std::array et. diff --git a/include/openPMD/Datatype.tpp b/include/openPMD/Datatype.tpp index 6685c62f73..e35f2e26b6 100644 --- a/include/openPMD/Datatype.tpp +++ b/include/openPMD/Datatype.tpp @@ -25,6 +25,7 @@ // comment to prevent clang-format from moving this #include up // datatype macros may be included and un-included in other headers #include "openPMD/DatatypeMacros.hpp" +#include "openPMD/auxiliary/TypeTraits.hpp" #include #include // std::void_t @@ -253,6 +254,56 @@ constexpr inline bool isSameChar(Datatype d) { return switchType>(d); } + +namespace detail +{ + struct IsSigned + { + template + static constexpr bool call() + { + if constexpr (auxiliary::IsVector_v || auxiliary::IsArray_v) + { + return call(); + } + else if constexpr (std::is_same_v) + { + return call(); + } + else + { + return std::is_signed_v; + } + } + + static constexpr char const *errorMsg = "IsSigned"; + }; +} // namespace detail + +constexpr inline bool isSigned(Datatype d) +{ + return switchType(d); +} + +constexpr inline bool isSameChar(Datatype d, Datatype e) +{ + return isChar(d) && isChar(e) && isSigned(d) == isSigned(e); +} + +constexpr bool isSame(openPMD::Datatype const d, openPMD::Datatype const e) +{ + return + // exact same type + static_cast(d) == static_cast(e) + // same int + || isSameInteger(d, e) + // same float + || isSameFloatingPoint(d, e) + // same complex floating point + || isSameComplexFloatingPoint(d, e) + // same char + || isSameChar(d, e); +} } // namespace openPMD #include "openPMD/UndefDatatypeMacros.hpp" diff --git a/include/openPMD/IO/AbstractIOHandlerImplCommon.hpp b/include/openPMD/IO/AbstractIOHandlerImplCommon.hpp index 7261b4bf71..4c995be817 100644 --- a/include/openPMD/IO/AbstractIOHandlerImplCommon.hpp +++ b/include/openPMD/IO/AbstractIOHandlerImplCommon.hpp @@ -28,10 +28,10 @@ #include "openPMD/auxiliary/StringManip.hpp" #include "openPMD/backend/Writable.hpp" +#include #include #include #include -#include namespace openPMD { @@ -50,7 +50,10 @@ class AbstractIOHandlerImplCommon : public AbstractIOHandlerImpl * without the OS path */ std::unordered_map m_files; - std::unordered_set m_dirty; + // MUST be an ordered set in order to consistently flush on different + // parallel processes (same logic cant apply to m_files since Writable* + // pointers are not predictable) + std::set m_dirty; enum PossiblyExisting { diff --git a/include/openPMD/IO/IOTask.hpp b/include/openPMD/IO/IOTask.hpp index 1ee7248b32..25e0d6ad54 100644 --- a/include/openPMD/IO/IOTask.hpp +++ b/include/openPMD/IO/IOTask.hpp @@ -558,6 +558,7 @@ struct OPENPMDAPI_EXPORT } // in parameters + bool queryOnly = false; // query if the backend supports this Offset offset; Extent extent; Datatype dtype = Datatype::UNDEFINED; diff --git a/include/openPMD/IO/InvalidatableFile.hpp b/include/openPMD/IO/InvalidatableFile.hpp index 6bdc24cbe6..6ac0051cbe 100644 --- a/include/openPMD/IO/InvalidatableFile.hpp +++ b/include/openPMD/IO/InvalidatableFile.hpp @@ -82,4 +82,14 @@ struct hash result_type operator()(argument_type const &s) const noexcept; }; + +template <> +struct less +{ + using first_argument_type = openPMD::InvalidatableFile; + using second_argument_type = first_argument_type; + using result_type = typename std::less::result_type; + result_type + operator()(first_argument_type const &, second_argument_type const &) const; +}; } // namespace std diff --git a/include/openPMD/Iteration.hpp b/include/openPMD/Iteration.hpp index c66199c46f..e2a4e6b1a5 100644 --- a/include/openPMD/Iteration.hpp +++ b/include/openPMD/Iteration.hpp @@ -129,6 +129,15 @@ namespace internal */ StepStatus m_stepStatus = StepStatus::NoStep; + /** + * Cached copy of the key under which this Iteration lives in + * Series::iterations. Populated when the iteration + * object is created/inserted. This allows constant-time lookup + * of the owning map entry instead of a linear scan in + * Series::indexOf(). + */ + std::optional m_iterationIndex = 0; + /** * Information on a parsing request that has not yet been executed. * Otherwise empty. @@ -153,6 +162,8 @@ class Iteration : public Attributable friend class Writable; friend class StatefulIterator; friend class StatefulSnapshotsContainer; + template + friend struct traits::GenerationPolicy; public: Iteration(Iteration const &) = default; @@ -276,6 +287,16 @@ class Iteration : public Attributable private: Iteration(); + /** + * @brief Get the cached iteration index. + * This is the key under which this iteration is stored in the + * Series::iterations map. Used internally for testing the index + * caching optimization. + * + * @return The cached iteration index. + */ + uint64_t getCachedIterationIndex() const; + using Data_t = internal::IterationData; std::shared_ptr m_iterationData; @@ -431,6 +452,20 @@ class Iteration : public Attributable void runDeferredParseAccess(); }; // Iteration +namespace traits +{ + template <> + struct GenerationPolicy + { + constexpr static bool is_noop = false; + template + void operator()(Iterator &it) + { + it->second.get().m_iterationIndex = it->first; + } + }; +} // namespace traits + extern template float Iteration::time() const; extern template double Iteration::time() const; diff --git a/include/openPMD/ParticleSpecies.hpp b/include/openPMD/ParticleSpecies.hpp index 4f309c0f2a..7309afddef 100644 --- a/include/openPMD/ParticleSpecies.hpp +++ b/include/openPMD/ParticleSpecies.hpp @@ -62,9 +62,9 @@ namespace traits { constexpr static bool is_noop = false; template - void operator()(T &ret) + void operator()(T &it) { - ret.particlePatches.linkHierarchy(ret.writable()); + it->second.particlePatches.linkHierarchy(it->second.writable()); } }; } // namespace traits diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 9d1d8332b4..b796ab1a93 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -85,14 +85,6 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) { verifyChunk(o, e); - /* - * The openPMD backend might not yet know about this dataset. - * Flush the openPMD hierarchy to the backend without flushing any actual - * data yet. - */ - seriesFlush_impl( - {FlushLevel::SkeletonOnly}); - size_t size = 1; for (auto ext : e) { @@ -102,33 +94,61 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) * Flushing the skeleton does not create datasets, * so we might need to do it now. */ - if (!written()) + auto &rc = get(); + if (!rc.m_dataset.has_value()) { - auto &rc = get(); - if (!rc.m_dataset.has_value()) - { - throw error::WrongAPIUsage( - "[RecordComponent] Must specify dataset type and extent before " - "using storeChunk() (see RecordComponent::resetDataset())."); - } - Parameter dCreate(rc.m_dataset.value()); - dCreate.name = Attributable::get().m_writable.ownKeyWithinParent; - IOHandler()->enqueue(IOTask(this, dCreate)); + throw error::WrongAPIUsage( + "[RecordComponent] Must specify dataset type and extent before " + "using storeChunk() (see RecordComponent::resetDataset())."); } + Parameter query; + query.queryOnly = true; + IOHandler()->enqueue(IOTask(this, query)); + IOHandler()->flush(internal::defaultFlushParams); + Parameter getBufferView; getBufferView.offset = o; getBufferView.extent = e; getBufferView.dtype = getDatatype(); - IOHandler()->enqueue(IOTask(this, getBufferView)); - IOHandler()->flush(internal::defaultFlushParams); - auto &out = *getBufferView.out; - if (!out.backendManagedBuffer) + + if (query.out->backendManagedBuffer) + { + // Need to initialize the dataset for the Span API + // But this is a non-collective call and initializing the dataset is + // collective in HDF5 So we do this only in backends that actually + // support the Span API (i.e. ADIOS2) which do not share this + // restriction + // TODO: Add some form of collective ::commitDefinitions() call to + // RecordComponents to be called by users before the Span API + if (!written()) + { + /* + * The openPMD backend might not yet know about this dataset. + * Flush the openPMD hierarchy to the backend without flushing any + * actual data yet. + */ + seriesFlush_impl( + {FlushLevel::SkeletonOnly}); + Parameter dCreate(rc.m_dataset.value()); + dCreate.name = Attributable::get().m_writable.ownKeyWithinParent; + IOHandler()->enqueue(IOTask(this, dCreate)); + + setWritten(true, EnqueueAsynchronously::OnlyAsync); + } + + IOHandler()->enqueue(IOTask(this, getBufferView)); + IOHandler()->flush(internal::defaultFlushParams); + } + + // The backend might still refuse the operation even if backend managed + // buffers are generally supported, so check again + if (!getBufferView.out->backendManagedBuffer) { // note that data might have either // type shared_ptr or shared_ptr auto data = std::forward(createBuffer)(size); - out.ptr = static_cast(data.get()); + getBufferView.out->ptr = static_cast(data.get()); if (size > 0) { storeChunk(std::move(data), std::move(o), std::move(e)); diff --git a/include/openPMD/auxiliary/Defer.hpp b/include/openPMD/auxiliary/Defer.hpp new file mode 100644 index 0000000000..a99bca1d37 --- /dev/null +++ b/include/openPMD/auxiliary/Defer.hpp @@ -0,0 +1,46 @@ +#pragma once + +#include +#include +#include + +namespace openPMD::auxiliary +{ +template +struct defer_type +{ + F functor; + bool do_run_this = true; + ~defer_type() + { + if (!do_run_this) + { + return; + } + do_run_this = false; + std::move(functor)(); + } + + auto to_opaque() && -> defer_type> + { + do_run_this = false; + if (!do_run_this) + { + return defer_type>{{}, false}; + } + else + { + return defer_type>{ + std::function{std::move(functor)}}; + } + } +}; + +using opaque_defer_type = defer_type>; + +template +auto defer(F &&functor) -> defer_type> +{ + return defer_type>{std::forward(functor)}; +} +} // namespace openPMD::auxiliary diff --git a/include/openPMD/auxiliary/Filesystem.hpp b/include/openPMD/auxiliary/Filesystem.hpp index 25239f65a3..6ad0288a5f 100644 --- a/include/openPMD/auxiliary/Filesystem.hpp +++ b/include/openPMD/auxiliary/Filesystem.hpp @@ -20,6 +20,7 @@ */ #pragma once +#include #include #include @@ -66,6 +67,19 @@ namespace auxiliary */ std::vector list_directory(std::string const &path); + /** List all contents of a directory at a given absolute or relative path. + * + * @note The equivalent of `ls path` + * @note Both contained files and directories are listed. + * `.` and `..` are not returned. + * @param path Absolute or relative path of directory to examine. + * @return Vector of all contained files and directories, + * or an empty option in case of an IO error, in which case the + * specific error will be stored in errno. + */ + std::optional> + list_directory_nothrow(std::string const &path); + /** Create all required directories to have a reachable given absolute or * relative path. * diff --git a/include/openPMD/backend/Attributable.hpp b/include/openPMD/backend/Attributable.hpp index 1c1d3db4a5..705080be84 100644 --- a/include/openPMD/backend/Attributable.hpp +++ b/include/openPMD/backend/Attributable.hpp @@ -589,9 +589,10 @@ OPENPMD_protected { return writable().written; } - enum class EnqueueAsynchronously : bool + enum class EnqueueAsynchronously : uint8_t { - Yes, + OnlyAsync, + Both, No }; /* diff --git a/include/openPMD/backend/ContainerImpl.tpp b/include/openPMD/backend/ContainerImpl.tpp index 0384333e10..de11c91f14 100644 --- a/include/openPMD/backend/ContainerImpl.tpp +++ b/include/openPMD/backend/ContainerImpl.tpp @@ -140,7 +140,8 @@ auto Container::operator[](key_type const &key) T t = T(); t.linkHierarchy(writable()); - auto &ret = container().insert({key, std::move(t)}).first->second; + auto inserted_iterator = container().insert({key, std::move(t)}).first; + auto &ret = inserted_iterator->second; if constexpr (std::is_same_v) { ret.writable().ownKeyWithinParent = key; @@ -150,7 +151,7 @@ auto Container::operator[](key_type const &key) ret.writable().ownKeyWithinParent = std::to_string(key); } traits::GenerationPolicy gen; - gen(ret); + gen(inserted_iterator); return ret; } } @@ -172,7 +173,8 @@ auto Container::operator[](key_type &&key) T t = T(); t.linkHierarchy(writable()); - auto &ret = container().insert({key, std::move(t)}).first->second; + auto inserted_iterator = container().insert({key, std::move(t)}).first; + auto &ret = inserted_iterator->second; if constexpr (std::is_same_v) { ret.writable().ownKeyWithinParent = std::move(key); @@ -182,7 +184,7 @@ auto Container::operator[](key_type &&key) ret.writable().ownKeyWithinParent = std::to_string(std::move(key)); } traits::GenerationPolicy gen; - gen(ret); + gen(inserted_iterator); return ret; } } diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index 63c8b34f92..eedd84e158 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -122,7 +122,8 @@ template inline void PatchRecordComponent::load(std::shared_ptr data) { Datatype dtype = determineDatatype(); - if (dtype != getDatatype()) + // Attention: Do NOT use operator==(), doesnt work properly on Windows! + if (!isSame(dtype, getDatatype())) throw std::runtime_error( "Type conversion during particle patch loading not yet " "implemented"); @@ -160,7 +161,7 @@ template inline void PatchRecordComponent::store(uint64_t idx, T data) { Datatype dtype = determineDatatype(); - if (dtype != getDatatype()) + if (!isSame(dtype, getDatatype())) { std::ostringstream oss; oss << "Datatypes of patch data (" << dtype << ") and dataset (" @@ -187,7 +188,7 @@ template inline void PatchRecordComponent::store(T data) { Datatype dtype = determineDatatype(); - if (dtype != getDatatype()) + if (!isSame(dtype, getDatatype())) { std::ostringstream oss; oss << "Datatypes of patch data (" << dtype << ") and dataset (" diff --git a/include/openPMD/binding/python/Numpy.hpp b/include/openPMD/binding/python/Numpy.hpp index c965f8c7de..062b76f560 100644 --- a/include/openPMD/binding/python/Numpy.hpp +++ b/include/openPMD/binding/python/Numpy.hpp @@ -32,9 +32,17 @@ namespace openPMD { inline Datatype dtype_from_numpy(pybind11::dtype const dt) { + // Use explicit PEP 3118 / struct format character codes for portable + // cross-platform behavior. This ensures consistency with + // dtype_from_bufferformat() and avoids platform-dependent interpretation + // of numpy type name strings. + // ref: https://docs.python.org/3/library/struct.html#format-characters + // ref: https://numpy.org/doc/stable/reference/arrays.interface.html // ref: https://docs.scipy.org/doc/numpy/user/basics.types.html - // ref: https://github.com/numpy/numpy/issues/10678#issuecomment-369363551 - if (dt.char_() == pybind11::dtype("b").char_()) + char const c = dt.char_(); + switch (c) + { + case 'b': // signed char if constexpr (std::is_signed_v) { return Datatype::CHAR; @@ -43,7 +51,7 @@ inline Datatype dtype_from_numpy(pybind11::dtype const dt) { return Datatype::SCHAR; } - else if (dt.char_() == pybind11::dtype("B").char_()) + case 'B': // unsigned char if constexpr (std::is_unsigned_v) { return Datatype::CHAR; @@ -52,42 +60,41 @@ inline Datatype dtype_from_numpy(pybind11::dtype const dt) { return Datatype::UCHAR; } - else if (dt.char_() == pybind11::dtype("short").char_()) + case 'h': // short return Datatype::SHORT; - else if (dt.char_() == pybind11::dtype("intc").char_()) - return Datatype::INT; - else if (dt.char_() == pybind11::dtype("int_").char_()) - return Datatype::LONG; - else if (dt.char_() == pybind11::dtype("longlong").char_()) - return Datatype::LONGLONG; - else if (dt.char_() == pybind11::dtype("ushort").char_()) + case 'H': // unsigned short return Datatype::USHORT; - else if (dt.char_() == pybind11::dtype("uintc").char_()) + case 'i': // int + return Datatype::INT; + case 'I': // unsigned int return Datatype::UINT; - else if (dt.char_() == pybind11::dtype("uint").char_()) + case 'l': // long + return Datatype::LONG; + case 'L': // unsigned long return Datatype::ULONG; - else if (dt.char_() == pybind11::dtype("ulonglong").char_()) + case 'q': // long long + return Datatype::LONGLONG; + case 'Q': // unsigned long long return Datatype::ULONGLONG; - else if (dt.char_() == pybind11::dtype("clongdouble").char_()) - return Datatype::CLONG_DOUBLE; - else if (dt.char_() == pybind11::dtype("cdouble").char_()) - return Datatype::CDOUBLE; - else if (dt.char_() == pybind11::dtype("csingle").char_()) - return Datatype::CFLOAT; - else if (dt.char_() == pybind11::dtype("longdouble").char_()) - return Datatype::LONG_DOUBLE; - else if (dt.char_() == pybind11::dtype("double").char_()) - return Datatype::DOUBLE; - else if (dt.char_() == pybind11::dtype("single").char_()) + case 'f': // float return Datatype::FLOAT; - else if (dt.char_() == pybind11::dtype("bool").char_()) + case 'd': // double + return Datatype::DOUBLE; + case 'g': // long double + return Datatype::LONG_DOUBLE; + case 'F': // complex float + return Datatype::CFLOAT; + case 'D': // complex double + return Datatype::CDOUBLE; + case 'G': // complex long double + return Datatype::CLONG_DOUBLE; + case '?': // bool return Datatype::BOOL; - else - { + default: pybind11::print(dt); throw std::runtime_error( - std::string("Datatype '") + dt.char_() + - std::string("' not known in 'dtype_from_numpy'!")); // _s.format(dt) + std::string("Datatype '") + c + + std::string("' not known in 'dtype_from_numpy'!")); } } @@ -159,6 +166,8 @@ inline Datatype dtype_from_bufferformat(std::string const &fmt) inline pybind11::dtype dtype_to_numpy(Datatype const dt) { + // Use explicit PEP 3118 format character codes for portable behavior. + // This ensures round-trip consistency with dtype_from_numpy(). using DT = Datatype; switch (dt) { @@ -177,80 +186,57 @@ inline pybind11::dtype dtype_to_numpy(Datatype const dt) case DT::SCHAR: case DT::VEC_SCHAR: return pybind11::dtype("b"); - break; case DT::UCHAR: case DT::VEC_UCHAR: return pybind11::dtype("B"); - break; - // case DT::SCHAR: - // case DT::VEC_SCHAR: - // pybind11::dtype("b"); - // break; case DT::SHORT: case DT::VEC_SHORT: - return pybind11::dtype("short"); - break; + return pybind11::dtype("h"); case DT::INT: case DT::VEC_INT: - return pybind11::dtype("intc"); - break; + return pybind11::dtype("i"); case DT::LONG: case DT::VEC_LONG: - return pybind11::dtype("int_"); - break; + return pybind11::dtype("l"); case DT::LONGLONG: case DT::VEC_LONGLONG: - return pybind11::dtype("longlong"); - break; + return pybind11::dtype("q"); case DT::USHORT: case DT::VEC_USHORT: - return pybind11::dtype("ushort"); - break; + return pybind11::dtype("H"); case DT::UINT: case DT::VEC_UINT: - return pybind11::dtype("uintc"); - break; + return pybind11::dtype("I"); case DT::ULONG: case DT::VEC_ULONG: - return pybind11::dtype("uint"); - break; + return pybind11::dtype("L"); case DT::ULONGLONG: case DT::VEC_ULONGLONG: - return pybind11::dtype("ulonglong"); - break; + return pybind11::dtype("Q"); case DT::FLOAT: case DT::VEC_FLOAT: - return pybind11::dtype("single"); - break; + return pybind11::dtype("f"); case DT::DOUBLE: case DT::VEC_DOUBLE: case DT::ARR_DBL_7: - return pybind11::dtype("double"); - break; + return pybind11::dtype("d"); case DT::LONG_DOUBLE: case DT::VEC_LONG_DOUBLE: - return pybind11::dtype("longdouble"); - break; + return pybind11::dtype("g"); case DT::CFLOAT: case DT::VEC_CFLOAT: - return pybind11::dtype("csingle"); - break; + return pybind11::dtype("F"); case DT::CDOUBLE: case DT::VEC_CDOUBLE: - return pybind11::dtype("cdouble"); - break; + return pybind11::dtype("D"); case DT::CLONG_DOUBLE: case DT::VEC_CLONG_DOUBLE: - return pybind11::dtype("clongdouble"); - break; + return pybind11::dtype("G"); case DT::BOOL: - return pybind11::dtype("bool"); // also "?" - break; + return pybind11::dtype("?"); case DT::UNDEFINED: default: - throw std::runtime_error( - "dtype_to_numpy: Invalid Datatype '{...}'!"); // _s.format(dt) - break; + throw std::runtime_error("dtype_to_numpy: Invalid Datatype!"); } } } // namespace openPMD diff --git a/share/openPMD/download_samples.ps1 b/share/openPMD/download_samples.ps1 index 9880aa891f..d479f57e75 100755 --- a/share/openPMD/download_samples.ps1 +++ b/share/openPMD/download_samples.ps1 @@ -24,10 +24,10 @@ Invoke-WebRequest https://github.com/openPMD/openPMD-example-datasets/raw/566b35 7z.exe x -r example-3d-bp4.tar 7z.exe x -r legacy_datasets.tar.gz 7z.exe x -r legacy_datasets.tar -Move-Item -Path example-3d\hdf5\* samples\git-sample\ -Move-Item -Path example-thetaMode\hdf5\* samples\git-sample\thetaMode\ -Move-Item -Path example-3d-bp4\* samples\git-sample\3d-bp4\ -Move-Item -Path legacy_datasets\* samples\git-sample\legacy\ +Move-Item -Path example-3d\hdf5\* -Destination samples\git-sample\ -Force +Move-Item -Path example-thetaMode\hdf5\* -Destination samples\git-sample\thetaMode\ -Force +Move-Item -Path example-3d-bp4\* -Destination samples\git-sample\3d-bp4\ -Force +Move-Item -Path legacy_datasets\* -Destination samples\git-sample\legacy\ -Force Remove-Item -Recurse -Force example-3d* Remove-Item -Recurse -Force example-thetaMode* Remove-Item -Recurse -Force example-3d-bp4* @@ -44,7 +44,7 @@ Remove-Item *.zip # Ref.: https://github.com/openPMD/openPMD-viewer/issues/296 Invoke-WebRequest https://github.com/openPMD/openPMD-viewer/files/5655027/diags.zip -OutFile empty_alternate_fbpic.zip Expand-Archive empty_alternate_fbpic.zip -Move-Item -Path empty_alternate_fbpic\diags\hdf5\data00000050.h5 samples\issue-sample\empty_alternate_fbpic_00000050.h5 +Move-Item -Path empty_alternate_fbpic\diags\hdf5\data00000050.h5 -Destination samples\issue-sample\empty_alternate_fbpic_00000050.h5 -Force Remove-Item -Recurse -Force empty_alternate_fbpic* cd $orgdir diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index 0ee10a77c4..96db1f948c 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -1308,6 +1308,12 @@ void ADIOS2IOHandlerImpl::getBufferView( parameters.out->backendManagedBuffer = false; return; } + else if (parameters.queryOnly) + { + parameters.out->backendManagedBuffer = true; + return; + } + setAndGetFilePosition(writable); auto file = refreshFileFromParent(writable, /* preferParentFile = */ false); detail::ADIOS2File &ba = getFileData(file, IfFileNotOpen::ThrowError); @@ -1733,7 +1739,7 @@ void ADIOS2IOHandlerImpl::listPaths( */ auto &fileData = getFileData(file, IfFileNotOpen::ThrowError); - std::unordered_set subdirs; + std::set subdirs; /* * When reading an attribute, we cannot distinguish * whether its containing "folder" is a group or a @@ -1875,7 +1881,7 @@ void ADIOS2IOHandlerImpl::listDatasets( auto &fileData = getFileData(file, IfFileNotOpen::ThrowError); - std::unordered_set subdirs; + std::set subdirs; for (auto var : fileData.availableVariablesPrefixed(myName)) { // if string still contains a slash, variable is a dataset below the diff --git a/src/IO/ADIOS/ADIOS2PreloadAttributes.cpp b/src/IO/ADIOS/ADIOS2PreloadAttributes.cpp index c3c2fbfbd7..a9adbb74d6 100644 --- a/src/IO/ADIOS/ADIOS2PreloadAttributes.cpp +++ b/src/IO/ADIOS/ADIOS2PreloadAttributes.cpp @@ -248,7 +248,7 @@ PreloadAdiosAttributes::getAttribute(std::string const &name) const } AttributeLocation const &location = it->second; Datatype determinedDatatype = determineDatatype(); - if (location.dt != determinedDatatype) + if (!isSame(location.dt, determinedDatatype)) { std::stringstream errorMsg; errorMsg << "[ADIOS2] Wrong datatype for attribute: " << name @@ -330,22 +330,13 @@ AttributeWithShapeAndResource::operator bool() const } #define OPENPMD_INSTANTIATE_GETATTRIBUTE(type) \ + template struct AttributeWithShape; \ + template struct AttributeWithShapeAndResource; \ template AttributeWithShape PreloadAdiosAttributes::getAttribute( \ std::string const &name) const; \ template auto AdiosAttributes::getAttribute( \ size_t step, adios2::IO &IO, std::string const &name) const \ - -> AttributeWithShapeAndResource; \ - template AttributeWithShapeAndResource< \ - type>::AttributeWithShapeAndResource(AttributeWithShape parent); \ - template AttributeWithShapeAndResource:: \ - AttributeWithShapeAndResource( \ - size_t len_in, \ - type const \ - *data_in, /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - std::optional> resource_in); \ - template AttributeWithShapeAndResource< \ - type>::AttributeWithShapeAndResource(adios2::Attribute attr); \ - template AttributeWithShapeAndResource::operator bool() const; + -> AttributeWithShapeAndResource; ADIOS2_FOREACH_TYPE_1ARG(OPENPMD_INSTANTIATE_GETATTRIBUTE) #undef OPENPMD_INSTANTIATE_GETATTRIBUTE } // namespace openPMD::detail diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index ee02020983..9539c98708 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -25,6 +25,7 @@ #include "openPMD/IO/Access.hpp" #include "openPMD/IO/FlushParametersInternal.hpp" #include "openPMD/IO/HDF5/HDF5IOHandlerImpl.hpp" +#include "openPMD/auxiliary/Defer.hpp" #include "openPMD/auxiliary/Environment.hpp" #include "openPMD/auxiliary/JSON_internal.hpp" #include "openPMD/auxiliary/Variant.hpp" @@ -388,6 +389,8 @@ void HDF5IOHandlerImpl::createPath( "[HDF5] Creating a path in a file opened as read only is not " "possible."); + herr_t status = 0; + hid_t gapl = H5Pcreate(H5P_GROUP_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI if (m_hdf5_collective_metadata) @@ -396,7 +399,15 @@ void HDF5IOHandlerImpl::createPath( } #endif - herr_t status; + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path creation." + << std::endl; + } + }); if (!writable->written) { @@ -426,6 +437,20 @@ void HDF5IOHandlerImpl::createPath( /* Create the path in the file */ std::stack groups; groups.push(node_id); + auto defer_close_groups = auxiliary::defer([&]() { + while (!groups.empty()) + { + status = H5Gclose(groups.top()); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during path creation." + << std::endl; + } + groups.pop(); + } + }); for (std::string const &folder : auxiliary::split(path, "/", false)) { // avoid creation of paths that already exist @@ -447,29 +472,12 @@ void HDF5IOHandlerImpl::createPath( groups.push(group_id); } - /* Close the groups */ - while (!groups.empty()) - { - status = H5Gclose(groups.top()); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during path " - "creation"); - groups.pop(); - } - writable->written = true; writable->abstractFilePosition = std::make_shared(path); m_fileNames[writable] = file.name; } - - status = H5Pclose(gapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during path " - "creation"); } namespace @@ -840,6 +848,7 @@ void HDF5IOHandlerImpl::createDataset( throw error::OperationUnsupportedInBackend( "HDF5", "No support for Datasets with undefined extent."); } + herr_t status = 0; if (!writable->written) { @@ -923,6 +932,27 @@ void HDF5IOHandlerImpl::createDataset( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during dataset " "creation"); + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during dataset creation." + << std::endl; + } + }); + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 property " + "during dataset creation." + << std::endl; + } + }); if (access::append(m_handler->m_backendAccess)) { @@ -938,7 +968,7 @@ void HDF5IOHandlerImpl::createDataset( // > should be able to detect and recycle the file space when no // > other reference to the deleted object exists // https://github.com/openPMD/openPMD-api/pull/1007#discussion_r867223316 - herr_t status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); + status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete old dataset '" + @@ -964,9 +994,29 @@ void HDF5IOHandlerImpl::createDataset( space >= 0, "[HDF5] Internal error: Failed to create dataspace during dataset " "creation"); + auto defer_close_space = auxiliary::defer([&]() { + status = H5Sclose(space); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset " + "space during dataset creation." + << std::endl; + } + }); /* enable chunking on the created dataspace */ hid_t datasetCreationProperty = H5Pcreate(H5P_DATASET_CREATE); + auto defer_close_datasetCreationProperty = auxiliary::defer([&]() { + status = H5Pclose(datasetCreationProperty); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset " + "creation property during dataset creation." + << std::endl; + } + }); H5Pset_fill_time(datasetCreationProperty, H5D_FILL_TIME_NEVER); @@ -1003,7 +1053,7 @@ void HDF5IOHandlerImpl::createDataset( } else { - herr_t status = H5Pset_chunk( + status = H5Pset_chunk( datasetCreationProperty, chunking->size(), chunking->data()); @@ -1016,7 +1066,7 @@ void HDF5IOHandlerImpl::createDataset( for (auto const &filter : filters) { - herr_t status = std::visit( + status = std::visit( auxiliary::overloaded{ [&](DatasetParams::ByID const &by_id) { return H5Pset_filter( @@ -1050,6 +1100,16 @@ void HDF5IOHandlerImpl::createDataset( datatype >= 0, "[HDF5] Internal error: Failed to get HDF5 datatype during dataset " "creation"); + auto defer_close_datatype = auxiliary::defer([&]() { + status = H5Tclose(datatype); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 datatype " + "during dataset creation." + << std::endl; + } + }); hid_t group_id = H5Dcreate( node_id, name.c_str(), @@ -1062,38 +1122,16 @@ void HDF5IOHandlerImpl::createDataset( group_id >= 0, "[HDF5] Internal error: Failed to create HDF5 group during dataset " "creation"); - - herr_t status; - status = H5Dclose(group_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset during " - "dataset creation"); - status = H5Tclose(datatype); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 datatype during " - "dataset creation"); - status = H5Pclose(datasetCreationProperty); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset creation " - "property during dataset creation"); - status = H5Sclose(space); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset space during " - "dataset creation"); - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during dataset " - "creation"); - status = H5Pclose(gapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during " - "dataset creation"); + auto defer_close_group_id = auxiliary::defer([&]() { + status = H5Dclose(group_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during dataset creation." + << std::endl; + } + }); writable->written = true; writable->abstractFilePosition = @@ -1134,6 +1172,16 @@ void HDF5IOHandlerImpl::extendDataset( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "extension"); + herr_t status = 0; + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during dataset extension." + << std::endl; + } + }); // Datasets may only be extended if they have chunked layout, so let's see // whether this one does @@ -1160,18 +1208,11 @@ void HDF5IOHandlerImpl::extendDataset( for (auto const &val : parameters.extent) size.push_back(static_cast(val)); - herr_t status; status = H5Dset_extent(dataset_id, size.data()); VERIFY( status == 0, "[HDF5] Internal error: Failed to extend HDF5 dataset during dataset " "extension"); - - status = H5Dclose(dataset_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset during dataset " - "extension"); } void HDF5IOHandlerImpl::availableChunks( @@ -1191,7 +1232,26 @@ void HDF5IOHandlerImpl::availableChunks( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "read"); + herr_t status = 0; + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during availableChunks task." + << std::endl; + } + }); hid_t dataset_space = H5Dget_space(dataset_id); + auto defer_close_dataset_space = auxiliary::defer([&]() { + status = H5Sclose(dataset_space); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "space during availableChunks task." + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(dataset_space); VERIFY( ndims >= 0, @@ -1229,19 +1289,6 @@ void HDF5IOHandlerImpl::availableChunks( extent.push_back(e); } parameters.chunks->emplace_back(std::move(offset), std::move(extent)); - - herr_t status; - status = H5Sclose(dataset_space); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset space during " - "availableChunks task"); - - status = H5Dclose(dataset_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset during " - "availableChunks task"); } void HDF5IOHandlerImpl::openFile( @@ -1329,6 +1376,16 @@ void HDF5IOHandlerImpl::openPath( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + herr_t status = 0; + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path opening." + << std::endl; + } + }); node_id = H5Gopen( file.id, concrete_h5_file_position(writable->parent).c_str(), gapl); @@ -1341,6 +1398,15 @@ void HDF5IOHandlerImpl::openPath( "[HDF5] Internal error: Failed to open HDF5 group during path " "opening"); } + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + "during path opening." + << std::endl; + } + }); /* Sanitize path */ std::string path = parameters.path; @@ -1360,40 +1426,17 @@ void HDF5IOHandlerImpl::openPath( "[HDF5] Internal error: Failed to open HDF5 group during path " "opening"); } - - herr_t status; - status = H5Gclose(path_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 group during path " - "opening"); - } - } - - herr_t status; - status = H5Gclose(node_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 group during path " - "opening"); - } - status = H5Pclose(gapl); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 property during path " - "opening"); + auto defer_close_path_id = + auxiliary::defer([&]() { + status = H5Gclose(path_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during path opening." + << std::endl; + } + }); } writable->written = true; @@ -1423,6 +1466,16 @@ void HDF5IOHandlerImpl::openDataset( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + herr_t status = 0; + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during dataset opening." + << std::endl; + } + }); node_id = H5Gopen( file.id, concrete_h5_file_position(writable->parent).c_str(), gapl); @@ -1435,6 +1488,15 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to open HDF5 group during dataset " "opening"); } + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + "during dataset opening." + << std::endl; + } + }); /* Sanitize name */ std::string name = parameters.name; @@ -1453,10 +1515,40 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to open HDF5 dataset during dataset " "opening"); } + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during dataset opening." + << std::endl; + } + }); hid_t dataset_type, dataset_space; dataset_type = H5Dget_type(dataset_id); + auto defer_close_dataset_type = auxiliary::defer([&]() { + status = H5Tclose(dataset_type); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset type " + "during dataset opening." + << std::endl; + } + }); + dataset_space = H5Dget_space(dataset_id); + auto defer_close_dataset_space = auxiliary::defer([&]() { + status = H5Sclose(dataset_space); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset space " + "during dataset opening." + << std::endl; + } + }); H5S_class_t dataset_class = H5Sget_simple_extent_type(dataset_space); @@ -1534,36 +1626,37 @@ void HDF5IOHandlerImpl::openDataset( "HDF5", "Unknown dataset type"); }; + if (remaining_tries == 0) { throw_error(); } + hid_t next_type = H5Tget_super(dataset_type); if (next_type == H5I_INVALID_HID) { throw_error(); } - else if (H5Tequal(dataset_type, next_type)) - { - H5Tclose(next_type); - throw_error(); - } - else - { - if (H5Tclose(dataset_type) != 0) + + auto defer_close_next_type = auxiliary::defer([&]() { + status = H5Tclose(next_type); + if (status != 0) { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset type " - "during " - "dataset opening"); + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 " + "dataset type during dataset opening." + << std::endl; } - dataset_type = next_type; - --remaining_tries; - repeat = true; + }); + + if (H5Tequal(dataset_type, next_type)) + { + throw_error(); } + + dataset_type = next_type; + --remaining_tries; + repeat = true; } } while (repeat); } @@ -1597,58 +1690,6 @@ void HDF5IOHandlerImpl::openDataset( *extent = e; } - herr_t status; - status = H5Sclose(dataset_space); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset space during " - "dataset opening"); - } - status = H5Tclose(dataset_type); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset type during " - "dataset opening"); - } - status = H5Dclose(dataset_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset during dataset " - "opening"); - } - status = H5Gclose(node_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 group during dataset " - "opening"); - } - status = H5Pclose(gapl); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 property during dataset " - "opening"); - } - writable->written = true; writable->abstractFilePosition = std::make_shared(name); @@ -1721,20 +1762,26 @@ void HDF5IOHandlerImpl::deletePath( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during path " "deletion"); + herr_t status = 0; + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during path deletion." + << std::endl; + } + }); path += static_cast( writable->abstractFilePosition.get()) ->location; - herr_t status = H5Ldelete(node_id, path.c_str(), H5P_DEFAULT); + status = H5Ldelete(node_id, path.c_str(), H5P_DEFAULT); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 group"); - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during path " - "deletion"); - writable->written = false; writable->abstractFilePosition.reset(); @@ -1773,20 +1820,26 @@ void HDF5IOHandlerImpl::deleteDataset( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during dataset " "deletion"); + herr_t status = 0; + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during dataset deletion." + << std::endl; + } + }); name += static_cast( writable->abstractFilePosition.get()) ->location; - herr_t status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); + status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 group"); - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during dataset " - "deletion"); - writable->written = false; writable->abstractFilePosition.reset(); @@ -1815,17 +1868,23 @@ void HDF5IOHandlerImpl::deleteAttribute( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during attribute " "deletion"); + herr_t status = 0; + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Oclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during attribute deletion." + << std::endl; + } + }); - herr_t status = H5Adelete(node_id, name.c_str()); + status = H5Adelete(node_id, name.c_str()); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 attribute"); - - status = H5Oclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during " - "attribute deletion"); } } @@ -1839,18 +1898,38 @@ void HDF5IOHandlerImpl::writeDataset( File file = requireFile("writeDataset", writable, /* checkParent = */ true); - hid_t dataset_id, filespace, memspace; herr_t status; + hid_t dataset_id, filespace, memspace; dataset_id = H5Dopen( file.id, concrete_h5_file_position(writable).c_str(), H5P_DEFAULT); VERIFY( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "write"); + auto defer_close_dataset = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close dataset " + + concrete_h5_file_position(writable) + + " during dataset write" + << std::endl; + } + }); filespace = H5Dget_space(dataset_id); + auto defer_close_filespace = auxiliary::defer([&]() { + status = H5Sclose(filespace); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close dataset file " + "space during dataset write" + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(filespace); + auxiliary::opaque_defer_type defer_close_memspace; if (ndims == 0) { if (parameters.offset != Offset{0} || parameters.extent != Extent{1}) @@ -1871,6 +1950,16 @@ void HDF5IOHandlerImpl::writeDataset( memspace > 0, "[HDF5] Internal error: Failed to create memspace during dataset " "write"); + defer_close_memspace = + auxiliary::defer([&]() { + status = H5Sclose(memspace); // + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + "dataset memory space during dataset write" + << std::endl; + } + }).to_opaque(); } else { @@ -1884,6 +1973,16 @@ void HDF5IOHandlerImpl::writeDataset( block.push_back(static_cast(val)); memspace = H5Screate_simple( static_cast(block.size()), block.data(), nullptr); + defer_close_memspace = + auxiliary::defer([&]() { + status = H5Sclose(memspace); // + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + "dataset memory space during dataset write" + << std::endl; + } + }).to_opaque(); status = H5Sselect_hyperslab( filespace, H5S_SELECT_SET, @@ -1914,6 +2013,15 @@ void HDF5IOHandlerImpl::writeDataset( dataType >= 0, "[HDF5] Internal error: Failed to get HDF5 datatype during dataset " "write"); + auto defer_close_dataType = auxiliary::defer([&]() { + status = H5Tclose(dataType); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close dataset " + "datatype during dataset write." + << std::endl; + } + }); switch (a.dtype) { using DT = Datatype; @@ -1952,26 +2060,6 @@ void HDF5IOHandlerImpl::writeDataset( default: throw std::runtime_error("[HDF5] Datatype not implemented in HDF5 IO"); } - status = H5Tclose(dataType); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset datatype during " - "dataset write"); - status = H5Sclose(filespace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset file space during " - "dataset write"); - status = H5Sclose(memspace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset memory space during " - "dataset write"); - status = H5Dclose(dataset_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset " + - concrete_h5_file_position(writable) + " during dataset write"); m_fileNames[writable] = file.name; } @@ -1993,6 +2081,7 @@ void HDF5IOHandlerImpl::writeAttribute( auto res = getFile(writable); File file = res ? res.value() : getFile(writable->parent).value(); hid_t node_id, attribute_id; + herr_t status = 0; hid_t fapl = H5Pcreate(H5P_LINK_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -2001,6 +2090,15 @@ void HDF5IOHandlerImpl::writeAttribute( H5Pset_all_coll_metadata_ops(fapl, true); } #endif + auto defer_close_fapl = auxiliary::defer([&]() { + status = H5Pclose(fapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "property during attribute write." + << std::endl; + } + }); node_id = H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl); @@ -2008,9 +2106,18 @@ void HDF5IOHandlerImpl::writeAttribute( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 object during attribute " "write"); + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Oclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + + concrete_h5_file_position(writable) + + " during attribute write." + << std::endl; + } + }); Attribute const att(Attribute::from_any, parameters.m_resource); Datatype dtype = parameters.dtype; - herr_t status; GetH5DataType getH5DataType({ {typeid(bool).name(), m_H5T_BOOL_ENUM}, {typeid(std::complex).name(), m_H5T_CFLOAT}, @@ -2022,6 +2129,15 @@ void HDF5IOHandlerImpl::writeAttribute( dataType >= 0, "[HDF5] Internal error: Failed to get HDF5 datatype during attribute " "write"); + auto defer_close_dataType = auxiliary::defer([&]() { + status = H5Tclose(dataType); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 datatype " + "during attribute write." + << std::endl; + } + }); std::string name = parameters.name; auto create_attribute_anew = [&]() { hid_t dataspace = getH5DataSpace(att); @@ -2029,6 +2145,15 @@ void HDF5IOHandlerImpl::writeAttribute( dataspace >= 0, "[HDF5] Internal error: Failed to get HDF5 dataspace during " "attribute write"); + auto defer_close_dataspace = auxiliary::defer([&]() { + status = H5Sclose(dataspace); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "dataspace during attribute write." + << std::endl; + } + }); attribute_id = H5Acreate( node_id, name.c_str(), @@ -2040,11 +2165,6 @@ void HDF5IOHandlerImpl::writeAttribute( node_id >= 0, "[HDF5] Internal error: Failed to create HDF5 attribute during " "attribute write"); - status = H5Sclose(dataspace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataspace during " - "attribute write"); }; if (H5Aexists(node_id, name.c_str()) != 0) { @@ -2066,7 +2186,7 @@ void HDF5IOHandlerImpl::writeAttribute( equal >= 0, "[HDF5] Internal error: Failed to compare HDF5 attribute types " "during attribute write"); - if (equal == 0) // unequal + if (equal == 0) // unequal - need to delete and recreate { status = H5Aclose(attribute_id); VERIFY( @@ -2087,6 +2207,16 @@ void HDF5IOHandlerImpl::writeAttribute( { create_attribute_anew(); } + auto defer_close_attribute_id = auxiliary::defer([&]() { + status = H5Aclose(attribute_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + + name + " at " + concrete_h5_file_position(writable) + + " during attribute write." + << std::endl; + } + }); using DT = Datatype; switch (dtype) @@ -2293,28 +2423,6 @@ void HDF5IOHandlerImpl::writeAttribute( "[HDF5] Internal error: Failed to write attribute " + name + " at " + concrete_h5_file_position(writable)); - status = H5Tclose(dataType); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 datatype during Attribute " - "write"); - - status = H5Aclose(attribute_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close attribute " + name + " at " + - concrete_h5_file_position(writable) + " during attribute write"); - status = H5Oclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close " + - concrete_h5_file_position(writable) + " during attribute write"); - status = H5Pclose(fapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during attribute " - "write"); - m_fileNames[writable] = file.name; } @@ -2330,8 +2438,28 @@ void HDF5IOHandlerImpl::readDataset( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "read"); + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset during " + "dataset read." + << std::endl; + } + }); filespace = H5Dget_space(dataset_id); + auto defer_close_filespace = auxiliary::defer([&]() { + status = H5Sclose(filespace); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset file space " + "during dataset read." + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(filespace); if (ndims == 0) @@ -2379,6 +2507,16 @@ void HDF5IOHandlerImpl::readDataset( "[HDF5] Internal error: Failed to select hyperslab during dataset " "read"); } + auto defer_close_memspace = auxiliary::defer([&]() { + status = H5Sclose(memspace); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset memory " + "space during dataset read." + << std::endl; + } + }); void *data = parameters.data.get(); @@ -2419,6 +2557,16 @@ void HDF5IOHandlerImpl::readDataset( {typeid(std::complex).name(), m_H5T_CLONG_DOUBLE}, }); hid_t dataType = getH5DataType(a); + auto defer_close_dataType = auxiliary::defer([&]() { + status = H5Tclose(dataType); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset datatype " + "during dataset read." + << std::endl; + } + }); if (H5Tequal(dataType, H5T_NATIVE_LDOUBLE)) { // We have previously determined in openDataset() that this dataset is @@ -2427,29 +2575,37 @@ void HDF5IOHandlerImpl::readDataset( // the worked-around m_H5T_LONG_DOUBLE_80_LE. // Check this. hid_t checkDatasetTypeAgain = H5Dget_type(dataset_id); + auto defer_close_checkDatasetTypeAgain = auxiliary::defer([&]() { + status = H5Tclose(checkDatasetTypeAgain); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "dataset type during dataset reading." + << std::endl; + } + }); if (!H5Tequal(checkDatasetTypeAgain, H5T_NATIVE_LDOUBLE)) { dataType = m_H5T_LONG_DOUBLE_80_LE; } - status = H5Tclose(checkDatasetTypeAgain); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset type during " - "dataset reading"); } else if (H5Tequal(dataType, m_H5T_CLONG_DOUBLE)) { // Same deal for m_H5T_CLONG_DOUBLE hid_t checkDatasetTypeAgain = H5Dget_type(dataset_id); + auto defer_close_checkDatasetTypeAgain = auxiliary::defer([&]() { + status = H5Tclose(checkDatasetTypeAgain); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "dataset type during dataset reading." + << std::endl; + } + }); if (!H5Tequal(checkDatasetTypeAgain, m_H5T_CLONG_DOUBLE)) { dataType = m_H5T_CLONG_DOUBLE_80_LE; } - status = H5Tclose(checkDatasetTypeAgain); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset type during " - "dataset reading"); } VERIFY( dataType >= 0, @@ -2463,26 +2619,6 @@ void HDF5IOHandlerImpl::readDataset( m_datasetTransferProperty, data); VERIFY(status == 0, "[HDF5] Internal error: Failed to read dataset"); - - status = H5Tclose(dataType); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset datatype during " - "dataset read"); - status = H5Sclose(filespace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset file space during " - "dataset read"); - status = H5Sclose(memspace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset memory space during " - "dataset read"); - status = H5Dclose(dataset_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset during dataset read"); } void HDF5IOHandlerImpl::readAttribute( @@ -2506,6 +2642,15 @@ void HDF5IOHandlerImpl::readAttribute( H5Pset_all_coll_metadata_ops(fapl, true); } #endif + auto defer_close_fapl = auxiliary::defer([&]() { + status = H5Pclose(fapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "attribute during attribute read." + << std::endl; + } + }); obj_id = H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl); @@ -2519,7 +2664,26 @@ void HDF5IOHandlerImpl::readAttribute( concrete_h5_file_position(writable).c_str() + "' during attribute read"); } + auto defer_close_obj_id = auxiliary::defer([&]() { + status = H5Oclose(obj_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + + concrete_h5_file_position(writable) + + " during attribute read." + << std::endl; + ; + } + }); std::string const &attr_name = parameters.name; + if (H5Aexists(obj_id, attr_name.c_str()) <= 0) + { + throw error::ReadError( + error::AffectedObject::Attribute, + error::Reason::NotFound, + "HDF5", + parameters.name); + } attr_id = H5Aopen(obj_id, attr_name.c_str(), H5P_DEFAULT); if (attr_id < 0) { @@ -2533,10 +2697,38 @@ void HDF5IOHandlerImpl::readAttribute( concrete_h5_file_position(writable).c_str() + ") during attribute read"); } + auto defer_close_attr_id = auxiliary::defer([&]() { + status = H5Aclose(attr_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + + attr_name + " at " + concrete_h5_file_position(writable) + + " during attribute read." + << std::endl; + } + }); hid_t attr_type, attr_space; attr_type = H5Aget_type(attr_id); + auto defer_close_attr_type = auxiliary::defer([&]() { + status = H5Tclose(attr_type); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + "file space during attribute read." + << std::endl; + } + }); attr_space = H5Aget_space(attr_id); + auto defer_close_attr_space = auxiliary::defer([&]() { + status = H5Sclose(attr_space); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + "datatype during attribute read." + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(attr_space); std::vector dims(ndims, 0); @@ -3065,63 +3257,9 @@ void HDF5IOHandlerImpl::readAttribute( " at " + concrete_h5_file_position(writable)); } - status = H5Tclose(attr_type); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close attribute datatype during " - "attribute read"); - } - status = H5Sclose(attr_space); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close attribute file space " - "during " - "attribute read"); - } - auto dtype = parameters.dtype; *dtype = a.dtype; *parameters.m_resource = a.getAny(); - - status = H5Aclose(attr_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close attribute " + attr_name + - " at " + concrete_h5_file_position(writable) + - " during attribute read"); - } - status = H5Oclose(obj_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close " + - concrete_h5_file_position(writable) + " during attribute read"); - } - status = H5Pclose(fapl); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 attribute during " - "attribute read"); - } } void HDF5IOHandlerImpl::listPaths( @@ -3133,6 +3271,7 @@ void HDF5IOHandlerImpl::listPaths( "listing"); File file = requireFile("listPaths", writable, /* checkParent = */ true); + herr_t status = 0; hid_t gapl = H5Pcreate(H5P_GROUP_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -3141,15 +3280,34 @@ void HDF5IOHandlerImpl::listPaths( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path listing." + << std::endl; + } + }); hid_t node_id = H5Gopen(file.id, concrete_h5_file_position(writable).c_str(), gapl); VERIFY( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during path listing"); + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + + concrete_h5_file_position(writable) + + " during path listing." + << std::endl; + } + }); H5G_info_t group_info; - herr_t status = H5Gget_info(node_id, &group_info); + status = H5Gget_info(node_id, &group_info); VERIFY( status == 0, "[HDF5] Internal error: Failed to get HDF5 group info for " + @@ -3166,17 +3324,6 @@ void HDF5IOHandlerImpl::listPaths( paths->emplace_back(name.data(), name_length); } } - - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group " + - concrete_h5_file_position(writable) + " during path listing"); - status = H5Pclose(gapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during path " - "listing"); } void HDF5IOHandlerImpl::listDatasets( @@ -3188,6 +3335,7 @@ void HDF5IOHandlerImpl::listDatasets( "listing"); File file = requireFile("listDatasets", writable, /* checkParent = */ true); + herr_t status = 0; hid_t gapl = H5Pcreate(H5P_GROUP_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -3196,6 +3344,15 @@ void HDF5IOHandlerImpl::listDatasets( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during dataset listing." + << std::endl; + } + }); hid_t node_id = H5Gopen(file.id, concrete_h5_file_position(writable).c_str(), gapl); @@ -3203,9 +3360,19 @@ void HDF5IOHandlerImpl::listDatasets( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during dataset " "listing"); + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + + concrete_h5_file_position(writable) + + " during dataset listing." + << std::endl; + } + }); H5G_info_t group_info; - herr_t status = H5Gget_info(node_id, &group_info); + status = H5Gget_info(node_id, &group_info); VERIFY( status == 0, "[HDF5] Internal error: Failed to get HDF5 group info for " + @@ -3222,17 +3389,6 @@ void HDF5IOHandlerImpl::listDatasets( datasets->emplace_back(name.data(), name_length); } } - - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group " + - concrete_h5_file_position(writable) + " during dataset listing"); - status = H5Pclose(gapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during dataset " - "listing"); } void HDF5IOHandlerImpl::listAttributes( @@ -3246,6 +3402,7 @@ void HDF5IOHandlerImpl::listAttributes( File file = requireFile("listAttributes", writable, /* checkParent = */ true); hid_t node_id; + herr_t status = 0; hid_t fapl = H5Pcreate(H5P_LINK_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -3254,6 +3411,15 @@ void HDF5IOHandlerImpl::listAttributes( H5Pset_all_coll_metadata_ops(fapl, true); } #endif + auto defer_close_fapl = auxiliary::defer([&]() { + status = H5Pclose(fapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during attribute listing." + << std::endl; + } + }); node_id = H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl); @@ -3261,8 +3427,16 @@ void HDF5IOHandlerImpl::listAttributes( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during attribute " "listing"); - - herr_t status; + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Oclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 object " + + concrete_h5_file_position(writable) + + " during attribute listing." + << std::endl; + } + }); #if H5_VERSION_GE(1, 12, 0) H5O_info2_t object_info; status = H5Oget_info3(node_id, &object_info, H5O_INFO_NUM_ATTRS); @@ -3299,17 +3473,6 @@ void HDF5IOHandlerImpl::listAttributes( H5P_DEFAULT); attributes->emplace_back(name.data(), name_length); } - - status = H5Oclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 object during attribute " - "listing"); - status = H5Pclose(fapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during dataset " - "listing"); } void HDF5IOHandlerImpl::deregister( diff --git a/src/IO/InvalidatableFile.cpp b/src/IO/InvalidatableFile.cpp index cb6930da2b..c4a5d2c69f 100644 --- a/src/IO/InvalidatableFile.cpp +++ b/src/IO/InvalidatableFile.cpp @@ -80,3 +80,9 @@ std::hash::operator()( return std::hash>{}( s.fileState); } +auto std::less::operator()( + first_argument_type const &first, second_argument_type const &second) const + -> result_type +{ + return less<>()(*first, *second); +} diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp index 59541c1e30..551b6c4358 100644 --- a/src/IO/JSON/JSONIOHandlerImpl.cpp +++ b/src/IO/JSON/JSONIOHandlerImpl.cpp @@ -2332,7 +2332,7 @@ auto JSONIOHandlerImpl::verifyDataset( } Datatype dt = stringToDatatype(j["datatype"].get()); VERIFY_ALWAYS( - dt == parameters.dtype, + isSame(dt, parameters.dtype), "[JSON] Read/Write request does not fit the dataset's type"); } catch (json::basic_json::type_error &) diff --git a/src/Iteration.cpp b/src/Iteration.cpp index ab35c1e3ae..1a5fa4e4e4 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -59,6 +59,16 @@ Iteration::Iteration() : Attributable(NoInit()) particles.writable().ownKeyWithinParent = "particles"; } +uint64_t Iteration::getCachedIterationIndex() const +{ + auto idx = get().m_iterationIndex; + if (!idx.has_value()) + { + throw error::Internal("Iteration index not known."); + } + return *idx; +} + template Iteration &Iteration::setTime(T newTime) { @@ -246,7 +256,7 @@ void Iteration::flushFileBased( * If it was written before, then in the context of another iteration. */ auto &attr = s.get().m_rankTable.m_attributable; - attr.setWritten(false, Attributable::EnqueueAsynchronously::Yes); + attr.setWritten(false, Attributable::EnqueueAsynchronously::Both); s.get() .m_rankTable.m_attributable.get() .m_writable.abstractFilePosition.reset(); @@ -841,7 +851,7 @@ auto Iteration::beginStep( { bool previous = series.iterations.written(); series.iterations.setWritten( - false, Attributable::EnqueueAsynchronously::Yes); + false, Attributable::EnqueueAsynchronously::Both); auto oldStatus = IOHandl->m_seriesStatus; IOHandl->m_seriesStatus = internal::SeriesStatus::Parsing; try @@ -858,7 +868,7 @@ auto Iteration::beginStep( } IOHandl->m_seriesStatus = oldStatus; series.iterations.setWritten( - previous, Attributable::EnqueueAsynchronously::Yes); + previous, Attributable::EnqueueAsynchronously::Both); } else if (thisObject.has_value()) { diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index eb41dddd19..9e841416db 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -647,7 +647,7 @@ void RecordComponent::verifyChunk( if (empty()) throw std::runtime_error( "Chunks cannot be written for an empty RecordComponent."); - if (dtype != getDatatype()) + if (!isSame(dtype, getDatatype())) { std::ostringstream oss; oss << "Datatypes of chunk data (" << dtype @@ -823,21 +823,19 @@ void RecordComponent::loadChunk(std::shared_ptr data, Offset o, Extent e) * JSON/TOML backends as they might implicitly turn a LONG into an INT in a * constant component. The frontend needs to catch such edge cases. * Ref. `if (constant())` branch. + * + * Attention: Do NOT use operator==(), doesnt work properly on Windows! */ - if (dtype != getDatatype() && !constant()) - if (!isSameInteger(getDatatype()) && - !isSameFloatingPoint(getDatatype()) && - !isSameComplexFloatingPoint(getDatatype()) && - !isSameChar(getDatatype())) - { - std::string const data_type_str = datatypeToString(getDatatype()); - std::string const requ_type_str = - datatypeToString(determineDatatype()); - std::string err_msg = - "Type conversion during chunk loading not yet implemented! "; - err_msg += "Data: " + data_type_str + "; Load as: " + requ_type_str; - throw std::runtime_error(err_msg); - } + if (!isSame(dtype, getDatatype()) && !constant()) + { + std::string const data_type_str = datatypeToString(getDatatype()); + std::string const requ_type_str = + datatypeToString(determineDatatype()); + std::string err_msg = + "Type conversion during chunk loading not yet implemented! "; + err_msg += "Data: " + data_type_str + "; Load as: " + requ_type_str; + throw std::runtime_error(err_msg); + } uint8_t dim = getDimensionality(); diff --git a/src/Series.cpp b/src/Series.cpp index 6d13de73c3..6cafe5988e 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -571,6 +571,11 @@ void Series::flushRankTable() [asRawPtr](char *) { delete asRawPtr; }}; writeDataset(std::move(put), /* num_lines = */ size); } + + // Must ensure that the Writable is consistently set to written on all + // ranks + series.m_rankTable.m_attributable.setWritten( + true, EnqueueAsynchronously::OnlyAsync); return; } #endif @@ -1503,9 +1508,9 @@ void Series::flushFileBased( * current iteration by the backend) */ this->setWritten( - false, Attributable::EnqueueAsynchronously::Yes); + false, Attributable::EnqueueAsynchronously::Both); series.iterations.setWritten( - false, Attributable::EnqueueAsynchronously::Yes); + false, Attributable::EnqueueAsynchronously::Both); setDirty(dirty() || it->second.dirty()); std::string filename = iterationFilename(it->first); @@ -1950,12 +1955,11 @@ void Series::readOneIterationFileBased(std::string const &filePath) readBase(); - using DT = Datatype; aRead.name = "iterationEncoding"; IOHandler()->enqueue(IOTask(this, aRead)); IOHandler()->flush(internal::defaultFlushParams); IterationEncoding encoding_out; - if (*aRead.dtype == DT::STRING) + if (isSame(*aRead.dtype, Datatype::STRING)) { std::string encoding = Attribute(Attribute::from_any, *aRead.m_resource) .get(); @@ -1997,7 +2001,7 @@ void Series::readOneIterationFileBased(std::string const &filePath) setWritten(false, Attributable::EnqueueAsynchronously::No); setIterationEncoding_internal( encoding_out, internal::default_or_explicit::explicit_); - setWritten(old_written, Attributable::EnqueueAsynchronously::Yes); + setWritten(old_written, Attributable::EnqueueAsynchronously::Both); } else throw std::runtime_error( @@ -2010,7 +2014,7 @@ void Series::readOneIterationFileBased(std::string const &filePath) aRead.name = "iterationFormat"; IOHandler()->enqueue(IOTask(this, aRead)); IOHandler()->flush(internal::defaultFlushParams); - if (*aRead.dtype == DT::STRING) + if (isSame(*aRead.dtype, Datatype::STRING)) { setWritten(false, Attributable::EnqueueAsynchronously::No); setIterationFormat(Attribute(Attribute::from_any, *aRead.m_resource) @@ -2604,16 +2608,25 @@ std::string Series::iterationFilename(IterationIndex_t i) Series::iterations_iterator Series::indexOf(Iteration const &iteration) { auto &series = get(); - for (auto it = series.iterations.begin(); it != series.iterations.end(); - ++it) + // first try the cached index; if it points to the correct entry return it + auto idx = iteration.get().m_iterationIndex; + if (!idx.has_value()) { - if (&it->second.Attributable::get() == &iteration.Attributable::get()) - { - return it; - } + throw error::Internal("Iteration index not known."); + } + + auto it = series.iterations.find(*idx); + if (it != series.iterations.end() && + &it->second.Attributable::get() == &iteration.Attributable::get()) + { + return it; + } + else + { + throw error::Internal( + "Iteration " + std::to_string(*idx) + + " no longer known by the Series?"); } - throw std::runtime_error( - "[Iteration::close] Iteration not found in Series."); } AdvanceStatus Series::advance( diff --git a/src/auxiliary/Filesystem.cpp b/src/auxiliary/Filesystem.cpp index 2a3e947a9e..b22117d171 100644 --- a/src/auxiliary/Filesystem.cpp +++ b/src/auxiliary/Filesystem.cpp @@ -67,6 +67,20 @@ bool file_exists(std::string const &path) } std::vector list_directory(std::string const &path) +{ + auto res = list_directory_nothrow(path); + if (!res) + { + throw std::system_error(std::error_code(errno, std::system_category())); + } + else + { + return *res; + } +} + +std::optional> +list_directory_nothrow(std::string const &path) { std::vector ret; #ifdef _WIN32 @@ -86,7 +100,9 @@ std::vector list_directory(std::string const &path) #else auto directory = opendir(path.c_str()); if (!directory) - throw std::system_error(std::error_code(errno, std::system_category())); + { + return std::nullopt; + } dirent *entry; while ((entry = readdir(directory)) != nullptr) if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) @@ -96,6 +112,46 @@ std::vector list_directory(std::string const &path) return ret; } +#ifndef _WIN32 +// Need to manually preserve sticky bit and setgid on Unix systems +namespace +{ + std::string get_parent(std::string const &path) + { + std::string parent = path; + size_t pos = parent.find_last_of(directory_separator); + if (pos != std::string::npos) + { + parent = parent.substr(0, pos); + if (parent.empty()) + parent = "/"; + } + else + { + parent.clear(); + } + return parent; + } + + mode_t get_permissions(std::string const &path) + { + std::string parent = get_parent(path); + if (parent.empty() || !directory_exists(parent)) + { + return 0; + } + + struct stat s; + if (stat(parent.c_str(), &s) != 0) + { + return 0; + } + + return s.st_mode & 07777; + } +} // namespace +#endif + bool create_directories(std::string const &path) { if (directory_exists(path)) @@ -106,10 +162,11 @@ bool create_directories(std::string const &path) return CreateDirectory(p.c_str(), nullptr); }; #else - mode_t mask = umask(0); - umask(mask); - auto mk = [mask](std::string const &p) -> bool { - return (0 == mkdir(p.c_str(), 0777 & ~mask)); + auto mk = [](std::string const &p) -> bool { + // preserve sticky and setgid from parent + mode_t parentPerms = + get_permissions(get_parent(p)) & (S_ISVTX | S_ISGID); + return (0 == mkdir(p.c_str(), 0777 | parentPerms)); }; #endif std::istringstream ss(path); @@ -150,14 +207,20 @@ bool remove_directory(std::string const &path) return (0 == remove(p.c_str())); }; #endif - for (auto const &entry : list_directory(path)) + auto entries = list_directory_nothrow(path); + // Check if some other process was faster deleting this + if (entries) { - auto partialPath = path; - partialPath.append(std::string(1, directory_separator)).append(entry); - if (directory_exists(partialPath)) - success &= remove_directory(partialPath); - else if (file_exists(partialPath)) - success &= remove_file(partialPath); + for (auto const &entry : *entries) + { + auto partialPath = path; + partialPath.append(std::string(1, directory_separator)) + .append(entry); + if (directory_exists(partialPath)) + success &= remove_directory(partialPath); + else if (file_exists(partialPath)) + success &= remove_file(partialPath); + } } success &= del(path); return success; diff --git a/src/backend/Attributable.cpp b/src/backend/Attributable.cpp index ce1c2936cb..dd9019b255 100644 --- a/src/backend/Attributable.cpp +++ b/src/backend/Attributable.cpp @@ -524,12 +524,18 @@ void Attributable::setWritten(bool val, EnqueueAsynchronously ea) switch (ea) { - case EnqueueAsynchronously::Yes: { + case EnqueueAsynchronously::OnlyAsync: { Parameter param; param.target_status = val; IOHandler()->enqueue(IOTask(this, param)); + return; + } + case EnqueueAsynchronously::Both: { + Parameter param; + param.target_status = val; + IOHandler()->enqueue(IOTask(this, param)); + break; } - break; case EnqueueAsynchronously::No: break; } diff --git a/src/binding/python/Attributable.cpp b/src/binding/python/Attributable.cpp index 72876867f1..23795276ce 100644 --- a/src/binding/python/Attributable.cpp +++ b/src/binding/python/Attributable.cpp @@ -618,7 +618,9 @@ void init_Attributable(py::module &m) "get_attribute", [](Attributable &attr, std::string const &key) { auto v = attr.getAttribute(key); - return v.getVariant(); + return std::visit( + [](auto const &val) { return py::cast(val); }, + v.getVariant()); // TODO instead of returning lists, return all arrays (ndim > 0) // as numpy arrays? }) diff --git a/src/binding/python/Iteration.cpp b/src/binding/python/Iteration.cpp index 846861d262..60ffe8b9f8 100644 --- a/src/binding/python/Iteration.cpp +++ b/src/binding/python/Iteration.cpp @@ -94,18 +94,22 @@ void init_Iteration(py::module &m) .def("set_dt", &Iteration::setDt) .def("set_time_unit_SI", &Iteration::setTimeUnitSI) - .def_readwrite( + .def_property_readonly( "meshes", - &Iteration::meshes, - py::return_value_policy::copy, - // garbage collection: return value must be freed before Iteration - py::keep_alive<1, 0>()) - .def_readwrite( + py::cpp_function( + [](Iteration &i) { return i.meshes; }, + py::return_value_policy::copy, + // garbage collection: return value must be freed before + // Iteration + py::keep_alive<0, 1>())) + .def_property_readonly( "particles", - &Iteration::particles, - py::return_value_policy::copy, - // garbage collection: return value must be freed before Iteration - py::keep_alive<1, 0>()); + py::cpp_function( + [](Iteration &i) { return i.particles; }, + py::return_value_policy::copy, + // garbage collection: return value must be freed before + // Iteration + py::keep_alive<0, 1>())); add_pickle( cl, [](openPMD::Series series, std::vector const &group) { diff --git a/src/binding/python/ParticleSpecies.cpp b/src/binding/python/ParticleSpecies.cpp index 1af708d206..a097c37e84 100644 --- a/src/binding/python/ParticleSpecies.cpp +++ b/src/binding/python/ParticleSpecies.cpp @@ -48,12 +48,13 @@ void init_ParticleSpecies(py::module &m) return stream.str(); }) - .def_readwrite( + .def_property_readonly( "particle_patches", - &ParticleSpecies::particlePatches, - py::return_value_policy::copy, - // garbage collection: return value must be freed before Series - py::keep_alive<1, 0>()); + py::cpp_function( + [](ParticleSpecies &ps) { return ps.particlePatches; }, + py::return_value_policy::copy, + // garbage collection: return value must be freed before Series + py::keep_alive<0, 1>())); add_pickle( cl, [](openPMD::Series series, std::vector const &group) { uint64_t const n_it = std::stoull(group.at(1)); diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index 232df5861d..382783a730 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -489,7 +489,14 @@ inline void store_chunk( check_buffer_is_contiguous(a); - // dtype_from_numpy(a.dtype()) + if (!dtype_to_numpy(r.getDatatype()).is(a.dtype())) + { + std::stringstream err; + err << "Attempting store from Python array of type '" + << dtype_from_numpy(a.dtype()) + << "' into Record Component of type '" << r.getDatatype() << "'."; + throw error::WrongAPIUsage(err.str()); + } switchDatasetType( r.getDatatype(), r, a, offset, extent); } @@ -770,6 +777,15 @@ inline void load_chunk( check_buffer_is_contiguous(a); + if (!dtype_to_numpy(r.getDatatype()).is(a.dtype())) + { + std::stringstream err; + err << "Attempting load into Python array of type '" + << dtype_from_numpy(a.dtype()) + << "' from Record Component of type '" << r.getDatatype() << "'."; + throw error::WrongAPIUsage(err.str()); + } + switchDatasetType( r.getDatatype(), r, a, offset, extent); } diff --git a/src/binding/python/Series.cpp b/src/binding/python/Series.cpp index 384462403d..6c95cf19b8 100644 --- a/src/binding/python/Series.cpp +++ b/src/binding/python/Series.cpp @@ -301,7 +301,8 @@ not possible once it has been closed. throw std::runtime_error("Unreachable"); }, // copy + keepalive - py::return_value_policy::copy) + py::return_value_policy::copy, + py::keep_alive<0, 1>()) .def( "current_iteration", [](Snapshots &s) -> std::optional { @@ -315,6 +316,7 @@ not possible once it has been closed. return std::nullopt; } }, + py::keep_alive<0, 1>(), "Return the iteration that is currently being written to, if " "it " "exists."); @@ -497,12 +499,13 @@ this method. .def("set_iteration_format", &Series::setIterationFormat) .def("set_name", &Series::setName) - .def_readwrite( + .def_property_readonly( "iterations", - &Series::iterations, - py::return_value_policy::copy, - // garbage collection: return value must be freed before Series - py::keep_alive<1, 0>()) + py::cpp_function( + [](Series &s) { return s.iterations; }, + py::return_value_policy::copy, + // garbage collection: return value must be freed before Series + py::keep_alive<0, 1>())) .def( "read_iterations", [](Series &s) { @@ -644,5 +647,18 @@ users to overwrite default options, while keeping any other ones. py::arg("comm"), docs_merge_json) #endif - ; + .def("__del__", [](Series &s) { + try + { + s.close(); + } + catch (std::exception const &e) + { + std::cerr << "Error during close: " << e.what() << std::endl; + } + catch (...) + { + std::cerr << "Unknown error during close." << std::endl; + } + }); } diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index ad22ae1cf8..ffeb22bfea 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -1748,6 +1748,11 @@ TEST_CASE("automatic_variable_encoding", "[adios2]") automatic_variable_encoding::automatic_variable_encoding(); } +TEST_CASE("read_nonexistent_attribute", "[core]") +{ + read_nonexistent_attribute::read_nonexistent_attribute(); +} + TEST_CASE("unique_ptr", "[core]") { auto stdptr = std::make_unique(5); diff --git a/test/Files_Core/CoreTests.hpp b/test/Files_Core/CoreTests.hpp index fa62279c82..f769beeb99 100644 --- a/test/Files_Core/CoreTests.hpp +++ b/test/Files_Core/CoreTests.hpp @@ -24,3 +24,8 @@ namespace automatic_variable_encoding { auto automatic_variable_encoding() -> void; } + +namespace read_nonexistent_attribute +{ +auto read_nonexistent_attribute() -> void; +} diff --git a/test/Files_Core/read_nonexistent_attribute.cpp b/test/Files_Core/read_nonexistent_attribute.cpp new file mode 100644 index 0000000000..08d584d0fc --- /dev/null +++ b/test/Files_Core/read_nonexistent_attribute.cpp @@ -0,0 +1,106 @@ + +/* Copyright 2026 Franz Poeschel + * + * This file is part of openPMD-api. + * + * openPMD-api is free software: you can redistribute it and/or modify + * it under the terms of of either the GNU General Public License or + * the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * openPMD-api is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License and the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License + * and the GNU Lesser General Public License along with openPMD-api. + * If not, see . + */ + +#include + +#if !openPMD_USE_INVASIVE_TESTS + +namespace read_nonexistent_attribute +{ +void read_nonexistent_attribute() +{} +} // namespace read_nonexistent_attribute + +#else + +#define OPENPMD_private public: +#define OPENPMD_protected public: + +#include "CoreTests.hpp" + +#include "openPMD/Error.hpp" +#include "openPMD/IO/AbstractIOHandler.hpp" +#include "openPMD/IO/IOTask.hpp" + +#include +namespace read_nonexistent_attribute +{ +using namespace openPMD; +static auto testedFileExtensions() -> std::vector +{ + auto allExtensions = getFileExtensions(); + auto newEnd = std::remove_if( + allExtensions.begin(), + allExtensions.end(), + []([[maybe_unused]] std::string const &ext) { + // sst and ssc need a receiver for testing + // bp5 is already tested via bp + // toml parsing is very slow and its implementation is equivalent to + // the json backend, so it is only activated for selected tests + return ext == "sst" || ext == "ssc" || ext == "bp5" || + ext == "toml"; + }); + return {allExtensions.begin(), newEnd}; +} + +void run(std::string const &ext) +{ + auto const filename = "../samples/read_nonexistent_attribute." + ext; + + auto do_create = [&filename]() { + Series write(filename, Access::CREATE); + write.close(); + }; + + do_create(); + + // Try reading an attribute from this Series which does not actually exist. + // This tests the bugs fixed in + // https://github.com/openPMD/openPMD-api/pull/1866: + // + // 1. The HDF5 backend should verify that the attribute exists before trying + // to read it. Otherwise, the read failure will print ugly backtraces. + // 2. The HDF5 backend should clean up resources also in case an operations + // returns early. Otherwise the second call to do_create() will fail, + // since the first HDF5 file will remain open (resource leak). + { + Series read(filename, Access::READ_ONLY); + Parameter readAttr; + readAttr.name = "this_attribute_does_hopefully_not_exist"; + read.IOHandler()->enqueue(IOTask(&read, readAttr)); + REQUIRE_THROWS_AS( + read.IOHandler()->flush(internal::defaultFlushParams), + error::ReadError); + } + + do_create(); +} + +void read_nonexistent_attribute() +{ + for (auto const &ext : testedFileExtensions()) + { + run(ext); + } +} +} // namespace read_nonexistent_attribute +#endif diff --git a/test/python/unittest/API/APITest.py b/test/python/unittest/API/APITest.py index 500752b9b1..97850c5aaf 100644 --- a/test/python/unittest/API/APITest.py +++ b/test/python/unittest/API/APITest.py @@ -1692,7 +1692,7 @@ def testDataset(self): extent = [1, 1, 1] obj = io.Dataset(data_type, extent) if found_numpy: - d = np.array((1, 1, 1, ), dtype=np.int_) + d = np.array((1, 1, 1, ), dtype="l") obj2 = io.Dataset(d.dtype, d.shape) assert data_type == io.determine_datatype(d.dtype) assert obj2.dtype == obj.dtype @@ -2026,6 +2026,9 @@ def makeAvailableChunksRoundTrip(self, ext): # Cleaner: write.close() # But let's keep this instance to test that that workflow stays # functional. + # Need to delete everything as garbage collection will keep `write` + # alive as long as E_x is around. + del E_x del write read = io.Series( @@ -2209,7 +2212,6 @@ def testError(self): def testCustomGeometries(self): DS = io.Dataset - DT = io.Datatype sample_data = np.ones([10], dtype=np.int_) write = io.Series("../samples/custom_geometries_python.json", @@ -2217,25 +2219,25 @@ def testCustomGeometries(self): E = write.iterations[0].meshes["E"] E.set_attribute("geometry", "other:customGeometry") E_x = E["x"] - E_x.reset_dataset(DS(DT.LONG, [10])) + E_x.reset_dataset(DS(np.dtype(np.int_), [10])) E_x[:] = sample_data B = write.iterations[0].meshes["B"] B.set_geometry("customGeometry") B_x = B["x"] - B_x.reset_dataset(DS(DT.LONG, [10])) + B_x.reset_dataset(DS(np.dtype(np.int_), [10])) B_x[:] = sample_data e_energyDensity = write.iterations[0].meshes["e_energyDensity"] e_energyDensity.set_geometry("other:customGeometry") e_energyDensity_x = e_energyDensity[io.Mesh_Record_Component.SCALAR] - e_energyDensity_x.reset_dataset(DS(DT.LONG, [10])) + e_energyDensity_x.reset_dataset(DS(np.dtype(np.int_), [10])) e_energyDensity_x[:] = sample_data e_chargeDensity = write.iterations[0].meshes["e_chargeDensity"] e_chargeDensity.set_geometry(io.Geometry.other) e_chargeDensity_x = e_chargeDensity[io.Mesh_Record_Component.SCALAR] - e_chargeDensity_x.reset_dataset(DS(DT.LONG, [10])) + e_chargeDensity_x.reset_dataset(DS(np.dtype(np.int_), [10])) e_chargeDensity_x[:] = sample_data self.assertTrue(write) @@ -2341,6 +2343,163 @@ def testScalarHdf5Fields(self): self.assertEqual(loaded_from_scalar, np.array([45])) series_read_again.close() + def testKeepaliveMeshComponent(self): + """Test keepalive for mesh component extraction.""" + for ext in tested_file_extensions: + self.backend_keepalive_mesh_component(ext) + + def testKeepaliveParticlePosition(self): + """Test keepalive for particle position component extraction.""" + for ext in tested_file_extensions: + self.backend_keepalive_particle_position(ext) + + def testKeepaliveParticlePatches(self): + """Test keepalive for particle patches component extraction.""" + for ext in tested_file_extensions: + self.backend_keepalive_particle_patches(ext) + + def backend_keepalive_mesh_component(self, file_ending): + """Helper function that tests keepalive + for mesh component extraction.""" + import gc + + filename = "unittest_py_keepalive_mesh." + file_ending + path = filename + + def get_component_only(): + series = io.Series(path, io.Access.create_linear) + backend = series.backend + iteration = series.snapshots()[0] + mesh = iteration.meshes["E"] + component = mesh["x"] + + mesh.axis_labels = ["x", "y"] + component.reset_dataset(io.Dataset(np.dtype("float"), [10, 10])) + + del iteration + del mesh + del series + gc.collect() + + return component, backend + + component, backend = get_component_only() + gc.collect() + + component[:, :] = np.reshape( + np.arange(100, dtype=np.dtype("float")), + [10, 10] + ) + + component.series_flush() + if backend == "ADIOS2": + del component + gc.collect() + + read = io.Series(path, io.Access.read_only) + loaded = read.snapshots()[0].meshes["E"]["x"][:] + read.flush() + np.testing.assert_array_equal( + loaded, + np.reshape(np.arange(100, dtype=np.dtype("float")), [10, 10]) + ) + + def backend_keepalive_particle_position(self, file_ending): + """Helper function that tests keepalive + for particle position component extraction.""" + import gc + + filename = "unittest_py_keepalive_particle." + file_ending + path = filename + num_particles = 100 + + def get_component_only(): + series = io.Series(path, io.Access.create_linear) + backend = series.backend + iteration = series.snapshots()[0] + particles = iteration.particles["electrons"] + position = particles["position"]["x"] + + position.reset_dataset( + io.Dataset(np.dtype("float"), [num_particles])) + + del iteration + del particles + del series + gc.collect() + + return position, backend + + position, backend = get_component_only() + gc.collect() + + position[:] = np.arange(num_particles, dtype=np.dtype("float")) + + position.series_flush() + if backend == "ADIOS2": + del position + gc.collect() + + read = io.Series(path, io.Access.read_only) + loaded = read.snapshots()[0] \ + .particles["electrons"]["position"]["x"][:] + read.flush() + np.testing.assert_array_equal( + loaded, + np.arange(num_particles, dtype=np.dtype("float")) + ) + + def backend_keepalive_particle_patches(self, file_ending): + """Helper function that tests keepalive + for particle patches extraction.""" + import gc + + filename = "unittest_py_keepalive_patches." + file_ending + path = filename + + def get_component_only(): + series = io.Series(path, io.Access.create_linear) + backend = series.backend + iteration = series.snapshots()[0] + particles = iteration.particles["electrons"] + + dset = io.Dataset(np.dtype(np.float32), [30]) + position_x = particles["position"]["x"] + position_x.reset_dataset(dset) + position_x[:] = np.arange(30, dtype=np.float32) + + dset = io.Dataset(np.dtype("uint64"), [2]) + num_particles_comp = particles.particle_patches["numParticles"] + num_particles_comp.reset_dataset(dset) + num_particles_comp.store(0, np.uint64(10)) + num_particles_comp.store(1, np.uint64(20)) + + del iteration + del particles + del series + gc.collect() + + return num_particles_comp, backend + + component, backend = get_component_only() + gc.collect() + + component.store(0, np.uint64(50)) + + component.series_flush() + if backend == "ADIOS2": + del component + gc.collect() + + read = io.Series(path, io.Access.read_only) + loaded = read.snapshots()[0] \ + .particles["electrons"].particle_patches["numParticles"].load() + read.flush() + np.testing.assert_array_equal( + loaded[0], + np.uint64(50) + ) + if __name__ == '__main__': unittest.main()