From 442481aaf0bd130f5d43b1e4ca1babf46a3dc378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 7 Nov 2023 03:56:21 +0100 Subject: [PATCH 01/11] Fix `unique_ptr` constructor of UniquePtrWithLambda (#1552) * Add test with copyable custom deleter * Remove wrong double application of `get_deleter()` * Test custom non-copyable deleter type * Consider that deleter may not be copy-constructible We then need to put them behind a shared_ptr to create a lambda for std::function * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add a comment and some cleanup --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/openPMD/auxiliary/UniquePtr.hpp | 39 +++++---- test/SerialIOTest.cpp | 106 +++++++++++++++++------- 2 files changed, 96 insertions(+), 49 deletions(-) diff --git a/include/openPMD/auxiliary/UniquePtr.hpp b/include/openPMD/auxiliary/UniquePtr.hpp index 4b99f36790..6a64763b13 100644 --- a/include/openPMD/auxiliary/UniquePtr.hpp +++ b/include/openPMD/auxiliary/UniquePtr.hpp @@ -30,24 +30,14 @@ namespace auxiliary public: using deleter_type = std::function; - deleter_type const &get_deleter() const - { - return *this; - } - deleter_type &get_deleter() - { - return *this; - } - /* * Default constructor: Use std::default_delete. * This ensures correct destruction of arrays by using delete[]. */ CustomDelete() - : deleter_type{[](T_decayed *ptr) { + : deleter_type{[]([[maybe_unused]] T_decayed *ptr) { if constexpr (std::is_void_v) { - (void)ptr; std::cerr << "[Warning] Cannot standard-delete a void-type " "pointer. Please specify a custom destructor. " "Will let the memory leak." @@ -144,12 +134,25 @@ UniquePtrWithLambda::UniquePtrWithLambda(std::unique_ptr stdPtr) template template UniquePtrWithLambda::UniquePtrWithLambda(std::unique_ptr ptr) - : BasePtr{ - ptr.release(), - auxiliary::CustomDelete{ - [deleter = std::move(ptr.get_deleter())](T_decayed *del_ptr) { - deleter.get_deleter()(del_ptr); - }}} + : BasePtr{ptr.release(), auxiliary::CustomDelete{[&]() { + if constexpr (std::is_copy_constructible_v) + { + return [deleter = std::move(ptr.get_deleter())]( + T_decayed *del_ptr) { deleter(del_ptr); }; + } + else + { + /* + * The constructor of std::function requires a copyable + * lambda. Since Del is not a copyable type, we cannot + * capture it directly, but need to put it into a + * shared_ptr to make it copyable. + */ + return [deleter = std::make_shared( + std::move(ptr.get_deleter()))]( + T_decayed *del_ptr) { (*deleter)(del_ptr); }; + } + }()}} {} template @@ -170,7 +173,7 @@ UniquePtrWithLambda UniquePtrWithLambda::static_cast_() && return UniquePtrWithLambda{ static_cast(this->release()), [deleter = std::move(this->get_deleter())](other_type *ptr) { - deleter.get_deleter()(static_cast(ptr)); + deleter(static_cast(ptr)); }}; } } // namespace openPMD diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 962ca636aa..2ab428c83f 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -579,6 +579,29 @@ TEST_CASE("close_iteration_interleaved_test", "[serial]") } } +namespace detail +{ +template +struct CopyableDeleter : std::function +{ + CopyableDeleter() + : std::function{[](T const *ptr) { delete[] ptr; }} + {} +}; + +template +struct NonCopyableDeleter : std::function +{ + NonCopyableDeleter() + : std::function{[](T const *ptr) { delete[] ptr; }} + {} + NonCopyableDeleter(NonCopyableDeleter const &) = delete; + NonCopyableDeleter &operator=(NonCopyableDeleter const &) = delete; + NonCopyableDeleter(NonCopyableDeleter &&) = default; + NonCopyableDeleter &operator=(NonCopyableDeleter &&) = default; +}; +} // namespace detail + void close_and_copy_attributable_test(std::string file_ending) { using position_t = int; @@ -685,6 +708,27 @@ void close_and_copy_attributable_test(std::string file_ending) {0}, {global_extent}); + // UniquePtrWithLambda from unique_ptr with custom delete type + auto pos_v = electronPositions["v"]; + pos_v.resetDataset(dataset); + std::unique_ptr> + ptr_v_copyable_deleter(new int[10]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); + pos_v.storeChunk( + UniquePtrWithLambda(std::move(ptr_v_copyable_deleter)), + {0}, + {global_extent}); + + // UniquePtrWithLambda from unique_ptr with non-copyable custom delete + // type + auto posOff_v = electronPositionsOffset["v"]; + posOff_v.resetDataset(dataset); + std::unique_ptr> + ptr_v_noncopyable_deleter(new int[10]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); + posOff_v.storeChunk( + UniquePtrWithLambda(std::move(ptr_v_noncopyable_deleter)), + {0}, + {global_extent}); + iteration_ptr->close(); // force re-flush of previous iterations series.flush(); @@ -940,7 +984,7 @@ inline void constant_scalar(std::string file_ending) s.iterations[1] .meshes["rho"][MeshRecordComponent::SCALAR] .getAttribute("shape") - .get >() == Extent{1, 2, 3}); + .get>() == Extent{1, 2, 3}); REQUIRE(s.iterations[1] .meshes["rho"][MeshRecordComponent::SCALAR] .containsAttribute("value")); @@ -962,7 +1006,7 @@ inline void constant_scalar(std::string file_ending) s.iterations[1] .meshes["E"]["x"] .getAttribute("shape") - .get >() == Extent{1, 2, 3}); + .get>() == Extent{1, 2, 3}); REQUIRE(s.iterations[1].meshes["E"]["x"].containsAttribute("value")); REQUIRE( s.iterations[1] @@ -990,7 +1034,7 @@ inline void constant_scalar(std::string file_ending) s.iterations[1] .particles["e"]["position"][RecordComponent::SCALAR] .getAttribute("shape") - .get >() == Extent{3, 2, 1}); + .get>() == Extent{3, 2, 1}); REQUIRE(s.iterations[1] .particles["e"]["position"][RecordComponent::SCALAR] .containsAttribute("value")); @@ -1014,7 +1058,7 @@ inline void constant_scalar(std::string file_ending) s.iterations[1] .particles["e"]["positionOffset"][RecordComponent::SCALAR] .getAttribute("shape") - .get >() == Extent{3, 2, 1}); + .get>() == Extent{3, 2, 1}); REQUIRE(s.iterations[1] .particles["e"]["positionOffset"][RecordComponent::SCALAR] .containsAttribute("value")); @@ -1036,7 +1080,7 @@ inline void constant_scalar(std::string file_ending) s.iterations[1] .particles["e"]["velocity"]["x"] .getAttribute("shape") - .get >() == Extent{3, 2, 1}); + .get>() == Extent{3, 2, 1}); REQUIRE( s.iterations[1].particles["e"]["velocity"]["x"].containsAttribute( "value")); @@ -1388,55 +1432,55 @@ inline void dtype_test(const std::string &backend) REQUIRE(s.getAttribute("emptyString").get().empty()); } REQUIRE( - s.getAttribute("vecChar").get >() == + s.getAttribute("vecChar").get>() == std::vector({'c', 'h', 'a', 'r'})); REQUIRE( - s.getAttribute("vecInt16").get >() == + s.getAttribute("vecInt16").get>() == std::vector({32766, 32767})); REQUIRE( - s.getAttribute("vecInt32").get >() == + s.getAttribute("vecInt32").get>() == std::vector({2147483646, 2147483647})); REQUIRE( - s.getAttribute("vecInt64").get >() == + s.getAttribute("vecInt64").get>() == std::vector({9223372036854775806, 9223372036854775807})); REQUIRE( - s.getAttribute("vecUchar").get >() == + s.getAttribute("vecUchar").get>() == std::vector({'u', 'c', 'h', 'a', 'r'})); REQUIRE( - s.getAttribute("vecSchar").get >() == + s.getAttribute("vecSchar").get>() == std::vector({'s', 'c', 'h', 'a', 'r'})); REQUIRE( - s.getAttribute("vecUint16").get >() == + s.getAttribute("vecUint16").get>() == std::vector({65534u, 65535u})); REQUIRE( - s.getAttribute("vecUint32").get >() == + s.getAttribute("vecUint32").get>() == std::vector({4294967294u, 4294967295u})); REQUIRE( - s.getAttribute("vecUint64").get >() == + s.getAttribute("vecUint64").get>() == std::vector({18446744073709551614u, 18446744073709551615u})); REQUIRE( - s.getAttribute("vecFloat").get >() == + s.getAttribute("vecFloat").get>() == std::vector({0.f, 3.40282e+38f})); REQUIRE( - s.getAttribute("vecDouble").get >() == + s.getAttribute("vecDouble").get>() == std::vector({0., 1.79769e+308})); if (test_long_double) { REQUIRE( - s.getAttribute("vecLongdouble").get >() == + s.getAttribute("vecLongdouble").get>() == std::vector( {0.L, std::numeric_limits::max()})); } REQUIRE( - s.getAttribute("vecString").get >() == + s.getAttribute("vecString").get>() == std::vector({"vector", "of", "strings"})); if (!adios1) { REQUIRE( - s.getAttribute("vecEmptyString").get >() == + s.getAttribute("vecEmptyString").get>() == std::vector({"", "", ""})); REQUIRE( - s.getAttribute("vecMixedString").get >() == + s.getAttribute("vecMixedString").get>() == std::vector({"hi", "", "ho"})); } REQUIRE(s.getAttribute("bool").get() == true); @@ -1648,7 +1692,7 @@ void test_complex(const std::string &backend) "longDoublesYouSay", std::complex(5.5, -4.55)); auto Cflt = o.iterations[0].meshes["Cflt"][RecordComponent::SCALAR]; - std::vector > cfloats(3); + std::vector> cfloats(3); cfloats.at(0) = {1., 2.}; cfloats.at(1) = {-3., 4.}; cfloats.at(2) = {5., -6.}; @@ -1656,14 +1700,14 @@ void test_complex(const std::string &backend) Cflt.storeChunk(cfloats, {0}); auto Cdbl = o.iterations[0].meshes["Cdbl"][RecordComponent::SCALAR]; - std::vector > cdoubles(3); + std::vector> cdoubles(3); cdoubles.at(0) = {2., 1.}; cdoubles.at(1) = {-4., 3.}; cdoubles.at(2) = {6., -5.}; Cdbl.resetDataset(Dataset(Datatype::CDOUBLE, {cdoubles.size()})); Cdbl.storeChunk(cdoubles, {0}); - std::vector > cldoubles(3); + std::vector> cldoubles(3); if (o.backend() != "ADIOS2" && o.backend() != "ADIOS1" && o.backend() != "MPI_ADIOS1") { @@ -1684,26 +1728,26 @@ void test_complex(const std::string &backend) Series i = Series( "../samples/serial_write_complex." + backend, Access::READ_ONLY); REQUIRE( - i.getAttribute("lifeIsComplex").get >() == + i.getAttribute("lifeIsComplex").get>() == std::complex(4.56, 7.89)); REQUIRE( - i.getAttribute("butComplexFloats").get >() == + i.getAttribute("butComplexFloats").get>() == std::complex(42.3, -99.3)); if (i.backend() != "ADIOS2" && i.backend() != "ADIOS1" && i.backend() != "MPI_ADIOS1") { REQUIRE( i.getAttribute("longDoublesYouSay") - .get >() == + .get>() == std::complex(5.5, -4.55)); } auto rcflt = i.iterations[0] .meshes["Cflt"][RecordComponent::SCALAR] - .loadChunk >(); + .loadChunk>(); auto rcdbl = i.iterations[0] .meshes["Cdbl"][RecordComponent::SCALAR] - .loadChunk >(); + .loadChunk>(); i.flush(); REQUIRE(rcflt.get()[1] == std::complex(-3., 4.)); @@ -1714,7 +1758,7 @@ void test_complex(const std::string &backend) { auto rcldbl = i.iterations[0] .meshes["Cldbl"][RecordComponent::SCALAR] - .loadChunk >(); + .loadChunk>(); i.flush(); REQUIRE(rcldbl.get()[2] == std::complex(7., -6.)); } @@ -4957,7 +5001,7 @@ void bp4_steps( auto E_x = E["x"]; REQUIRE( E.getAttribute("vector_of_string") - .get >() == + .get>() == std::vector{"vector", "of", "string"}); REQUIRE(E_x.getDimensionality() == 1); REQUIRE(E_x.getExtent()[0] == 10); @@ -5191,7 +5235,7 @@ struct AreEqual }; template -struct AreEqual > +struct AreEqual> { static bool areEqual(std::vector v1, std::vector v2) { From ac316677f0463ff705a72825100b856ab4423836 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Mon, 6 Nov 2023 18:56:49 -0800 Subject: [PATCH 02/11] Doc: Python 3.12 (#1549) Document that we support Python 3.12. --- Dockerfile | 21 ++++++++++++++++--- README.md | 2 +- docs/source/dev/dependencies.rst | 2 +- setup.py | 1 + .../pybind11/tools/pybind11Tools.cmake | 2 +- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5f08b571c0..a6d1fcbfbc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,8 +5,8 @@ FROM quay.io/pypa/manylinux2010_x86_64 as build-env # FROM quay.io/pypa/manylinux1_x86_64 as build-env ENV DEBIAN_FRONTEND noninteractive -# Python 3.8-3.11 via "38 39 311" -ARG PY_VERSIONS="38 39 310 311" +# Python 3.8-3.12 via "38 39 311 312" +ARG PY_VERSIONS="38 39 310 311 312" # static libs need relocatable symbols for linking to shared python lib ENV CFLAGS="-fPIC ${CFLAGS}" @@ -162,7 +162,7 @@ FROM debian:bullseye ENV DEBIAN_FRONTEND noninteractive COPY --from=build-env /wheelhouse/openPMD_api-*-cp311-cp311-manylinux2010_x86_64.whl . RUN apt-get update \ - && apt-get install -y --no-install-recommends python3.10 python3-distutils ca-certificates curl \ + && apt-get install -y --no-install-recommends python3.11 python3-distutils ca-certificates curl \ && rm -rf /var/lib/apt/lists/* RUN python3.11 --version \ && curl https://bootstrap.pypa.io/get-pip.py -o get-pip.py \ @@ -172,6 +172,21 @@ RUN python3.11 -c "import openpmd_api as io; print(io.__version__); print RUN python3.11 -m openpmd_api.ls --help RUN openpmd-ls --help +# test in fresh env: Debian:Bullseye + Python 3.12 +FROM debian:bullseye +ENV DEBIAN_FRONTEND noninteractive +COPY --from=build-env /wheelhouse/openPMD_api-*-cp312-cp312-manylinux2010_x86_64.whl . +RUN apt-get update \ + && apt-get install -y --no-install-recommends python3.12 python3-distutils ca-certificates curl \ + && rm -rf /var/lib/apt/lists/* +RUN python3.12 --version \ + && curl https://bootstrap.pypa.io/get-pip.py -o get-pip.py \ + && python3.12 get-pip.py \ + && python3.12 -m pip install openPMD_api-*-cp312-cp312-manylinux2010_x86_64.whl +RUN python3.12 -c "import openpmd_api as io; print(io.__version__); print(io.variants)" +RUN python3.12 -m openpmd_api.ls --help +RUN openpmd-ls --help + # copy binary artifacts (wheels) FROM quay.io/pypa/manylinux2010_x86_64 MAINTAINER Axel Huebl diff --git a/README.md b/README.md index ace6eb47a4..5576107d83 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ while those can be built either with or without: Optional language bindings: * Python: - * Python 3.8 - 3.11 + * Python 3.8 - 3.12 * pybind11 2.11.1+ * numpy 1.15+ * mpi4py 2.1+ (optional, for MPI) diff --git a/docs/source/dev/dependencies.rst b/docs/source/dev/dependencies.rst index cc66b85587..00629fc58a 100644 --- a/docs/source/dev/dependencies.rst +++ b/docs/source/dev/dependencies.rst @@ -39,7 +39,7 @@ Optional: language bindings * Python: - * Python 3.8 - 3.11 + * Python 3.8 - 3.12 * pybind11 2.11.1+ * numpy 1.15+ * mpi4py 2.1+ (optional, for MPI) diff --git a/setup.py b/setup.py index 31a7fdc97f..2053f1bd22 100644 --- a/setup.py +++ b/setup.py @@ -226,6 +226,7 @@ def build_extension(self, ext): 'Programming Language :: Python :: 3.9', 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3.11', + 'Programming Language :: Python :: 3.12', ('License :: OSI Approved :: ' 'GNU Lesser General Public License v3 or later (LGPLv3+)'), ], diff --git a/share/openPMD/thirdParty/pybind11/tools/pybind11Tools.cmake b/share/openPMD/thirdParty/pybind11/tools/pybind11Tools.cmake index 66ad00a478..48050966a4 100644 --- a/share/openPMD/thirdParty/pybind11/tools/pybind11Tools.cmake +++ b/share/openPMD/thirdParty/pybind11/tools/pybind11Tools.cmake @@ -43,7 +43,7 @@ endif() # A user can set versions manually too set(Python_ADDITIONAL_VERSIONS - "3.11;3.10;3.9;3.8;3.7;3.6" + "3.12;3.11;3.10;3.9;3.8" CACHE INTERNAL "") list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}") From 9ad33b07a9102abb2cd38ca1c7b524693c16de4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 7 Nov 2023 04:07:21 +0100 Subject: [PATCH 03/11] Fix Attribute copy/move constructors (#1545) * Add failing test that Attribute constructors work properly * Ensure that constructor doesnt override default constructors * Use macro instead --- include/openPMD/DatatypeMacros.hpp | 113 ++++++++++++++++++++++++ include/openPMD/UndefDatatypeMacros.hpp | 24 +++++ include/openPMD/backend/Attribute.hpp | 18 ++-- test/CoreTest.cpp | 8 ++ 4 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 include/openPMD/DatatypeMacros.hpp create mode 100644 include/openPMD/UndefDatatypeMacros.hpp diff --git a/include/openPMD/DatatypeMacros.hpp b/include/openPMD/DatatypeMacros.hpp new file mode 100644 index 0000000000..aa78879aab --- /dev/null +++ b/include/openPMD/DatatypeMacros.hpp @@ -0,0 +1,113 @@ +/* Copyright 2023 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 . + */ +#pragma once + +#include +#include +#include + +// Need to alias this to avoid the comma +// Cannot use a namespace, otherwise the macro will work either only within +// or outside the namespace +// Defining macros means polluting the global namespace anyway +using openpmd_array_double_7 = std::array; + +#define OPENPMD_FOREACH_DATATYPE(MACRO) \ + MACRO(char) \ + MACRO(unsigned char) \ + MACRO(signed char) \ + MACRO(short) \ + MACRO(int) \ + MACRO(long) \ + MACRO(long long) \ + MACRO(unsigned short) \ + MACRO(unsigned int) \ + MACRO(unsigned long) \ + MACRO(unsigned long long) \ + MACRO(float) \ + MACRO(double) \ + MACRO(long double) \ + MACRO(std::complex) \ + MACRO(std::complex) \ + MACRO(std::complex) \ + MACRO(std::string) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(std::vector>) \ + MACRO(std::vector>) \ + MACRO(std::vector>) \ + MACRO(std::vector) \ + MACRO(std::vector) \ + MACRO(openpmd_array_double_7) \ + MACRO(bool) + +#define OPENPMD_FOREACH_NONVECTOR_DATATYPE(MACRO) \ + MACRO(char) \ + MACRO(unsigned char) \ + MACRO(signed char) \ + MACRO(short) \ + MACRO(int) \ + MACRO(long) \ + MACRO(long long) \ + MACRO(unsigned short) \ + MACRO(unsigned int) \ + MACRO(unsigned long) \ + MACRO(unsigned long long) \ + MACRO(float) \ + MACRO(double) \ + MACRO(long double) \ + MACRO(std::complex) \ + MACRO(std::complex) \ + MACRO(std::complex) \ + MACRO(std::string) \ + MACRO(array_double_7) \ + MACRO(bool) + +#define OPENPMD_FOREACH_DATASET_DATATYPE(MACRO) \ + MACRO(char) \ + MACRO(unsigned char) \ + MACRO(signed char) \ + MACRO(short) \ + MACRO(int) \ + MACRO(long) \ + MACRO(long long) \ + MACRO(unsigned short) \ + MACRO(unsigned int) \ + MACRO(unsigned long) \ + MACRO(unsigned long long) \ + MACRO(float) \ + MACRO(double) \ + MACRO(long double) \ + MACRO(std::complex) \ + MACRO(std::complex) \ + MACRO(std::complex) \ + MACRO(std::array) diff --git a/include/openPMD/UndefDatatypeMacros.hpp b/include/openPMD/UndefDatatypeMacros.hpp new file mode 100644 index 0000000000..f55303ca11 --- /dev/null +++ b/include/openPMD/UndefDatatypeMacros.hpp @@ -0,0 +1,24 @@ +/* Copyright 2023 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 . + */ + +#undef OPENPMD_FOREACH_DATATYPE +#undef OPENPMD_FOREACH_NONVECTOR_DATATYPE +#undef OPENPMD_FOREACH_DATASET_DATATYPE diff --git a/include/openPMD/backend/Attribute.hpp b/include/openPMD/backend/Attribute.hpp index b2cee727ae..c34d7c1847 100644 --- a/include/openPMD/backend/Attribute.hpp +++ b/include/openPMD/backend/Attribute.hpp @@ -21,6 +21,7 @@ #pragma once #include "openPMD/Datatype.hpp" +#include "openPMD/DatatypeMacros.hpp" #include "openPMD/auxiliary/TypeTraits.hpp" #include "openPMD/auxiliary/Variant.hpp" @@ -85,9 +86,9 @@ class Attribute std::vector, std::vector, std::vector, - std::vector >, - std::vector >, - std::vector >, + std::vector>, + std::vector>, + std::vector>, std::vector, std::vector, std::array, @@ -106,10 +107,15 @@ class Attribute * * Fix by explicitly instantiating resource */ - template - Attribute(T &&val) : Variant(resource(std::forward(val))) + +#define OPENPMD_ATTRIBUTE_CONSTRUCTOR_FROM_VARIANT(TYPE) \ + Attribute(TYPE val) : Variant(resource(std::move(val))) \ {} + OPENPMD_FOREACH_DATATYPE(OPENPMD_ATTRIBUTE_CONSTRUCTOR_FROM_VARIANT) + +#undef OPENPMD_ATTRIBUTE_CONSTRUCTOR_FROM_VARIANT + /** Retrieve a stored specific Attribute and cast if convertible. * * @note This performs a static_cast and might introduce precision loss if @@ -297,3 +303,5 @@ std::optional Attribute::getOptional() const std::move(eitherValueOrError)); } } // namespace openPMD + +#include "openPMD/UndefDatatypeMacros.hpp" diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index d660e29ec4..f6f93e065c 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -47,6 +47,14 @@ TEST_CASE("attribute_dtype_test", "[core]") { Attribute a = Attribute(static_cast(' ')); REQUIRE(Datatype::CHAR == a.dtype); + { + // check that copy constructor works + [[maybe_unused]] Attribute b = a; + } + { + // check that move constructor works + [[maybe_unused]] Attribute b = std::move(a); + } a = Attribute(static_cast(' ')); REQUIRE(Datatype::UCHAR == a.dtype); a = Attribute(static_cast(0)); From 991d0d9e726cc1356610652abbdc5205e44645b2 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 16 Nov 2023 07:52:13 -0800 Subject: [PATCH 04/11] Doc: HELPMI Support (#1555) Add support by HELMPI (Helmholtz Association) for Franz. --- README.md | 1 + docs/source/index.rst | 1 + 2 files changed, 2 insertions(+) diff --git a/README.md b/README.md index 5576107d83..19bb8a5be9 100644 --- a/README.md +++ b/README.md @@ -428,6 +428,7 @@ Previously supported by the Consortium for Advanced Modeling of Particles Accele Supported by the Exascale Computing Project (17-SC-20-SC), a collaborative effort of two U.S. Department of Energy organizations (Office of Science and the National Nuclear Security Administration). This project has received funding from the European Unions Horizon 2020 research and innovation programme under grant agreement No 654220. This work was partially funded by the Center of Advanced Systems Understanding (CASUS), which is financed by Germany's Federal Ministry of Education and Research (BMBF) and by the Saxon Ministry for Science, Culture and Tourism (SMWK) with tax funds on the basis of the budget approved by the Saxon State Parliament. +Supported by the HElmholtz Laser Plasma Metadata Initiative (HELPMI) project (ZT-I-PF-3-066), funded by the "Initiative and Networking Fund" of the Helmholtz Association in the framework of the "Helmholtz Metadata Collaboration" project call 2022. ### Transitive Contributions diff --git a/docs/source/index.rst b/docs/source/index.rst index 7b7f050069..4f79cd9d78 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -58,6 +58,7 @@ Previously supported by the Consortium for Advanced Modeling of Particles Accele Supported by the Exascale Computing Project (17-SC-20-SC), a collaborative effort of two U.S. Department of Energy organizations (Office of Science and the National Nuclear Security Administration). This project has received funding from the European Unions Horizon 2020 research and innovation programme under grant agreement No 654220. This work was partially funded by the Center of Advanced Systems Understanding (CASUS), which is financed by Germany's Federal Ministry of Education and Research (BMBF) and by the Saxon Ministry for Science, Culture and Tourism (SMWK) with tax funds on the basis of the budget approved by the Saxon State Parliament. +Supported by the HElmholtz Laser Plasma Metadata Initiative (HELPMI) project (ZT-I-PF-3-066), funded by the "Initiative and Networking Fund" of the Helmholtz Association in the framework of the "Helmholtz Metadata Collaboration" project call 2022. .. toctree:: From 71aa0e98d7a957a5b050577ad9c563e72e8a77f8 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 20 Nov 2023 16:23:52 +0100 Subject: [PATCH 05/11] Add all the performance-* clang-tidy checks (#1532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add all the performance-* clang-tidy checks * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * address residual issues found with clang-tidy * enforce east const * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix bug * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * cleaning * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * remove unused variable * Fix some corrections that were too ambitious * Some little forgotten clang-tidy checks * Fixes after merging the dev * Replace auxiliary::forget() with NOLINT comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Franz Pöschel --- .clang-tidy | 2 +- examples/10_streaming_read.cpp | 4 +- examples/8_benchmark_parallel.cpp | 5 +- include/openPMD/Datatype.hpp | 2 +- include/openPMD/Error.hpp | 5 +- include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 11 ++-- include/openPMD/IO/AbstractIOHandlerImpl.hpp | 4 +- include/openPMD/IO/HDF5/HDF5Auxiliary.hpp | 2 +- .../openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp | 3 +- include/openPMD/IO/JSON/JSONIOHandler.hpp | 2 +- include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp | 18 +++---- include/openPMD/Iteration.hpp | 4 +- include/openPMD/ReadIterations.hpp | 3 +- include/openPMD/Series.hpp | 14 ++++-- include/openPMD/ThrowError.hpp | 4 +- include/openPMD/backend/Attributable.hpp | 2 +- include/openPMD/backend/Writable.hpp | 2 +- include/openPMD/cli/ls.hpp | 4 +- src/Dataset.cpp | 10 ++-- src/Datatype.cpp | 2 +- src/Error.cpp | 11 ++-- src/IO/ADIOS/ADIOS2IOHandler.cpp | 50 +++++++++++-------- src/IO/AbstractIOHandlerHelper.cpp | 25 +++++----- src/IO/AbstractIOHandlerImpl.cpp | 2 +- src/IO/HDF5/HDF5Auxiliary.cpp | 2 +- src/IO/HDF5/HDF5IOHandler.cpp | 6 ++- src/IO/HDF5/ParallelHDF5IOHandler.cpp | 14 ++++-- src/IO/InvalidatableFile.cpp | 6 +-- src/IO/JSON/JSONIOHandler.cpp | 2 +- src/IO/JSON/JSONIOHandlerImpl.cpp | 30 +++++------ src/Iteration.cpp | 2 +- src/ReadIterations.cpp | 7 +-- src/Series.cpp | 20 ++++---- src/WriteIterations.cpp | 4 +- src/auxiliary/Filesystem.cpp | 3 +- src/auxiliary/JSON.cpp | 4 +- src/backend/Attributable.cpp | 4 +- src/backend/Writable.cpp | 2 +- src/binding/python/Attributable.cpp | 31 ++++++------ src/binding/python/ChunkInfo.cpp | 2 +- src/binding/python/Dataset.cpp | 8 +-- src/binding/python/Datatype.cpp | 2 +- src/binding/python/PatchRecordComponent.cpp | 2 +- src/binding/python/RecordComponent.cpp | 5 +- src/cli/ls.cpp | 1 + src/version.cpp | 9 ++-- test/AuxiliaryTest.cpp | 7 +-- test/CoreTest.cpp | 1 + test/ParallelIOTest.cpp | 21 ++++---- test/SerialIOTest.cpp | 49 ++++++++++-------- 50 files changed, 240 insertions(+), 195 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 7998697f29..034a863660 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -2,5 +2,5 @@ # FIXME: all performance-* reports # FIXME: all cert-* reports # FIXME: all bugprone-* reports -Checks: -*,bugprone-*,-bugprone-unhandled-self-assignment,-bugprone-parent-virtual-call,-bugprone-narrowing-conversions,-bugprone-exception-escape,-bugprone-string-literal-with-embedded-nul,cppcoreguidelines-slicing,mpi-*,readability-non-const-parameter,performance-for-range-copy,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list +Checks: -*,bugprone-*,-bugprone-unhandled-self-assignment,-bugprone-parent-virtual-call,-bugprone-narrowing-conversions,-bugprone-exception-escape,-bugprone-string-literal-with-embedded-nul,cppcoreguidelines-slicing,mpi-*,readability-non-const-parameter,performance-*,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list HeaderFilterRegex: '((^(?!\/share\/openPMD\/).*)*include\/openPMD\/.+\.hpp|src\/^(?!binding).+\.cpp$)' diff --git a/examples/10_streaming_read.cpp b/examples/10_streaming_read.cpp index 6e6aba1fd9..99da2c5a5c 100644 --- a/examples/10_streaming_read.cpp +++ b/examples/10_streaming_read.cpp @@ -46,7 +46,7 @@ int main() for (size_t i = 0; i < 3; ++i) { - std::string dim = dimensions[i]; + std::string const &dim = dimensions[i]; RecordComponent rc = electronPositions[dim]; loadedChunks[i] = rc.loadChunk( Offset(rc.getDimensionality(), 0), rc.getExtent()); @@ -60,7 +60,7 @@ int main() for (size_t i = 0; i < 3; ++i) { - std::string dim = dimensions[i]; + std::string const &dim = dimensions[i]; Extent const &extent = extents[i]; std::cout << "\ndim: " << dim << "\n" << std::endl; auto chunk = loadedChunks[i]; diff --git a/examples/8_benchmark_parallel.cpp b/examples/8_benchmark_parallel.cpp index 5adf2512ff..2dc63632ff 100644 --- a/examples/8_benchmark_parallel.cpp +++ b/examples/8_benchmark_parallel.cpp @@ -12,7 +12,7 @@ #include #if openPMD_HAVE_MPI -inline void print_help(std::string const program_name) +inline void print_help(std::string const &program_name) { std::cout << "Usage: " << program_name << "\n"; std::cout << "Run a simple parallel write and read benchmark.\n\n"; @@ -27,7 +27,7 @@ inline void print_help(std::string const program_name) std::cout << " " << program_name << " # for a strong scaling\n"; } -inline void print_version(std::string const program_name) +inline void print_version(std::string const &program_name) { std::cout << program_name << " (openPMD-api) " << openPMD::getVersion() << "\n"; @@ -46,6 +46,7 @@ int main(int argc, char *argv[]) // CLI parsing std::vector str_argv; + str_argv.reserve(argc); for (int i = 0; i < argc; ++i) str_argv.emplace_back(argv[i]); bool weak_scaling = false; diff --git a/include/openPMD/Datatype.hpp b/include/openPMD/Datatype.hpp index 05d0ddefbb..c83c865e59 100644 --- a/include/openPMD/Datatype.hpp +++ b/include/openPMD/Datatype.hpp @@ -739,7 +739,7 @@ Datatype toVectorType(Datatype dt); std::string datatypeToString(Datatype dt); -Datatype stringToDatatype(std::string s); +Datatype stringToDatatype(const std::string &s); void warnWrongDtype(std::string const &key, Datatype store, Datatype request); diff --git a/include/openPMD/Error.hpp b/include/openPMD/Error.hpp index c50e2918b6..3e516e16ec 100644 --- a/include/openPMD/Error.hpp +++ b/include/openPMD/Error.hpp @@ -50,7 +50,8 @@ namespace error { public: std::string backend; - OperationUnsupportedInBackend(std::string backend_in, std::string what); + OperationUnsupportedInBackend( + std::string backend_in, std::string const &what); }; /** @@ -62,7 +63,7 @@ namespace error class WrongAPIUsage : public Error { public: - WrongAPIUsage(std::string what); + WrongAPIUsage(std::string const &what); }; class BackendConfigSchema : public Error diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index 86afffbbdf..e63c6a493b 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp @@ -391,9 +391,10 @@ class ADIOS2IOHandlerImpl ThrowError }; - detail::BufferedActions &getFileData(InvalidatableFile file, IfFileNotOpen); + detail::BufferedActions & + getFileData(InvalidatableFile const &file, IfFileNotOpen); - void dropFileData(InvalidatableFile file); + void dropFileData(InvalidatableFile const &file); /* * Prepare a variable that already exists for an IO @@ -465,7 +466,7 @@ namespace detail ADIOS2IOHandlerImpl &, adios2::IO &IO, std::string name, - std::shared_ptr resource); + Attribute::resource &resource); template static Datatype call(Params &&...); @@ -488,8 +489,8 @@ namespace detail template static void call( ADIOS2IOHandlerImpl *impl, - InvalidatableFile, - const std::string &varName, + InvalidatableFile const &, + std::string const &varName, Parameter ¶meters); static constexpr char const *errorMsg = "ADIOS2: openDataset()"; diff --git a/include/openPMD/IO/AbstractIOHandlerImpl.hpp b/include/openPMD/IO/AbstractIOHandlerImpl.hpp index 8d13f3feb8..dafbc24111 100644 --- a/include/openPMD/IO/AbstractIOHandlerImpl.hpp +++ b/include/openPMD/IO/AbstractIOHandlerImpl.hpp @@ -390,8 +390,8 @@ class AbstractIOHandlerImpl * Using the default implementation (which copies the abstractFilePath * into the current writable) should be enough for all backends. */ - void - keepSynchronous(Writable *, Parameter param); + void keepSynchronous( + Writable *, Parameter const ¶m); /** Notify the backend that the Writable has been / will be deallocated. * diff --git a/include/openPMD/IO/HDF5/HDF5Auxiliary.hpp b/include/openPMD/IO/HDF5/HDF5Auxiliary.hpp index da7ff2f68f..dc040f7053 100644 --- a/include/openPMD/IO/HDF5/HDF5Auxiliary.hpp +++ b/include/openPMD/IO/HDF5/HDF5Auxiliary.hpp @@ -62,5 +62,5 @@ std::string concrete_h5_file_position(Writable *w); * @return array for resulting chunk dimensions */ std::vector -getOptimalChunkDims(std::vector const dims, size_t const typeSize); +getOptimalChunkDims(std::vector const &dims, size_t const typeSize); } // namespace openPMD diff --git a/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp b/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp index 18d43c93ab..cd951be5d2 100644 --- a/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp +++ b/include/openPMD/IO/HDF5/ParallelHDF5IOHandler.hpp @@ -39,7 +39,8 @@ class ParallelHDF5IOHandler : public AbstractIOHandler ParallelHDF5IOHandler( std::string path, Access, MPI_Comm, json::TracingJSON config); #else - ParallelHDF5IOHandler(std::string path, Access, json::TracingJSON config); + ParallelHDF5IOHandler( + std::string const &path, Access, json::TracingJSON config); #endif ~ParallelHDF5IOHandler() override; diff --git a/include/openPMD/IO/JSON/JSONIOHandler.hpp b/include/openPMD/IO/JSON/JSONIOHandler.hpp index 452098137e..7fdea5b6f0 100644 --- a/include/openPMD/IO/JSON/JSONIOHandler.hpp +++ b/include/openPMD/IO/JSON/JSONIOHandler.hpp @@ -30,7 +30,7 @@ class JSONIOHandler : public AbstractIOHandler { public: JSONIOHandler( - std::string path, + std::string const &path, Access at, openPMD::json::TracingJSON config, JSONIOHandlerImpl::FileFormat, diff --git a/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp b/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp index c935647665..5ce9d057c3 100644 --- a/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp +++ b/include/openPMD/IO/JSON/JSONIOHandlerImpl.hpp @@ -84,7 +84,7 @@ struct File return fileState->valid; } - File &operator=(std::string s) + File &operator=(std::string const &s) { if (fileState) { @@ -259,10 +259,10 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl // else null. first tuple element needs to be a pointer, since the casted // streams are references only. std::tuple, std::istream *, std::ostream *> - getFilehandle(File, Access access); + getFilehandle(File const &, Access access); // full operating system path of the given file - std::string fullPath(File); + std::string fullPath(File const &); std::string fullPath(std::string const &); @@ -304,7 +304,7 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl // make sure that the given path exists in proper form in // the passed json value - static void ensurePath(nlohmann::json *json, std::string path); + static void ensurePath(nlohmann::json *json, std::string const &path); // In order not to insert the same file name into the data structures // with a new pointer (e.g. when reopening), search for a possibly @@ -312,24 +312,24 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl // The bool is true iff the pointer has been newly-created. // The iterator is an iterator for m_files std::tuple::iterator, bool> - getPossiblyExisting(std::string file); + getPossiblyExisting(std::string const &file); // get the json value representing the whole file, possibly reading // from disk - std::shared_ptr obtainJsonContents(File); + std::shared_ptr obtainJsonContents(File const &); // get the json value at the writable's fileposition nlohmann::json &obtainJsonContents(Writable *writable); // write to disk the json contents associated with the file // remove from m_dirty if unsetDirty == true - void putJsonContents(File, bool unsetDirty = true); + void putJsonContents(File const &, bool unsetDirty = true); // figure out the file position of the writable // (preferring the parent's file position) and extend it // by extend. return the modified file position. std::shared_ptr - setAndGetFilePosition(Writable *, std::string extend); + setAndGetFilePosition(Writable *, std::string const &extend); // figure out the file position of the writable // (preferring the parent's file position) @@ -345,7 +345,7 @@ class JSONIOHandlerImpl : public AbstractIOHandlerImpl void associateWithFile(Writable *writable, File); // need to check the name too in order to exclude "attributes" key - static bool isGroup(nlohmann::json::const_iterator it); + static bool isGroup(nlohmann::json::const_iterator const &it); static bool isDataset(nlohmann::json const &j); diff --git a/include/openPMD/Iteration.hpp b/include/openPMD/Iteration.hpp index fd1f14cd00..49e8c3296c 100644 --- a/include/openPMD/Iteration.hpp +++ b/include/openPMD/Iteration.hpp @@ -284,7 +284,9 @@ class Iteration : public Attributable */ void reread(std::string const &path); void readFileBased( - std::string filePath, std::string const &groupPath, bool beginStep); + std::string const &filePath, + std::string const &groupPath, + bool beginStep); void readGorVBased(std::string const &groupPath, bool beginStep); void read_impl(std::string const &groupPath); void readMeshes(std::string const &meshesPath); diff --git a/include/openPMD/ReadIterations.hpp b/include/openPMD/ReadIterations.hpp index a7cf0d18b5..35f2f740ce 100644 --- a/include/openPMD/ReadIterations.hpp +++ b/include/openPMD/ReadIterations.hpp @@ -77,7 +77,8 @@ class SeriesIterator explicit SeriesIterator(); SeriesIterator( - Series, std::optional parsePreference); + Series const &, + std::optional const &parsePreference); SeriesIterator &operator++(); diff --git a/include/openPMD/Series.hpp b/include/openPMD/Series.hpp index 1d99b54a84..094ed24fbf 100644 --- a/include/openPMD/Series.hpp +++ b/include/openPMD/Series.hpp @@ -241,7 +241,13 @@ class Series : public Attributable Access at, std::string const &options = "{}"); - virtual ~Series() = default; + Series(Series const &) = default; + Series(Series &&) = default; + + Series &operator=(Series const &) = default; + Series &operator=(Series &&) = default; + + ~Series() override = default; /** * An unsigned integer type, used to identify Iterations in a Series. @@ -637,12 +643,12 @@ OPENPMD_private std::future flush_impl( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler = true); void flushFileBased( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler = true); /* * Group-based and variable-based iteration layouts share a lot of logic @@ -654,7 +660,7 @@ OPENPMD_private void flushGorVBased( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler = true); void flushMeshesPath(); void flushParticlesPath(); diff --git a/include/openPMD/ThrowError.hpp b/include/openPMD/ThrowError.hpp index f2695f7ae0..3888c40b12 100644 --- a/include/openPMD/ThrowError.hpp +++ b/include/openPMD/ThrowError.hpp @@ -50,8 +50,8 @@ enum class Reason [[noreturn]] OPENPMDAPI_EXPORT void throwBackendConfigSchema(std::vector jsonPath, std::string what); -[[noreturn]] OPENPMDAPI_EXPORT void -throwOperationUnsupportedInBackend(std::string backend, std::string what); +[[noreturn]] OPENPMDAPI_EXPORT void throwOperationUnsupportedInBackend( + std::string backend, std::string const &what); [[noreturn]] OPENPMDAPI_EXPORT void throwReadError( AffectedObject affectedObject, diff --git a/include/openPMD/backend/Attributable.hpp b/include/openPMD/backend/Attributable.hpp index db7aec37de..2b45ac0e45 100644 --- a/include/openPMD/backend/Attributable.hpp +++ b/include/openPMD/backend/Attributable.hpp @@ -253,7 +253,7 @@ OPENPMD_protected Iteration &containingIteration(); /** @} */ - void seriesFlush(internal::FlushParams); + void seriesFlush(internal::FlushParams const &); void flushAttributes(internal::FlushParams const &); diff --git a/include/openPMD/backend/Writable.hpp b/include/openPMD/backend/Writable.hpp index 7aeead3dfe..5e1272317e 100644 --- a/include/openPMD/backend/Writable.hpp +++ b/include/openPMD/backend/Writable.hpp @@ -119,7 +119,7 @@ class Writable final OPENPMD_private // clang-format on - void seriesFlush(internal::FlushParams); + void seriesFlush(internal::FlushParams const &); /* * These members need to be shared pointers since distinct instances of * Writable may share them. diff --git a/include/openPMD/cli/ls.hpp b/include/openPMD/cli/ls.hpp index 1d2313e250..b61e34e13d 100644 --- a/include/openPMD/cli/ls.hpp +++ b/include/openPMD/cli/ls.hpp @@ -34,7 +34,7 @@ namespace cli { namespace ls { - inline void print_help(std::string const program_name) + inline void print_help(std::string const &program_name) { std::cout << "Usage: " << program_name << " openPMD-series\n"; std::cout << "List information about an openPMD data series.\n\n"; @@ -54,7 +54,7 @@ namespace cli << " ./samples/serial_patch.bp\n"; } - inline void print_version(std::string const program_name) + inline void print_version(std::string const &program_name) { std::cout << program_name << " (openPMD-api) " << getVersion() << "\n"; diff --git a/src/Dataset.cpp b/src/Dataset.cpp index 587598db63..662bd2d29f 100644 --- a/src/Dataset.cpp +++ b/src/Dataset.cpp @@ -26,11 +26,11 @@ namespace openPMD { Dataset::Dataset(Datatype d, Extent e, std::string options_in) - : extent{e} - , dtype{d} - , rank{static_cast(e.size())} - , options{std::move(options_in)} -{} + : extent{std::move(e)}, dtype{d}, options{std::move(options_in)} +{ + // avoid initialization order issues + rank = static_cast(extent.size()); +} Dataset::Dataset(Extent e) : Dataset(Datatype::UNDEFINED, std::move(e)) {} diff --git a/src/Datatype.cpp b/src/Datatype.cpp index f0f26f7ae9..298bcc8790 100644 --- a/src/Datatype.cpp +++ b/src/Datatype.cpp @@ -163,7 +163,7 @@ std::ostream &operator<<(std::ostream &os, openPMD::Datatype const &d) return os; } -Datatype stringToDatatype(std::string s) +Datatype stringToDatatype(const std::string &s) { static std::unordered_map m{ {"CHAR", Datatype::CHAR}, diff --git a/src/Error.cpp b/src/Error.cpp index a8e83338ed..f2e27a0213 100644 --- a/src/Error.cpp +++ b/src/Error.cpp @@ -12,19 +12,18 @@ const char *Error::what() const noexcept namespace error { OperationUnsupportedInBackend::OperationUnsupportedInBackend( - std::string backend_in, std::string what) + std::string backend_in, std::string const &what) : Error("Operation unsupported in " + backend_in + ": " + what) , backend{std::move(backend_in)} {} - void - throwOperationUnsupportedInBackend(std::string backend, std::string what) + void throwOperationUnsupportedInBackend( + std::string backend, std::string const &what) { - throw OperationUnsupportedInBackend( - std::move(backend), std::move(what)); + throw OperationUnsupportedInBackend(std::move(backend), what); } - WrongAPIUsage::WrongAPIUsage(std::string what) + WrongAPIUsage::WrongAPIUsage(std::string const &what) : Error("Wrong API usage: " + what) {} diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index a9af92e4a0..8cddd578a0 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -1111,7 +1111,7 @@ void ADIOS2IOHandlerImpl::readAttribute( } Datatype ret = switchType( - type, *this, ba.m_IO, name, parameters.resource); + type, *this, ba.m_IO, name, *parameters.resource); *parameters.dtype = ret; } @@ -1241,7 +1241,7 @@ void ADIOS2IOHandlerImpl::listPaths( } for (auto &path : subdirs) { - parameters.paths->emplace_back(std::move(path)); + parameters.paths->emplace_back(path); } } @@ -1284,7 +1284,7 @@ void ADIOS2IOHandlerImpl::listDatasets( } for (auto &dataset : subdirs) { - parameters.datasets->emplace_back(std::move(dataset)); + parameters.datasets->emplace_back(dataset); } } @@ -1345,7 +1345,7 @@ void ADIOS2IOHandlerImpl::closePath( return; } auto position = setAndGetFilePosition(writable); - auto const positionString = filePositionToString(position); + auto positionString = filePositionToString(position); VERIFY( !auxiliary::ends_with(positionString, '/'), "[ADIOS2] Position string has unexpected format. This is a bug " @@ -1354,7 +1354,8 @@ void ADIOS2IOHandlerImpl::closePath( for (auto const &attr : fileData.availableAttributesPrefixed(positionString)) { - fileData.m_IO.RemoveAttribute(positionString + '/' + attr); + fileData.m_IO.RemoveAttribute( + std::string(positionString).append("/").append(attr)); } } @@ -1491,8 +1492,8 @@ std::string ADIOS2IOHandlerImpl::nameOfAttribute(Writable *writable, std::string attribute) { auto pos = setAndGetFilePosition(writable); - return filePositionToString( - extendFilePosition(pos, auxiliary::removeSlashes(attribute))); + return filePositionToString(extendFilePosition( + pos, auxiliary::removeSlashes(std::move(attribute)))); } GroupOrDataset ADIOS2IOHandlerImpl::groupOrDataset(Writable *writable) @@ -1500,8 +1501,8 @@ GroupOrDataset ADIOS2IOHandlerImpl::groupOrDataset(Writable *writable) return setAndGetFilePosition(writable)->gd; } -detail::BufferedActions & -ADIOS2IOHandlerImpl::getFileData(InvalidatableFile file, IfFileNotOpen flag) +detail::BufferedActions &ADIOS2IOHandlerImpl::getFileData( + InvalidatableFile const &file, IfFileNotOpen flag) { VERIFY_ALWAYS( file.valid(), @@ -1515,8 +1516,7 @@ ADIOS2IOHandlerImpl::getFileData(InvalidatableFile file, IfFileNotOpen flag) case IfFileNotOpen::OpenImplicitly: { auto res = m_fileData.emplace( - std::move(file), - std::make_unique(*this, file)); + file, std::make_unique(*this, file)); return *res.first->second; } case IfFileNotOpen::ThrowError: @@ -1531,7 +1531,7 @@ ADIOS2IOHandlerImpl::getFileData(InvalidatableFile file, IfFileNotOpen flag) } } -void ADIOS2IOHandlerImpl::dropFileData(InvalidatableFile file) +void ADIOS2IOHandlerImpl::dropFileData(InvalidatableFile const &file) { auto it = m_fileData.find(file); if (it != m_fileData.end()) @@ -1615,7 +1615,7 @@ namespace detail ADIOS2IOHandlerImpl &impl, adios2::IO &IO, std::string name, - std::shared_ptr resource) + Attribute::resource &resource) { (void)impl; /* @@ -1653,11 +1653,11 @@ namespace detail auto meta = IO.InquireAttribute(metaAttr); if (meta.Data().size() == 1 && meta.Data()[0] == 1) { - *resource = bool_repr::fromRep(attr.Data()[0]); + resource = bool_repr::fromRep(attr.Data()[0]); return determineDatatype(); } } - *resource = attr.Data()[0]; + resource = attr.Data()[0]; } else if constexpr (IsUnsupportedComplex_v) { @@ -1674,7 +1674,7 @@ namespace detail "[ADIOS2] Internal error: Failed reading attribute '" + name + "'."); } - *resource = attr.Data(); + resource = attr.Data(); } else if constexpr (auxiliary::IsArray_v) { @@ -1691,7 +1691,7 @@ namespace detail { res[i] = data[i]; } - *resource = res; + resource = res; } else if constexpr (std::is_same_v) { @@ -1707,7 +1707,7 @@ namespace detail "[ADIOS2] Internal error: Failed reading attribute '" + name + "'."); } - *resource = attr.Data()[0]; + resource = attr.Data()[0]; } return determineDatatype(); @@ -1892,7 +1892,7 @@ namespace detail template void DatasetOpener::call( ADIOS2IOHandlerImpl *impl, - InvalidatableFile file, + InvalidatableFile const &file, const std::string &varName, Parameter ¶meters) { @@ -3438,8 +3438,11 @@ ADIOS2IOHandler::ADIOS2IOHandler( std::string path, Access at, MPI_Comm comm, + // NOLINTNEXTLINE(performance-unnecessary-value-param) json::TracingJSON, + // NOLINTNEXTLINE(performance-unnecessary-value-param) std::string, + // NOLINTNEXTLINE(performance-unnecessary-value-param) std::string) : AbstractIOHandler(std::move(path), at, comm) {} @@ -3447,7 +3450,14 @@ ADIOS2IOHandler::ADIOS2IOHandler( #endif // openPMD_HAVE_MPI ADIOS2IOHandler::ADIOS2IOHandler( - std::string path, Access at, json::TracingJSON, std::string, std::string) + std::string path, + Access at, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + json::TracingJSON, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + std::string, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + std::string) : AbstractIOHandler(std::move(path), at) {} diff --git a/src/IO/AbstractIOHandlerHelper.cpp b/src/IO/AbstractIOHandlerHelper.cpp index c6cd69f8a5..699dfd3619 100644 --- a/src/IO/AbstractIOHandlerHelper.cpp +++ b/src/IO/AbstractIOHandlerHelper.cpp @@ -69,7 +69,6 @@ std::unique_ptr createIOHandler( Access access, Format format, std::string originalExtension, - MPI_Comm comm, json::TracingJSON options, std::string const &pathAsItWasSpecifiedInTheConstructor) @@ -83,7 +82,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -92,7 +91,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP4: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -101,7 +100,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP5: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -110,7 +109,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SST: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -119,7 +118,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SSC: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, comm, std::move(options), @@ -155,7 +154,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "file", @@ -163,7 +162,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP4: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "bp4", @@ -171,7 +170,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_BP5: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "bp5", @@ -179,7 +178,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SST: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "sst", @@ -187,7 +186,7 @@ std::unique_ptr createIOHandler( case Format::ADIOS2_SSC: return constructIOHandler( "ADIOS2", - path, + std::move(path), access, std::move(options), "ssc", @@ -195,7 +194,7 @@ std::unique_ptr createIOHandler( case Format::JSON: return constructIOHandler( "JSON", - path, + std::move(path), access, std::move(options), JSONIOHandlerImpl::FileFormat::Json, @@ -203,7 +202,7 @@ std::unique_ptr createIOHandler( case Format::TOML: return constructIOHandler( "JSON", - path, + std::move(path), access, std::move(options), JSONIOHandlerImpl::FileFormat::Toml, diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index af827704a1..01b489f4dd 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/src/IO/AbstractIOHandlerImpl.cpp @@ -38,7 +38,7 @@ AbstractIOHandlerImpl::AbstractIOHandlerImpl(AbstractIOHandler *handler) } void AbstractIOHandlerImpl::keepSynchronous( - Writable *writable, Parameter param) + Writable *writable, Parameter const ¶m) { writable->abstractFilePosition = param.otherWritable->abstractFilePosition; writable->written = true; diff --git a/src/IO/HDF5/HDF5Auxiliary.cpp b/src/IO/HDF5/HDF5Auxiliary.cpp index 53fb1fb390..75207720cd 100644 --- a/src/IO/HDF5/HDF5Auxiliary.cpp +++ b/src/IO/HDF5/HDF5Auxiliary.cpp @@ -314,7 +314,7 @@ std::string openPMD::concrete_h5_file_position(Writable *w) } std::vector openPMD::getOptimalChunkDims( - std::vector const dims, size_t const typeSize) + std::vector const &dims, size_t const typeSize) { auto const ndims = dims.size(); std::vector chunk_dims(dims.size()); diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index a0faac5fe8..e2fa63d9b3 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -2814,8 +2814,12 @@ std::future HDF5IOHandler::flush(internal::ParsedFlushParams &) return m_impl->flush(); } #else + HDF5IOHandler::HDF5IOHandler( - std::string path, Access at, json::TracingJSON /* config */) + std::string path, + Access at, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] json::TracingJSON config) : AbstractIOHandler(std::move(path), at) { throw std::runtime_error("openPMD-api built without HDF5 support"); diff --git a/src/IO/HDF5/ParallelHDF5IOHandler.cpp b/src/IO/HDF5/ParallelHDF5IOHandler.cpp index f7a6dc1a1c..47a7764480 100644 --- a/src/IO/HDF5/ParallelHDF5IOHandler.cpp +++ b/src/IO/HDF5/ParallelHDF5IOHandler.cpp @@ -180,17 +180,25 @@ ParallelHDF5IOHandlerImpl::~ParallelHDF5IOHandlerImpl() } } #else + #if openPMD_HAVE_MPI ParallelHDF5IOHandler::ParallelHDF5IOHandler( - std::string path, Access at, MPI_Comm comm, json::TracingJSON /* config */) + std::string path, + Access at, + MPI_Comm comm, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] json::TracingJSON config) : AbstractIOHandler(std::move(path), at, comm) { throw std::runtime_error("openPMD-api built without HDF5 support"); } #else ParallelHDF5IOHandler::ParallelHDF5IOHandler( - std::string path, Access at, json::TracingJSON /* config */) - : AbstractIOHandler(std::move(path), at) + std::string const &path, + Access at, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] json::TracingJSON config) + : AbstractIOHandler(path, at) { throw std::runtime_error( "openPMD-api built without parallel support and without HDF5 support"); diff --git a/src/IO/InvalidatableFile.cpp b/src/IO/InvalidatableFile.cpp index 8d4b688673..4d512f8064 100644 --- a/src/IO/InvalidatableFile.cpp +++ b/src/IO/InvalidatableFile.cpp @@ -22,7 +22,7 @@ #include "openPMD/IO/InvalidatableFile.hpp" openPMD::InvalidatableFile::InvalidatableFile(std::string s) - : fileState{std::make_shared(s)} + : fileState{std::make_shared(std::move(s))} {} void openPMD::InvalidatableFile::invalidate() @@ -39,11 +39,11 @@ openPMD::InvalidatableFile &openPMD::InvalidatableFile::operator=(std::string s) { if (fileState) { - fileState->name = s; + fileState->name = std::move(s); } else { - fileState = std::make_shared(s); + fileState = std::make_shared(std::move(s)); } return *this; } diff --git a/src/IO/JSON/JSONIOHandler.cpp b/src/IO/JSON/JSONIOHandler.cpp index 7eb8a57278..041b236340 100644 --- a/src/IO/JSON/JSONIOHandler.cpp +++ b/src/IO/JSON/JSONIOHandler.cpp @@ -26,7 +26,7 @@ namespace openPMD JSONIOHandler::~JSONIOHandler() = default; JSONIOHandler::JSONIOHandler( - std::string path, + std::string const &path, Access at, openPMD::json::TracingJSON jsonCfg, JSONIOHandlerImpl::FileFormat format, diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp index 73d50366f7..a4e1bb39ab 100644 --- a/src/IO/JSON/JSONIOHandlerImpl.cpp +++ b/src/IO/JSON/JSONIOHandlerImpl.cpp @@ -124,16 +124,14 @@ namespace JSONIOHandlerImpl::JSONIOHandlerImpl( AbstractIOHandler *handler, - openPMD::json::TracingJSON config, + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [[maybe_unused]] openPMD::json::TracingJSON config, FileFormat format, std::string originalExtension) : AbstractIOHandlerImpl(handler) , m_fileFormat{format} , m_originalExtension{std::move(originalExtension)} -{ - // Currently unused - (void)config; -} +{} JSONIOHandlerImpl::~JSONIOHandlerImpl() = default; @@ -1011,13 +1009,13 @@ void JSONIOHandlerImpl::deregister( m_files.erase(writable); } -auto JSONIOHandlerImpl::getFilehandle(File fileName, Access access) +auto JSONIOHandlerImpl::getFilehandle(File const &fileName, Access access) -> std::tuple, std::istream *, std::ostream *> { VERIFY_ALWAYS( fileName.valid(), "[JSON] Tried opening a file that has been overwritten or deleted.") - auto path = fullPath(std::move(fileName)); + auto path = fullPath(fileName); auto fs = std::make_unique(); std::istream *istream = nullptr; std::ostream *ostream = nullptr; @@ -1057,7 +1055,7 @@ auto JSONIOHandlerImpl::getFilehandle(File fileName, Access access) return std::make_tuple(std::move(fs), istream, ostream); } -std::string JSONIOHandlerImpl::fullPath(File fileName) +std::string JSONIOHandlerImpl::fullPath(File const &fileName) { return fullPath(*fileName); } @@ -1189,7 +1187,8 @@ bool JSONIOHandlerImpl::hasKey(nlohmann::json const &j, KeyT &&key) return j.find(std::forward(key)) != j.end(); } -void JSONIOHandlerImpl::ensurePath(nlohmann::json *jsonp, std::string path) +void JSONIOHandlerImpl::ensurePath( + nlohmann::json *jsonp, std::string const &path) { auto groups = auxiliary::split(path, "/"); for (std::string &group : groups) @@ -1206,7 +1205,7 @@ void JSONIOHandlerImpl::ensurePath(nlohmann::json *jsonp, std::string path) } std::tuple::iterator, bool> -JSONIOHandlerImpl::getPossiblyExisting(std::string file) +JSONIOHandlerImpl::getPossiblyExisting(std::string const &file) { auto it = std::find_if( @@ -1233,7 +1232,8 @@ JSONIOHandlerImpl::getPossiblyExisting(std::string file) std::move(name), it, newlyCreated); } -std::shared_ptr JSONIOHandlerImpl::obtainJsonContents(File file) +std::shared_ptr +JSONIOHandlerImpl::obtainJsonContents(File const &file) { VERIFY_ALWAYS( file.valid(), @@ -1270,7 +1270,7 @@ nlohmann::json &JSONIOHandlerImpl::obtainJsonContents(Writable *writable) } void JSONIOHandlerImpl::putJsonContents( - File filename, + File const &filename, bool unsetDirty // = true ) { @@ -1305,8 +1305,8 @@ void JSONIOHandlerImpl::putJsonContents( } } -std::shared_ptr -JSONIOHandlerImpl::setAndGetFilePosition(Writable *writable, std::string extend) +std::shared_ptr JSONIOHandlerImpl::setAndGetFilePosition( + Writable *writable, std::string const &extend) { std::string path; if (writable->abstractFilePosition) @@ -1388,7 +1388,7 @@ bool JSONIOHandlerImpl::isDataset(nlohmann::json const &j) return i != j.end() && i.value().is_array(); } -bool JSONIOHandlerImpl::isGroup(nlohmann::json::const_iterator it) +bool JSONIOHandlerImpl::isGroup(nlohmann::json::const_iterator const &it) { auto &j = it.value(); if (it.key() == "attributes" || it.key() == "platform_byte_widths" || diff --git a/src/Iteration.cpp b/src/Iteration.cpp index ce35aee652..9e402f419d 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -372,7 +372,7 @@ void Iteration::reread(std::string const &path) } void Iteration::readFileBased( - std::string filePath, std::string const &groupPath, bool doBeginStep) + std::string const &filePath, std::string const &groupPath, bool doBeginStep) { if (doBeginStep) { diff --git a/src/ReadIterations.cpp b/src/ReadIterations.cpp index 41edd4ed05..9457a22421 100644 --- a/src/ReadIterations.cpp +++ b/src/ReadIterations.cpp @@ -123,11 +123,12 @@ void SeriesIterator::close() } SeriesIterator::SeriesIterator( - Series series_in, std::optional parsePreference) + Series const &series_in, + std::optional const &parsePreference) : m_data{std::make_shared>(std::in_place)} { auto &data = get(); - data.parsePreference = std::move(parsePreference); + data.parsePreference = parsePreference; /* * Since the iterator is stored in * internal::SeriesData::m_sharedStatefulIterator, @@ -628,7 +629,7 @@ ReadIterations::ReadIterations( Series series, Access access, std::optional parsePreference) - : m_series(std::move(series)), m_parsePreference(std::move(parsePreference)) + : m_series(std::move(series)), m_parsePreference(parsePreference) { auto &data = m_series.get(); if (access == Access::READ_LINEAR && !data.m_sharedStatefulIterator) diff --git a/src/Series.cpp b/src/Series.cpp index 7d9a8413fb..b7c807eb4c 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -508,7 +508,7 @@ namespace */ template int autoDetectPadding( - std::function isPartOfSeries, + std::function const &isPartOfSeries, std::string const &directory, MappingFunction &&mappingFunction) { @@ -539,11 +539,11 @@ namespace } int autoDetectPadding( - std::function isPartOfSeries, + std::function const &isPartOfSeries, std::string const &directory) { return autoDetectPadding( - std::move(isPartOfSeries), + isPartOfSeries, directory, [](Series::IterationIndex_t index, std::string const &filename) { (void)index; @@ -707,7 +707,7 @@ void Series::initDefaults(IterationEncoding ie, bool initAll) std::future Series::flush_impl( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler) { IOHandler()->m_lastFlushSuccessful = true; @@ -745,7 +745,7 @@ std::future Series::flush_impl( void Series::flushFileBased( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler) { auto &series = get(); @@ -862,7 +862,7 @@ void Series::flushFileBased( void Series::flushGorVBased( iterations_iterator begin, iterations_iterator end, - internal::FlushParams flushParams, + internal::FlushParams const &flushParams, bool flushIOHandler) { auto &series = get(); @@ -1020,7 +1020,7 @@ void Series::readFileBased() series.m_filenameExtension); int padding = autoDetectPadding( - std::move(isPartOfSeries), + isPartOfSeries, IOHandler()->directory, // foreach found file with `filename` and `index`: [&series](IterationIndex_t index, std::string const &filename) { @@ -1457,7 +1457,7 @@ creating new iterations. auto readSingleIteration = [&series, &pOpen, this]( IterationIndex_t index, - std::string path, + std::string const &path, bool guardAgainstRereading, bool beginStep) -> std::optional { if (series.iterations.contains(index)) @@ -2386,7 +2386,9 @@ ReadIterations Series::readIterations() // Use private constructor instead of copy constructor to avoid // object slicing return { - this->m_series, IOHandler()->m_frontendAccess, get().m_parsePreference}; + Series(this->m_series), + IOHandler()->m_frontendAccess, + get().m_parsePreference}; } void Series::parseBase() diff --git a/src/WriteIterations.cpp b/src/WriteIterations.cpp index 334df0ec3a..0ae7246ae0 100644 --- a/src/WriteIterations.cpp +++ b/src/WriteIterations.cpp @@ -75,7 +75,7 @@ WriteIterations::mapped_type &WriteIterations::operator[](key_type &&key) auto lastIteration_v = lastIteration.value(); if (lastIteration_v.iterationIndex == key) { - return s.iterations.at(std::move(key)); + return s.iterations.at(std::forward(key)); } else { @@ -83,7 +83,7 @@ WriteIterations::mapped_type &WriteIterations::operator[](key_type &&key) } } s.currentlyOpen = key; - auto &res = s.iterations[std::move(key)]; + auto &res = s.iterations[std::forward(key)]; if (res.getStepStatus() == StepStatus::NoStep) { try diff --git a/src/auxiliary/Filesystem.cpp b/src/auxiliary/Filesystem.cpp index 38d8e209f8..cce80b9d17 100644 --- a/src/auxiliary/Filesystem.cpp +++ b/src/auxiliary/Filesystem.cpp @@ -152,7 +152,8 @@ bool remove_directory(std::string const &path) #endif for (auto const &entry : list_directory(path)) { - std::string partialPath = path + directory_separator + entry; + 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)) diff --git a/src/auxiliary/JSON.cpp b/src/auxiliary/JSON.cpp index 168cab7bf6..dd0825c33c 100644 --- a/src/auxiliary/JSON.cpp +++ b/src/auxiliary/JSON.cpp @@ -226,7 +226,7 @@ namespace res[pair.key()] = jsonToToml(pair.value(), currentPath); currentPath.pop_back(); } - return toml::value(std::move(res)); + return toml::value(res); } case nlohmann::json::value_t::array: { toml::value::array_type res; @@ -238,7 +238,7 @@ namespace res.emplace_back(jsonToToml(entry, currentPath)); currentPath.pop_back(); } - return toml::value(std::move(res)); + return toml::value(res); } case nlohmann::json::value_t::string: return val.get(); diff --git a/src/backend/Attributable.cpp b/src/backend/Attributable.cpp index 2d97dd0ff3..52ddda28d3 100644 --- a/src/backend/Attributable.cpp +++ b/src/backend/Attributable.cpp @@ -207,7 +207,7 @@ auto Attributable::myPath() const -> MyPath return res; } -void Attributable::seriesFlush(internal::FlushParams flushParams) +void Attributable::seriesFlush(internal::FlushParams const &flushParams) { writable().seriesFlush(flushParams); } @@ -311,7 +311,7 @@ void Attributable::readAttributes(ReadMode mode) } std::array arr; std::copy_n(vector.begin(), 7, arr.begin()); - setAttribute(key, std::move(arr)); + setAttribute(key, arr); } else { diff --git a/src/backend/Writable.cpp b/src/backend/Writable.cpp index f886e94046..97d13ce5f0 100644 --- a/src/backend/Writable.cpp +++ b/src/backend/Writable.cpp @@ -47,7 +47,7 @@ void Writable::seriesFlush(std::string backendConfig) seriesFlush({FlushLevel::UserFlush, std::move(backendConfig)}); } -void Writable::seriesFlush(internal::FlushParams flushParams) +void Writable::seriesFlush(internal::FlushParams const &flushParams) { auto series = Attributable({attributable, [](auto const *) {}}).retrieveSeries(); diff --git a/src/binding/python/Attributable.cpp b/src/binding/python/Attributable.cpp index 88dbb95cbc..a3fa63cdca 100644 --- a/src/binding/python/Attributable.cpp +++ b/src/binding/python/Attributable.cpp @@ -155,61 +155,61 @@ bool setAttributeFromBufferInfo( ) ); else */ // std::cout << "+++++++++++ BUFFER: " << buf.format << std::endl; - if (buf.format.find("b") != std::string::npos) + if (buf.format.find('b') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("h") != std::string::npos) + else if (buf.format.find('h') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("i") != std::string::npos) + else if (buf.format.find('i') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("l") != std::string::npos) + else if (buf.format.find('l') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("q") != std::string::npos) + else if (buf.format.find('q') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("B") != std::string::npos) + else if (buf.format.find('B') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("H") != std::string::npos) + else if (buf.format.find('H') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("I") != std::string::npos) + else if (buf.format.find('I') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("L") != std::string::npos) + else if (buf.format.find('L') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("Q") != std::string::npos) + else if (buf.format.find('Q') != std::string::npos) return attr.setAttribute( key, std::vector( @@ -234,19 +234,19 @@ bool setAttributeFromBufferInfo( static_cast *>(buf.ptr), static_cast *>(buf.ptr) + buf.size)); - else if (buf.format.find("f") != std::string::npos) + else if (buf.format.find('f') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("d") != std::string::npos) + else if (buf.format.find('d') != std::string::npos) return attr.setAttribute( key, std::vector( static_cast(buf.ptr), static_cast(buf.ptr) + buf.size)); - else if (buf.format.find("g") != std::string::npos) + else if (buf.format.find('g') != std::string::npos) return attr.setAttribute( key, std::vector( @@ -349,7 +349,7 @@ bool setAttributeFromObject( py::object &obj, pybind11::dtype datatype) { - Datatype requestedDatatype = dtype_from_numpy(datatype); + Datatype requestedDatatype = dtype_from_numpy(std::move(datatype)); return switchNonVectorType( requestedDatatype, attr, key, obj); } @@ -401,7 +401,8 @@ void init_Attributable(py::module &m) std::string const &key, py::object &obj, pybind11::dtype datatype) { - return setAttributeFromObject(attr, key, obj, datatype); + return setAttributeFromObject( + attr, key, obj, std::move(datatype)); }, py::arg("key"), py::arg("value"), diff --git a/src/binding/python/ChunkInfo.cpp b/src/binding/python/ChunkInfo.cpp index d86a579905..86bcb0128a 100644 --- a/src/binding/python/ChunkInfo.cpp +++ b/src/binding/python/ChunkInfo.cpp @@ -61,7 +61,7 @@ void init_Chunk(py::module &m) }, // __setstate__ - [](py::tuple t) { + [](py::tuple const &t) { // our state tuple has exactly three values if (t.size() != 3) throw std::runtime_error("Invalid state!"); diff --git a/src/binding/python/Dataset.cpp b/src/binding/python/Dataset.cpp index a67d7f1221..656cd59ea8 100644 --- a/src/binding/python/Dataset.cpp +++ b/src/binding/python/Dataset.cpp @@ -32,8 +32,8 @@ void init_Dataset(py::module &m) .def(py::init(), py::arg("dtype"), py::arg("extent")) .def(py::init(), py::arg("extent")) .def( - py::init([](py::dtype dt, Extent e) { - auto const d = dtype_from_numpy(dt); + py::init([](py::dtype dt, Extent const &e) { + auto const d = dtype_from_numpy(std::move(dt)); return new Dataset{d, e}; }), py::arg("dtype"), @@ -44,8 +44,8 @@ void init_Dataset(py::module &m) py::arg("extent"), py::arg("options")) .def( - py::init([](py::dtype dt, Extent e, std::string options) { - auto const d = dtype_from_numpy(dt); + py::init([](py::dtype dt, Extent const &e, std::string options) { + auto const d = dtype_from_numpy(std::move(dt)); return new Dataset{d, e, std::move(options)}; }), py::arg("dtype"), diff --git a/src/binding/python/Datatype.cpp b/src/binding/python/Datatype.cpp index 9d53fba5ca..1a603855b0 100644 --- a/src/binding/python/Datatype.cpp +++ b/src/binding/python/Datatype.cpp @@ -58,7 +58,7 @@ void init_Datatype(py::module &m) .value("BOOL", Datatype::BOOL) .value("UNDEFINED", Datatype::UNDEFINED); - m.def("determine_datatype", [](py::dtype const dt) { + m.def("determine_datatype", [](py::dtype const &dt) { return dtype_from_numpy(dt); }); m.def("determine_datatype", [](py::array const &a) { diff --git a/src/binding/python/PatchRecordComponent.cpp b/src/binding/python/PatchRecordComponent.cpp index 08059826e7..33b04e733c 100644 --- a/src/binding/python/PatchRecordComponent.cpp +++ b/src/binding/python/PatchRecordComponent.cpp @@ -95,7 +95,7 @@ void init_PatchRecordComponent(py::module &m) // all buffer types .def( "store", - [](PatchRecordComponent &prc, uint64_t idx, py::buffer a) { + [](PatchRecordComponent &prc, uint64_t idx, py::buffer const &a) { py::buffer_info buf = a.request(); auto const dtype = dtype_from_bufferformat(buf.format); diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index f5ef98f6f6..9b8154e906 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -932,7 +932,7 @@ void init_RecordComponent(py::module &m) .def( "make_empty", [](RecordComponent &rc, - pybind11::dtype const dt, + pybind11::dtype const &dt, uint8_t dimensionality) { return rc.makeEmpty(dtype_from_numpy(dt), dimensionality); }) @@ -1114,7 +1114,8 @@ void init_RecordComponent(py::module &m) py::arg_v("extent", Extent(1, -1u), "array.shape")) .def_property_readonly_static( - "SCALAR", [](py::object) { return RecordComponent::SCALAR; }) + "SCALAR", + [](py::object const &) { return RecordComponent::SCALAR; }) // TODO remove in future versions (deprecated) .def("set_unit_SI", &RecordComponent::setUnitSI) // deprecated diff --git a/src/cli/ls.cpp b/src/cli/ls.cpp index 65bb226602..3f13770f56 100644 --- a/src/cli/ls.cpp +++ b/src/cli/ls.cpp @@ -27,6 +27,7 @@ int main(int argc, char *argv[]) { std::vector str_argv; + str_argv.reserve(argc); for (int i = 0; i < argc; ++i) str_argv.emplace_back(argv[i]); diff --git a/src/version.cpp b/src/version.cpp index 6fa0a9ecfa..c2e8809a32 100644 --- a/src/version.cpp +++ b/src/version.cpp @@ -30,8 +30,7 @@ std::string openPMD::getVersion() << OPENPMDAPI_VERSION_PATCH; if (std::string(OPENPMDAPI_VERSION_LABEL).size() > 0) api << "-" << OPENPMDAPI_VERSION_LABEL; - std::string const apistr = api.str(); - return apistr; + return api.str(); } std::string openPMD::getStandard() @@ -39,8 +38,7 @@ std::string openPMD::getStandard() std::stringstream standard; standard << OPENPMD_STANDARD_MAJOR << "." << OPENPMD_STANDARD_MINOR << "." << OPENPMD_STANDARD_PATCH; - std::string const standardstr = standard.str(); - return standardstr; + return standard.str(); } std::string openPMD::getStandardMinimum() @@ -49,6 +47,5 @@ std::string openPMD::getStandardMinimum() standardMin << OPENPMD_STANDARD_MIN_MAJOR << "." << OPENPMD_STANDARD_MIN_MINOR << "." << OPENPMD_STANDARD_MIN_PATCH; - std::string const standardMinstr = standardMin.str(); - return standardMinstr; + return standardMin.str(); } diff --git a/test/AuxiliaryTest.cpp b/test/AuxiliaryTest.cpp index 491bb37794..7cc88f3b65 100644 --- a/test/AuxiliaryTest.cpp +++ b/test/AuxiliaryTest.cpp @@ -64,6 +64,7 @@ TEST_CASE("deref_cast_test", "[auxiliary]") B const value = {123.45}; B const *const ptr = &value; + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) auto const a = deref_dynamic_cast(ptr); auto const &ra = deref_dynamic_cast(ptr); (void)a; @@ -175,7 +176,7 @@ struct structure : public TestHelper } structure &setText(std::string newText) { - setAttribute("text", newText); + setAttribute("text", std::move(newText)); return *this; } }; @@ -308,7 +309,7 @@ struct AttributedWidget : public TestHelper AttributedWidget() : TestHelper() {} - Attribute::resource get(std::string key) + Attribute::resource get(std::string const &key) { return getAttribute(key).getResource(); } @@ -380,7 +381,7 @@ struct Dotty : public TestHelper } Dotty &setAtt3(std::string s) { - setAttribute("att3", s); + setAttribute("att3", std::move(s)); return *this; } }; diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index f6f93e065c..58505ec85f 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -49,6 +49,7 @@ TEST_CASE("attribute_dtype_test", "[core]") REQUIRE(Datatype::CHAR == a.dtype); { // check that copy constructor works + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) [[maybe_unused]] Attribute b = a; } { diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index d884766f15..eb9765375a 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -75,9 +75,8 @@ TEST_CASE("parallel_multi_series_test", "[parallel]") // have multiple serial series alive at the same time for (auto const sn : {1, 2, 3}) { - for (auto const &t : myBackends) + for (auto const &file_ending : myBackends) { - auto const file_ending = t; std::cout << file_ending << std::endl; allSeries.emplace_back( std::string("../samples/parallel_multi_open_test_") @@ -115,7 +114,7 @@ TEST_CASE("parallel_multi_series_test", "[parallel]") void write_test_zero_extent( bool fileBased, - std::string file_ending, + std::string const &file_ending, bool writeAllChunks, bool declareFromAll) { @@ -379,7 +378,7 @@ TEST_CASE("no_parallel_hdf5", "[parallel][hdf5]") #endif #if openPMD_HAVE_ADIOS2 && openPMD_HAVE_MPI -void available_chunks_test(std::string file_ending) +void available_chunks_test(std::string const &file_ending) { int r_mpi_rank{-1}, r_mpi_size{-1}; MPI_Comm_rank(MPI_COMM_WORLD, &r_mpi_rank); @@ -628,7 +627,7 @@ TEST_CASE("hzdr_adios_sample_content_test", "[parallel][adios2][bp3]") #endif #if openPMD_HAVE_MPI -void write_4D_test(std::string file_ending) +void write_4D_test(std::string const &file_ending) { int mpi_s{-1}; int mpi_r{-1}; @@ -662,7 +661,7 @@ TEST_CASE("write_4D_test", "[parallel]") } } -void write_makeconst_some(std::string file_ending) +void write_makeconst_some(std::string const &file_ending) { int mpi_s{-1}; int mpi_r{-1}; @@ -695,7 +694,7 @@ TEST_CASE("write_makeconst_some", "[parallel]") } } -void close_iteration_test(std::string file_ending) +void close_iteration_test(std::string const &file_ending) { int i_mpi_rank{-1}, i_mpi_size{-1}; MPI_Comm_rank(MPI_COMM_WORLD, &i_mpi_rank); @@ -765,7 +764,7 @@ TEST_CASE("close_iteration_test", "[parallel]") } } -void file_based_write_read(std::string file_ending) +void file_based_write_read(std::string const &file_ending) { namespace io = openPMD; @@ -872,7 +871,7 @@ TEST_CASE("file_based_write_read", "[parallel]") } } -void hipace_like_write(std::string file_ending) +void hipace_like_write(std::string const &file_ending) { namespace io = openPMD; @@ -1397,7 +1396,7 @@ void append_mode( std::string const &extension, bool variableBased, ParseMode parseMode, - std::string jsonConfig = "{}") + std::string const &jsonConfig = "{}") { std::string filename = (variableBased ? "../samples/append/append_variablebased." @@ -1415,7 +1414,7 @@ void append_mode( std::vector data(10, 999); auto writeSomeIterations = [&data, mpi_size, mpi_rank]( WriteIterations &&writeIterations, - std::vector indices) { + std::vector const &indices) { for (auto index : indices) { auto it = writeIterations[index]; diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 2ab428c83f..52ee3c387a 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -67,7 +67,7 @@ std::vector testedBackends() if (lookup != extensions.end()) { std::string extension = lookup->second; - res.push_back({std::move(pair.first), std::move(extension)}); + res.push_back({pair.first, std::move(extension)}); } } } @@ -237,9 +237,8 @@ TEST_CASE("multi_series_test", "[serial]") // have multiple serial series alive at the same time for (auto const sn : {1, 2, 3}) { - for (auto const &t : myfileExtensions) + for (auto const &file_ending : myfileExtensions) { - auto const file_ending = t; std::cout << file_ending << std::endl; allSeries.emplace_back( std::string("../samples/multi_open_test_") @@ -414,7 +413,7 @@ TEST_CASE("multiple_series_handles_test", "[serial]") #endif } -void close_iteration_test(std::string file_ending) +void close_iteration_test(std::string const &file_ending) { std::string name = "../samples/close_iterations_%T." + file_ending; @@ -500,7 +499,7 @@ TEST_CASE("close_iteration_test", "[serial]") } void close_iteration_interleaved_test( - std::string const file_ending, IterationEncoding const it_encoding) + std::string const &file_ending, IterationEncoding const it_encoding) { std::string name = "../samples/close_iterations_interleaved_"; if (it_encoding == IterationEncoding::fileBased) @@ -597,12 +596,18 @@ struct NonCopyableDeleter : std::function {} NonCopyableDeleter(NonCopyableDeleter const &) = delete; NonCopyableDeleter &operator=(NonCopyableDeleter const &) = delete; + /* + * MSVC does not recognize these when declaring noexcept and this is + * for a test only anyway. + */ + // NOLINTNEXTLINE(performance-noexcept-move-constructor) NonCopyableDeleter(NonCopyableDeleter &&) = default; + // NOLINTNEXTLINE(performance-noexcept-move-constructor) NonCopyableDeleter &operator=(NonCopyableDeleter &&) = default; }; } // namespace detail -void close_and_copy_attributable_test(std::string file_ending) +void close_and_copy_attributable_test(std::string const &file_ending) { using position_t = int; @@ -794,7 +799,7 @@ TEST_CASE("close_iteration_throws_test", "[serial]") } #endif -inline void empty_dataset_test(std::string file_ending) +inline void empty_dataset_test(std::string const &file_ending) { { Series series( @@ -898,7 +903,7 @@ TEST_CASE("empty_dataset_test", "[serial]") } } -inline void constant_scalar(std::string file_ending) +inline void constant_scalar(std::string const &file_ending) { Mesh::Geometry const geometry = Mesh::Geometry::spherical; std::string const geometryParameters = "dummyGeometryParameters"; @@ -1170,7 +1175,7 @@ TEST_CASE("flush_without_position_positionOffset", "[serial]") } } -inline void particle_patches(std::string file_ending) +inline void particle_patches(std::string const &file_ending) { constexpr auto SCALAR = openPMD::RecordComponent::SCALAR; @@ -2176,7 +2181,7 @@ TEST_CASE("fileBased_write_test", "[serial]") } } -inline void sample_write_thetaMode(std::string file_ending) +inline void sample_write_thetaMode(std::string const &file_ending) { Series o = Series( std::string("../samples/thetaMode_%05T.").append(file_ending), @@ -2512,8 +2517,8 @@ inline void optional_paths_110_test(const std::string &backend) } void git_early_chunk_query( - std::string const filename, - std::string const species, + std::string const &filename, + std::string const &species, int const step, std::string const &jsonConfig = "{}") { @@ -5839,7 +5844,7 @@ void variableBasedParticleData() for (size_t i = 0; i < 3; ++i) { - std::string dim = dimensions[i]; + std::string const &dim = dimensions[i]; RecordComponent rc = electronPositions[dim]; loadedChunks[i] = rc.loadChunk( Offset(rc.getDimensionality(), 0), rc.getExtent()); @@ -5850,7 +5855,6 @@ void variableBasedParticleData() for (size_t i = 0; i < 3; ++i) { - std::string dim = dimensions[i]; Extent const &extent = extents[i]; auto chunk = loadedChunks[i]; for (size_t j = 0; j < extent[0]; ++j) @@ -6041,7 +6045,9 @@ TEST_CASE("automatically_deactivate_span", "[serial][adios2]") // @todo Upon switching to ADIOS2 2.7.0, test this the other way around also void iterate_nonstreaming_series( - std::string const &file, bool variableBasedLayout, std::string jsonConfig) + std::string const &file, + bool variableBasedLayout, + std::string const &jsonConfig) { constexpr size_t extent = 100; { @@ -6461,9 +6467,10 @@ void deferred_parsing(std::string const &extension) { padding += "0"; } - infix = padding + infix; + infix = padding.append(infix); std::ofstream file; - file.open(basename + infix + "." + extension); + file.open(std::string(basename).append(infix).append(".").append( + extension)); file.close(); } } @@ -6850,7 +6857,7 @@ TEST_CASE("late_setting_of_iterationencoding", "[serial]") auxiliary::file_exists("../samples/change_name_and_encoding_10.json")); } -void varying_pattern(std::string const file_ending) +void varying_pattern(std::string const &file_ending) { { std::string filename = "../samples/varying_pattern_%06T." + file_ending; @@ -6938,7 +6945,7 @@ void append_mode( std::string const &filename, bool variableBased, ParseMode parseMode, - std::string jsonConfig = "{}") + std::string const &jsonConfig = "{}") { if (auxiliary::directory_exists("../samples/append")) { @@ -6947,7 +6954,7 @@ void append_mode( std::vector data(10, 999); auto writeSomeIterations = [&data]( WriteIterations &&writeIterations, - std::vector indices) { + std::vector const &indices) { for (auto index : indices) { auto it = writeIterations[index]; @@ -7314,7 +7321,7 @@ void append_mode_filebased(std::string const &extension) } })END"; auto writeSomeIterations = [](WriteIterations &&writeIterations, - std::vector indices) { + std::vector const &indices) { for (auto index : indices) { auto it = writeIterations[index]; From d3aedffe62fcbc9c953b457d712ebcf68290ea3d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 20 Nov 2023 17:40:04 -0800 Subject: [PATCH 06/11] [pre-commit.ci] pre-commit autoupdate (#1559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-clang-format: v17.0.4 → v17.0.5](https://github.com/pre-commit/mirrors-clang-format/compare/v17.0.4...v17.0.5) - [github.com/hadialqattan/pycln: v2.3.0 → v2.4.0](https://github.com/hadialqattan/pycln/compare/v2.3.0...v2.4.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 82119873ac..086ac0f078 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -66,7 +66,7 @@ repos: # clang-format v13 # to run manually, use .github/workflows/clang-format/clang-format.sh - repo: https://github.com/pre-commit/mirrors-clang-format - rev: v17.0.4 + rev: v17.0.5 hooks: - id: clang-format # By default, the clang-format hook configures: @@ -80,7 +80,7 @@ repos: # Autoremoves unused Python imports - repo: https://github.com/hadialqattan/pycln - rev: v2.3.0 + rev: v2.4.0 hooks: - id: pycln name: pycln (python) From 72ab3d12d8c2cfc61fcb02570f0048911e914a41 Mon Sep 17 00:00:00 2001 From: Davide Terzani <23354403+titoiride@users.noreply.github.com> Date: Mon, 20 Nov 2023 18:27:38 -0800 Subject: [PATCH 07/11] Update cmake minimum required (#1558) --- share/openPMD/cmake/FindADIOS.cmake | 2 +- share/openPMD/thirdParty/json/CMakeLists.txt | 2 +- share/openPMD/thirdParty/toml11/CMakeLists.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/share/openPMD/cmake/FindADIOS.cmake b/share/openPMD/cmake/FindADIOS.cmake index 5c39052778..a7fc253a91 100644 --- a/share/openPMD/cmake/FindADIOS.cmake +++ b/share/openPMD/cmake/FindADIOS.cmake @@ -88,7 +88,7 @@ # Required cmake version ############################################################################### -cmake_minimum_required(VERSION 2.8.12) +cmake_minimum_required(VERSION 3.5) ############################################################################### diff --git a/share/openPMD/thirdParty/json/CMakeLists.txt b/share/openPMD/thirdParty/json/CMakeLists.txt index fa77a5aed2..014324dbd6 100644 --- a/share/openPMD/thirdParty/json/CMakeLists.txt +++ b/share/openPMD/thirdParty/json/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.1) +cmake_minimum_required(VERSION 3.5) ## ## PROJECT diff --git a/share/openPMD/thirdParty/toml11/CMakeLists.txt b/share/openPMD/thirdParty/toml11/CMakeLists.txt index 911874cbe2..e157632f6f 100644 --- a/share/openPMD/thirdParty/toml11/CMakeLists.txt +++ b/share/openPMD/thirdParty/toml11/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.1) +cmake_minimum_required(VERSION 3.5) enable_testing() project(toml11 VERSION 3.7.1) From 599a1cb4dbaf9ade8533832720b1559d85f978fb Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Tue, 5 Dec 2023 10:54:19 -0800 Subject: [PATCH 08/11] Remove `FindADIOS.cmake` (#1560) * Remove `FindADIOS.cmake` We removed ADIOS1 support in the upcoming 0.16 release series. * More ADIOS1 Cleanups --- Singularity | 8 +- docs/Doxyfile | 3 +- docs/source/analysis/paraview.rst | 2 +- docs/source/dev/dependencies.rst | 1 - examples/8a_benchmark_write_parallel.cpp | 3 +- examples/8b_benchmark_read_parallel.cpp | 3 +- openPMDConfig.cmake.in | 8 +- setup.py | 5 +- share/openPMD/cmake/FindADIOS.cmake | 311 ----------------------- test/SerialIOTest.cpp | 225 ++-------------- 10 files changed, 27 insertions(+), 542 deletions(-) delete mode 100644 share/openPMD/cmake/FindADIOS.cmake diff --git a/Singularity b/Singularity index fb26fdf3bb..f58b39a2d5 100644 --- a/Singularity +++ b/Singularity @@ -7,7 +7,7 @@ From: debian:unstable Welcome to the openPMD-api container. This container contains a pre-installed openPMD-api library. This container provides serial I/O. -Supported backends are HDF5 and ADIOS1. +Supported backends are HDF5 and ADIOS2. Supported frontends are C++11 and Python3. %setup @@ -25,7 +25,6 @@ Supported frontends are C++11 and Python3. ipython3 \ python3-dev \ pybind11-dev \ - libadios-bin libadios-dev \ libglib2.0-dev libbz2-dev libibverbs-dev libnetcdf-dev \ libhdf5-dev && \ rm -rf /var/lib/apt/lists/* @@ -33,16 +32,12 @@ Supported frontends are C++11 and Python3. # python3-numpy # missing: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=900804 - # libadios-openmpi-dev # libopenmpi-dev libhdf5-openmpi-dev - # libadios2-dev - cd $(mktemp -d) cmake /opt/openpmd-api \ -DopenPMD_USE_MPI=OFF \ -DopenPMD_USE_HDF5=ON \ - -DopenPMD_USE_ADIOS1=ON \ -DopenPMD_USE_ADIOS2=OFF \ -DopenPMD_USE_PYTHON=ON \ -DopenPMD_BUILD_TESTING=OFF \ @@ -61,6 +56,5 @@ Supported frontends are C++11 and Python3. %labels openPMD_HAVE_MPI OFF openPMD_HAVE_HDF5 ON - openPMD_HAVE_ADIOS1 ON openPMD_HAVE_ADIOS2 OFF openPMD_HAVE_PYTHON ON diff --git a/docs/Doxyfile b/docs/Doxyfile index d1835b60ed..471c3cef28 100644 --- a/docs/Doxyfile +++ b/docs/Doxyfile @@ -1,7 +1,7 @@ PROJECT_NAME = "openPMD-api" XML_OUTPUT = xml INPUT = ../src ../include ../README.md -EXCLUDE_PATTERNS = *CommonADIOS1IOHandler.cpp +#EXCLUDE_PATTERNS = *CommonADIOS1IOHandler.cpp # TAGFILES += "cppreference-doxygen-web.tag.xml=http://en.cppreference.com/w/" BUILTIN_STL_SUPPORT = YES @@ -24,7 +24,6 @@ MACRO_EXPANSION = YES PREDEFINED = DOXYGEN_SHOULD_SKIP_THIS \ openPMD_HAVE_MPI=1 \ openPMD_HAVE_HDF5=1 \ - openPMD_HAVE_ADIOS1=1 \ openPMD_HAVE_ADIOS2=1 \ openPMD_HAVE_PYTHON=1 \ OPENPMD_private:=private \ diff --git a/docs/source/analysis/paraview.rst b/docs/source/analysis/paraview.rst index 696f08bbb2..e0c837ca0a 100644 --- a/docs/source/analysis/paraview.rst +++ b/docs/source/analysis/paraview.rst @@ -22,7 +22,7 @@ openPMD ------- openPMD files can be visualized with ParaView 5.9+, using 5.11+ is recommended. -ParaView supports ADIOS1, ADIOS2 and HDF5 files, as it implements against the Python bindings of openPMD-api. +ParaView supports ADIOS2 and HDF5 files, as it implements against the Python bindings of openPMD-api. For openPMD output to be recognized, create a small textfile with ``.pmd`` ending per data series, which can be opened with ParaView: diff --git a/docs/source/dev/dependencies.rst b/docs/source/dev/dependencies.rst index 00629fc58a..8e0cda7b97 100644 --- a/docs/source/dev/dependencies.rst +++ b/docs/source/dev/dependencies.rst @@ -27,7 +27,6 @@ Optional: I/O backends * `JSON `_ * `HDF5 `_ 1.8.13+ -* `ADIOS1 `_ 1.13.1+ (deprecated) * `ADIOS2 `_ 2.7.0+ while those can be build either with or without: diff --git a/examples/8a_benchmark_write_parallel.cpp b/examples/8a_benchmark_write_parallel.cpp index c509c879ff..6219fbb549 100644 --- a/examples/8a_benchmark_write_parallel.cpp +++ b/examples/8a_benchmark_write_parallel.cpp @@ -240,8 +240,7 @@ std::vector getBackends() { std::vector res; #if openPMD_HAVE_ADIOS2 - if (auxiliary::getEnvString("OPENPMD_BP_BACKEND", "NOT_SET") != "ADIOS1") - res.emplace_back(".bp"); + res.emplace_back(".bp"); #endif #if openPMD_HAVE_HDF5 diff --git a/examples/8b_benchmark_read_parallel.cpp b/examples/8b_benchmark_read_parallel.cpp index 98cd81add2..0b5466eb13 100644 --- a/examples/8b_benchmark_read_parallel.cpp +++ b/examples/8b_benchmark_read_parallel.cpp @@ -198,8 +198,7 @@ std::vector getBackends() { std::vector res; #if openPMD_HAVE_ADIOS2 - if (auxiliary::getEnvString("OPENPMD_BP_BACKEND", "NOT_SET") != "ADIOS1") - res.emplace_back(".bp"); + res.emplace_back(".bp"); if (auxiliary::getEnvString("OPENPMD_BENCHMARK_USE_BACKEND", "NOT_SET") == "ADIOS") return res; diff --git a/openPMDConfig.cmake.in b/openPMDConfig.cmake.in index eea485f2b8..3f6902e503 100644 --- a/openPMDConfig.cmake.in +++ b/openPMDConfig.cmake.in @@ -8,7 +8,7 @@ if(POLICY CMP0074) cmake_policy(SET CMP0074 NEW) endif() -# locate the installed FindADIOS.cmake module for ADIOS1 +# locate the installed CMake modules list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/Modules") # optional dependencies @@ -29,12 +29,6 @@ if(openPMD_HAVE_HDF5) endif() set(openPMD_HDF5_FOUND ${openPMD_HAVE_HDF5}) -set(openPMD_HAVE_ADIOS1 @openPMD_HAVE_ADIOS1@) -if(openPMD_HAVE_ADIOS1) - find_dependency(ADIOS) -endif() -set(openPMD_ADIOS1_FOUND ${openPMD_HAVE_ADIOS1}) - set(openPMD_HAVE_ADIOS2 @openPMD_HAVE_ADIOS2@) if(openPMD_HAVE_ADIOS2) find_dependency(ADIOS2) diff --git a/setup.py b/setup.py index 2053f1bd22..27e4cc4916 100644 --- a/setup.py +++ b/setup.py @@ -61,10 +61,8 @@ def build_extension(self, ext): # static/shared libs '-DopenPMD_BUILD_SHARED_LIBS:BOOL=' + BUILD_SHARED_LIBS, '-DHDF5_USE_STATIC_LIBRARIES:BOOL=' + HDF5_USE_STATIC_LIBRARIES, - '-DADIOS_USE_STATIC_LIBS:BOOL=' + ADIOS_USE_STATIC_LIBS, # Unix: rpath to current dir when packaged - # needed for shared (here non-default) builds and ADIOS1 - # wrapper libraries + # needed for shared (here non-default) builds '-DCMAKE_BUILD_WITH_INSTALL_RPATH:BOOL=ON', '-DCMAKE_INSTALL_RPATH_USE_LINK_PATH:BOOL=OFF', # Windows: has no RPath concept, all `.dll`s must be in %PATH% @@ -129,7 +127,6 @@ def build_extension(self, ext): # note: changed default for SHARED, MPI, TESTING and EXAMPLES openPMD_USE_MPI = os.environ.get('openPMD_USE_MPI', 'OFF') HDF5_USE_STATIC_LIBRARIES = os.environ.get('HDF5_USE_STATIC_LIBRARIES', 'OFF') -ADIOS_USE_STATIC_LIBS = os.environ.get('ADIOS_USE_STATIC_LIBS', 'OFF') # deprecated: backwards compatibility to <= 0.13.* BUILD_SHARED_LIBS = os.environ.get('BUILD_SHARED_LIBS', 'OFF') BUILD_TESTING = os.environ.get('BUILD_TESTING', 'OFF') diff --git a/share/openPMD/cmake/FindADIOS.cmake b/share/openPMD/cmake/FindADIOS.cmake deleted file mode 100644 index a7fc253a91..0000000000 --- a/share/openPMD/cmake/FindADIOS.cmake +++ /dev/null @@ -1,311 +0,0 @@ -# - Find ADIOS library, routines for scientific, parallel IO -# https://www.olcf.ornl.gov/center-projects/adios/ -# -# Use this module by invoking find_package with the form: -# find_package(ADIOS -# [version] [EXACT] # Minimum or EXACT version, e.g. 1.6.0 -# [REQUIRED] # Fail with an error if ADIOS or a required -# # component is not found -# [QUIET] # ... -# [COMPONENTS <...>] # Compiled in components: fortran, readonly, -# # sequential (all are case-insensitive) -# ) -# -# Module that finds the includes and libraries for a working ADIOS install. -# This module invokes the `adios_config` script that should be installed with -# the other ADIOS tools. -# -# To provide a hint to the module where to find the ADIOS installation, -# set the ADIOS_ROOT environment variable. -# -# If this variable is not set, make sure that at least the according `bin/` -# directory of ADIOS is in your PATH environment variable. -# -# Set the following CMake variables BEFORE calling find_packages to -# influence this module: -# ADIOS_USE_STATIC_LIBS - Set to ON to force the use of static -# libraries. Default: OFF -# -# This module will define the following variables: -# ADIOS_INCLUDE_DIRS - Include directories for the ADIOS headers. -# ADIOS_LIBRARIES - ADIOS libraries. -# ADIOS_DEFINITIONS - ADIOS compile definitions. -# ADIOS_FOUND - TRUE if FindADIOS found a working install -# ADIOS_VERSION - Version in format Major.Minor.Patch -# ADIOS_HAVE_SEQUENTIAL - TRUE if found library links as sequential only -# -# Not used for now: -# ADIOS_DEFINITIONS - Compiler definitions you should add with -# add_definitions(${ADIOS_DEFINITIONS}) -# -# Example to find ADIOS (default) -# find_package(ADIOS) -# if(ADIOS_FOUND) -# include_directories(${ADIOS_INCLUDE_DIRS}) -# add_executable(foo foo.c) -# target_link_libraries(foo ${ADIOS_LIBRARIES}) -# endif() - -# Example to find ADIOS using component -# find_package(ADIOS COMPONENTS fortran) -# if(ADIOS_FOUND) -# include_directories(${ADIOS_INCLUDE_DIRS}) -# add_executable(foo foo.c) -# target_link_libraries(foo ${ADIOS_LIBRARIES}) -# endif() -############################################################################### -#Copyright (c) 2014-2020, Axel Huebl and Felix Schmitt from http://picongpu.hzdr.de -#All rights reserved. - -#Redistribution and use in source and binary forms, with or without -#modification, are permitted provided that the following conditions are met: - -#1. Redistributions of source code must retain the above copyright notice, this -#list of conditions and the following disclaimer. - -#2. Redistributions in binary form must reproduce the above copyright notice, -#this list of conditions and the following disclaimer in the documentation -#and/or other materials provided with the distribution. - -#3. Neither the name of the copyright holder nor the names of its contributors -#may be used to endorse or promote products derived from this software without -#specific prior written permission. - -#THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -#AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -#IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE -#DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE -#FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL -#DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -#SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -#CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, -#OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -#OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -############################################################################### - - -############################################################################### -# Required cmake version -############################################################################### - -cmake_minimum_required(VERSION 3.5) - - -############################################################################### -# ADIOS -############################################################################### -# get flags for adios_config, -l is the default -#-f for fortran, -r for readonly, -s for sequential (nompi) -set(OPTLIST "") -if(ADIOS_FIND_COMPONENTS) - foreach(COMP ${ADIOS_FIND_COMPONENTS}) - string(TOLOWER ${COMP} comp) - if(comp STREQUAL "fortran") - set(OPTLIST "${OPTLIST}f") - elseif(comp STREQUAL "readonly") - set(OPTLIST "${OPTLIST}r") - elseif(comp STREQUAL "sequential") - set(OPTLIST "${OPTLIST}s") - else() - if(NOT ADIOS_FIND_QUIETLY) - message("ADIOS component ${COMP} is not supported. Please use fortran, readonly, or sequential") - endif() - endif() - endforeach() -endif() -set(LINKOPTLIST "-l${OPTLIST}") -set(COMPILEOPTLIST "-c${OPTLIST}") - -# we start by assuming we found ADIOS and falsify it if some -# dependencies are missing (or if we did not find ADIOS at all) -set(ADIOS_FOUND TRUE) - - -# find `adios_config` program ################################################# -# check the ADIOS_ROOT hint and the normal PATH -find_file(ADIOS_CONFIG - NAME adios_config - PATHS $ENV{ADIOS_ROOT}/bin $ENV{ADIOS_DIR}/bin $ENV{INSTALL_PREFIX}/bin $ENV{PATH}) - -if(ADIOS_CONFIG) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "Found 'adios_config': ${ADIOS_CONFIG}") - endif() -else(ADIOS_CONFIG) - set(ADIOS_FOUND FALSE) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "Can NOT find 'adios_config' - set ADIOS_ROOT, ADIOS_DIR or INSTALL_PREFIX, or check your PATH") - endif() -endif(ADIOS_CONFIG) - -# check `adios_config` program ################################################ -if(ADIOS_FOUND) - execute_process(COMMAND ${ADIOS_CONFIG} ${LINKOPTLIST} - OUTPUT_VARIABLE ADIOS_LINKFLAGS - RESULT_VARIABLE ADIOS_CONFIG_RETURN - OUTPUT_STRIP_TRAILING_WHITESPACE) - if(NOT ADIOS_CONFIG_RETURN EQUAL 0) - set(ADIOS_FOUND FALSE) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "Can NOT execute 'adios_config' - check file permissions") - endif() - endif() - - execute_process(COMMAND ${ADIOS_CONFIG} ${COMPILEOPTLIST} - OUTPUT_VARIABLE ADIOS_COMPILEFLAGS - RESULT_VARIABLE ADIOS_CONFIG_RETURN - OUTPUT_STRIP_TRAILING_WHITESPACE) - if(NOT ADIOS_CONFIG_RETURN EQUAL 0) - set(ADIOS_FOUND FALSE) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "Can NOT execute 'adios_config' - check file permissions") - endif() - endif() - - # find ADIOS_ROOT_DIR - execute_process(COMMAND ${ADIOS_CONFIG} -d - OUTPUT_VARIABLE ADIOS_ROOT_DIR - OUTPUT_STRIP_TRAILING_WHITESPACE) - if(NOT IS_DIRECTORY "${ADIOS_ROOT_DIR}") - set(ADIOS_FOUND FALSE) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "The directory provided by 'adios_config -d' does not exist: ${ADIOS_ROOT_DIR}") - endif() - endif() -endif(ADIOS_FOUND) - -# option: use only static libs ################################################ -if(ADIOS_USE_STATIC_LIBS) - # careful, we have to restore the original path in the end - set(_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES}) - set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_STATIC_LIBRARY_SUFFIX}) -endif() - - -# we found something in ADIOS_ROOT_DIR and adios_config works ################# -set(ADIOS_INCLUDE_DIRS) -set(ADIOS_DEFINITIONS) -set(ADIOS_LIBRARIES) -if(ADIOS_FOUND) - # ADIOS headers - list(APPEND ADIOS_INCLUDE_DIRS ${ADIOS_ROOT_DIR}/include) - - # check for compiled in dependencies, recomve ";" in ADIOS_LINKFLAGS (from cmake build) - string(REGEX REPLACE ";" " " ADIOS_LINKFLAGS "${ADIOS_LINKFLAGS}") - string(REGEX REPLACE ";" " " ADIOS_COMPILEFLAGS "${ADIOS_COMPILEFLAGS}") - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "ADIOS linker flags (unparsed): ${ADIOS_LINKFLAGS}") - message(STATUS "ADIOS compiler flags (unparsed): ${ADIOS_COMPILEFLAGS}") - endif() - - # find all library paths -L - # note: this can cause trouble if some libs are specified twice from - # different sources (quite unlikely) - # http://www.cmake.org/pipermail/cmake/2008-November/025128.html - set(ADIOS_LIBRARY_DIRS) - string(REGEX MATCHALL " -L([A-Za-z_0-9/\\.-]+)" _ADIOS_LIBDIRS " ${ADIOS_LINKFLAGS}") - foreach(_LIBDIR ${_ADIOS_LIBDIRS}) - string(REPLACE " -L" "" _LIBDIR ${_LIBDIR}) - list(APPEND ADIOS_LIBRARY_DIRS ${_LIBDIR}) - endforeach() - # we could append ${CMAKE_PREFIX_PATH} now but that is not really necessary - - # determine whether found library links as serial only - set(ADIOS_HAVE_SEQUENTIAL FALSE) - - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "ADIOS DIRS to look for libs: ${ADIOS_LIBRARY_DIRS}") - endif() - - # parse all -lname libraries and find an absolute path for them - string(REGEX MATCHALL " -l([A-Za-z_0-9\\.\\-\\+]+)" _ADIOS_LIBS " ${ADIOS_LINKFLAGS}") - foreach(_LIB ${_ADIOS_LIBS}) - string(REPLACE " -l" "" _LIB ${_LIB}) - - # find static lib: absolute path in -L then default - if(_LIB MATCHES "^glib") - find_library(_LIB_DIR NAMES ${_LIB} PATHS ${ADIOS_LIBRARY_DIRS} NAMES glib-2.0) - else() - find_library(_LIB_DIR NAMES ${_LIB} PATHS ${ADIOS_LIBRARY_DIRS}) - endif() - - # pthread should not be linked statically, allow fallback to shared - if(NOT _LIB_DIR AND _LIB MATCHES "pthread|m|rt") - if(ADIOS_USE_STATIC_LIBS) - set(CMAKE_FIND_LIBRARY_SUFFIXES ${_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES}) - find_library(_LIB_DIR NAMES ${_LIB} PATHS ${ADIOS_LIBRARY_DIRS}) - set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_STATIC_LIBRARY_SUFFIX}) - endif() - endif() - - if(_LIB MATCHES "^.*nompi.*$") - set(ADIOS_HAVE_SEQUENTIAL TRUE) - endif() - - # found? - if(_LIB_DIR) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "Found ${_LIB} in ${_LIB_DIR}") - endif() - list(APPEND ADIOS_LIBRARIES "${_LIB_DIR}") - else(_LIB_DIR) - set(ADIOS_FOUND FALSE) - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "ADIOS: Could NOT find library '${_LIB}'") - endif() - endif(_LIB_DIR) - - # clean cached var - unset(_LIB_DIR CACHE) - unset(_LIB_DIR) - endforeach() - - #add libraries which are already using cmake format - string(REGEX MATCHALL "/([A-Za-z_0-9/\\.\\-\\+]+)\\.([a|so]+)" _ADIOS_LIBS_SUB "${ADIOS_LINKFLAGS}") - foreach(foo ${_ADIOS_LIBS_SUB}) - if (EXISTS ${foo}) - if(NOT ADIOS_FIND_QUIETLY) - message("Appending: ${foo}") - endif() - list(APPEND ADIOS_LIBRARIES "${foo}") - endif() - endforeach(foo) - - # find all compiler definitions _D - string(REGEX MATCHALL "(-D[A-Za-z_0-9/\\.-]+)" _ADIOS_DEFINES " ${ADIOS_COMPILEFLAGS}") - string(REGEX REPLACE ";" " " ADIOS_DEFINITIONS "${_ADIOS_DEFINES}") - - if(NOT ADIOS_FIND_QUIETLY) - message(STATUS "ADIOS compile definitions: ${ADIOS_DEFINITIONS}") - endif() - - # add the version string - execute_process(COMMAND ${ADIOS_CONFIG} -v - OUTPUT_VARIABLE ADIOS_VERSION - OUTPUT_STRIP_TRAILING_WHITESPACE) -endif(ADIOS_FOUND) - -# unset checked variables if not found -if(NOT ADIOS_FOUND) - unset(ADIOS_INCLUDE_DIRS) - unset(ADIOS_LIBRARIES) -endif(NOT ADIOS_FOUND) - - -# restore CMAKE_FIND_LIBRARY_SUFFIXES if manipulated by this module ########### -if(ADIOS_USE_STATIC_LIBS) - set(CMAKE_FIND_LIBRARY_SUFFIXES ${_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES}) -endif() - - -############################################################################### -# FindPackage Options -############################################################################### - -# handles the REQUIRED, QUIET and version-related arguments for find_package -include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(ADIOS - FOUND_VAR ADIOS_FOUND - REQUIRED_VARS ADIOS_LIBRARIES ADIOS_INCLUDE_DIRS - VERSION_VAR ADIOS_VERSION -) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 52ee3c387a..7002d31162 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -420,7 +420,6 @@ void close_iteration_test(std::string const &file_ending) std::vector data{2, 4, 6, 8}; // { // we do *not* need these parentheses Series write(name, Access::CREATE); - bool isAdios1 = write.backend() == "ADIOS1"; { Iteration it0 = write.iterations[0]; auto E_x = it0.meshes["E"]["x"]; @@ -431,13 +430,6 @@ void close_iteration_test(std::string const &file_ending) write.flush(); // } - if (isAdios1) - { - // run a simplified test for Adios1 since Adios1 has issues opening - // twice in the same process - REQUIRE(auxiliary::file_exists("../samples/close_iterations_0.bp")); - } - else { Series read(name, Access::READ_ONLY); Iteration it0 = read.iterations[0]; @@ -463,13 +455,6 @@ void close_iteration_test(std::string const &file_ending) REQUIRE_THROWS(write.flush()); } - if (isAdios1) - { - // run a simplified test for Adios1 since Adios1 has issues opening - // twice in the same process - REQUIRE(auxiliary::file_exists("../samples/close_iterations_1.bp")); - } - else { Series read(name, Access::READ_ONLY); Iteration it1 = read.iterations[1]; @@ -1284,7 +1269,6 @@ inline void dtype_test(const std::string &backend) bool test_long_long = (backend != "json") || sizeof(long long) <= 8; { Series s = Series("../samples/dtype_test." + backend, Access::CREATE); - bool adios1 = s.backend() == "ADIOS1" || s.backend() == "MPI_ADIOS1"; char c = 'c'; s.setAttribute("char", c); @@ -1315,10 +1299,7 @@ inline void dtype_test(const std::string &backend) } std::string str = "string"; s.setAttribute("string", str); - if (!adios1) - { - s.setAttribute("emptyString", ""); - } + s.setAttribute("emptyString", ""); s.setAttribute("vecChar", std::vector({'c', 'h', 'a', 'r'})); s.setAttribute("vecInt16", std::vector({32766, 32767})); s.setAttribute( @@ -1348,13 +1329,9 @@ inline void dtype_test(const std::string &backend) } s.setAttribute( "vecString", std::vector({"vector", "of", "strings"})); - if (!adios1) - { - s.setAttribute( - "vecEmptyString", std::vector{"", "", ""}); - s.setAttribute( - "vecMixedString", std::vector{"hi", "", "ho"}); - } + s.setAttribute("vecEmptyString", std::vector{"", "", ""}); + s.setAttribute( + "vecMixedString", std::vector{"hi", "", "ho"}); s.setAttribute("bool", true); s.setAttribute("boolF", false); @@ -1414,7 +1391,6 @@ inline void dtype_test(const std::string &backend) } Series s = Series("../samples/dtype_test." + backend, Access::READ_ONLY); - bool adios1 = s.backend() == "ADIOS1" || s.backend() == "MPI_ADIOS1"; REQUIRE(s.getAttribute("char").get() == 'c'); REQUIRE(s.getAttribute("uchar").get() == 'u'); @@ -1432,10 +1408,7 @@ inline void dtype_test(const std::string &backend) REQUIRE(s.getAttribute("longdouble").get() == 1.e80L); } REQUIRE(s.getAttribute("string").get() == "string"); - if (!adios1) - { - REQUIRE(s.getAttribute("emptyString").get().empty()); - } + REQUIRE(s.getAttribute("emptyString").get().empty()); REQUIRE( s.getAttribute("vecChar").get>() == std::vector({'c', 'h', 'a', 'r'})); @@ -1479,15 +1452,12 @@ inline void dtype_test(const std::string &backend) REQUIRE( s.getAttribute("vecString").get>() == std::vector({"vector", "of", "strings"})); - if (!adios1) - { - REQUIRE( - s.getAttribute("vecEmptyString").get>() == - std::vector({"", "", ""})); - REQUIRE( - s.getAttribute("vecMixedString").get>() == - std::vector({"hi", "", "ho"})); - } + REQUIRE( + s.getAttribute("vecEmptyString").get>() == + std::vector({"", "", ""})); + REQUIRE( + s.getAttribute("vecMixedString").get>() == + std::vector({"hi", "", "ho"})); REQUIRE(s.getAttribute("bool").get() == true); REQUIRE(s.getAttribute("boolF").get() == false); @@ -1691,8 +1661,7 @@ void test_complex(const std::string &backend) "../samples/serial_write_complex." + backend, Access::CREATE); o.setAttribute("lifeIsComplex", std::complex(4.56, 7.89)); o.setAttribute("butComplexFloats", std::complex(42.3, -99.3)); - if (o.backend() != "ADIOS2" && o.backend() != "ADIOS1" && - o.backend() != "MPI_ADIOS1") + if (o.backend() != "ADIOS2") o.setAttribute( "longDoublesYouSay", std::complex(5.5, -4.55)); @@ -1713,8 +1682,7 @@ void test_complex(const std::string &backend) Cdbl.storeChunk(cdoubles, {0}); std::vector> cldoubles(3); - if (o.backend() != "ADIOS2" && o.backend() != "ADIOS1" && - o.backend() != "MPI_ADIOS1") + if (o.backend() != "ADIOS2") { auto Cldbl = o.iterations[0].meshes["Cldbl"][RecordComponent::SCALAR]; @@ -1738,8 +1706,7 @@ void test_complex(const std::string &backend) REQUIRE( i.getAttribute("butComplexFloats").get>() == std::complex(42.3, -99.3)); - if (i.backend() != "ADIOS2" && i.backend() != "ADIOS1" && - i.backend() != "MPI_ADIOS1") + if (i.backend() != "ADIOS2") { REQUIRE( i.getAttribute("longDoublesYouSay") @@ -1758,8 +1725,7 @@ void test_complex(const std::string &backend) REQUIRE(rcflt.get()[1] == std::complex(-3., 4.)); REQUIRE(rcdbl.get()[2] == std::complex(6, -5.)); - if (i.backend() != "ADIOS2" && i.backend() != "ADIOS1" && - i.backend() != "MPI_ADIOS1") + if (i.backend() != "ADIOS2") { auto rcldbl = i.iterations[0] .meshes["Cldbl"][RecordComponent::SCALAR] @@ -1779,7 +1745,7 @@ void test_complex(const std::string &backend) TEST_CASE("test_complex", "[serial]") { // Notes: - // - ADIOS1 and ADIOS 2.7.0 have no complex long double + // - ADIOS 2.7.0 has no complex long double // - JSON read-back not distinguishable yet from N+1 shaped data set for (auto const &t : testedFileExtensions()) { @@ -2407,7 +2373,7 @@ TEST_CASE("deletion_test", "[serial]") { if (t == "bp" || t == "bp4" || t == "bp5") { - continue; // deletion not implemented in ADIOS1 backend + continue; // deletion not implemented in ADIOS2 backend } deletion_test(t); } @@ -3894,12 +3860,12 @@ TEST_CASE("no_serial_hdf5", "[serial][hdf5]") REQUIRE(true); } #endif -#if openPMD_HAVE_ADIOS1 +#if openPMD_HAVE_ADIOS2 -TEST_CASE("hzdr_adios1_sample_content_test", "[serial][adios1]") +TEST_CASE("hzdr_bp3_sample_content_test", "[serial][adios2][bp3]") { // since this file might not be publicly available, gracefully handle errors - /** @todo add bp example files to + /** @todo add bp3 example files to * https://github.com/openPMD/openPMD-example-datasets */ try { @@ -4070,108 +4036,6 @@ TEST_CASE("hzdr_adios1_sample_content_test", "[serial][adios1]") } } } - -#else -TEST_CASE("no_serial_adios1", "[serial][adios]") -{ - REQUIRE(true); -} -#endif - -#if openPMD_HAVE_ADIOS1 -TEST_CASE("serial_adios1_json_config", "[serial][adios1]") -{ - std::string globalConfig = R"END( -{ - "backend": "adios1", - "adios1": { - "dataset": { - "transform": "deliberately use a wrong configuration..." - } - } -})END"; - - std::string globalConfigWithoutCompression = R"END( -{ - "backend": "adios1" -})END"; - - std::string localConfig = R"END( -{ - "adios1": { - "dataset": { - "transform": "...so we can check for the resulting errors" - } - } -})END"; - - auto test1 = [&]() { - Series write( - "../samples/adios1_dataset_transform.bp", - Access::CREATE, - globalConfig); - auto meshes = write.writeIterations()[0].meshes; - - auto defaultConfiguredMesh = - meshes["defaultConfigured"][RecordComponent::SCALAR]; - - Dataset ds{Datatype::INT, {10}}; - - defaultConfiguredMesh.resetDataset(ds); - - std::vector data(10, 2345); - defaultConfiguredMesh.storeChunk(data, {0}, {10}); - - write.flush(); - }; - REQUIRE_THROWS_WITH( - test1(), - Catch::Equals("[ADIOS1] Internal error: Failed to set ADIOS transform " - "during Dataset creation")); - - auto test2 = [&]() { - Series write( - "../samples/adios1_dataset_transform.bp", - Access::CREATE, - globalConfig); - auto meshes = write.writeIterations()[0].meshes; - auto overridenTransformMesh = - meshes["overridenConfig"][RecordComponent::SCALAR]; - - Dataset ds{Datatype::INT, {10}}; - ds.options = localConfig; - overridenTransformMesh.resetDataset(ds); - - std::vector data(10, 2345); - overridenTransformMesh.storeChunk(data, {0}, {10}); - - write.flush(); - }; - REQUIRE_THROWS_WITH( - test2(), - Catch::Equals("[ADIOS1] Internal error: Failed to set ADIOS transform " - "during Dataset creation")); - - auto test3 = [&]() { - // use no dataset transform at all - Series write( - "../samples/adios1_dataset_transform.bp", - Access::CREATE, - globalConfigWithoutCompression); - auto meshes = write.writeIterations()[0].meshes; - auto defaultConfiguredMesh = - meshes["defaultConfigured"][RecordComponent::SCALAR]; - - Dataset ds{Datatype::INT, {10}}; - defaultConfiguredMesh.resetDataset(ds); - - std::vector data(10, 2345); - defaultConfiguredMesh.storeChunk(data, {0}, {10}); - - write.flush(); - }; - test3(); // should run without exception -} #endif #if openPMD_HAVE_ADIOS2 @@ -6791,17 +6655,6 @@ TEST_CASE("unfinished_iteration_test", "[serial]") unfinished_iteration_test( "bp", IterationEncoding::fileBased, R"({"backend": "adios2"})"); #endif -#if openPMD_HAVE_ADIOS1 - /* - * Catching errors from ADIOS1 is not generally supported - */ - // unfinished_iteration_test( - // "adios1.bp", IterationEncoding::groupBased, R"({"backend": - // "adios1"})"); - // unfinished_iteration_test( - // "adios1.bp", IterationEncoding::fileBased, R"({"backend": - // "adios1"})"); -#endif #if openPMD_HAVE_HDF5 unfinished_iteration_test("h5", IterationEncoding::groupBased); unfinished_iteration_test("h5", IterationEncoding::fileBased); @@ -6984,16 +6837,6 @@ void append_mode( { write.setIterationEncoding(IterationEncoding::variableBased); } - if (write.backend() == "ADIOS1") - { - REQUIRE_THROWS_WITH( - write.flush(), - Catch::Equals( - "Operation unsupported in ADIOS1: Appending to existing " - "file on disk (use Access::CREATE to overwrite)")); - // destructor will be noisy now - return; - } writeSomeIterations( write.writeIterations(), std::vector{3, 2}); @@ -7012,16 +6855,6 @@ void append_mode( { write.setIterationEncoding(IterationEncoding::variableBased); } - if (write.backend() == "ADIOS1") - { - REQUIRE_THROWS_WITH( - write.flush(), - Catch::Equals( - "Operation unsupported in ADIOS1: Appending to existing " - "file on disk (use Access::CREATE to overwrite)")); - // destructor will be noisy now - return; - } writeSomeIterations( write.writeIterations(), std::vector{4, 3, 10}); @@ -7033,13 +6866,6 @@ void append_mode( { write.setIterationEncoding(IterationEncoding::variableBased); } - if (write.backend() == "ADIOS1") - { - REQUIRE_THROWS_AS( - write.flush(), error::OperationUnsupportedInBackend); - // destructor will be noisy now - return; - } writeSomeIterations( write.writeIterations(), std::vector{7, 1, 11}); @@ -7172,17 +6998,6 @@ void append_mode( { write.setIterationEncoding(IterationEncoding::variableBased); } - if (write.backend() == "ADIOS1") - { - REQUIRE_THROWS_WITH( - write.flush(), - Catch::Equals( - "Operation unsupported in ADIOS1: Appending to " - "existing " - "file on disk (use Access::CREATE to overwrite)")); - // destructor will be noisy now - return; - } writeSomeIterations( write.writeIterations(), std::vector{4, 5}); From 4de431c46b75ede4aa205fa639291c4b993a5616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 5 Dec 2023 21:16:36 +0100 Subject: [PATCH 09/11] ADIOS2: Optionally write attributes only from given ranks (#1542) * ADIOS2: Optionally write attributes only from given ranks Ref. https://github.com/ornladios/ADIOS2/issues/3846#issuecomment-1769255906 * ADIOS2 < v2.9 compatibility in tests * Documentation --- docs/source/backends/adios2.rst | 23 +++++++ docs/source/details/adios2.json | 4 +- docs/source/details/adios2.toml | 7 ++ docs/source/details/backendconfig.rst | 7 +- docs/source/details/mpi.rst | 2 + include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp | 6 +- src/IO/ADIOS/ADIOS2IOHandler.cpp | 68 ++++++++++++++++++-- test/ParallelIOTest.cpp | 11 +++- 8 files changed, 116 insertions(+), 12 deletions(-) diff --git a/docs/source/backends/adios2.rst b/docs/source/backends/adios2.rst index de6b47aaad..13e357022c 100644 --- a/docs/source/backends/adios2.rst +++ b/docs/source/backends/adios2.rst @@ -105,6 +105,11 @@ The default behavior may be restored by setting the :ref:`JSON parameter ` ``adios2.attribute_writing_ranks`` can be used to restrict attribute writing to only a select handful of ranks (most typically a single one). +The ADIOS2 backend of the openPMD-api will then ignore attributes from all other MPI ranks. + +.. tip:: + + Treat metadata specification as a collective operation in order to retain compatibility with HDF5, and then specify ``adios2.attribute_writing_ranks = 0`` in order to achieve best performance in ADIOS2. + +.. warning:: + + The ADIOS2 backend may also use attributes to encode openPMD groups (ref. "group table"). + The ``adios2.attribute_writing_ranks`` key also applies to those attributes, i.e. also group creation must be treated as collective then (at least on the specified ranks). + Experimental group table feature -------------------------------- diff --git a/docs/source/details/adios2.json b/docs/source/details/adios2.json index d71061c0bc..c817eb0d7a 100644 --- a/docs/source/details/adios2.json +++ b/docs/source/details/adios2.json @@ -2,6 +2,7 @@ "adios2": { "engine": { "type": "sst", + "preferred_flush_target": "disk", "parameters": { "BufferGrowthFactor": "2.0", "QueueLimit": "2" @@ -17,6 +18,7 @@ } } ] - } + }, + "attribute_writing_ranks": 0 } } diff --git a/docs/source/details/adios2.toml b/docs/source/details/adios2.toml index 863a5021fa..20ef9e827f 100644 --- a/docs/source/details/adios2.toml +++ b/docs/source/details/adios2.toml @@ -1,5 +1,12 @@ +[adios2] +# ignore all attribute writes not issued on these ranks +# can also be a list if multiple ranks need to be given +# however rank 0 should be the most common option here +attribute_writing_ranks = 0 + [adios2.engine] type = "sst" +preferred_flush_target = "disk" [adios2.engine.parameters] BufferGrowthFactor = "2.0" diff --git a/docs/source/details/backendconfig.rst b/docs/source/details/backendconfig.rst index f1c47c78bd..57d577af10 100644 --- a/docs/source/details/backendconfig.rst +++ b/docs/source/details/backendconfig.rst @@ -113,7 +113,7 @@ A full configuration of the ADIOS2 backend: .. literalinclude:: adios2.toml :language: toml -All keys found under ``adios2.dataset`` are applicable globally as well as per dataset, keys found under ``adios2.engine`` only globally. +All keys found under ``adios2.dataset`` are applicable globally as well as per dataset, any other keys such as those found under ``adios2.engine`` only globally. Explanation of the single keys: * ``adios2.engine.type``: A string that is passed directly to ``adios2::IO:::SetEngine`` for choosing the ADIOS2 engine to be used. @@ -142,6 +142,11 @@ Explanation of the single keys: The openPMD-api will automatically use a fallback implementation for the span-based Put() API if any operator is added to a dataset. This workaround is enabled on a per-dataset level. The workaround can be completely deactivated by specifying ``{"adios2": {"use_span_based_put": true}}`` or it can alternatively be activated indiscriminately for all datasets by specifying ``{"adios2": {"use_span_based_put": false}}``. +* ``adios2.attribute_writing_ranks``: A list of MPI ranks that define metadata. ADIOS2 attributes will be written only from those ranks, any other ranks will be ignored. Can be either a list of integers or a single integer. + +.. hint:: + + Specifying ``adios2.attribute_writing_ranks`` can lead to serious serialization performance improvements at large scale. Operations specified inside ``adios2.dataset.operators`` will be applied to ADIOS2 datasets in writing as well as in reading. Beginning with ADIOS2 2.8.0, this can be used to specify decompressor settings: diff --git a/docs/source/details/mpi.rst b/docs/source/details/mpi.rst index b94e886fac..ea4ec0551e 100644 --- a/docs/source/details/mpi.rst +++ b/docs/source/details/mpi.rst @@ -42,6 +42,8 @@ Functionality Behavior Description If you want to support all backends equally, treat as a collective operation. Note that openPMD represents constant record components with attributes, thus inheriting this for ``::makeConstant``. + When treating attribute definitions as collective, we advise specifying the ADIOS2 :ref:`JSON/TOML key ` ``adios2.attribute_writing_ranks`` for metadata aggregation scalabilty, typically as ``adios2.attribute_writing_ranks = 0``. + .. [4] We usually open iterations delayed on first access. This first access is usually the ``flush()`` call after a ``storeChunk``/``loadChunk`` operation. If the first access is non-collective, an explicit, collective ``Iteration::open()`` can be used to have the files already open. Alternatively, iterations might be accessed for the first time by immediate operations such as ``::availableChunks()``. diff --git a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp index e63c6a493b..cdd7983312 100644 --- a/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp +++ b/include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp @@ -274,6 +274,8 @@ class ADIOS2IOHandlerImpl return m_useGroupTable.value(); } + bool m_writeAttributesFromThisRank = true; + struct ParameterizedOperator { adios2::Operator op; @@ -285,7 +287,9 @@ class ADIOS2IOHandlerImpl json::TracingJSON m_config; static json::TracingJSON nullvalue; - void init(json::TracingJSON config); + template + void + init(json::TracingJSON config, Callback &&callbackWriteAttributesFromRank); template json::TracingJSON config(Key &&key, json::TracingJSON &cfg) diff --git a/src/IO/ADIOS/ADIOS2IOHandler.cpp b/src/IO/ADIOS/ADIOS2IOHandler.cpp index 8cddd578a0..534e364f53 100644 --- a/src/IO/ADIOS/ADIOS2IOHandler.cpp +++ b/src/IO/ADIOS/ADIOS2IOHandler.cpp @@ -79,7 +79,43 @@ ADIOS2IOHandlerImpl::ADIOS2IOHandlerImpl( , m_engineType(std::move(engineType)) , m_userSpecifiedExtension{std::move(specifiedExtension)} { - init(std::move(cfg)); + init( + std::move(cfg), + /* callbackWriteAttributesFromRank = */ + [communicator, this](nlohmann::json const &attribute_writing_ranks) { + int rank = 0; + MPI_Comm_rank(communicator, &rank); + auto throw_error = []() { + throw error::BackendConfigSchema( + {"adios2", "attribute_writing_ranks"}, + "Type must be either an integer or an array of integers."); + }; + if (attribute_writing_ranks.is_array()) + { + m_writeAttributesFromThisRank = false; + for (auto const &val : attribute_writing_ranks) + { + if (!val.is_number()) + { + throw_error(); + } + if (val.get() == rank) + { + m_writeAttributesFromThisRank = true; + break; + } + } + } + else if (attribute_writing_ranks.is_number()) + { + m_writeAttributesFromThisRank = + attribute_writing_ranks.get() == rank; + } + else + { + throw_error(); + } + }); } #endif // openPMD_HAVE_MPI @@ -94,7 +130,7 @@ ADIOS2IOHandlerImpl::ADIOS2IOHandlerImpl( , m_engineType(std::move(engineType)) , m_userSpecifiedExtension(std::move(specifiedExtension)) { - init(std::move(cfg)); + init(std::move(cfg), [](auto const &...) {}); } ADIOS2IOHandlerImpl::~ADIOS2IOHandlerImpl() @@ -135,7 +171,9 @@ ADIOS2IOHandlerImpl::~ADIOS2IOHandlerImpl() } } -void ADIOS2IOHandlerImpl::init(json::TracingJSON cfg) +template +void ADIOS2IOHandlerImpl::init( + json::TracingJSON cfg, Callback &&callbackWriteAttributesFromRank) { // allow overriding through environment variable m_engineType = @@ -181,6 +219,12 @@ void ADIOS2IOHandlerImpl::init(json::TracingJSON cfg) : ModifiableAttributes::No; } + if (m_config.json().contains("attribute_writing_ranks")) + { + callbackWriteAttributesFromRank( + m_config["attribute_writing_ranks"].json()); + } + auto engineConfig = config(ADIOS2Defaults::str_engine); if (!engineConfig.json().is_null()) { @@ -915,6 +959,10 @@ void ADIOS2IOHandlerImpl::writeDataset( void ADIOS2IOHandlerImpl::writeAttribute( Writable *writable, const Parameter ¶meters) { + if (!m_writeAttributesFromThisRank) + { + return; + } #if openPMD_HAS_ADIOS_2_9 switch (useGroupTable()) { @@ -3033,7 +3081,11 @@ namespace detail if (!initializedDefaults) { // Currently only schema 0 supported - m_IO.DefineAttribute(ADIOS2Defaults::str_adios2Schema, 0); + if (m_impl->m_writeAttributesFromThisRank) + { + m_IO.DefineAttribute( + ADIOS2Defaults::str_adios2Schema, 0); + } initializedDefaults = true; } @@ -3168,7 +3220,8 @@ namespace detail { if (writeOnly(m_mode) && !m_IO.InquireAttribute( - ADIOS2Defaults::str_usesstepsAttribute)) + ADIOS2Defaults::str_usesstepsAttribute) && + m_impl->m_writeAttributesFromThisRank) { m_IO.DefineAttribute( ADIOS2Defaults::str_usesstepsAttribute, 0); @@ -3189,7 +3242,8 @@ namespace detail */ if (calledExplicitly && writeOnly(m_mode) && !m_IO.InquireAttribute( - ADIOS2Defaults::str_usesstepsAttribute)) + ADIOS2Defaults::str_usesstepsAttribute) && + m_impl->m_writeAttributesFromThisRank) { m_IO.DefineAttribute( ADIOS2Defaults::str_usesstepsAttribute, 1); @@ -3356,7 +3410,7 @@ namespace detail case UseGroupTable::Yes: #if openPMD_HAS_ADIOS_2_9 { - if (writeOnly(m_mode)) + if (writeOnly(m_mode) && m_impl->m_writeAttributesFromThisRank) { requireActiveStep(); auto currentStepBuffered = currentStep(); diff --git a/test/ParallelIOTest.cpp b/test/ParallelIOTest.cpp index eb9765375a..ce7d6cc565 100644 --- a/test/ParallelIOTest.cpp +++ b/test/ParallelIOTest.cpp @@ -1,6 +1,7 @@ /* Running this test in parallel with MPI requires MPI::Init. * To guarantee a correct call to Init, launch the tests manually. */ +#include "openPMD/IO/ADIOS/macros.hpp" #include "openPMD/auxiliary/Environment.hpp" #include "openPMD/auxiliary/Filesystem.hpp" #include "openPMD/openPMD.hpp" @@ -1170,10 +1171,16 @@ clevel = "1" doshuffle = "BLOSC_BITSHUFFLE" )END"; - std::string writeConfigBP4 = R"END( + std::string writeConfigBP4 = + R"END( [adios2] unused = "parameter" - +attribute_writing_ranks = 0 +)END" +#if openPMD_HAS_ADIOS_2_9 + "use_group_table = true" +#endif + R"END( [adios2.engine] type = "bp4" unused = "as well" From d44f981c1c6c49d011a8cc74d736c62d4b49d5ea Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 5 Dec 2023 14:03:12 -0800 Subject: [PATCH 10/11] [pre-commit.ci] pre-commit autoupdate (#1564) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-clang-format: v17.0.5 → v17.0.6](https://github.com/pre-commit/mirrors-clang-format/compare/v17.0.5...v17.0.6) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 086ac0f078..bb87d59e01 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -66,7 +66,7 @@ repos: # clang-format v13 # to run manually, use .github/workflows/clang-format/clang-format.sh - repo: https://github.com/pre-commit/mirrors-clang-format - rev: v17.0.5 + rev: v17.0.6 hooks: - id: clang-format # By default, the clang-format hook configures: From 6b963badb868a22ae1b888ba30754ab7b2d4e7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 7 Dec 2023 13:42:03 +0100 Subject: [PATCH 11/11] Workaround for current CI issues with clang14 on MacOS (#1565) * Use mpirun wrapper to circumvent parser bug in mpirun The bug sounds similar to this one https://github.com/open-mpi/ompi/issues/6372, though that is supposedly fixed in Open MPI 5.. This creates a tmp script to call the launched application instead of calling it directly on the command line. This way, mpirun does not see the command line arguments and cannot try to wrongly parse them. * Add mechanism to get rid of the workaround again in future --- .github/workflows/macos.yml | 3 +- .github/workflows/mpirun_workaround.sh | 51 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100755 .github/workflows/mpirun_workaround.sh diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index bfc70bf508..f7b332fbde 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -40,7 +40,8 @@ jobs: -DopenPMD_USE_MPI=ON \ -DopenPMD_USE_HDF5=ON \ -DopenPMD_USE_ADIOS2=ON \ - -DopenPMD_USE_INVASIVE_TESTS=ON + -DopenPMD_USE_INVASIVE_TESTS=ON \ + -DMPIEXEC_EXECUTABLE=".github/workflows/mpirun_workaround.sh" cmake --build build --parallel 3 ctest --test-dir build --verbose diff --git a/.github/workflows/mpirun_workaround.sh b/.github/workflows/mpirun_workaround.sh new file mode 100755 index 0000000000..cae5a9e791 --- /dev/null +++ b/.github/workflows/mpirun_workaround.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash + +# mpiexec currently seems to have a bug where it tries to parse parameters +# of the launched application when they start with a dash, e.g. +# `mpiexec python ./openpmd-pipe --infile in.bp --outfile out.bp` +# leads to: +# > An unrecognized option was included on the mpiexec command line: +# > +# > Option: --infile +# > +# > Please use the "mpiexec --help" command to obtain a list of all +# > supported options. +# +# This script provides a workaround by putting the called sub-command into +# a script in a temporary file. + +mpiexec -n 1 ls --all \ + && echo "MPIRUN WORKING AGAIN, PLEASE REMOVE WORKAROUND" >&2 \ + && exit 1 \ + || true + +mpirun_args=() + +script_file="$(mktemp)" + +cleanup() { + rm "$script_file" +} +trap cleanup EXIT + +while true; do + case "$1" in + -c | -np | --np | -n | --n ) + mpirun_args+=("$1" "$2") + shift + shift + ;; + *) + break + ;; + esac +done + +echo -e '#!/usr/bin/env bash\n' > "$script_file" +for item in "$@"; do + echo -n "'$item' " >> "$script_file" +done + +chmod +x "$script_file" + +mpirun "${mpirun_args[@]}" "$script_file"