From b4cbe37ee03a3bf91e0b942b62bdec3eb107cffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 20 Mar 2023 14:39:08 +0100 Subject: [PATCH 01/22] Add helper: openPMD::auxiliary::overloaded --- include/openPMD/auxiliary/Variant.hpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/openPMD/auxiliary/Variant.hpp b/include/openPMD/auxiliary/Variant.hpp index 89190b882e..5b8eb2a305 100644 --- a/include/openPMD/auxiliary/Variant.hpp +++ b/include/openPMD/auxiliary/Variant.hpp @@ -90,5 +90,17 @@ namespace auxiliary private: resource m_data; }; + + /* + * Helper type for std::visit, + * see https://en.cppreference.com/w/cpp/utility/variant/visit + */ + template + struct overloaded : Ts... + { + using Ts::operator()...; + }; + template + overloaded(Ts...) -> overloaded; } // namespace auxiliary } // namespace openPMD From 16ff34602e10273a4be86027ae95f913fa3707a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 20 Mar 2023 16:03:02 +0100 Subject: [PATCH 02/22] Prepare Attributable for virtual inheritance Use only zero-param constructors to avoid diamond initialization pitfalls --- include/openPMD/Iteration.hpp | 14 +++++--- include/openPMD/RecordComponent.hpp | 16 ++++------ include/openPMD/Series.hpp | 25 ++++++++++----- include/openPMD/backend/Attributable.hpp | 10 +++--- include/openPMD/backend/BaseRecord.hpp | 26 ++++++--------- .../openPMD/backend/BaseRecordComponent.hpp | 18 +++++------ include/openPMD/backend/Container.hpp | 24 ++++++-------- .../openPMD/backend/PatchRecordComponent.hpp | 18 +++++------ src/Iteration.cpp | 4 +-- src/Record.cpp | 4 +-- src/RecordComponent.cpp | 17 +++------- src/Series.cpp | 32 +++++++------------ src/backend/Attributable.cpp | 22 ++++++++++--- src/backend/BaseRecordComponent.cpp | 12 +++---- src/backend/PatchRecordComponent.cpp | 16 +++------- src/backend/Writable.cpp | 5 +-- 16 files changed, 122 insertions(+), 141 deletions(-) diff --git a/include/openPMD/Iteration.hpp b/include/openPMD/Iteration.hpp index 49e8c3296c..3ede7a6a37 100644 --- a/include/openPMD/Iteration.hpp +++ b/include/openPMD/Iteration.hpp @@ -244,19 +244,25 @@ class Iteration : public Attributable private: Iteration(); - std::shared_ptr m_iterationData{ - new internal::IterationData}; + using Data_t = internal::IterationData; + std::shared_ptr m_iterationData; - inline internal::IterationData const &get() const + inline Data_t const &get() const { return *m_iterationData; } - inline internal::IterationData &get() + inline Data_t &get() { return *m_iterationData; } + inline void setData(std::shared_ptr data) + { + m_iterationData = std::move(data); + Attributable::setData(m_iterationData); + } + void flushFileBased( std::string const &, IterationIndex_t, internal::FlushParams const &); void flushGroupBased(IterationIndex_t, internal::FlushParams const &); diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 3443b27d7a..87b7915407 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -105,8 +105,6 @@ class RecordComponent : public BaseRecordComponent friend class ParticleSpecies; template friend class BaseRecord; - template - friend class BaseRecordInterface; friend class Record; friend class Mesh; template @@ -431,23 +429,21 @@ class RecordComponent : public BaseRecordComponent */ bool dirtyRecursive() const; - std::shared_ptr m_recordComponentData{ - new internal::RecordComponentData()}; - - RecordComponent(); - // clang-format off OPENPMD_protected // clang-format on - RecordComponent(std::shared_ptr); + using Data_t = internal::RecordComponentData; + std::shared_ptr m_recordComponentData; + + RecordComponent(); - inline internal::RecordComponentData const &get() const + inline Data_t const &get() const { return *m_recordComponentData; } - inline internal::RecordComponentData &get() + inline Data_t &get() { return *m_recordComponentData; } diff --git a/include/openPMD/Series.hpp b/include/openPMD/Series.hpp index 094ed24fbf..abc6bdf0ff 100644 --- a/include/openPMD/Series.hpp +++ b/include/openPMD/Series.hpp @@ -64,8 +64,13 @@ namespace internal * * (Not movable or copyable) * + * Class is final since our std::shared_ptr pattern has the little + * disadvantage that child constructors overwrite the parent constructors. + * Since the SeriesData constructor does some initialization, making this + * class final avoids stumbling over this pitfall. + * */ - class SeriesData : public AttributableData + class SeriesData final : public AttributableData { public: explicit SeriesData() = default; @@ -212,10 +217,6 @@ class Series : public Attributable friend class internal::SeriesData; friend class WriteIterations; -protected: - // Should not be called publicly, only by implementing classes - Series(std::shared_ptr); - public: explicit Series(); @@ -585,9 +586,10 @@ OPENPMD_private using iterations_t = decltype(internal::SeriesData::iterations); using iterations_iterator = iterations_t::iterator; - std::shared_ptr m_series = nullptr; + using Data_t = internal::SeriesData; + std::shared_ptr m_series = nullptr; - inline internal::SeriesData &get() + inline Data_t &get() { if (m_series) { @@ -600,7 +602,7 @@ OPENPMD_private } } - inline internal::SeriesData const &get() const + inline Data_t const &get() const { if (m_series) { @@ -613,6 +615,13 @@ OPENPMD_private } } + inline void setData(std::shared_ptr series) + { + m_series = std::move(series); + iterations = m_series->iterations; + Attributable::setData(m_series); + } + std::unique_ptr parseInput(std::string); /** * @brief Parse non-backend-specific configuration in JSON config. diff --git a/include/openPMD/backend/Attributable.hpp b/include/openPMD/backend/Attributable.hpp index 2b45ac0e45..1a247722c4 100644 --- a/include/openPMD/backend/Attributable.hpp +++ b/include/openPMD/backend/Attributable.hpp @@ -111,14 +111,16 @@ class Attributable friend class WriteIterations; protected: - std::shared_ptr m_attri{ - new internal::AttributableData()}; + // tag for internal constructor + struct NoInit + {}; - // Should not be called publicly, only by implementing classes - Attributable(std::shared_ptr); + using Data_t = internal::AttributableData; + std::shared_ptr m_attri; public: Attributable(); + Attributable(NoInit); virtual ~Attributable() = default; diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index b35709b240..d775f81074 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -63,15 +63,15 @@ class BaseRecord : public Container friend class Record; friend class Mesh; - std::shared_ptr > m_baseRecordData{ - new internal::BaseRecordData()}; + using Data_t = internal::BaseRecordData; + std::shared_ptr m_baseRecordData; - inline internal::BaseRecordData &get() + inline Data_t const &get() const { return *m_baseRecordData; } - inline internal::BaseRecordData const &get() const + inline Data_t &get() { return *m_baseRecordData; } @@ -79,9 +79,7 @@ class BaseRecord : public Container BaseRecord(); protected: - BaseRecord(std::shared_ptr >); - - inline void setData(internal::BaseRecordData *data) + inline void setData(std::shared_ptr data) { m_baseRecordData = std::move(data); Container::setData(m_baseRecordData); @@ -137,7 +135,6 @@ class BaseRecord : public Container bool scalar() const; protected: - BaseRecord(internal::BaseRecordData *); void readBase(); private: @@ -164,7 +161,8 @@ namespace internal template BaseRecordData::BaseRecordData() { - Attributable impl{{this, [](auto const *) {}}}; + Attributable impl; + impl.setData({this, [](auto const *) {}}); impl.setAttribute( "unitDimension", std::array{{0., 0., 0., 0., 0., 0., 0.}}); @@ -172,17 +170,11 @@ namespace internal } // namespace internal template -BaseRecord::BaseRecord() : Container{nullptr} +BaseRecord::BaseRecord() : Container(Attributable::NoInit()) { - Container::setData(m_baseRecordData); + setData(std::make_shared()); } -template -BaseRecord::BaseRecord( - std::shared_ptr > data) - : Container{data}, m_baseRecordData{std::move(data)} -{} - template inline typename BaseRecord::mapped_type & BaseRecord::operator[](key_type const &key) diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index fd8279eb05..f62056adcf 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -35,7 +35,7 @@ namespace openPMD { namespace internal { - class BaseRecordComponentData : public AttributableData + class BaseRecordComponentData : virtual public AttributableData { public: /** @@ -61,7 +61,7 @@ namespace internal }; } // namespace internal -class BaseRecordComponent : public Attributable +class BaseRecordComponent : virtual public Attributable { template friend class Container; @@ -102,29 +102,27 @@ class BaseRecordComponent : public Attributable ChunkTable availableChunks(); protected: - std::shared_ptr - m_baseRecordComponentData{new internal::BaseRecordComponentData()}; + using Data_t = internal::BaseRecordComponentData; + std::shared_ptr m_baseRecordComponentData; - inline internal::BaseRecordComponentData const &get() const + inline Data_t const &get() const { return *m_baseRecordComponentData; } - inline internal::BaseRecordComponentData &get() + inline Data_t &get() { return *m_baseRecordComponentData; } - inline void setData(std::shared_ptr data) + inline void setData(std::shared_ptr data) { m_baseRecordComponentData = std::move(data); Attributable::setData(m_baseRecordComponentData); } - BaseRecordComponent(std::shared_ptr); - -private: BaseRecordComponent(); + BaseRecordComponent(NoInit); }; // BaseRecordComponent namespace detail diff --git a/include/openPMD/backend/Container.hpp b/include/openPMD/backend/Container.hpp index c697593a12..32a31809cb 100644 --- a/include/openPMD/backend/Container.hpp +++ b/include/openPMD/backend/Container.hpp @@ -65,7 +65,7 @@ namespace internal typename T, typename T_key = std::string, typename T_container = std::map > - class ContainerData : public AttributableData + class ContainerData : virtual public AttributableData { public: using InternalContainer = T_container; @@ -128,7 +128,7 @@ template < typename T, typename T_key = std::string, typename T_container = std::map > -class Container : public Attributable +class Container : virtual public Attributable { static_assert( std::is_base_of::value, @@ -147,7 +147,7 @@ class Container : public Attributable using ContainerData = internal::ContainerData; using InternalContainer = T_container; - std::shared_ptr m_containerData{new ContainerData()}; + std::shared_ptr m_containerData; inline void setData(std::shared_ptr containerData) { @@ -428,10 +428,6 @@ class Container : public Attributable OPENPMD_protected // clang-format on - Container(std::shared_ptr containerData) - : Attributable{containerData}, m_containerData{std::move(containerData)} - {} - void clear_unchecked() { if (written()) @@ -454,14 +450,13 @@ OPENPMD_protected flushAttributes(flushParams); } - // clang-format off -OPENPMD_private - // clang-format on - - Container() : Attributable{nullptr} + Container() : Attributable(NoInit()) { - Attributable::setData(m_containerData); + setData(std::make_shared()); } + + Container(NoInit) : Attributable(NoInit()) + {} }; namespace internal @@ -532,7 +527,8 @@ namespace internal ~EraseStaleEntries() { auto &map = m_originalContainer.container(); - using iterator_t = typename BareContainer_t::const_iterator; + using iterator_t = + typename BareContainer_t::InternalContainer::const_iterator; std::vector deleteMe; deleteMe.reserve(map.size() - m_accessedKeys.size()); for (iterator_t it = map.begin(); it != map.end(); ++it) diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index be5b37ec26..41f3239d3f 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -112,29 +112,27 @@ OPENPMD_private */ bool dirtyRecursive() const; - std::shared_ptr - m_patchRecordComponentData{new internal::PatchRecordComponentData()}; - - PatchRecordComponent(); - // clang-format off OPENPMD_protected // clang-format on - PatchRecordComponent(std::shared_ptr); + using Data_t = internal::PatchRecordComponentData; + + std::shared_ptr m_patchRecordComponentData; + + PatchRecordComponent(); - inline internal::PatchRecordComponentData const &get() const + inline Data_t const &get() const { return *m_patchRecordComponentData; } - inline internal::PatchRecordComponentData &get() + inline Data_t &get() { return *m_patchRecordComponentData; } - inline void - setData(std::shared_ptr data) + inline void setData(std::shared_ptr data) { m_patchRecordComponentData = std::move(data); BaseRecordComponent::setData(m_patchRecordComponentData); diff --git a/src/Iteration.cpp b/src/Iteration.cpp index 9e402f419d..339928d4f3 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -36,9 +36,9 @@ namespace openPMD using internal::CloseStatus; using internal::DeferredParseAccess; -Iteration::Iteration() : Attributable{nullptr} +Iteration::Iteration() : Attributable(NoInit()) { - Attributable::setData(m_iterationData); + setData(std::make_shared()); setTime(static_cast(0)); setDt(static_cast(1)); setTimeUnitSI(1); diff --git a/src/Record.cpp b/src/Record.cpp index dc6949efdb..b886af2cd4 100644 --- a/src/Record.cpp +++ b/src/Record.cpp @@ -176,7 +176,5 @@ void Record::read() readAttributes(ReadMode::FullyReread); } -template <> -BaseRecord::mapped_type & -BaseRecord::operator[](std::string &&key); +template class BaseRecord; } // namespace openPMD diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 2d72f4c150..fcee8cdb3d 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -37,24 +37,15 @@ namespace openPMD { namespace internal { - RecordComponentData::RecordComponentData() - { - RecordComponent impl{ - std::shared_ptr{this, [](auto const *) {}}}; - impl.setUnitSI(1); - } + RecordComponentData::RecordComponentData() = default; } // namespace internal -RecordComponent::RecordComponent() : BaseRecordComponent{nullptr} +RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) { - BaseRecordComponent::setData(m_recordComponentData); + setData(std::make_shared()); + setUnitSI(1); } -RecordComponent::RecordComponent( - std::shared_ptr data) - : BaseRecordComponent{data}, m_recordComponentData{std::move(data)} -{} - // We need to instantiate this somewhere otherwise there might be linker issues // despite this thing actually being constepxr constexpr char const *const RecordComponent::SCALAR; diff --git a/src/Series.cpp b/src/Series.cpp index b7c807eb4c..3719952c79 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -2292,7 +2292,8 @@ namespace internal * `Series` is needlessly flushed a second time. Otherwise, error * messages can get very confusing. */ - Series impl{{this, [](auto const *) {}}}; + Series impl; + impl.setData({this, [](auto const *) {}}); if (auto IOHandler = impl.IOHandler(); IOHandler && IOHandler->m_lastFlushSuccessful) { @@ -2319,25 +2320,18 @@ namespace internal } } // namespace internal -Series::Series() : Attributable{nullptr}, iterations{} +Series::Series() : Attributable(NoInit()), iterations{} {} -Series::Series(std::shared_ptr data) - : Attributable{data}, m_series{std::move(data)} -{ - iterations = m_series->iterations; -} - #if openPMD_HAVE_MPI Series::Series( std::string const &filepath, Access at, MPI_Comm comm, std::string const &options) - : Attributable{nullptr}, m_series{new internal::SeriesData} + : Attributable(NoInit()) { - Attributable::setData(m_series); - iterations = m_series->iterations; + setData(std::make_shared()); json::TracingJSON optionsJson = json::parseOptions(options, comm, /* considerFiles = */ true); auto input = parseInput(filepath); @@ -2357,10 +2351,9 @@ Series::Series( Series::Series( std::string const &filepath, Access at, std::string const &options) - : Attributable{nullptr}, m_series{new internal::SeriesData} + : Attributable(NoInit()) { - Attributable::setData(m_series); - iterations = m_series->iterations; + setData(std::make_shared()); json::TracingJSON optionsJson = json::parseOptions(options, /* considerFiles = */ true); auto input = parseInput(filepath); @@ -2378,17 +2371,17 @@ Series::Series( Series::operator bool() const { - return m_series.operator bool(); + return m_attri.operator bool(); } ReadIterations Series::readIterations() { // Use private constructor instead of copy constructor to avoid // object slicing - return { - Series(this->m_series), - IOHandler()->m_frontendAccess, - get().m_parsePreference}; + Series res; + res.setData(std::dynamic_pointer_cast(this->m_attri)); + return ReadIterations{ + std::move(res), IOHandler()->m_frontendAccess, get().m_parsePreference}; } void Series::parseBase() @@ -2409,7 +2402,6 @@ WriteIterations Series::writeIterations() void Series::close() { get().close(); - m_series.reset(); m_attri.reset(); } diff --git a/src/backend/Attributable.cpp b/src/backend/Attributable.cpp index 52ddda28d3..00e41f5d63 100644 --- a/src/backend/Attributable.cpp +++ b/src/backend/Attributable.cpp @@ -37,10 +37,17 @@ namespace internal {} } // namespace internal -Attributable::Attributable() = default; +Attributable::Attributable() +{ + // Might already be initialized by inheriting classes due to virtual + // inheritance + if (!m_attri) + { + m_attri = std::make_shared(); + } +} -Attributable::Attributable(std::shared_ptr attri) - : m_attri{std::move(attri)} +Attributable::Attributable(NoInit) {} Attribute Attributable::getAttribute(std::string const &key) const @@ -120,7 +127,10 @@ Series Attributable::retrieveSeries() const } auto seriesData = &auxiliary::deref_dynamic_cast( findSeries->attributable); - return Series{{seriesData, [](auto const *) {}}}; + Series res; + res.setData( + std::shared_ptr{seriesData, [](auto const *) {}}); + return res; } Iteration const &Attributable::containingIteration() const @@ -199,7 +209,9 @@ auto Attributable::myPath() const -> MyPath std::reverse(res.group.begin(), res.group.end()); auto &seriesData = auxiliary::deref_dynamic_cast( findSeries->attributable); - Series series{{&seriesData, [](auto const *) {}}}; + Series series; + series.setData(std::shared_ptr{ + &seriesData, [](auto const *) {}}); res.seriesName = series.name(); res.seriesExtension = suffix(seriesData.m_format); res.directory = IOHandler()->directory; diff --git a/src/backend/BaseRecordComponent.cpp b/src/backend/BaseRecordComponent.cpp index 79890b95ab..9c4216e051 100644 --- a/src/backend/BaseRecordComponent.cpp +++ b/src/backend/BaseRecordComponent.cpp @@ -85,13 +85,11 @@ ChunkTable BaseRecordComponent::availableChunks() return std::move(*param.chunks); } -BaseRecordComponent::BaseRecordComponent( - std::shared_ptr data) - : Attributable{data}, m_baseRecordComponentData{std::move(data)} -{} - -BaseRecordComponent::BaseRecordComponent() : Attributable{nullptr} +BaseRecordComponent::BaseRecordComponent() : Attributable(NoInit()) { - Attributable::setData(m_baseRecordComponentData); + setData(std::make_shared()); } + +BaseRecordComponent::BaseRecordComponent(NoInit) : Attributable(NoInit()) +{} } // namespace openPMD diff --git a/src/backend/PatchRecordComponent.cpp b/src/backend/PatchRecordComponent.cpp index d5df665084..2710e96c45 100644 --- a/src/backend/PatchRecordComponent.cpp +++ b/src/backend/PatchRecordComponent.cpp @@ -27,11 +27,7 @@ namespace openPMD { namespace internal { - PatchRecordComponentData::PatchRecordComponentData() - { - PatchRecordComponent impl{{this, [](auto const *) {}}}; - impl.setUnitSI(1); - } + PatchRecordComponentData::PatchRecordComponentData() = default; } // namespace internal PatchRecordComponent &PatchRecordComponent::setUnitSI(double usi) @@ -78,16 +74,12 @@ Extent PatchRecordComponent::getExtent() const } } -PatchRecordComponent::PatchRecordComponent() : BaseRecordComponent{nullptr} +PatchRecordComponent::PatchRecordComponent() : BaseRecordComponent(NoInit()) { - BaseRecordComponent::setData(m_patchRecordComponentData); + setData(std::make_shared()); + setUnitSI(1); } -PatchRecordComponent::PatchRecordComponent( - std::shared_ptr data) - : BaseRecordComponent{data}, m_patchRecordComponentData{std::move(data)} -{} - void PatchRecordComponent::flush( std::string const &name, internal::FlushParams const &flushParams) { diff --git a/src/backend/Writable.cpp b/src/backend/Writable.cpp index 97d13ce5f0..c294f8efc8 100644 --- a/src/backend/Writable.cpp +++ b/src/backend/Writable.cpp @@ -49,8 +49,9 @@ void Writable::seriesFlush(std::string backendConfig) void Writable::seriesFlush(internal::FlushParams const &flushParams) { - auto series = - Attributable({attributable, [](auto const *) {}}).retrieveSeries(); + Attributable impl; + impl.setData({attributable, [](auto const *) {}}); + auto series = impl.retrieveSeries(); series.flush_impl( series.iterations.begin(), series.iterations.end(), flushParams); } From e8daa0de9c49bbb706f15a5e6328d0860f8587e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 12 Apr 2023 16:41:32 +0200 Subject: [PATCH 03/22] Fix default constructors/operators of Container and BaseRecordComponent These derive virtually from Attributable, and this avoids that pitfalls propagate to user code. --- .../openPMD/backend/BaseRecordComponent.hpp | 45 +++++++++++++++++++ include/openPMD/backend/Container.hpp | 45 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index f62056adcf..a6d4a5b70f 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -67,6 +67,51 @@ class BaseRecordComponent : virtual public Attributable friend class Container; public: + /* + * Need to define these manually due to the virtual inheritance from + * Attributable. + * Otherwise, they would only run from the most derived class + * if explicitly called. + * If not defining these, a user could destroy copy/move constructors/ + * assignment operators by deriving from any class that has a virtual + * Attributable somewhere. + * Care must be taken in move constructors/assignment operators to not move + * multiple times (which could happen in diamond inheritance situations). + */ + + BaseRecordComponent(BaseRecordComponent const &other) + : Attributable(NoInit()) + { + m_attri = other.m_attri; + m_baseRecordComponentData = other.m_baseRecordComponentData; + } + + BaseRecordComponent(BaseRecordComponent &&other) : Attributable(NoInit()) + { + if (other.m_attri) + { + m_attri = std::move(other.m_attri); + } + m_baseRecordComponentData = std::move(other.m_baseRecordComponentData); + } + + BaseRecordComponent &operator=(BaseRecordComponent const &other) + { + m_attri = other.m_attri; + m_baseRecordComponentData = other.m_baseRecordComponentData; + return *this; + } + + BaseRecordComponent &operator=(BaseRecordComponent &&other) + { + if (other.m_attri) + { + m_attri = std::move(other.m_attri); + } + m_baseRecordComponentData = std::move(other.m_baseRecordComponentData); + return *this; + } + double unitSI() const; BaseRecordComponent &resetDatatype(Datatype); diff --git a/include/openPMD/backend/Container.hpp b/include/openPMD/backend/Container.hpp index 32a31809cb..6671c62c9a 100644 --- a/include/openPMD/backend/Container.hpp +++ b/include/openPMD/backend/Container.hpp @@ -457,6 +457,51 @@ OPENPMD_protected Container(NoInit) : Attributable(NoInit()) {} + +public: + /* + * Need to define these manually due to the virtual inheritance from + * Attributable. + * Otherwise, they would only run from the most derived class + * if explicitly called. + * If not defining these, a user could destroy copy/move constructors/ + * assignment operators by deriving from any class that has a virtual + * Attributable somewhere. + * Care must be taken in move constructors/assignment operators to not move + * multiple times (which could happen in diamond inheritance situations). + */ + + Container(Container const &other) : Attributable(NoInit()) + { + m_attri = other.m_attri; + m_containerData = other.m_containerData; + } + + Container(Container &&other) : Attributable(NoInit()) + { + if (other.m_attri) + { + m_attri = std::move(other.m_attri); + } + m_containerData = std::move(other.m_containerData); + } + + Container &operator=(Container const &other) + { + m_attri = other.m_attri; + m_containerData = other.m_containerData; + return *this; + } + + Container &operator=(Container &&other) + { + if (other.m_attri) + { + m_attri = std::move(other.m_attri); + } + m_containerData = std::move(other.m_containerData); + return *this; + } }; namespace internal From 44db18dcaa09774a4529444cc0e8c655007c7c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 20 Mar 2023 16:46:29 +0100 Subject: [PATCH 04/22] Add m_datasetDefined See in-code documentation. --- include/openPMD/RecordComponent.hpp | 3 +++ include/openPMD/backend/BaseRecordComponent.hpp | 11 +++++++++++ include/openPMD/backend/PatchRecordComponent.hpp | 3 +++ src/backend/BaseRecordComponent.cpp | 12 ++++++++++++ 4 files changed, 29 insertions(+) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 87b7915407..cb22b6a336 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -440,11 +440,14 @@ OPENPMD_protected inline Data_t const &get() const { + // cannot call this in the const overload + // datasetDefined(*m_recordComponentData); return *m_recordComponentData; } inline Data_t &get() { + setDatasetDefined(*m_recordComponentData); return *m_recordComponentData; } diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index a6d4a5b70f..75ba5e8448 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -49,6 +49,13 @@ namespace internal * instead defined via light-weight attributes. */ bool m_isConstant = false; + /** + * Tracks if there was any write access to the record component. + * Necessary in BaseRecord to track if the scalar component has been + * used and is used by BaseRecord to determine the return value of + * the BaseRecord::scalar() method. + */ + bool m_datasetDefined = false; BaseRecordComponentData(BaseRecordComponentData const &) = delete; BaseRecordComponentData(BaseRecordComponentData &&) = delete; @@ -166,6 +173,10 @@ class BaseRecordComponent : virtual public Attributable Attributable::setData(m_baseRecordComponentData); } + void setDatasetDefined(Data_t &); + + bool datasetDefined() const; + BaseRecordComponent(); BaseRecordComponent(NoInit); }; // BaseRecordComponent diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index 41f3239d3f..f2e3540f29 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -124,11 +124,14 @@ OPENPMD_protected inline Data_t const &get() const { + // cannot call this in the const overload + // setDatasetDefined(*m_recordComponentData); return *m_patchRecordComponentData; } inline Data_t &get() { + setDatasetDefined(*m_patchRecordComponentData); return *m_patchRecordComponentData; } diff --git a/src/backend/BaseRecordComponent.cpp b/src/backend/BaseRecordComponent.cpp index 9c4216e051..4de7eb797e 100644 --- a/src/backend/BaseRecordComponent.cpp +++ b/src/backend/BaseRecordComponent.cpp @@ -92,4 +92,16 @@ BaseRecordComponent::BaseRecordComponent() : Attributable(NoInit()) BaseRecordComponent::BaseRecordComponent(NoInit) : Attributable(NoInit()) {} + +void BaseRecordComponent::setDatasetDefined( + internal::BaseRecordComponentData &data) +{ + data.m_datasetDefined = true; +} + +bool BaseRecordComponent::datasetDefined() const +{ + auto & data = get(); + return data.m_datasetDefined; +} } // namespace openPMD From e1c34ef4503f9790d8909cd329760473c78aa4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 20 Mar 2023 19:12:39 +0100 Subject: [PATCH 05/22] Prepare class structure without applying logic yet BaseRecord is now derived by its contained RecordComponent type. If it is scalar, the idea is that the BaseRecord itself is used as a RecordComponent, without needing to retrieve the [SCALAR] entry. No logic implemented yet around this, this just prepares the class structure. Notice that this will write some unnecessary attributes since the RecordComponent types initialize some default attributes upon construction. --- include/openPMD/ParticleSpecies.hpp | 1 + include/openPMD/RecordComponent.hpp | 7 ++- include/openPMD/backend/Attributable.hpp | 4 +- include/openPMD/backend/BaseRecord.hpp | 49 ++++++++++++++----- include/openPMD/backend/Container.hpp | 1 + .../openPMD/backend/MeshRecordComponent.hpp | 3 ++ .../openPMD/backend/PatchRecordComponent.hpp | 6 +++ src/ReadIterations.cpp | 3 +- src/RecordComponent.cpp | 3 ++ src/backend/MeshRecordComponent.cpp | 3 ++ src/backend/PatchRecordComponent.cpp | 4 ++ 11 files changed, 67 insertions(+), 17 deletions(-) diff --git a/include/openPMD/ParticleSpecies.hpp b/include/openPMD/ParticleSpecies.hpp index 0257cd474f..eb2a2bda9b 100644 --- a/include/openPMD/ParticleSpecies.hpp +++ b/include/openPMD/ParticleSpecies.hpp @@ -61,6 +61,7 @@ namespace traits template <> struct GenerationPolicy { + constexpr static bool is_noop = false; template void operator()(T &ret) { diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index cb22b6a336..11773902d9 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -95,6 +95,8 @@ namespace internal */ bool m_hasBeenExtended = false; }; + template + class BaseRecordData; } // namespace internal class RecordComponent : public BaseRecordComponent @@ -103,8 +105,10 @@ class RecordComponent : public BaseRecordComponent friend class Container; friend class Iteration; friend class ParticleSpecies; - template + template friend class BaseRecord; + template + friend class internal::BaseRecordData; friend class Record; friend class Mesh; template @@ -437,6 +441,7 @@ OPENPMD_protected std::shared_ptr m_recordComponentData; RecordComponent(); + RecordComponent(NoInit); inline Data_t const &get() const { diff --git a/include/openPMD/backend/Attributable.hpp b/include/openPMD/backend/Attributable.hpp index 1a247722c4..c7b92b8b44 100644 --- a/include/openPMD/backend/Attributable.hpp +++ b/include/openPMD/backend/Attributable.hpp @@ -81,7 +81,7 @@ namespace internal A_MAP m_attributes; }; - template + template class BaseRecordData; } // namespace internal @@ -99,7 +99,7 @@ class Attributable friend class BaseRecord; template friend class BaseRecordInterface; - template + template friend class internal::BaseRecordData; template friend class Container; diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index d775f81074..42213d711c 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -32,8 +32,16 @@ namespace openPMD { namespace internal { - template - class BaseRecordData : public ContainerData + template < + typename T_elem, // = T_RecordComponent + /* + * Technically not necessary, but some old compilers ignore friend + * declarations at this place, so we specify the data class explicitly + */ + typename T_RecordComponentData = typename T_elem::Data_t> + class BaseRecordData final + : public ContainerData + , public T_RecordComponentData { public: /** @@ -55,17 +63,29 @@ namespace internal } // namespace internal template -class BaseRecord : public Container +class BaseRecord + : public Container + , public T_elem // T_RecordComponent { + using T_RecordComponent = T_elem; + using T_Container = Container; + using T_Self = BaseRecord; friend class Iteration; friend class ParticleSpecies; friend class PatchRecord; friend class Record; friend class Mesh; + template + friend class internal::BaseRecordData; - using Data_t = internal::BaseRecordData; + using Data_t = + internal::BaseRecordData; std::shared_ptr m_baseRecordData; + static_assert( + traits::GenerationPolicy::is_noop, + "Internal error: Scalar components cannot have generation policies."); + inline Data_t const &get() const { return *m_baseRecordData; @@ -82,7 +102,8 @@ class BaseRecord : public Container inline void setData(std::shared_ptr data) { m_baseRecordData = std::move(data); - Container::setData(m_baseRecordData); + T_Container::setData(m_baseRecordData); + T_RecordComponent::setData(m_baseRecordData); } public: @@ -96,8 +117,8 @@ class BaseRecord : public Container using const_reference = typename Container::const_reference; using pointer = typename Container::pointer; using const_pointer = typename Container::const_pointer; - using iterator = typename Container::iterator; - using const_iterator = typename Container::const_iterator; + using iterator = typename T_Container::iterator; + using const_iterator = typename T_Container::const_iterator; virtual ~BaseRecord() = default; @@ -158,8 +179,8 @@ class BaseRecord : public Container namespace internal { - template - BaseRecordData::BaseRecordData() + template + BaseRecordData::BaseRecordData() { Attributable impl; impl.setData({this, [](auto const *) {}}); @@ -170,7 +191,9 @@ namespace internal } // namespace internal template -BaseRecord::BaseRecord() : Container(Attributable::NoInit()) +BaseRecord::BaseRecord() + : T_Container(Attributable::NoInit()) + , T_RecordComponent(Attributable::NoInit()) { setData(std::make_shared()); } @@ -291,7 +314,7 @@ template inline std::array BaseRecord::unitDimension() const { return this->getAttribute("unitDimension") - .template get >(); + .template get>(); } template @@ -310,7 +333,7 @@ inline void BaseRecord::readBase() this->IOHandler()->enqueue(IOTask(this, aRead)); this->IOHandler()->flush(internal::defaultFlushParams); if (auto val = - Attribute(*aRead.resource).getOptional >(); + Attribute(*aRead.resource).getOptional>(); val.has_value()) this->setAttribute("unitDimension", val.value()); else @@ -339,7 +362,7 @@ template inline void BaseRecord::flush( std::string const &name, internal::FlushParams const &flushParams) { - if (!this->written() && this->empty()) + if (!this->written() && this->T_Container::empty()) throw std::runtime_error( "A Record can not be written without any contained " "RecordComponents: " + diff --git a/include/openPMD/backend/Container.hpp b/include/openPMD/backend/Container.hpp index 6671c62c9a..c0df662a9f 100644 --- a/include/openPMD/backend/Container.hpp +++ b/include/openPMD/backend/Container.hpp @@ -49,6 +49,7 @@ namespace traits template struct GenerationPolicy { + constexpr static bool is_noop = true; template void operator()(T &) {} diff --git a/include/openPMD/backend/MeshRecordComponent.hpp b/include/openPMD/backend/MeshRecordComponent.hpp index 20f5a9b42a..4623303e35 100644 --- a/include/openPMD/backend/MeshRecordComponent.hpp +++ b/include/openPMD/backend/MeshRecordComponent.hpp @@ -30,11 +30,14 @@ class MeshRecordComponent : public RecordComponent { template friend class Container; + template + friend class BaseRecord; friend class Mesh; private: MeshRecordComponent(); + MeshRecordComponent(NoInit); void read() override; public: diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index f2e3540f29..bfa4fb979d 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -56,6 +56,9 @@ namespace internal PatchRecordComponentData(); }; + + template + class BaseRecordData; } // namespace internal /** @@ -67,6 +70,8 @@ class PatchRecordComponent : public BaseRecordComponent friend class Container; template friend class BaseRecord; + template + friend class internal::BaseRecordData; friend class ParticlePatches; friend class PatchRecord; friend class ParticleSpecies; @@ -121,6 +126,7 @@ OPENPMD_protected std::shared_ptr m_patchRecordComponentData; PatchRecordComponent(); + PatchRecordComponent(NoInit); inline Data_t const &get() const { diff --git a/src/ReadIterations.cpp b/src/ReadIterations.cpp index 9457a22421..74e963eb55 100644 --- a/src/ReadIterations.cpp +++ b/src/ReadIterations.cpp @@ -137,7 +137,8 @@ SeriesIterator::SeriesIterator( * This is ok due to the usual C++ iterator invalidation workflows * (deleting the original container invalidates the iterator). */ - data.series = Series(std::shared_ptr( + data.series = Series(); + data.series->setData(std::shared_ptr( series_in.m_series.get(), [](auto const *) {})); auto &series = data.series.value(); if (series.IOHandler()->m_frontendAccess == Access::READ_LINEAR && diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index fcee8cdb3d..e8deac4864 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -46,6 +46,9 @@ RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) setUnitSI(1); } +RecordComponent::RecordComponent(NoInit) : BaseRecordComponent(NoInit()) +{} + // We need to instantiate this somewhere otherwise there might be linker issues // despite this thing actually being constepxr constexpr char const *const RecordComponent::SCALAR; diff --git a/src/backend/MeshRecordComponent.cpp b/src/backend/MeshRecordComponent.cpp index e7eef6e047..50d300b1a0 100644 --- a/src/backend/MeshRecordComponent.cpp +++ b/src/backend/MeshRecordComponent.cpp @@ -27,6 +27,9 @@ MeshRecordComponent::MeshRecordComponent() : RecordComponent() setPosition(std::vector{0}); } +MeshRecordComponent::MeshRecordComponent(NoInit) : RecordComponent(NoInit()) +{} + void MeshRecordComponent::read() { using DT = Datatype; diff --git a/src/backend/PatchRecordComponent.cpp b/src/backend/PatchRecordComponent.cpp index 2710e96c45..e319d99e04 100644 --- a/src/backend/PatchRecordComponent.cpp +++ b/src/backend/PatchRecordComponent.cpp @@ -80,6 +80,10 @@ PatchRecordComponent::PatchRecordComponent() : BaseRecordComponent(NoInit()) setUnitSI(1); } +PatchRecordComponent::PatchRecordComponent(NoInit) + : BaseRecordComponent(NoInit()) +{} + void PatchRecordComponent::flush( std::string const &name, internal::FlushParams const &flushParams) { From 52a4d4eefd86e28df062d88616973c9e4d273002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 22 Mar 2023 21:14:51 +0100 Subject: [PATCH 06/22] No longer use map entry SCALAR in application logic Not yet supported: Backward compatibility for still allowing legacy access to scalar entries --- include/openPMD/ParticleSpecies.hpp | 8 +- include/openPMD/backend/BaseRecord.hpp | 87 +++++++++++-------- .../openPMD/backend/MeshRecordComponent.hpp | 1 + .../openPMD/backend/PatchRecordComponent.hpp | 2 +- src/Iteration.cpp | 6 +- src/Mesh.cpp | 28 +++--- src/ParticlePatches.cpp | 8 +- src/ParticleSpecies.cpp | 8 +- src/Record.cpp | 28 +++--- src/RecordComponent.cpp | 5 +- src/backend/BaseRecordComponent.cpp | 2 +- src/backend/MeshRecordComponent.cpp | 15 +++- src/backend/PatchRecord.cpp | 4 +- src/backend/PatchRecordComponent.cpp | 4 + test/SerialIOTest.cpp | 2 +- 15 files changed, 110 insertions(+), 98 deletions(-) diff --git a/include/openPMD/ParticleSpecies.hpp b/include/openPMD/ParticleSpecies.hpp index eb2a2bda9b..808412d0b9 100644 --- a/include/openPMD/ParticleSpecies.hpp +++ b/include/openPMD/ParticleSpecies.hpp @@ -68,13 +68,9 @@ namespace traits ret.particlePatches.linkHierarchy(ret.writable()); auto &np = ret.particlePatches["numParticles"]; - auto &npc = np[RecordComponent::SCALAR]; - npc.resetDataset(Dataset(determineDatatype(), {1})); - npc.parent() = np.parent(); + np.resetDataset(Dataset(determineDatatype(), {1})); auto &npo = ret.particlePatches["numParticlesOffset"]; - auto &npoc = npo[RecordComponent::SCALAR]; - npoc.resetDataset(Dataset(determineDatatype(), {1})); - npoc.parent() = npo.parent(); + npo.resetDataset(Dataset(determineDatatype(), {1})); } }; } // namespace traits diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index 42213d711c..f4ef05874e 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -44,14 +44,6 @@ namespace internal , public T_RecordComponentData { public: - /** - * True if this Record contains a scalar record component. - * If so, then that record component is the only component contained, - * and the last hierarchical layer is skipped (i.e. only one OPEN_PATH - * task for Record and RecordComponent). - */ - bool m_containsScalar = false; - BaseRecordData(); BaseRecordData(BaseRecordData const &) = delete; @@ -124,6 +116,8 @@ class BaseRecord mapped_type &operator[](key_type const &key) override; mapped_type &operator[](key_type &&key) override; + mapped_type &at(key_type const &key); + mapped_type const &at(key_type const &key) const; size_type erase(key_type const &key) override; iterator erase(iterator res) override; //! @todo add also, as soon as added in Container: @@ -162,7 +156,6 @@ class BaseRecord void flush(std::string const &, internal::FlushParams const &) final; virtual void flush_impl(std::string const &, internal::FlushParams const &) = 0; - virtual void read() = 0; /** * @brief Check recursively whether this BaseRecord is dirty. @@ -214,12 +207,15 @@ BaseRecord::operator[](key_type const &key) "A scalar component can not be contained at " "the same time as one or more regular components."); - mapped_type &ret = Container::operator[](key); if (keyScalar) { - get().m_containsScalar = true; - ret.parent() = this->parent(); + /* + * This activates the RecordComponent API of this object. + */ + T_RecordComponent::get(); } + mapped_type &ret = keyScalar ? static_cast(*this) + : T_Container::operator[](key); return ret; } } @@ -240,16 +236,45 @@ BaseRecord::operator[](key_type &&key) "A scalar component can not be contained at " "the same time as one or more regular components."); - mapped_type &ret = Container::operator[](std::move(key)); if (keyScalar) { - get().m_containsScalar = true; - ret.parent() = this->parent(); + /* + * This activates the RecordComponent API of this object. + */ + T_RecordComponent::get(); } + mapped_type &ret = keyScalar ? static_cast(*this) + : T_Container::operator[](std::move(key)); return ret; } } +template +auto BaseRecord::at(key_type const &key) -> mapped_type & +{ + return const_cast( + static_cast const *>(this)->at(key)); +} + +template +auto BaseRecord::at(key_type const &key) const -> mapped_type const & +{ + bool const keyScalar = (key == RecordComponent::SCALAR); + if (keyScalar) + { + if (!get().m_datasetDefined) + { + throw std::out_of_range( + "[at()] Requested scalar entry from non-scalar record."); + } + return static_cast(*this); + } + else + { + return T_Container::at(key); + } +} + template inline typename BaseRecord::size_type BaseRecord::erase(key_type const &key) @@ -260,22 +285,21 @@ BaseRecord::erase(key_type const &key) res = Container::erase(key); else { - mapped_type &rc = this->find(RecordComponent::SCALAR)->second; - if (rc.written()) + if (this->written()) { Parameter dDelete; dDelete.name = "."; - this->IOHandler()->enqueue(IOTask(&rc, dDelete)); + this->IOHandler()->enqueue(IOTask(this, dDelete)); this->IOHandler()->flush(internal::defaultFlushParams); } - res = Container::erase(key); + res = this->datasetDefined() ? 1 : 0; } if (keyScalar) { this->written() = false; this->writable().abstractFilePosition.reset(); - this->get().m_containsScalar = false; + this->get().m_datasetDefined = false; } return res; } @@ -290,23 +314,11 @@ BaseRecord::erase(iterator res) ret = Container::erase(res); else { - mapped_type &rc = this->find(RecordComponent::SCALAR)->second; - if (rc.written()) - { - Parameter dDelete; - dDelete.name = "."; - this->IOHandler()->enqueue(IOTask(&rc, dDelete)); - this->IOHandler()->flush(internal::defaultFlushParams); - } - ret = Container::erase(res); + throw std::runtime_error( + "Unreachable! Iterators do not yet cover scalars (they will in a " + "later commit)."); } - if (keyScalar) - { - this->written() = false; - this->writable().abstractFilePosition.reset(); - this->get().m_containsScalar = false; - } return ret; } @@ -320,7 +332,7 @@ inline std::array BaseRecord::unitDimension() const template inline bool BaseRecord::scalar() const { - return get().m_containsScalar; + return this->datasetDefined(); } template @@ -362,7 +374,8 @@ template inline void BaseRecord::flush( std::string const &name, internal::FlushParams const &flushParams) { - if (!this->written() && this->T_Container::empty()) + if (!this->written() && this->T_Container::empty() && + !this->datasetDefined()) throw std::runtime_error( "A Record can not be written without any contained " "RecordComponents: " + diff --git a/include/openPMD/backend/MeshRecordComponent.hpp b/include/openPMD/backend/MeshRecordComponent.hpp index 4623303e35..2e9e259ce0 100644 --- a/include/openPMD/backend/MeshRecordComponent.hpp +++ b/include/openPMD/backend/MeshRecordComponent.hpp @@ -39,6 +39,7 @@ class MeshRecordComponent : public RecordComponent MeshRecordComponent(); MeshRecordComponent(NoInit); void read() override; + void flush(std::string const &, internal::FlushParams const &); public: ~MeshRecordComponent() override = default; diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index bfa4fb979d..1e91f68830 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -105,7 +105,7 @@ OPENPMD_private // clang-format on void flush(std::string const &, internal::FlushParams const &); - void read(); + virtual void read(); /** * @brief Check recursively whether this RecordComponent is dirty. diff --git a/src/Iteration.cpp b/src/Iteration.cpp index 339928d4f3..7f5cb5e4c6 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -586,8 +586,7 @@ void Iteration::readMeshes(std::string const &meshesPath) auto shape = std::find(att_begin, att_end, "shape"); if (value != att_end && shape != att_end) { - MeshRecordComponent &mrc = m[MeshRecordComponent::SCALAR]; - mrc.parent() = m.parent(); + MeshRecordComponent &mrc = m; IOHandler()->enqueue(IOTask(&mrc, pOpen)); IOHandler()->flush(internal::defaultFlushParams); mrc.get().m_isConstant = true; @@ -617,8 +616,7 @@ void Iteration::readMeshes(std::string const &meshesPath) dOpen.name = mesh_name; IOHandler()->enqueue(IOTask(&m, dOpen)); IOHandler()->flush(internal::defaultFlushParams); - MeshRecordComponent &mrc = m[MeshRecordComponent::SCALAR]; - mrc.parent() = m.parent(); + MeshRecordComponent &mrc = m; IOHandler()->enqueue(IOTask(&mrc, dOpen)); IOHandler()->flush(internal::defaultFlushParams); mrc.written() = false; diff --git a/src/Mesh.cpp b/src/Mesh.cpp index 7454ff005e..fa32d24374 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -219,8 +219,16 @@ void Mesh::flush_impl( { if (access::readOnly(IOHandler()->m_frontendAccess)) { - for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + auto &m = get(); + if (m.m_datasetDefined) + { + T_RecordComponent::flush(SCALAR, flushParams); + } + else + { + for (auto &comp : *this) + comp.second.flush(comp.first, flushParams); + } } else { @@ -228,12 +236,8 @@ void Mesh::flush_impl( { if (scalar()) { - MeshRecordComponent &mrc = at(RecordComponent::SCALAR); - mrc.parent() = parent(); + MeshRecordComponent &mrc = *this; mrc.flush(name, flushParams); - Parameter pSynchronize; - pSynchronize.otherWritable = &mrc.writable(); - IOHandler()->enqueue(IOTask(this, pSynchronize)); } else { @@ -251,12 +255,7 @@ void Mesh::flush_impl( { if (scalar()) { - for (auto &comp : *this) - { - comp.second.flush(name, flushParams); - writable().abstractFilePosition = - comp.second.writable().abstractFilePosition; - } + T_RecordComponent::flush(name, flushParams); } else { @@ -400,8 +399,7 @@ void Mesh::read() if (scalar()) { - /* using operator[] will incorrectly update parent */ - map.at(MeshRecordComponent::SCALAR).read(); + T_RecordComponent::read(); } else { diff --git a/src/ParticlePatches.cpp b/src/ParticlePatches.cpp index 8cd48e212f..5d84b6cd32 100644 --- a/src/ParticlePatches.cpp +++ b/src/ParticlePatches.cpp @@ -32,7 +32,7 @@ size_t ParticlePatches::numPatches() const if (this->empty()) return 0; - return this->at("numParticles").at(RecordComponent::SCALAR).getExtent()[0]; + return this->at("numParticles").getExtent()[0]; } void ParticlePatches::read() @@ -78,10 +78,8 @@ void ParticlePatches::read() } PatchRecord &pr = Container::operator[](component_name); - PatchRecordComponent &prc = pr[RecordComponent::SCALAR]; - prc.parent() = pr.parent(); + PatchRecordComponent &prc = pr; dOpen.name = component_name; - IOHandler()->enqueue(IOTask(&pr, dOpen)); IOHandler()->enqueue(IOTask(&prc, dOpen)); IOHandler()->flush(internal::defaultFlushParams); @@ -102,7 +100,7 @@ void ParticlePatches::read() pr.dirty() = false; try { - prc.read(); + prc.PatchRecordComponent::read(); } catch (error::ReadError const &err) { diff --git a/src/ParticleSpecies.cpp b/src/ParticleSpecies.cpp index eb502ae371..59aff4b6ec 100644 --- a/src/ParticleSpecies.cpp +++ b/src/ParticleSpecies.cpp @@ -79,9 +79,7 @@ void ParticleSpecies::read() auto shape = std::find(att_begin, att_end, "shape"); if (value != att_end && shape != att_end) { - internal::EraseStaleEntries scalarMap(r); - RecordComponent &rc = scalarMap[RecordComponent::SCALAR]; - rc.parent() = r.parent(); + RecordComponent &rc = r; IOHandler()->enqueue(IOTask(&rc, pOpen)); IOHandler()->flush(internal::defaultFlushParams); rc.get().m_isConstant = true; @@ -122,9 +120,7 @@ void ParticleSpecies::read() dOpen.name = record_name; IOHandler()->enqueue(IOTask(&r, dOpen)); IOHandler()->flush(internal::defaultFlushParams); - internal::EraseStaleEntries scalarMap(r); - RecordComponent &rc = scalarMap[RecordComponent::SCALAR]; - rc.parent() = r.parent(); + RecordComponent &rc = r; IOHandler()->enqueue(IOTask(&rc, dOpen)); IOHandler()->flush(internal::defaultFlushParams); rc.written() = false; diff --git a/src/Record.cpp b/src/Record.cpp index b886af2cd4..939930d12c 100644 --- a/src/Record.cpp +++ b/src/Record.cpp @@ -48,8 +48,15 @@ void Record::flush_impl( { if (access::readOnly(IOHandler()->m_frontendAccess)) { - for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + if (scalar()) + { + T_RecordComponent::flush(SCALAR, flushParams); + } + else + { + for (auto &comp : *this) + comp.second.flush(comp.first, flushParams); + } } else { @@ -57,12 +64,8 @@ void Record::flush_impl( { if (scalar()) { - RecordComponent &rc = at(RecordComponent::SCALAR); - rc.parent() = parent(); + RecordComponent &rc = *this; rc.flush(name, flushParams); - Parameter pSynchronize; - pSynchronize.otherWritable = &rc.writable(); - IOHandler()->enqueue(IOTask(this, pSynchronize)); } else { @@ -81,12 +84,7 @@ void Record::flush_impl( if (scalar()) { - for (auto &comp : *this) - { - comp.second.flush(name, flushParams); - writable().abstractFilePosition = - comp.second.writable().abstractFilePosition; - } + T_RecordComponent::flush(name, flushParams); } else { @@ -104,17 +102,15 @@ void Record::read() if (scalar()) { /* using operator[] will incorrectly update parent */ - auto &scalarComponent = this->at(RecordComponent::SCALAR); try { - scalarComponent.read(); + T_RecordComponent::read(); } catch (error::ReadError const &err) { std::cerr << "Cannot read scalar record component and will skip it " "due to read error:\n" << err.what() << std::endl; - this->container().erase(RecordComponent::SCALAR); } } else diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index e8deac4864..d6110ed50d 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -43,7 +43,6 @@ namespace internal RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) { setData(std::make_shared()); - setUnitSI(1); } RecordComponent::RecordComponent(NoInit) : BaseRecordComponent(NoInit()) @@ -252,6 +251,10 @@ void RecordComponent::flush( "before flushing (see RecordComponent::resetDataset())."); } } + if (!containsAttribute("unitSI")) + { + setUnitSI(1); + } if (!written()) { if (constant()) diff --git a/src/backend/BaseRecordComponent.cpp b/src/backend/BaseRecordComponent.cpp index 4de7eb797e..96b38beed5 100644 --- a/src/backend/BaseRecordComponent.cpp +++ b/src/backend/BaseRecordComponent.cpp @@ -101,7 +101,7 @@ void BaseRecordComponent::setDatasetDefined( bool BaseRecordComponent::datasetDefined() const { - auto & data = get(); + auto &data = get(); return data.m_datasetDefined; } } // namespace openPMD diff --git a/src/backend/MeshRecordComponent.cpp b/src/backend/MeshRecordComponent.cpp index 50d300b1a0..8d65783018 100644 --- a/src/backend/MeshRecordComponent.cpp +++ b/src/backend/MeshRecordComponent.cpp @@ -23,9 +23,7 @@ namespace openPMD { MeshRecordComponent::MeshRecordComponent() : RecordComponent() -{ - setPosition(std::vector{0}); -} +{} MeshRecordComponent::MeshRecordComponent(NoInit) : RecordComponent(NoInit()) {} @@ -61,6 +59,17 @@ void MeshRecordComponent::read() readBase(); } +void MeshRecordComponent::flush( + std::string const &name, internal::FlushParams const ¶ms) +{ + if (access::write(IOHandler()->m_frontendAccess) && + !containsAttribute("position")) + { + setPosition(std::vector{0}); + } + RecordComponent::flush(name, params); +} + template MeshRecordComponent &MeshRecordComponent::setPosition(std::vector pos) { diff --git a/src/backend/PatchRecord.cpp b/src/backend/PatchRecord.cpp index 8f9e64012c..25d72d62b6 100644 --- a/src/backend/PatchRecord.cpp +++ b/src/backend/PatchRecord.cpp @@ -41,7 +41,7 @@ PatchRecord::setUnitDimension(std::map const &udim) void PatchRecord::flush_impl( std::string const &path, internal::FlushParams const &flushParams) { - if (this->find(RecordComponent::SCALAR) == this->end()) + if (!this->datasetDefined()) { if (IOHandler()->m_frontendAccess != Access::READ_ONLY) Container::flush( @@ -51,7 +51,7 @@ void PatchRecord::flush_impl( comp.second.flush(comp.first, flushParams); } else - this->operator[](RecordComponent::SCALAR).flush(path, flushParams); + T_RecordComponent::flush(path, flushParams); if (flushParams.flushLevel == FlushLevel::UserFlush) { this->dirty() = false; diff --git a/src/backend/PatchRecordComponent.cpp b/src/backend/PatchRecordComponent.cpp index e319d99e04..33aeb433b5 100644 --- a/src/backend/PatchRecordComponent.cpp +++ b/src/backend/PatchRecordComponent.cpp @@ -117,6 +117,10 @@ void PatchRecordComponent::flush( "RecordComponent::resetDataset())."); } } + if (!containsAttribute("unitSI")) + { + setUnitSI(1); + } if (!written()) { Parameter dCreate; diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 52ee3c387a..eb3181c0c8 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -1748,7 +1748,7 @@ void test_complex(const std::string &backend) } auto rcflt = i.iterations[0] - .meshes["Cflt"][RecordComponent::SCALAR] + .meshes["Cflt"] //[RecordComponent::SCALAR] .loadChunk>(); auto rcdbl = i.iterations[0] .meshes["Cdbl"][RecordComponent::SCALAR] From 7d37e5a39b8e3b2426345185ebc1143ebc33ec50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 24 Mar 2023 12:10:14 +0100 Subject: [PATCH 07/22] Remove KEEP_SYNCHRONOUS task No longer needed, as one object in the openPMD hierarchy is no longer represented by possibly multiple Writable objects. --- include/openPMD/IO/AbstractIOHandlerImpl.hpp | 8 ---- include/openPMD/IO/IOTask.hpp | 46 +++----------------- src/IO/AbstractIOHandlerImpl.cpp | 20 --------- 3 files changed, 5 insertions(+), 69 deletions(-) diff --git a/include/openPMD/IO/AbstractIOHandlerImpl.hpp b/include/openPMD/IO/AbstractIOHandlerImpl.hpp index dafbc24111..7fc2e4cca0 100644 --- a/include/openPMD/IO/AbstractIOHandlerImpl.hpp +++ b/include/openPMD/IO/AbstractIOHandlerImpl.hpp @@ -385,14 +385,6 @@ class AbstractIOHandlerImpl virtual void listAttributes(Writable *, Parameter &) = 0; - /** Treat the current Writable as equivalent to that in the parameter object - * - * Using the default implementation (which copies the abstractFilePath - * into the current writable) should be enough for all backends. - */ - void keepSynchronous( - Writable *, Parameter const ¶m); - /** Notify the backend that the Writable has been / will be deallocated. * * The backend should remove all references to this Writable from internal diff --git a/include/openPMD/IO/IOTask.hpp b/include/openPMD/IO/IOTask.hpp index 8ded6f1775..f62e4360eb 100644 --- a/include/openPMD/IO/IOTask.hpp +++ b/include/openPMD/IO/IOTask.hpp @@ -47,36 +47,19 @@ Writable *getWritable(Attributable *); /** Type of IO operation between logical and persistent data. */ OPENPMDAPI_EXPORT_ENUM_CLASS(Operation){ - CREATE_FILE, - CHECK_FILE, - OPEN_FILE, - CLOSE_FILE, + CREATE_FILE, CHECK_FILE, OPEN_FILE, CLOSE_FILE, DELETE_FILE, - CREATE_PATH, - CLOSE_PATH, - OPEN_PATH, - DELETE_PATH, + CREATE_PATH, CLOSE_PATH, OPEN_PATH, DELETE_PATH, LIST_PATHS, - CREATE_DATASET, - EXTEND_DATASET, - OPEN_DATASET, - DELETE_DATASET, - WRITE_DATASET, - READ_DATASET, - LIST_DATASETS, - GET_BUFFER_VIEW, + CREATE_DATASET, EXTEND_DATASET, OPEN_DATASET, DELETE_DATASET, + WRITE_DATASET, READ_DATASET, LIST_DATASETS, GET_BUFFER_VIEW, - DELETE_ATT, - WRITE_ATT, - READ_ATT, - LIST_ATTS, + DELETE_ATT, WRITE_ATT, READ_ATT, LIST_ATTS, ADVANCE, AVAILABLE_CHUNKS, //!< Query chunks that can be loaded in a dataset - KEEP_SYNCHRONOUS, //!< Keep two items in the object model synchronous with - //!< each other DEREGISTER //!< Inform the backend that an object has been deleted. }; // note: if you change the enum members here, please update // docs/source/dev/design.rst @@ -657,25 +640,6 @@ struct OPENPMDAPI_EXPORT Parameter std::shared_ptr chunks = std::make_shared(); }; -template <> -struct OPENPMDAPI_EXPORT Parameter - : public AbstractParameter -{ - Parameter() = default; - Parameter(Parameter &&) = default; - Parameter(Parameter const &) = default; - Parameter &operator=(Parameter &&) = default; - Parameter &operator=(Parameter const &) = default; - - std::unique_ptr to_heap() && override - { - return std::make_unique>( - std::move(*this)); - } - - Writable *otherWritable; -}; - template <> struct OPENPMDAPI_EXPORT Parameter : public AbstractParameter diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index 01b489f4dd..78e9bc3fc7 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/src/IO/AbstractIOHandlerImpl.cpp @@ -37,13 +37,6 @@ AbstractIOHandlerImpl::AbstractIOHandlerImpl(AbstractIOHandler *handler) } } -void AbstractIOHandlerImpl::keepSynchronous( - Writable *writable, Parameter const ¶m) -{ - writable->abstractFilePosition = param.otherWritable->abstractFilePosition; - writable->written = true; -} - template void AbstractIOHandlerImpl::writeToStderr([[maybe_unused]] Args &&...args) const { @@ -341,19 +334,6 @@ std::future AbstractIOHandlerImpl::flush() availableChunks(i.writable, parameter); break; } - case O::KEEP_SYNCHRONOUS: { - auto ¶meter = - deref_dynamic_cast>( - i.parameter.get()); - writeToStderr( - "[", - i.writable->parent, - "->", - i.writable, - "] KEEP_SYNCHRONOUS"); - keepSynchronous(i.writable, parameter); - break; - } case O::DEREGISTER: { auto ¶meter = deref_dynamic_cast>( i.parameter.get()); From 9817075502d9ab53df90f32686fff54ce973d3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 24 Mar 2023 12:41:58 +0100 Subject: [PATCH 08/22] Adapt Coretests --- test/CoreTest.cpp | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index 58505ec85f..1ca4e06869 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -300,8 +300,7 @@ TEST_CASE("myPath", "[core]") "particles", "e", "particlePatches", - "numParticles", - RecordComponent::SCALAR}); + "numParticles"}); #endif } @@ -450,16 +449,21 @@ TEST_CASE("record_constructor_test", "[core]") ps["position"][RecordComponent::SCALAR].resetDataset(dset); ps["positionOffset"][RecordComponent::SCALAR].resetDataset(dset); - REQUIRE(r["x"].resetDataset(dset).unitSI() == 1); - REQUIRE(r["x"].numAttributes() == 1); /* unitSI */ - REQUIRE(r["y"].resetDataset(dset).unitSI() == 1); - REQUIRE(r["y"].numAttributes() == 1); /* unitSI */ - REQUIRE(r["z"].resetDataset(dset).unitSI() == 1); - REQUIRE(r["z"].numAttributes() == 1); /* unitSI */ + // unitSI is set upon flushing + // REQUIRE(r["x"].unitSI() == 1); + REQUIRE(r["x"].resetDataset(dset).numAttributes() == 0); /* unitSI */ + // REQUIRE(r["y"].unitSI() == 1); + REQUIRE(r["y"].resetDataset(dset).numAttributes() == 0); /* unitSI */ + // REQUIRE(r["z"].unitSI() == 1); + REQUIRE(r["z"].resetDataset(dset).numAttributes() == 0); /* unitSI */ std::array zeros{{0., 0., 0., 0., 0., 0., 0.}}; REQUIRE(r.unitDimension() == zeros); REQUIRE(r.timeOffset() == static_cast(0)); REQUIRE(r.numAttributes() == 2); /* timeOffset, unitDimension */ + o.flush(); + REQUIRE(r["x"].unitSI() == 1); + REQUIRE(r["y"].unitSI() == 1); + REQUIRE(r["z"].unitSI() == 1); } TEST_CASE("record_modification_test", "[core]") @@ -518,15 +522,11 @@ TEST_CASE("mesh_constructor_test", "[core]") Mesh &m = o.iterations[42].meshes["E"]; std::vector pos{0}; - REQUIRE(m["x"].resetDataset(globalDataset).unitSI() == 1); - REQUIRE(m["x"].numAttributes() == 2); /* unitSI, position */ - REQUIRE(m["x"].position() == pos); - REQUIRE(m["y"].resetDataset(globalDataset).unitSI() == 1); - REQUIRE(m["y"].numAttributes() == 2); /* unitSI, position */ - REQUIRE(m["y"].position() == pos); - REQUIRE(m["z"].resetDataset(globalDataset).unitSI() == 1); - REQUIRE(m["z"].numAttributes() == 2); /* unitSI, position */ - REQUIRE(m["z"].position() == pos); + /* unitSI and position are set to default values upon flushing */ + REQUIRE(m["x"].resetDataset(globalDataset).numAttributes() == 0); + REQUIRE(m["y"].resetDataset(globalDataset).numAttributes() == 0); + REQUIRE(m["z"].resetDataset(globalDataset).numAttributes() == 0); + REQUIRE(m.geometry() == Mesh::Geometry::cartesian); REQUIRE(m.dataOrder() == Mesh::DataOrder::C); std::vector al{"x"}; @@ -540,6 +540,17 @@ TEST_CASE("mesh_constructor_test", "[core]") m.numAttributes() == 8); /* axisLabels, dataOrder, geometry, gridGlobalOffset, gridSpacing, gridUnitSI, timeOffset, unitDimension */ + + o.flush(); + REQUIRE(m["x"].unitSI() == 1); + REQUIRE(m["x"].numAttributes() == 2); /* unitSI, position */ + REQUIRE(m["x"].position() == pos); + REQUIRE(m["y"].unitSI() == 1); + REQUIRE(m["y"].numAttributes() == 2); /* unitSI, position */ + REQUIRE(m["y"].position() == pos); + REQUIRE(m["z"].unitSI() == 1); + REQUIRE(m["z"].numAttributes() == 2); /* unitSI, position */ + REQUIRE(m["z"].position() == pos); } TEST_CASE("mesh_modification_test", "[core]") @@ -1023,6 +1034,8 @@ TEST_CASE("empty_record_test", "[core]") "RecordComponents: E")); o.iterations[1].meshes["E"][RecordComponent::SCALAR].resetDataset( Dataset(Datatype::DOUBLE, {1})); + auto B = o.iterations[1].meshes["B"]; + B.resetDataset(Dataset(Datatype::DOUBLE, {1})); o.flush(); } From 8ad6c8d99115176d323e5b1f552c2a963aea3a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 13:41:07 +0100 Subject: [PATCH 09/22] No virtual methods in Container class Either this way, or make all of them virtual --- include/openPMD/backend/BaseRecord.hpp | 8 ++++---- include/openPMD/backend/Container.hpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index f4ef05874e..f4207a4bb4 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -114,12 +114,12 @@ class BaseRecord virtual ~BaseRecord() = default; - mapped_type &operator[](key_type const &key) override; - mapped_type &operator[](key_type &&key) override; + mapped_type &operator[](key_type const &key); + mapped_type &operator[](key_type &&key); mapped_type &at(key_type const &key); mapped_type const &at(key_type const &key) const; - size_type erase(key_type const &key) override; - iterator erase(iterator res) override; + size_type erase(key_type const &key); + iterator erase(iterator res); //! @todo add also, as soon as added in Container: // iterator erase(const_iterator first, const_iterator last) override; diff --git a/include/openPMD/backend/Container.hpp b/include/openPMD/backend/Container.hpp index c0df662a9f..dea7bf741b 100644 --- a/include/openPMD/backend/Container.hpp +++ b/include/openPMD/backend/Container.hpp @@ -283,7 +283,7 @@ class Container : virtual public Attributable * @throws std::out_of_range error if in READ_ONLY mode and key does not * exist, otherwise key will be created */ - virtual mapped_type &operator[](key_type const &key) + mapped_type &operator[](key_type const &key) { auto it = container().find(key); if (it != container().end()) @@ -318,7 +318,7 @@ class Container : virtual public Attributable * @throws std::out_of_range error if in READ_ONLY mode and key does not * exist, otherwise key will be created */ - virtual mapped_type &operator[](key_type &&key) + mapped_type &operator[](key_type &&key) { auto it = container().find(key); if (it != container().end()) @@ -382,7 +382,7 @@ class Container : virtual public Attributable * @param key Key of the element to remove. * @return Number of elements removed (either 0 or 1). */ - virtual size_type erase(key_type const &key) + size_type erase(key_type const &key) { if (Access::READ_ONLY == IOHandler()->m_frontendAccess) throw std::runtime_error( @@ -400,7 +400,7 @@ class Container : virtual public Attributable } //! @todo why does const_iterator not work compile with pybind11? - virtual iterator erase(iterator res) + iterator erase(iterator res) { if (Access::READ_ONLY == IOHandler()->m_frontendAccess) throw std::runtime_error( From 95b8315249dd3948b9a8297df17fb5c3136ea627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 13:55:00 +0100 Subject: [PATCH 10/22] Fully override Container methods in BaseRecord Special care for legacy usage of SCALAR constant. Implement iteration API such that it works for scalar components as well. --- include/openPMD/RecordComponent.hpp | 10 + include/openPMD/backend/BaseRecord.hpp | 651 +++++++++++++++++- .../openPMD/backend/BaseRecordComponent.hpp | 8 +- .../openPMD/backend/MeshRecordComponent.hpp | 8 + .../openPMD/backend/PatchRecordComponent.hpp | 6 + test/CoreTest.cpp | 21 + 6 files changed, 675 insertions(+), 29 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 11773902d9..c145dabad6 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -94,6 +94,16 @@ namespace internal * flushed to the backend */ bool m_hasBeenExtended = false; + + void reset() override + { + BaseRecordComponentData::reset(); + m_chunks = std::queue(); + m_constantValue = -1; + m_name = std::string(); + m_isEmpty = false; + m_hasBeenExtended = false; + } }; template class BaseRecordData; diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index f4207a4bb4..0f90a7e1c4 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -22,14 +22,19 @@ #include "openPMD/RecordComponent.hpp" #include "openPMD/UnitDimension.hpp" +#include "openPMD/auxiliary/Variant.hpp" #include "openPMD/backend/Container.hpp" #include #include #include +#include // std::remove_reference_t +#include // std::declval namespace openPMD { +template +class BaseRecord; namespace internal { template < @@ -43,6 +48,8 @@ namespace internal : public ContainerData , public T_RecordComponentData { + using T_RecordComponent = T_elem; + public: BaseRecordData(); @@ -52,6 +59,150 @@ namespace internal BaseRecordData &operator=(BaseRecordData const &) = delete; BaseRecordData &operator=(BaseRecordData &&) = delete; }; + + // @todo change T_InternalContainer to direct iterator type + template < + typename T_BaseRecord_, + typename T_BaseRecordData_, + typename T_BaseIterator> + class ScalarIterator + { + /* + * Allow other template instantiations of ScalarIterators member access. + */ + template + friend class ScalarIterator; + + using T_BaseRecord = T_BaseRecord_; + using T_BaseRecordData = T_BaseRecordData_; + using T_RecordComponent = typename T_BaseRecord::T_RecordComponent; + using T_Container = typename T_BaseRecord::T_Container; + using T_Value = + std::remove_reference_t())>; + using Left = T_BaseIterator; + struct Right + { /*Empty*/ + constexpr bool operator==(Right const &) const noexcept + { + return true; + } + constexpr bool operator!=(Right const &) const noexcept + { + return false; + } + }; + + template + friend class openPMD::BaseRecord; + + T_BaseRecordData *m_baseRecordData = nullptr; + using ScalarTuple = + std::optional>; + ScalarTuple m_scalarTuple; + std::variant m_iterator; + + explicit ScalarIterator() = default; + + ScalarIterator(T_BaseRecord *baseRecord) + : m_baseRecordData(&baseRecord->get()) + , m_scalarTuple(std::make_pair( + RecordComponent::SCALAR, T_RecordComponent(*baseRecord))) + , m_iterator(Right()) + {} + ScalarIterator(T_BaseRecord *baseRecord, Left iterator) + : m_baseRecordData(&baseRecord->get()) + , m_scalarTuple(std::make_pair( + RecordComponent::SCALAR, T_RecordComponent(*baseRecord))) + , m_iterator(std::move(iterator)) + {} + + public: + /** + * Auto-convert normal to const iterator. + * This is necessary to have things like: + * > B.insert(B.find("x"), std::pair{"y", E["y"]}) + * ^^^^^^^^^^^ + * returns a normal iterator + * but insert-with-hint requires a const + * + * @tparam Other A ScalarIterator with other template parameters. + * @tparam SFINAE Implementation detail. + * @param other Copy from this non-const iterator. + */ + template < + typename Other, + /* + * We need this in order to not accidentally register this as a copy + * constructor. + */ + typename SFINAE = std::enable_if_t< + !std::is_same_v>> + ScalarIterator(Other const &other) + : m_baseRecordData(other.m_baseRecordData) + , m_scalarTuple( + other.m_scalarTuple.has_value() + ? ScalarTuple(std::make_pair( + RecordComponent::SCALAR, + T_RecordComponent( + other.m_scalarTuple.value().second))) + : ScalarTuple(std::nullopt)) + , m_iterator(std::visit( + auxiliary::overloaded{ + [](typename Other::Left const &left) { + // This converts the STL iterator to an + // STL const_iterator + return std::variant(left); + }, + [](typename Other::Right const &) { + return std::variant(Right()); + }}, + other.m_iterator)) + {} + + ScalarIterator &operator++() + { + std::visit( + auxiliary::overloaded{ + [](Left &left) { ++left; }, + [this](Right &) { + m_iterator = m_baseRecordData->m_container.end(); + }}, + m_iterator); + return *this; + } + + T_Value *operator->() + { + return std::visit( + auxiliary::overloaded{ + [](Left &left) -> T_Value * { return left.operator->(); }, + [this](Right &) -> T_Value * { + /* + * We cannot create this value on the fly since we only + * give out a pointer, so that would be use-after-free. + * Instead, we just keep one value around inside + * BaseRecordData and give it out when needed. + */ + return &m_scalarTuple.value(); + }}, + m_iterator); + } + + T_Value &operator*() + { + return *operator->(); + } + + bool operator==(ScalarIterator const &other) const + { + return this->m_iterator == other.m_iterator; + } + + bool operator!=(ScalarIterator const &other) const + { + return !operator==(other); + } + }; } // namespace internal template @@ -69,6 +220,8 @@ class BaseRecord friend class Mesh; template friend class internal::BaseRecordData; + template + friend class internal::ScalarIterator; using Data_t = internal::BaseRecordData; @@ -109,8 +262,148 @@ class BaseRecord using const_reference = typename Container::const_reference; using pointer = typename Container::pointer; using const_pointer = typename Container::const_pointer; - using iterator = typename T_Container::iterator; - using const_iterator = typename T_Container::const_iterator; + + using iterator = internal::ScalarIterator< + T_Self, + Data_t, + typename T_Container::InternalContainer::iterator>; + using const_iterator = internal::ScalarIterator< + T_Self const, + Data_t const, + typename T_Container::InternalContainer::const_iterator>; + using reverse_iterator = internal::ScalarIterator< + T_Self, + Data_t, + typename T_Container::InternalContainer::reverse_iterator>; + using const_reverse_iterator = internal::ScalarIterator< + T_Self const, + Data_t const, + typename T_Container::InternalContainer::const_reverse_iterator>; + +private: + template + iterator makeIterator(Arg &&...arg) + { + return iterator{this, std::forward(arg)...}; + } + template + const_iterator makeIterator(Arg &&...arg) const + { + return const_iterator{this, std::forward(arg)...}; + } + template + reverse_iterator makeReverseIterator(Arg &&...arg) + { + return reverse_iterator{this, std::forward(arg)...}; + } + template + const_reverse_iterator makeReverseIterator(Arg &&...arg) const + { + return const_reverse_iterator{this, std::forward(arg)...}; + } + +public: + iterator begin() + { + if (get().m_datasetDefined) + { + return makeIterator(); + } + else + { + return makeIterator(T_Container::begin()); + } + } + + const_iterator begin() const + { + if (get().m_datasetDefined) + { + return makeIterator(); + } + else + { + return makeIterator(T_Container::begin()); + } + } + + const_iterator cbegin() const + { + if (get().m_datasetDefined) + { + return makeIterator(); + } + else + { + return makeIterator(T_Container::cbegin()); + } + } + + iterator end() + { + return makeIterator(T_Container::end()); + } + + const_iterator end() const + { + return makeIterator(T_Container::end()); + } + + const_iterator cend() const + { + return makeIterator(T_Container::cend()); + } + + reverse_iterator rbegin() + { + if (get().m_datasetDefined) + { + return makeReverseIterator(); + } + else + { + return makeReverseIterator(this->container().rbegin()); + } + } + + const_reverse_iterator rbegin() const + { + if (get().m_datasetDefined) + { + return makeReverseIterator(); + } + else + { + return makeReverseIterator(this->container().rbegin()); + } + } + + const_reverse_iterator crbegin() const + { + if (get().m_datasetDefined) + { + return makeReverseIterator(); + } + else + { + return makeReverseIterator(this->container().crbegin()); + } + } + + reverse_iterator rend() + { + return makeReverseIterator(this->container().rend()); + } + + const_reverse_iterator rend() const + { + return makeReverseIterator(this->container().rend()); + } + + const_reverse_iterator crend() const + { + return makeReverseIterator(this->container().crend()); + } virtual ~BaseRecord() = default; @@ -120,6 +413,24 @@ class BaseRecord mapped_type const &at(key_type const &key) const; size_type erase(key_type const &key); iterator erase(iterator res); + bool empty() const noexcept; + iterator find(key_type const &key); + const_iterator find(key_type const &key) const; + size_type count(key_type const &key) const; + size_type size() const; + void clear(); + std::pair insert(value_type const &value); + std::pair insert(value_type &&value); + iterator insert(const_iterator hint, value_type const &value); + iterator insert(const_iterator hint, value_type &&value); + template + void insert(InputIt first, InputIt last); + void insert(std::initializer_list ilist); + void swap(BaseRecord &other); + bool contains(key_type const &key) const; + template + auto emplace(Args &&...args) -> std::pair; + //! @todo add also, as soon as added in Container: // iterator erase(const_iterator first, const_iterator last) override; @@ -166,6 +477,7 @@ class BaseRecord * @return false Otherwise. */ bool dirtyRecursive() const; + void eraseScalar(); }; // BaseRecord // implementation @@ -192,12 +504,25 @@ BaseRecord::BaseRecord() } template -inline typename BaseRecord::mapped_type & -BaseRecord::operator[](key_type const &key) +auto BaseRecord::operator[](key_type const &key) -> mapped_type & { auto it = this->find(key); if (it != this->end()) - return it->second; + { + return std::visit( + auxiliary::overloaded{ + [](typename iterator::Left &l) -> mapped_type & { + return l->second; + }, + [this](typename iterator::Right &) -> mapped_type & { + /* + * Do not use the iterator result, as it is a non-owning + * handle + */ + return static_cast(*this); + }}, + it.m_iterator); + } else { bool const keyScalar = (key == RecordComponent::SCALAR); @@ -221,12 +546,25 @@ BaseRecord::operator[](key_type const &key) } template -inline typename BaseRecord::mapped_type & -BaseRecord::operator[](key_type &&key) +auto BaseRecord::operator[](key_type &&key) -> mapped_type & { auto it = this->find(key); if (it != this->end()) - return it->second; + { + return std::visit( + auxiliary::overloaded{ + [](typename iterator::Left &l) -> mapped_type & { + return l->second; + }, + [this](typename iterator::Right &) -> mapped_type & { + /* + * Do not use the iterator result, as it is a non-owning + * handle + */ + return static_cast(*this); + }}, + it.m_iterator); + } else { bool const keyScalar = (key == RecordComponent::SCALAR); @@ -276,8 +614,7 @@ auto BaseRecord::at(key_type const &key) const -> mapped_type const & } template -inline typename BaseRecord::size_type -BaseRecord::erase(key_type const &key) +auto BaseRecord::erase(key_type const &key) -> size_type { bool const keyScalar = (key == RecordComponent::SCALAR); size_type res; @@ -285,14 +622,8 @@ BaseRecord::erase(key_type const &key) res = Container::erase(key); else { - if (this->written()) - { - Parameter dDelete; - dDelete.name = "."; - this->IOHandler()->enqueue(IOTask(this, dDelete)); - this->IOHandler()->flush(internal::defaultFlushParams); - } res = this->datasetDefined() ? 1 : 0; + eraseScalar(); } if (keyScalar) @@ -305,21 +636,271 @@ BaseRecord::erase(key_type const &key) } template -inline typename BaseRecord::iterator -BaseRecord::erase(iterator res) +auto BaseRecord::erase(iterator it) -> iterator +{ + return std::visit( + auxiliary::overloaded{ + [this](typename iterator::Left &left) { + return makeIterator(T_Container::erase(left)); + }, + [this](typename iterator::Right &) { + eraseScalar(); + return end(); + }}, + it.m_iterator); +} + +template +auto BaseRecord::empty() const noexcept -> bool +{ + return !scalar() && T_Container::empty(); +} + +template +auto BaseRecord::find(key_type const &key) -> iterator +{ + auto &r = get(); + if (key == RecordComponent::SCALAR && get().m_datasetDefined) + { + if (r.m_datasetDefined) + { + return begin(); + } + else + { + return end(); + } + } + else + { + return makeIterator(r.m_container.find(key)); + } +} + +template +auto BaseRecord::find(key_type const &key) const -> const_iterator +{ + auto &r = get(); + if (key == RecordComponent::SCALAR && get().m_datasetDefined) + { + if (r.m_datasetDefined) + { + return begin(); + } + else + { + return end(); + } + } + else + { + return makeIterator(r.m_container.find(key)); + } +} + +template +auto BaseRecord::count(key_type const &key) const -> size_type +{ + if (key == RecordComponent::SCALAR) + { + return get().m_datasetDefined ? 1 : 0; + } + else + { + return T_Container::count(key); + } +} + +template +auto BaseRecord::size() const -> size_type { - bool const keyScalar = (res->first == RecordComponent::SCALAR); - iterator ret; - if (!keyScalar || (keyScalar && this->at(res->first).constant())) - ret = Container::erase(res); + if (scalar()) + { + return 1; + } else { + return T_Container::size(); + } +} + +template +auto BaseRecord::clear() -> void +{ + if (Access::READ_ONLY == this->IOHandler()->m_frontendAccess) throw std::runtime_error( - "Unreachable! Iterators do not yet cover scalars (they will in a " - "later commit)."); + "Can not clear a container in a read-only Series."); + if (scalar()) + { + eraseScalar(); + } + else + { + T_Container::clear_unchecked(); + } +} + +namespace detail +{ + constexpr char const *const NO_SCALAR_INSERT = + "[BaseRecord] emplace()/insert()/swap() API invalid for scalar " + "records. Use the Record directly as a RecordComponent."; + + template + void verifyNonscalar(BaseRecord *self) + { + if (self->scalar()) + { + throw error::WrongAPIUsage(NO_SCALAR_INSERT); + } + } +} // namespace detail + +template +auto BaseRecord::insert(value_type const &value) + -> std::pair +{ + detail::verifyNonscalar(this); + auto res = this->container().insert(value); + if (res.first->first == RecordComponent::SCALAR) + { + this->container().erase(res.first); + throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + } + return {makeIterator(std::move(res.first)), res.second}; +} + +template +auto BaseRecord::insert(value_type &&value) -> std::pair +{ + detail::verifyNonscalar(this); + auto res = this->container().insert(std::move(value)); + if (res.first->first == RecordComponent::SCALAR) + { + this->container().erase(res.first); + throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + } + return {makeIterator(std::move(res.first)), res.second}; +} + +template +auto BaseRecord::insert(const_iterator hint, value_type const &value) + -> iterator +{ + detail::verifyNonscalar(this); + auto base_hint = std::visit( + auxiliary::overloaded{ + [](typename const_iterator::Left left) { return left; }, + [this](typename const_iterator::Right) { + return static_cast const *>(this) + ->container() + .begin(); + }}, + hint.m_iterator); + auto res = this->container().insert(base_hint, value); + if (res->first == RecordComponent::SCALAR) + { + this->container().erase(res); + throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + } + return makeIterator(res); +} + +template +auto BaseRecord::insert(const_iterator hint, value_type &&value) + -> iterator +{ + detail::verifyNonscalar(this); + auto base_hint = std::visit( + auxiliary::overloaded{ + [](typename const_iterator::Left left) { return left; }, + [this](typename const_iterator::Right) { + return static_cast const *>(this) + ->container() + .begin(); + }}, + hint.m_iterator); + auto res = this->container().insert(base_hint, std::move(value)); + if (res->first == RecordComponent::SCALAR) + { + this->container().erase(res); + throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + } + return makeIterator(res); +} + +template +template +auto BaseRecord::insert(InputIt first, InputIt last) -> void +{ + detail::verifyNonscalar(this); + this->container().insert(first, last); + /* + * We skip this check as it changes the runtime of this call from + * O(last-first) to O(container().size()). + */ + // for (auto it = this->container().begin(); it != end; ++it) + // { + // if (it->first == RecordComponent::SCALAR) + // { + // this->container().erase(it); + // throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + // } + // } +} + +template +auto BaseRecord::insert(std::initializer_list ilist) -> void +{ + detail::verifyNonscalar(this); + this->container().insert(std::move(ilist)); + /* + * We skip this check as it changes the runtime of this call from + * O(last-first) to O(container().size()). + */ + // for (auto it = this->container().begin(); it != end; ++it) + // { + // if (it->first == RecordComponent::SCALAR) + // { + // this->container().erase(it); + // throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + // } + // } +} + +template +auto BaseRecord::swap(BaseRecord &other) -> void +{ + detail::verifyNonscalar(this); + detail::verifyNonscalar(&other); + this->container().swap(other.container()); +} + +template +auto BaseRecord::contains(key_type const &key) const -> bool +{ + if (scalar()) + { + return key == RecordComponent::SCALAR; } + else + { + return T_Container::contains(key); + } +} - return ret; +template +template +auto BaseRecord::emplace(Args &&...args) -> std::pair +{ + detail::verifyNonscalar(this); + auto res = this->container().emplace(std::forward(args)...); + if (res.first->first == RecordComponent::SCALAR) + { + this->container().erase(res.first); + throw error::WrongAPIUsage(detail::NO_SCALAR_INSERT); + } + return {makeIterator(std::move(res.first)), res.second}; } template @@ -374,8 +955,7 @@ template inline void BaseRecord::flush( std::string const &name, internal::FlushParams const &flushParams) { - if (!this->written() && this->T_Container::empty() && - !this->datasetDefined()) + if (!this->written() && this->empty() && !this->datasetDefined()) throw std::runtime_error( "A Record can not be written without any contained " "RecordComponents: " + @@ -402,4 +982,19 @@ inline bool BaseRecord::dirtyRecursive() const } return false; } + +template +void BaseRecord::eraseScalar() +{ + if (this->written()) + { + Parameter dDelete; + dDelete.name = "."; + this->IOHandler()->enqueue(IOTask(this, dDelete)); + this->IOHandler()->flush(internal::defaultFlushParams); + } + auto &data = T_RecordComponent::get(); + data.reset(); + this->writable().abstractFilePosition.reset(); +} } // namespace openPMD diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index 75ba5e8448..96c8e86af7 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -59,12 +59,18 @@ namespace internal BaseRecordComponentData(BaseRecordComponentData const &) = delete; BaseRecordComponentData(BaseRecordComponentData &&) = delete; - BaseRecordComponentData & operator=(BaseRecordComponentData const &) = delete; BaseRecordComponentData &operator=(BaseRecordComponentData &&) = delete; BaseRecordComponentData() = default; + + virtual void reset() + { + m_dataset = std::nullopt; + m_isConstant = false; + m_datasetDefined = false; + } }; } // namespace internal diff --git a/include/openPMD/backend/MeshRecordComponent.hpp b/include/openPMD/backend/MeshRecordComponent.hpp index 2e9e259ce0..97acb26e43 100644 --- a/include/openPMD/backend/MeshRecordComponent.hpp +++ b/include/openPMD/backend/MeshRecordComponent.hpp @@ -26,12 +26,20 @@ namespace openPMD { +namespace internal +{ + template + class BaseRecordData; +} + class MeshRecordComponent : public RecordComponent { template friend class Container; template friend class BaseRecord; + template + friend class internal::BaseRecordData; friend class Mesh; diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index 1e91f68830..7bf9e7b81e 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -55,6 +55,12 @@ namespace internal operator=(PatchRecordComponentData &&) = delete; PatchRecordComponentData(); + + void reset() override + { + BaseRecordComponentData::reset(); + m_chunks = std::queue(); + } }; template diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index 1ca4e06869..4507c92b15 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -986,6 +986,27 @@ TEST_CASE("wrapper_test", "[core]") #endif } +TEST_CASE("baserecord_test", "[core]") +{ + Series o("../samples/testBaseRecord.json", Access::CREATE); + auto E = o.iterations[100].meshes["E"]; + Mesh B = o.iterations[100].meshes["B"]; + Dataset ds(Datatype::INT, {16, 16}); + for (auto const &component : {"x", "y", "z"}) + { + E[component].makeConstant(5); + E[component].resetDataset(ds); + } + for (auto const &component : {"x", /* "y", */ "z"}) + { + B[component].makeConstant(5); + B[component].resetDataset(ds); + } + auto inserted = B.insert(B.find("x"), std::pair{"y", E["y"]}); + REQUIRE(inserted->first == "y"); + B.erase(inserted); +} + TEST_CASE("use_count_test", "[core]") { Series o = Series("./new_openpmd_output.json", Access::CREATE); From 882195fc16284eec5ea15b6cbeab38fe7054f90f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 29 Mar 2023 16:21:53 +0200 Subject: [PATCH 11/22] Adapt Container API to C++17 insert() and reverse iterators --- include/openPMD/backend/Container.hpp | 35 ++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/include/openPMD/backend/Container.hpp b/include/openPMD/backend/Container.hpp index dea7bf741b..02b0875eb8 100644 --- a/include/openPMD/backend/Container.hpp +++ b/include/openPMD/backend/Container.hpp @@ -179,6 +179,9 @@ class Container : virtual public Attributable using const_pointer = typename InternalContainer::const_pointer; using iterator = typename InternalContainer::iterator; using const_iterator = typename InternalContainer::const_iterator; + using reverse_iterator = typename InternalContainer::reverse_iterator; + using const_reverse_iterator = + typename InternalContainer::const_reverse_iterator; iterator begin() noexcept { @@ -206,6 +209,32 @@ class Container : virtual public Attributable return container().cend(); } + reverse_iterator rbegin() noexcept + { + return container().rbegin(); + } + const_reverse_iterator rbegin() const noexcept + { + return container().rbegin(); + } + const_reverse_iterator crbegin() const noexcept + { + return container().crbegin(); + } + + reverse_iterator rend() noexcept + { + return container().rend(); + } + const_reverse_iterator rend() const noexcept + { + return container().rend(); + } + const_reverse_iterator crend() const noexcept + { + return container().crend(); + } + bool empty() const noexcept { return container().empty(); @@ -235,8 +264,7 @@ class Container : virtual public Attributable { return container().insert(value); } - template - std::pair insert(P &&value) + std::pair insert(value_type &&value) { return container().insert(value); } @@ -244,8 +272,7 @@ class Container : virtual public Attributable { return container().insert(hint, value); } - template - iterator insert(const_iterator hint, P &&value) + iterator insert(const_iterator hint, value_type &&value) { return container().insert(hint, value); } From 1a5583e2b3ee96c0e15eff1a3655c550f96be015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 14:14:05 +0100 Subject: [PATCH 12/22] Adapt myPath() functionality --- CMakeLists.txt | 1 - include/openPMD/backend/Container.hpp | 51 ++++++++--------------- include/openPMD/backend/Writable.hpp | 8 ++-- src/Iteration.cpp | 4 +- src/ParticleSpecies.cpp | 2 +- src/Series.cpp | 2 +- src/backend/Attributable.cpp | 8 +--- src/backend/Container.cpp | 58 --------------------------- test/CoreTest.cpp | 15 +------ 9 files changed, 27 insertions(+), 122 deletions(-) delete mode 100644 src/backend/Container.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a1bdfb4b62..253b7b8bbe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -462,7 +462,6 @@ set(CORE_SOURCE src/auxiliary/JSON.cpp src/backend/Attributable.cpp src/backend/BaseRecordComponent.cpp - src/backend/Container.cpp src/backend/MeshRecordComponent.cpp src/backend/PatchRecord.cpp src/backend/PatchRecordComponent.cpp diff --git a/include/openPMD/backend/Container.hpp b/include/openPMD/backend/Container.hpp index 02b0875eb8..ae18b0182c 100644 --- a/include/openPMD/backend/Container.hpp +++ b/include/openPMD/backend/Container.hpp @@ -86,36 +86,6 @@ namespace internal }; } // namespace internal -namespace detail -{ - /* - * This converts the key (first parameter) to its string name within the - * openPMD hierarchy. - * If the key is found to be equal to RecordComponent::SCALAR, the parentKey - * will be returned, adding RecordComponent::SCALAR to its back. - * Reason: Scalar record components do not link their containing record as - * parent, but rather the parent's parent, so the own key within the - * "apparent" parent must be given as two steps. - */ - template - std::vector - keyAsString(T &&key, std::vector const &parentKey) - { - (void)parentKey; - return {std::to_string(std::forward(key))}; - } - - // moved to a *.cpp file so we don't need to include RecordComponent.hpp - // here - template <> - std::vector keyAsString( - std::string const &key, std::vector const &parentKey); - - template <> - std::vector keyAsString( - std::string &&key, std::vector const &parentKey); -} // namespace detail - /** @brief Map-like container that enforces openPMD requirements and handles IO. * * @see http://en.cppreference.com/w/cpp/container/map @@ -328,8 +298,14 @@ class Container : virtual public Attributable T t = T(); t.linkHierarchy(writable()); auto &ret = container().insert({key, std::move(t)}).first->second; - ret.writable().ownKeyWithinParent = - detail::keyAsString(key, writable().ownKeyWithinParent); + if constexpr (std::is_same_v) + { + ret.writable().ownKeyWithinParent = key; + } + else + { + ret.writable().ownKeyWithinParent = std::to_string(key); + } traits::GenerationPolicy gen; gen(ret); return ret; @@ -363,8 +339,15 @@ class Container : virtual public Attributable T t = T(); t.linkHierarchy(writable()); auto &ret = container().insert({key, std::move(t)}).first->second; - ret.writable().ownKeyWithinParent = detail::keyAsString( - std::move(key), writable().ownKeyWithinParent); + if constexpr (std::is_same_v) + { + ret.writable().ownKeyWithinParent = std::move(key); + } + else + { + ret.writable().ownKeyWithinParent = + std::to_string(std::move(key)); + } traits::GenerationPolicy gen; gen(ret); return ret; diff --git a/include/openPMD/backend/Writable.hpp b/include/openPMD/backend/Writable.hpp index 5e1272317e..28554d0cf9 100644 --- a/include/openPMD/backend/Writable.hpp +++ b/include/openPMD/backend/Writable.hpp @@ -137,12 +137,10 @@ OPENPMD_private Writable *parent = nullptr; bool dirty = true; /** - * If parent is not null, then this is a vector of keys such that: - * &(*parent)[key_1]...[key_n] == this - * (Notice that scalar record components do not link their direct parent, - * but instead their parent's parent, hence a vector of keys) + * If parent is not null, then this is a key such that: + * &(*parent)[key] == this */ - std::vector ownKeyWithinParent; + std::string ownKeyWithinParent; /** * @brief Whether a Writable has been written to the backend. * diff --git a/src/Iteration.cpp b/src/Iteration.cpp index 7f5cb5e4c6..593e38066f 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -42,8 +42,8 @@ Iteration::Iteration() : Attributable(NoInit()) setTime(static_cast(0)); setDt(static_cast(1)); setTimeUnitSI(1); - meshes.writable().ownKeyWithinParent = {"meshes"}; - particles.writable().ownKeyWithinParent = {"particles"}; + meshes.writable().ownKeyWithinParent = "meshes"; + particles.writable().ownKeyWithinParent = "particles"; } template diff --git a/src/ParticleSpecies.cpp b/src/ParticleSpecies.cpp index 59aff4b6ec..7f57450acf 100644 --- a/src/ParticleSpecies.cpp +++ b/src/ParticleSpecies.cpp @@ -30,7 +30,7 @@ namespace openPMD { ParticleSpecies::ParticleSpecies() { - particlePatches.writable().ownKeyWithinParent = {"particlePatches"}; + particlePatches.writable().ownKeyWithinParent = "particlePatches"; } void ParticleSpecies::read() diff --git a/src/Series.cpp b/src/Series.cpp index 3719952c79..05516ab483 100644 --- a/src/Series.cpp +++ b/src/Series.cpp @@ -561,7 +561,7 @@ void Series::init( std::make_shared>>( std::move(ioHandler)); series.iterations.linkHierarchy(writable()); - series.iterations.writable().ownKeyWithinParent = {"iterations"}; + series.iterations.writable().ownKeyWithinParent = "iterations"; series.m_name = input->name; diff --git a/src/backend/Attributable.cpp b/src/backend/Attributable.cpp index 00e41f5d63..7eaf47dd07 100644 --- a/src/backend/Attributable.cpp +++ b/src/backend/Attributable.cpp @@ -197,13 +197,7 @@ auto Attributable::myPath() const -> MyPath // so it's alright that this loop doesn't ask the key of the last found // Writable - // push these in reverse because we're building the list from the back - for (auto it = findSeries->ownKeyWithinParent.rbegin(); - it != findSeries->ownKeyWithinParent.rend(); - ++it) - { - res.group.push_back(*it); - } + res.group.push_back(findSeries->ownKeyWithinParent); findSeries = findSeries->parent; } std::reverse(res.group.begin(), res.group.end()); diff --git a/src/backend/Container.cpp b/src/backend/Container.cpp deleted file mode 100644 index 8df16f5ea9..0000000000 --- a/src/backend/Container.cpp +++ /dev/null @@ -1,58 +0,0 @@ -/* Copyright 2017-2021 Franz Poeschel - * - * This file is part of openPMD-api. - * - * openPMD-api is free software: you can redistribute it and/or modify - * it under the terms of of either the GNU General Public License or - * the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * openPMD-api is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License and the GNU Lesser General Public License - * for more details. - * - * You should have received a copy of the GNU General Public License - * and the GNU Lesser General Public License along with openPMD-api. - * If not, see . - */ - -#include "openPMD/backend/Container.hpp" -#include "openPMD/RecordComponent.hpp" - -namespace openPMD::detail -{ -template <> -std::vector keyAsString( - std::string const &key, std::vector const &parentKey) -{ - if (key == RecordComponent::SCALAR) - { - auto ret = parentKey; - ret.emplace_back(RecordComponent::SCALAR); - return ret; - } - else - { - return {key}; - } -} - -template <> -std::vector keyAsString( - std::string &&key, std::vector const &parentKey) -{ - if (key == RecordComponent::SCALAR) - { - auto ret = parentKey; - ret.emplace_back(RecordComponent::SCALAR); - return ret; - } - else - { - return {std::move(key)}; - } -} -} // namespace openPMD::detail diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index 4507c92b15..d5562380ca 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -202,12 +202,7 @@ TEST_CASE("myPath", "[core]") auto scalarMeshComponent = scalarMesh[RecordComponent::SCALAR]; REQUIRE( pathOf(scalarMeshComponent) == - vec_t{ - "iterations", - "1234", - "meshes", - "e_chargeDensity", - RecordComponent::SCALAR}); + vec_t{"iterations", "1234", "meshes", "e_chargeDensity"}); writeSomething(scalarMeshComponent); auto vectorMesh = iteration.meshes["E"]; @@ -243,13 +238,7 @@ TEST_CASE("myPath", "[core]") auto speciesWeightingX = speciesWeighting[RecordComponent::SCALAR]; REQUIRE( pathOf(speciesWeightingX) == - vec_t{ - "iterations", - "1234", - "particles", - "e", - "weighting", - RecordComponent::SCALAR}); + vec_t{"iterations", "1234", "particles", "e", "weighting"}); writeSomething(speciesWeightingX); REQUIRE( From 96310c920d709a94bf904e7819bc4042f76ed622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 16:17:25 +0100 Subject: [PATCH 13/22] Factor out create_and_bind_container() function template Will later be called by Record-type classes, too --- include/openPMD/binding/python/Common.hpp | 2 - include/openPMD/binding/python/Container.H | 4 + include/openPMD/binding/python/Container.hpp | 152 +++++++++++++++++++ src/binding/python/BaseRecordComponent.cpp | 5 - 4 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 include/openPMD/binding/python/Container.hpp diff --git a/include/openPMD/binding/python/Common.hpp b/include/openPMD/binding/python/Common.hpp index 39fed57238..ea6fff2832 100644 --- a/include/openPMD/binding/python/Common.hpp +++ b/include/openPMD/binding/python/Common.hpp @@ -42,7 +42,6 @@ using PyPatchRecordContainer = Container; using PyRecordComponentContainer = Container; using PyMeshRecordComponentContainer = Container; using PyPatchRecordComponentContainer = Container; -using PyBaseRecordComponentContainer = Container; PYBIND11_MAKE_OPAQUE(PyIterationContainer) PYBIND11_MAKE_OPAQUE(PyMeshContainer) PYBIND11_MAKE_OPAQUE(PyPartContainer) @@ -52,4 +51,3 @@ PYBIND11_MAKE_OPAQUE(PyPatchRecordContainer) PYBIND11_MAKE_OPAQUE(PyRecordComponentContainer) PYBIND11_MAKE_OPAQUE(PyMeshRecordComponentContainer) PYBIND11_MAKE_OPAQUE(PyPatchRecordComponentContainer) -PYBIND11_MAKE_OPAQUE(PyBaseRecordComponentContainer) diff --git a/include/openPMD/binding/python/Container.H b/include/openPMD/binding/python/Container.H index 724d24c435..28d0abedd1 100644 --- a/include/openPMD/binding/python/Container.H +++ b/include/openPMD/binding/python/Container.H @@ -26,6 +26,10 @@ #pragma once #include "openPMD/backend/Container.hpp" +#include "openPMD/backend/MeshRecordComponent.hpp" +#include "openPMD/backend/PatchRecord.hpp" +#include "openPMD/backend/PatchRecordComponent.hpp" +#include "openPMD/binding/python/Container.hpp" #include "openPMD/binding/python/Common.hpp" diff --git a/include/openPMD/binding/python/Container.hpp b/include/openPMD/binding/python/Container.hpp new file mode 100644 index 0000000000..0b24951108 --- /dev/null +++ b/include/openPMD/binding/python/Container.hpp @@ -0,0 +1,152 @@ +/* Copyright 2018-2022 Axel Huebl and 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 . + * + * The function `bind_container` is based on std_bind.h in pybind11 + * Copyright (c) 2016 Sergey Lyskov and Wenzel Jakob + * + * BSD-style license, see pybind11 LICENSE file. + */ + +#pragma once + +#include "openPMD/backend/Attributable.hpp" + +#include +#include +#include + +#include +#include +#include +#include + +namespace py = pybind11; + +namespace openPMD::detail +{ +/* based on std_bind.h in pybind11 + * + * Copyright (c) 2016 Sergey Lyskov and Wenzel Jakob + * + * BSD-style license, see pybind11 LICENSE file. + */ +template +Class_ bind_container(Class_ &cl, std::string const &name) +{ + using KeyType = typename Map::key_type; + using MappedType = typename Map::mapped_type; + + // Register stream insertion operator (if possible) + py::detail::map_if_insertion_operator(cl, name); + + cl.def( + "__bool__", + [](const Map &m) -> bool { return !m.empty(); }, + "Check whether the container is nonempty"); + + cl.def( + "__iter__", + [](Map &m) { return py::make_key_iterator(m.begin(), m.end()); }, + // keep container alive while iterator exists + py::keep_alive<0, 1>()); + + cl.def("__repr__", [name](Map const &m) { + std::stringstream stream; + stream << ""; + return stream.str(); + }); + + cl.def( + "items", + [](Map &m) { return py::make_iterator(m.begin(), m.end()); }, + // keep container alive while iterator exists + py::keep_alive<0, 1>()); + + // keep same policy as Container class: missing keys are created + cl.def( + "__getitem__", + [](Map &m, KeyType const &k) -> MappedType & { return m[k]; }, + // copy + keepalive + // All objects in the openPMD object model are handles, so using a copy + // is safer and still performant. + py::return_value_policy::copy, + py::keep_alive<0, 1>()); + + // Assignment provided only if the type is copyable + py::detail::map_assignment(cl); + + cl.def("__delitem__", [](Map &m, KeyType const &k) { + auto it = m.find(k); + if (it == m.end()) + throw py::key_error(); + m.erase(it); + }); + + cl.def("__len__", &Map::size); + + cl.def("_ipython_key_completions_", [](Map &m) { + auto l = py::list(); + for (const auto &myPair : m) + l.append(myPair.first); + return l; + }); + + return cl; +} + +template +py::class_, Args...> +create_and_bind_container(py::handle scope, std::string const &name) +{ + using holder_type = std::unique_ptr; + using KeyType = typename Map::key_type; + using MappedType = typename Map::mapped_type; + using Class_ = py::class_; + + // If either type is a non-module-local bound type then make the map + // binding non-local as well; otherwise (e.g. both types are either + // module-local or converting) the map will be module-local. + auto tinfo = py::detail::get_type_info(typeid(MappedType)); + bool local = !tinfo || tinfo->module_local; + if (local) + { + tinfo = py::detail::get_type_info(typeid(KeyType)); + local = !tinfo || tinfo->module_local; + } + + Class_ cl( + scope, + name.c_str(), + py::module_local(local), + py::multiple_inheritance()); + + return bind_container(cl, name); +} +} // namespace openPMD::detail diff --git a/src/binding/python/BaseRecordComponent.cpp b/src/binding/python/BaseRecordComponent.cpp index cf6f2d829c..a4a651c69d 100644 --- a/src/binding/python/BaseRecordComponent.cpp +++ b/src/binding/python/BaseRecordComponent.cpp @@ -29,9 +29,6 @@ void init_BaseRecordComponent(py::module &m) { - auto py_brc_cont = declare_container( - m, "Base_Record_Component_Container"); - py::class_(m, "Base_Record_Component") .def( "__repr__", @@ -50,6 +47,4 @@ void init_BaseRecordComponent(py::module &m) .def_property_readonly("dtype", [](BaseRecordComponent &brc) { return dtype_to_numpy(brc.getDatatype()); }); - - finalize_container(py_brc_cont); } From bb03e16ad1dd657d6f86ac6aac2f447d9ff3b429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 16:26:54 +0100 Subject: [PATCH 14/22] Factor out RecordComponent.__setitem__ and __getitem__ Similarly to the Container API, we will need to apply this to Record-type classes. Defining `__setitem__` and `__getitem__` for them is sufficient, as all other members are inherited from RecordComponent. `__setitem__` and `__getitem__` need special care, as they are inherited from Container AND from RecordComponent, so some conflict resolution is needed. --- .../binding/python/RecordComponent.hpp | 103 ++++++++++++++++++ src/binding/python/RecordComponent.cpp | 61 ++--------- 2 files changed, 112 insertions(+), 52 deletions(-) create mode 100644 include/openPMD/binding/python/RecordComponent.hpp diff --git a/include/openPMD/binding/python/RecordComponent.hpp b/include/openPMD/binding/python/RecordComponent.hpp new file mode 100644 index 0000000000..70a957791a --- /dev/null +++ b/include/openPMD/binding/python/RecordComponent.hpp @@ -0,0 +1,103 @@ +/* Copyright 2018-2022 Axel Huebl and 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 . + * + * The function `bind_container` is based on std_bind.h in pybind11 + * Copyright (c) 2016 Sergey Lyskov and Wenzel Jakob + * + * BSD-style license, see pybind11 LICENSE file. + */ + +#pragma once + +#include "openPMD/RecordComponent.hpp" + +#include +#include +#include + +#include + +namespace py = pybind11; +using namespace openPMD; + +/* + * Definitions for these functions `load_chunk` and `store_chunk` found in + * python/RecordComponent.cpp. + * No need to pull them here, as they are not templates. + */ +py::array load_chunk(RecordComponent &r, py::tuple const &slices); + +void store_chunk(RecordComponent &r, py::array &a, py::tuple const &slices); + +template +Class &&addRecordComponentSetGet(Class &&class_) +{ + // TODO if we also want to support scalar arrays, we have to switch + // py::array for py::buffer as in Attributable + // https://github.com/pybind/pybind11/pull/1537 + + // slicing protocol + class_ + .def( + "__getitem__", + [](RecordComponent &r, py::tuple const &slices) { + return load_chunk(r, slices); + }, + py::arg("tuple of index slices")) + .def( + "__getitem__", + [](RecordComponent &r, py::slice const &slice_obj) { + auto const slices = py::make_tuple(slice_obj); + return load_chunk(r, slices); + }, + py::arg("slice")) + .def( + "__getitem__", + [](RecordComponent &r, py::int_ const &slice_obj) { + auto const slices = py::make_tuple(slice_obj); + return load_chunk(r, slices); + }, + py::arg("axis index")) + + .def( + "__setitem__", + [](RecordComponent &r, py::tuple const &slices, py::array &a) { + store_chunk(r, a, slices); + }, + py::arg("tuple of index slices"), + py::arg("array with values to assign")) + .def( + "__setitem__", + [](RecordComponent &r, py::slice const &slice_obj, py::array &a) { + auto const slices = py::make_tuple(slice_obj); + store_chunk(r, a, slices); + }, + py::arg("slice"), + py::arg("array with values to assign")) + .def( + "__setitem__", + [](RecordComponent &r, py::int_ const &slice_obj, py::array &a) { + auto const slices = py::make_tuple(slice_obj); + store_chunk(r, a, slices); + }, + py::arg("axis index"), + py::arg("array with values to assign")); + return std::forward(class_); +} diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index 9b8154e906..b39fd4c01f 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -18,7 +18,12 @@ * and the GNU Lesser General Public License along with openPMD-api. * If not, see . */ -#include "openPMD/RecordComponent.hpp" +#include +#include +#include + +#include "openPMD/binding/python/RecordComponent.hpp" + #include "openPMD/DatatypeHelpers.hpp" #include "openPMD/Error.hpp" #include "openPMD/Series.hpp" @@ -714,7 +719,7 @@ inline void load_chunk( * * Called with a py::tuple of slices. */ -inline py::array load_chunk(RecordComponent &r, py::tuple const &slices) +py::array load_chunk(RecordComponent &r, py::tuple const &slices) { uint8_t ndim = r.getDimensionality(); auto const full_extent = r.getExtent(); @@ -937,56 +942,6 @@ void init_RecordComponent(py::module &m) return rc.makeEmpty(dtype_from_numpy(dt), dimensionality); }) - // TODO if we also want to support scalar arrays, we have to switch - // py::array for py::buffer as in Attributable - // https://github.com/pybind/pybind11/pull/1537 - - // slicing protocol - .def( - "__getitem__", - [](RecordComponent &r, py::tuple const &slices) { - return load_chunk(r, slices); - }, - py::arg("tuple of index slices")) - .def( - "__getitem__", - [](RecordComponent &r, py::slice const &slice_obj) { - auto const slices = py::make_tuple(slice_obj); - return load_chunk(r, slices); - }, - py::arg("slice")) - .def( - "__getitem__", - [](RecordComponent &r, py::int_ const &slice_obj) { - auto const slices = py::make_tuple(slice_obj); - return load_chunk(r, slices); - }, - py::arg("axis index")) - - .def( - "__setitem__", - [](RecordComponent &r, py::tuple const &slices, py::array &a) { - store_chunk(r, a, slices); - }, - py::arg("tuple of index slices"), - py::arg("array with values to assign")) - .def( - "__setitem__", - [](RecordComponent &r, py::slice const &slice_obj, py::array &a) { - auto const slices = py::make_tuple(slice_obj); - store_chunk(r, a, slices); - }, - py::arg("slice"), - py::arg("array with values to assign")) - .def( - "__setitem__", - [](RecordComponent &r, py::int_ const &slice_obj, py::array &a) { - auto const slices = py::make_tuple(slice_obj); - store_chunk(r, a, slices); - }, - py::arg("axis index"), - py::arg("array with values to assign")) - // deprecated: pass-through C++ API .def( "load_chunk", @@ -1127,6 +1082,8 @@ void init_RecordComponent(py::module &m) .particles[group.at(3)][group.at(4)][group.at(5)]; }); + addRecordComponentSetGet(cl); + finalize_container(py_rc_cnt); py::enum_(m, "Allocation") From 0ccf5ba1dadf82ea821cb9fd449383fc1d210f48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 16:33:24 +0100 Subject: [PATCH 15/22] Consistently use copy semantics in Python API --- src/binding/python/ParticleSpecies.cpp | 7 ++++++- src/binding/python/Series.cpp | 7 +------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/binding/python/ParticleSpecies.cpp b/src/binding/python/ParticleSpecies.cpp index 7c3112029a..ae8f08b711 100644 --- a/src/binding/python/ParticleSpecies.cpp +++ b/src/binding/python/ParticleSpecies.cpp @@ -47,7 +47,12 @@ void init_ParticleSpecies(py::module &m) return stream.str(); }) - .def_readwrite("particle_patches", &ParticleSpecies::particlePatches); + .def_readwrite( + "particle_patches", + &ParticleSpecies::particlePatches, + py::return_value_policy::copy, + // garbage collection: return value must be freed before Series + py::keep_alive<1, 0>()); add_pickle( cl, [](openPMD::Series &series, std::vector const &group) { uint64_t const n_it = std::stoull(group.at(1)); diff --git a/src/binding/python/Series.cpp b/src/binding/python/Series.cpp index 1a163e3a4f..ed2a4180a8 100644 --- a/src/binding/python/Series.cpp +++ b/src/binding/python/Series.cpp @@ -334,12 +334,7 @@ this method. .def_readwrite( "iterations", &Series::iterations, - /* - * Need to keep reference return policy here for now to further - * support legacy `del series` workflows that works despite children - * still being alive. - */ - py::return_value_policy::reference, + py::return_value_policy::copy, // garbage collection: return value must be freed before Series py::keep_alive<1, 0>()) .def( From bf58bf7346c349e54a9cdacf7d4d89dd5455236e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 16:36:12 +0100 Subject: [PATCH 16/22] Apply new class structure to Python API as well --- src/binding/python/BaseRecord.cpp | 38 ++++++++++------------ src/binding/python/BaseRecordComponent.cpp | 3 +- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/binding/python/BaseRecord.cpp b/src/binding/python/BaseRecord.cpp index 233ad90512..dd90006c23 100644 --- a/src/binding/python/BaseRecord.cpp +++ b/src/binding/python/BaseRecord.cpp @@ -25,6 +25,8 @@ #include "openPMD/backend/PatchRecordComponent.hpp" #include "openPMD/binding/python/Common.hpp" +#include "openPMD/binding/python/Container.hpp" +#include "openPMD/binding/python/RecordComponent.hpp" #include "openPMD/binding/python/UnitDimension.hpp" void init_BaseRecord(py::module &m) @@ -33,31 +35,25 @@ void init_BaseRecord(py::module &m) Returns true if this record only contains a single component. )docstr"; - py::class_< - BaseRecord, - Container >(m, "Base_Record_Base_Record_Component") - .def_property_readonly( - "unit_dimension", - &BaseRecord::unitDimension, - python::doc_unit_dimension) - .def_property_readonly( - "scalar", &BaseRecord::scalar, doc_scalar); - - py::class_, Container >( - m, "Base_Record_Record_Component") + addRecordComponentSetGet( + detail::create_and_bind_container< + BaseRecord, + Container, + RecordComponent>(m, "Base_Record_Record_Component")) .def_property_readonly( "scalar", &BaseRecord::scalar, doc_scalar); - - py::class_< - BaseRecord, - Container >(m, "Base_Record_Mesh_Record_Component") + addRecordComponentSetGet( + detail::create_and_bind_container< + BaseRecord, + Container, + MeshRecordComponent>(m, "Base_Record_Mesh_Record_Component")) .def_property_readonly( "scalar", &BaseRecord::scalar, doc_scalar); - - py::class_< - BaseRecord, - Container >( - m, "Base_Record_Patch_Record_Component") + addRecordComponentSetGet( + detail::create_and_bind_container< + BaseRecord, + Container, + PatchRecordComponent>(m, "Base_Record_Patch_Record_Component")) .def_property_readonly( "scalar", &BaseRecord::scalar, doc_scalar); } diff --git a/src/binding/python/BaseRecordComponent.cpp b/src/binding/python/BaseRecordComponent.cpp index a4a651c69d..f069beb8f4 100644 --- a/src/binding/python/BaseRecordComponent.cpp +++ b/src/binding/python/BaseRecordComponent.cpp @@ -29,7 +29,8 @@ void init_BaseRecordComponent(py::module &m) { - py::class_(m, "Base_Record_Component") + py::class_( + m, "Base_Record_Component", py::multiple_inheritance()) .def( "__repr__", [](BaseRecordComponent const &brc) { From 5160f31d35dfbe5c135fe2ff576a9c7309adcb7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 23 Mar 2023 16:54:49 +0100 Subject: [PATCH 17/22] Adapt openpmd-pipe to new design This somewhat demonstrates that this change is slightly API-breaking. Since openpmd-pipe acts directly on the class structure via `instanceof()`, fixes are necessary. --- src/binding/python/openpmd_api/pipe/__main__.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/binding/python/openpmd_api/pipe/__main__.py b/src/binding/python/openpmd_api/pipe/__main__.py index f226bbb51a..32a1df6513 100644 --- a/src/binding/python/openpmd_api/pipe/__main__.py +++ b/src/binding/python/openpmd_api/pipe/__main__.py @@ -265,6 +265,11 @@ def __copy(self, src, dest, current_path="/data/"): io.Mesh_Container, io.Particle_Container, io.ParticleSpecies, io.Record, io.Mesh, io.Particle_Patches, io.Patch_Record ] + is_container = any([ + isinstance(src, container_type) + for container_type in container_types + ]) + if isinstance(src, io.Series): # main loop: read iterations of src, write to dest write_iterations = dest.write_iterations() @@ -302,7 +307,8 @@ def __copy(self, src, dest, current_path="/data/"): self.__particle_patches.clear() self.loads.clear() sys.stdout.flush() - elif isinstance(src, io.Record_Component): + elif isinstance(src, io.Record_Component) and (not is_container + or src.scalar): shape = src.shape offset = [0 for _ in shape] dtype = src.dtype @@ -327,7 +333,8 @@ def __copy(self, src, dest, current_path="/data/"): self.loads.append( deferred_load(src, span, local_chunk.offset, local_chunk.extent)) - elif isinstance(src, io.Patch_Record_Component): + elif isinstance(src, io.Patch_Record_Component) and (not is_container + or src.scalar): dest.reset_dataset(io.Dataset(src.dtype, src.shape)) if self.comm.rank == 0: self.__particle_patches.append( @@ -336,10 +343,7 @@ def __copy(self, src, dest, current_path="/data/"): self.__copy(src.meshes, dest.meshes, current_path + "meshes/") self.__copy(src.particles, dest.particles, current_path + "particles/") - elif any([ - isinstance(src, container_type) - for container_type in container_types - ]): + elif is_container: for key in src: self.__copy(src[key], dest[key], current_path + key + "/") if isinstance(src, io.ParticleSpecies): From 1cd6c6b0ea2889ba471c2350d1d1e4868c977579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 27 Apr 2023 15:31:36 +0200 Subject: [PATCH 18/22] Safeguard: No scalar and vector components side by side "A scalar component can not be contained at the same time as one or more regular components." --- include/openPMD/RecordComponent.hpp | 2 +- include/openPMD/backend/BaseRecord.hpp | 36 +++++++++++++--- .../openPMD/backend/BaseRecordComponent.hpp | 2 +- .../openPMD/backend/PatchRecordComponent.hpp | 2 +- test/CoreTest.cpp | 43 +++++++++++++++++++ 5 files changed, 76 insertions(+), 9 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index c145dabad6..6d89a49115 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -154,7 +154,7 @@ class RecordComponent : public BaseRecordComponent * * @return RecordComponent& */ - RecordComponent &resetDataset(Dataset); + virtual RecordComponent &resetDataset(Dataset); uint8_t getDimensionality() const; Extent getExtent() const; diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index 0f90a7e1c4..be67f24aa9 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -20,9 +20,11 @@ */ #pragma once +#include "openPMD/Error.hpp" #include "openPMD/RecordComponent.hpp" #include "openPMD/UnitDimension.hpp" #include "openPMD/auxiliary/Variant.hpp" +#include "openPMD/backend/BaseRecordComponent.hpp" #include "openPMD/backend/Container.hpp" #include @@ -454,6 +456,17 @@ class BaseRecord */ std::array unitDimension() const; + void setDatasetDefined(BaseRecordComponent::Data_t &data) override + { + if (!T_Container::empty()) + { + throw error::WrongAPIUsage( + "A scalar component can not be contained at the same time as " + "one or more regular components."); + } + T_RecordComponent::setDatasetDefined(data); + } + /** Returns true if this record only contains a single component * * @return true if a record with only a single component @@ -528,9 +541,9 @@ auto BaseRecord::operator[](key_type const &key) -> mapped_type & bool const keyScalar = (key == RecordComponent::SCALAR); if ((keyScalar && !Container::empty() && !scalar()) || (scalar() && !keyScalar)) - throw std::runtime_error( - "A scalar component can not be contained at " - "the same time as one or more regular components."); + throw error::WrongAPIUsage( + "A scalar component can not be contained at the same time as " + "one or more regular components."); if (keyScalar) { @@ -570,9 +583,9 @@ auto BaseRecord::operator[](key_type &&key) -> mapped_type & bool const keyScalar = (key == RecordComponent::SCALAR); if ((keyScalar && !Container::empty() && !scalar()) || (scalar() && !keyScalar)) - throw std::runtime_error( - "A scalar component can not be contained at " - "the same time as one or more regular components."); + throw error::WrongAPIUsage( + "A scalar component can not be contained at the same time as " + "one or more regular components."); if (keyScalar) { @@ -961,6 +974,17 @@ inline void BaseRecord::flush( "RecordComponents: " + name); + /* + * Defensive programming. Normally, this error should yield as soon as + * possible. + */ + if (scalar() && !T_Container::empty()) + { + throw error::WrongAPIUsage( + "A scalar component can not be contained at the same time as " + "one or more regular components."); + } + this->flush_impl(name, flushParams); // flush_impl must take care to correctly set the dirty() flag so this // method doesn't do it diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index 96c8e86af7..3acaceb19c 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -179,7 +179,7 @@ class BaseRecordComponent : virtual public Attributable Attributable::setData(m_baseRecordComponentData); } - void setDatasetDefined(Data_t &); + virtual void setDatasetDefined(Data_t &); bool datasetDefined() const; diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index 7bf9e7b81e..cf5061b66e 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -86,7 +86,7 @@ class PatchRecordComponent : public BaseRecordComponent public: PatchRecordComponent &setUnitSI(double); - PatchRecordComponent &resetDataset(Dataset); + virtual PatchRecordComponent &resetDataset(Dataset); uint8_t getDimensionality() const; Extent getExtent() const; diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index d5562380ca..084d118578 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -1,4 +1,6 @@ // expose private and protected members for invasive testing +#include "openPMD/Datatype.hpp" +#include "openPMD/Error.hpp" #if openPMD_USE_INVASIVE_TESTS #define OPENPMD_private public: #define OPENPMD_protected public: @@ -1388,3 +1390,44 @@ TEST_CASE("unique_ptr", "[core]") UniquePtrWithLambda arrptrFilledCustom{ new int[5]{}, [](int const *p) { delete[] p; }}; } + +TEST_CASE("scalar_and_vector", "[core]") +{ + { + Series series("../samples/scalar_and_vector.json", Access::CREATE); + auto E = series.iterations[0].meshes["E"]; + E["x"].makeEmpty(Datatype::FLOAT, 3); + REQUIRE_THROWS_AS( + E.makeEmpty(Datatype::FLOAT, 3), error::WrongAPIUsage); + } + { + Series series("scalar_and_vector.json", Access::CREATE); + auto E = series.iterations[0].meshes["E"]; + E.makeEmpty(Datatype::FLOAT, 3); + REQUIRE_THROWS_AS(E["x"], error::WrongAPIUsage); + } + { + Series series("../samples/scalar_and_vector.json", Access::CREATE); + auto E = series.iterations[0].meshes["E"]; + E["x"].makeEmpty(Datatype::FLOAT, 3); + } + { + Series read("../samples/scalar_and_vector.json", Access::READ_ONLY); + auto E = read.iterations[0].meshes["E"]; + REQUIRE(E.size() == 1); + REQUIRE(!E.scalar()); + REQUIRE(E.contains("x")); + } + { + Series series("../samples/scalar_and_vector.json", Access::CREATE); + auto E = series.iterations[0].meshes["E"]; + E.makeEmpty(Datatype::FLOAT, 3); + } + { + Series read("../samples/scalar_and_vector.json", Access::READ_ONLY); + auto E = read.iterations[0].meshes["E"]; + REQUIRE(E.size() == 1); + REQUIRE(E.scalar()); + REQUIRE(!E.contains("x")); + } +} From b117863bf057855a36411337d24a69b306d9427d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 27 Apr 2023 15:50:28 +0200 Subject: [PATCH 19/22] Remove [SCALAR] from all examples --- examples/10_streaming_write.py | 2 +- examples/12_span_write.cpp | 3 +-- examples/13_write_dynamic_configuration.cpp | 3 +-- examples/13_write_dynamic_configuration.py | 2 +- examples/1_structure.cpp | 3 +-- examples/2_read_serial.cpp | 4 +--- examples/2_read_serial.py | 2 +- examples/3_write_serial.cpp | 3 +-- examples/3_write_serial.py | 2 +- examples/5_write_parallel.cpp | 3 +-- examples/5_write_parallel.py | 2 +- examples/7_extended_write_serial.cpp | 15 +++++---------- examples/7_extended_write_serial.py | 10 +++++----- examples/8a_benchmark_write_parallel.cpp | 10 ++++------ examples/8b_benchmark_read_parallel.cpp | 2 +- examples/9_particle_write_serial.py | 8 ++++---- 16 files changed, 30 insertions(+), 44 deletions(-) diff --git a/examples/10_streaming_write.py b/examples/10_streaming_write.py index e0b830bd35..575079ea6d 100755 --- a/examples/10_streaming_write.py +++ b/examples/10_streaming_write.py @@ -69,7 +69,7 @@ temperature.axis_labels = ["x", "y"] temperature.grid_spacing = [1., 1.] # temperature has no x,y,z components, so skip the last layer: - temperature_dataset = temperature[io.Mesh_Record_Component.SCALAR] + temperature_dataset = temperature # let's say we are in a 3x3 mesh temperature_dataset.reset_dataset( io.Dataset(np.dtype("double"), [3, 3])) diff --git a/examples/12_span_write.cpp b/examples/12_span_write.cpp index 089ceddff3..45379c77e9 100644 --- a/examples/12_span_write.cpp +++ b/examples/12_span_write.cpp @@ -90,8 +90,7 @@ void span_write(std::string const &filename) using mesh_type = position_t; - RecordComponent chargeDensity = - iteration.meshes["e_chargeDensity"][RecordComponent::SCALAR]; + Mesh chargeDensity = iteration.meshes["e_chargeDensity"]; /* * A similar memory optimization is possible by using a unique_ptr type diff --git a/examples/13_write_dynamic_configuration.cpp b/examples/13_write_dynamic_configuration.cpp index b6e7f3694c..73ce0be095 100644 --- a/examples/13_write_dynamic_configuration.cpp +++ b/examples/13_write_dynamic_configuration.cpp @@ -124,8 +124,7 @@ chunks = "auto" Dataset differentlyCompressedDataset{Datatype::INT, {10}}; differentlyCompressedDataset.options = differentCompressionSettings; - auto someMesh = iteration.meshes["differentCompressionSettings"] - [RecordComponent::SCALAR]; + auto someMesh = iteration.meshes["differentCompressionSettings"]; someMesh.resetDataset(differentlyCompressedDataset); std::vector dataVec(10, i); someMesh.storeChunk(dataVec, {0}, {10}); diff --git a/examples/13_write_dynamic_configuration.py b/examples/13_write_dynamic_configuration.py index bb7e81ce4a..f3e1e63bd5 100644 --- a/examples/13_write_dynamic_configuration.py +++ b/examples/13_write_dynamic_configuration.py @@ -130,7 +130,7 @@ def main(): temperature.axis_labels = ["x", "y"] temperature.grid_spacing = [1., 1.] # temperature has no x,y,z components, so skip the last layer: - temperature_dataset = temperature[io.Mesh_Record_Component.SCALAR] + temperature_dataset = temperature # let's say we are in a 3x3 mesh dataset = io.Dataset(np.dtype("double"), [3, 3]) dataset.options = json.dumps(config) diff --git a/examples/1_structure.cpp b/examples/1_structure.cpp index 6e595c56ba..2ecb909722 100644 --- a/examples/1_structure.cpp +++ b/examples/1_structure.cpp @@ -54,10 +54,9 @@ int main() * differently. * https://github.com/openPMD/openPMD-standard/blob/latest/STANDARD.md#scalar-vector-and-tensor-records*/ Record mass = electrons["mass"]; - RecordComponent mass_scalar = mass[RecordComponent::SCALAR]; Dataset dataset = Dataset(Datatype::DOUBLE, Extent{1}); - mass_scalar.resetDataset(dataset); + mass.resetDataset(dataset); /* Required Records and RecordComponents are created automatically. * Initialization has to be done explicitly by the user. */ diff --git a/examples/2_read_serial.cpp b/examples/2_read_serial.cpp index 8fb3ccb190..8843dd45ed 100644 --- a/examples/2_read_serial.cpp +++ b/examples/2_read_serial.cpp @@ -58,9 +58,7 @@ int main() } openPMD::ParticleSpecies electrons = i.particles["electrons"]; - std::shared_ptr charge = - electrons["charge"][openPMD::RecordComponent::SCALAR] - .loadChunk(); + std::shared_ptr charge = electrons["charge"].loadChunk(); series.flush(); cout << "And the first electron particle has a charge = " << charge.get()[0]; diff --git a/examples/2_read_serial.py b/examples/2_read_serial.py index 87b5568306..ce24f3e941 100644 --- a/examples/2_read_serial.py +++ b/examples/2_read_serial.py @@ -34,7 +34,7 @@ # printing a scalar value electrons = i.particles["electrons"] - charge = electrons["charge"][io.Mesh_Record_Component.SCALAR] + charge = electrons["charge"] series.flush() print("And the first electron particle has a charge {}" .format(charge[0])) diff --git a/examples/3_write_serial.cpp b/examples/3_write_serial.cpp index aeb62aef6c..7a91f37973 100644 --- a/examples/3_write_serial.cpp +++ b/examples/3_write_serial.cpp @@ -49,8 +49,7 @@ int main(int argc, char *argv[]) // in streaming setups, e.g. an iteration cannot be opened again once // it has been closed. // `Series::iterations` can be directly accessed in random-access workflows. - MeshRecordComponent rho = - series.writeIterations()[1].meshes["rho"][MeshRecordComponent::SCALAR]; + Mesh rho = series.writeIterations()[1].meshes["rho"]; cout << "Created a scalar mesh Record with all required openPMD " "attributes\n"; diff --git a/examples/3_write_serial.py b/examples/3_write_serial.py index b1bdd2d063..8e3b3bfe5e 100644 --- a/examples/3_write_serial.py +++ b/examples/3_write_serial.py @@ -34,7 +34,7 @@ # it has been closed. # `Series.iterations` can be directly accessed in random-access workflows. rho = series.write_iterations()[1]. \ - meshes["rho"][io.Mesh_Record_Component.SCALAR] + meshes["rho"] dataset = io.Dataset(data.dtype, data.shape) diff --git a/examples/5_write_parallel.cpp b/examples/5_write_parallel.cpp index 2b70c775cb..8587175fe7 100644 --- a/examples/5_write_parallel.cpp +++ b/examples/5_write_parallel.cpp @@ -64,8 +64,7 @@ int main(int argc, char *argv[]) // in streaming setups, e.g. an iteration cannot be opened again once // it has been closed. series.iterations[1].open(); - MeshRecordComponent mymesh = - series.iterations[1].meshes["mymesh"][MeshRecordComponent::SCALAR]; + Mesh mymesh = series.iterations[1].meshes["mymesh"]; // example 1D domain decomposition in first index Datatype datatype = determineDatatype(); diff --git a/examples/5_write_parallel.py b/examples/5_write_parallel.py index 27aa01f94c..ace0cd6e63 100644 --- a/examples/5_write_parallel.py +++ b/examples/5_write_parallel.py @@ -48,7 +48,7 @@ # `Series.iterations` can be directly accessed in random-access workflows. series.iterations[1].open() mymesh = series.iterations[1]. \ - meshes["mymesh"][io.Mesh_Record_Component.SCALAR] + meshes["mymesh"] # example 1D domain decomposition in first index global_extent = [comm.size * 10, 300] diff --git a/examples/7_extended_write_serial.cpp b/examples/7_extended_write_serial.cpp index 580894a34f..6b69635fc6 100644 --- a/examples/7_extended_write_serial.cpp +++ b/examples/7_extended_write_serial.cpp @@ -83,7 +83,7 @@ int main() {{io::UnitDimension::M, 1}}); electrons["displacement"]["x"].setUnitSI(1e-6); electrons.erase("displacement"); - electrons["weighting"][io::RecordComponent::SCALAR] + electrons["weighting"] .resetDataset({io::Datatype::FLOAT, {1}}) .makeConstant(1.e-5); } @@ -150,11 +150,8 @@ int main() electrons["positionOffset"]["x"].resetDataset(d); auto dset = io::Dataset(io::determineDatatype(), {2}); - electrons.particlePatches["numParticles"][io::RecordComponent::SCALAR] - .resetDataset(dset); - electrons - .particlePatches["numParticlesOffset"][io::RecordComponent::SCALAR] - .resetDataset(dset); + electrons.particlePatches["numParticles"].resetDataset(dset); + electrons.particlePatches["numParticlesOffset"].resetDataset(dset); dset = io::Dataset(io::Datatype::FLOAT, {2}); electrons.particlePatches["offset"].setUnitDimension( @@ -204,12 +201,10 @@ int main() electrons["positionOffset"]["x"].storeChunk( partial_particleOff, o, e); - electrons - .particlePatches["numParticles"][io::RecordComponent::SCALAR] - .store(i, numParticles); + electrons.particlePatches["numParticles"].store(i, numParticles); electrons .particlePatches["numParticlesOffset"] - [io::RecordComponent::SCALAR] + .store(i, numParticlesOffset); electrons.particlePatches["offset"]["x"].store( diff --git a/examples/7_extended_write_serial.py b/examples/7_extended_write_serial.py index e16b4b993a..d9b50693cb 100755 --- a/examples/7_extended_write_serial.py +++ b/examples/7_extended_write_serial.py @@ -90,7 +90,7 @@ electrons["displacement"].unit_dimension = {Unit_Dimension.M: 1} electrons["displacement"]["x"].unit_SI = 1.e-6 del electrons["displacement"] - electrons["weighting"][SCALAR] \ + electrons["weighting"] \ .reset_dataset(Dataset(np.dtype("float32"), extent=[1])) \ .make_constant(1.e-5) @@ -137,8 +137,8 @@ electrons["positionOffset"]["x"].reset_dataset(d) dset = Dataset(np.dtype("uint64"), extent=[2]) - electrons.particle_patches["numParticles"][SCALAR].reset_dataset(dset) - electrons.particle_patches["numParticlesOffset"][SCALAR]. \ + electrons.particle_patches["numParticles"].reset_dataset(dset) + electrons.particle_patches["numParticlesOffset"]. \ reset_dataset(dset) dset = Dataset(partial_particlePos.dtype, extent=[2]) @@ -185,9 +185,9 @@ electrons["position"]["x"][o:u] = partial_particlePos electrons["positionOffset"]["x"][o:u] = partial_particleOff - electrons.particle_patches["numParticles"][SCALAR].store( + electrons.particle_patches["numParticles"].store( i, np.array([numParticles], dtype=np.uint64)) - electrons.particle_patches["numParticlesOffset"][SCALAR].store( + electrons.particle_patches["numParticlesOffset"].store( i, np.array([numParticlesOffset], dtype=np.uint64)) electrons.particle_patches["offset"]["x"].store( diff --git a/examples/8a_benchmark_write_parallel.cpp b/examples/8a_benchmark_write_parallel.cpp index c509c879ff..bd7ea10773 100644 --- a/examples/8a_benchmark_write_parallel.cpp +++ b/examples/8a_benchmark_write_parallel.cpp @@ -821,8 +821,8 @@ void AbstractPattern::storeParticles(ParticleSpecies &currSpecies, int &step) openPMD::Dataset(openPMD::determineDatatype(), {np}); auto const realDataSet = openPMD::Dataset(openPMD::determineDatatype(), {np}); - currSpecies["id"][RecordComponent::SCALAR].resetDataset(intDataSet); - currSpecies["charge"][RecordComponent::SCALAR].resetDataset(realDataSet); + currSpecies["id"].resetDataset(intDataSet); + currSpecies["charge"].resetDataset(realDataSet); currSpecies["position"]["x"].resetDataset(realDataSet); @@ -838,12 +838,10 @@ void AbstractPattern::storeParticles(ParticleSpecies &currSpecies, int &step) if (count > 0) { auto ids = createData(count, offset, 1); - currSpecies["id"][RecordComponent::SCALAR].storeChunk( - ids, {offset}, {count}); + currSpecies["id"].storeChunk(ids, {offset}, {count}); auto charges = createData(count, 0.1 * step, 0.0001); - currSpecies["charge"][RecordComponent::SCALAR].storeChunk( - charges, {offset}, {count}); + currSpecies["charge"].storeChunk(charges, {offset}, {count}); auto mx = createData(count, 1.0 * step, 0.0002); currSpecies["position"]["x"].storeChunk(mx, {offset}, {count}); diff --git a/examples/8b_benchmark_read_parallel.cpp b/examples/8b_benchmark_read_parallel.cpp index 98cd81add2..cb9d6a88c4 100644 --- a/examples/8b_benchmark_read_parallel.cpp +++ b/examples/8b_benchmark_read_parallel.cpp @@ -751,7 +751,7 @@ class TestInput } openPMD::ParticleSpecies p = iter.particles.begin()->second; - RecordComponent idVal = p["id"][RecordComponent::SCALAR]; + Record idVal = p["id"]; Extent pExtent = idVal.getExtent(); diff --git a/examples/9_particle_write_serial.py b/examples/9_particle_write_serial.py index 5af8796d49..10a9557d21 100644 --- a/examples/9_particle_write_serial.py +++ b/examples/9_particle_write_serial.py @@ -39,14 +39,14 @@ # let's set a weird user-defined record this time electrons["displacement"].unit_dimension = {Unit_Dimension.M: 1} - electrons["displacement"][SCALAR].unit_SI = 1.e-6 + electrons["displacement"].unit_SI = 1.e-6 dset = Dataset(np.dtype("float64"), extent=[n_particles]) - electrons["displacement"][SCALAR].reset_dataset(dset) - electrons["displacement"][SCALAR].make_constant(42.43) + electrons["displacement"].reset_dataset(dset) + electrons["displacement"].make_constant(42.43) # don't like it anymore? remove it with: # del electrons["displacement"] - electrons["weighting"][SCALAR] \ + electrons["weighting"] \ .reset_dataset(Dataset(np.dtype("float32"), extent=[n_particles])) \ .make_constant(1.e-5) From 3142ef4f6437bd82b6e9edcd9805b45cd1cb133c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 3 May 2023 17:10:51 +0200 Subject: [PATCH 20/22] Avoid object slicing when using records as scalar components --- include/openPMD/RecordComponent.hpp | 12 ++++++++++++ include/openPMD/backend/BaseRecord.hpp | 3 +++ include/openPMD/backend/BaseRecordComponent.hpp | 3 +++ include/openPMD/backend/MeshRecordComponent.hpp | 9 +++++++++ include/openPMD/backend/PatchRecordComponent.hpp | 9 +++++++++ src/RecordComponent.cpp | 7 +++++++ src/backend/MeshRecordComponent.cpp | 8 ++++++++ src/backend/PatchRecordComponent.cpp | 8 ++++++++ 8 files changed, 59 insertions(+) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 6d89a49115..bb01dfe13f 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -109,6 +109,9 @@ namespace internal class BaseRecordData; } // namespace internal +template +class BaseRecord; + class RecordComponent : public BaseRecordComponent { template @@ -134,6 +137,15 @@ class RecordComponent : public BaseRecordComponent AUTO }; // Allocation + /** + * @brief Avoid object slicing when using a Record as a scalar Record + * Component. + * + * It's still preferred to directly use the Record, or alternatively a + * Record-Component-type reference to a Record. + */ + RecordComponent(BaseRecord const &); + RecordComponent &setUnitSI(double); /** diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index be67f24aa9..25606f9f76 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -212,8 +212,11 @@ class BaseRecord : public Container , public T_elem // T_RecordComponent { +public: using T_RecordComponent = T_elem; using T_Container = Container; + +private: using T_Self = BaseRecord; friend class Iteration; friend class ParticleSpecies; diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index 3acaceb19c..0288e9bb9a 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -74,6 +74,9 @@ namespace internal }; } // namespace internal +template +class BaseRecord; + class BaseRecordComponent : virtual public Attributable { template diff --git a/include/openPMD/backend/MeshRecordComponent.hpp b/include/openPMD/backend/MeshRecordComponent.hpp index 97acb26e43..3d10cedacd 100644 --- a/include/openPMD/backend/MeshRecordComponent.hpp +++ b/include/openPMD/backend/MeshRecordComponent.hpp @@ -52,6 +52,15 @@ class MeshRecordComponent : public RecordComponent public: ~MeshRecordComponent() override = default; + /** + * @brief Avoid object slicing when using a Record as a scalar Record + * Component. + * + * It's still preferred to directly use the Record, or alternatively a + * Record-Component-type reference to a Record. + */ + MeshRecordComponent(BaseRecord const &); + /** Position on an element * * Relative on an element (node/cell/voxel) of the mesh diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp index cf5061b66e..10ca725e33 100644 --- a/include/openPMD/backend/PatchRecordComponent.hpp +++ b/include/openPMD/backend/PatchRecordComponent.hpp @@ -84,6 +84,15 @@ class PatchRecordComponent : public BaseRecordComponent friend class internal::PatchRecordComponentData; public: + /** + * @brief Avoid object slicing when using a Record as a scalar Record + * Component. + * + * It's still preferred to directly use the Record, or alternatively a + * Record-Component-type reference to a Record. + */ + PatchRecordComponent(BaseRecord const &); + PatchRecordComponent &setUnitSI(double); virtual PatchRecordComponent &resetDataset(Dataset); diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index d6110ed50d..5833e53af6 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -25,6 +25,7 @@ #include "openPMD/IO/Format.hpp" #include "openPMD/Series.hpp" #include "openPMD/auxiliary/Memory.hpp" +#include "openPMD/backend/BaseRecord.hpp" #include #include @@ -48,6 +49,12 @@ RecordComponent::RecordComponent() : BaseRecordComponent(NoInit()) RecordComponent::RecordComponent(NoInit) : BaseRecordComponent(NoInit()) {} +RecordComponent::RecordComponent(BaseRecord const &baseRecord) + : BaseRecordComponent(NoInit()) +{ + setData(baseRecord.m_recordComponentData); +} + // We need to instantiate this somewhere otherwise there might be linker issues // despite this thing actually being constepxr constexpr char const *const RecordComponent::SCALAR; diff --git a/src/backend/MeshRecordComponent.cpp b/src/backend/MeshRecordComponent.cpp index 8d65783018..2be3879ea9 100644 --- a/src/backend/MeshRecordComponent.cpp +++ b/src/backend/MeshRecordComponent.cpp @@ -19,6 +19,7 @@ * If not, see . */ #include "openPMD/backend/MeshRecordComponent.hpp" +#include "openPMD/backend/BaseRecord.hpp" namespace openPMD { @@ -28,6 +29,13 @@ MeshRecordComponent::MeshRecordComponent() : RecordComponent() MeshRecordComponent::MeshRecordComponent(NoInit) : RecordComponent(NoInit()) {} +MeshRecordComponent::MeshRecordComponent( + BaseRecord const &baseRecord) + : RecordComponent(NoInit()) +{ + setData(baseRecord.m_recordComponentData); +} + void MeshRecordComponent::read() { using DT = Datatype; diff --git a/src/backend/PatchRecordComponent.cpp b/src/backend/PatchRecordComponent.cpp index 33aeb433b5..3277bac550 100644 --- a/src/backend/PatchRecordComponent.cpp +++ b/src/backend/PatchRecordComponent.cpp @@ -20,6 +20,7 @@ */ #include "openPMD/backend/PatchRecordComponent.hpp" #include "openPMD/auxiliary/Memory.hpp" +#include "openPMD/backend/BaseRecord.hpp" #include @@ -74,6 +75,13 @@ Extent PatchRecordComponent::getExtent() const } } +PatchRecordComponent::PatchRecordComponent( + BaseRecord const &baseRecord) + : BaseRecordComponent(NoInit()) +{ + setData(baseRecord.m_patchRecordComponentData); +} + PatchRecordComponent::PatchRecordComponent() : BaseRecordComponent(NoInit()) { setData(std::make_shared()); From 796da2fdfa42ffc6453b8f187d0c3f813d84d3d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 5 May 2023 15:25:35 +0200 Subject: [PATCH 21/22] Documentation --- docs/source/usage/concepts.rst | 3 +++ include/openPMD/backend/BaseRecord.hpp | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/docs/source/usage/concepts.rst b/docs/source/usage/concepts.rst index 43e9ace4dc..91253f8945 100644 --- a/docs/source/usage/concepts.rst +++ b/docs/source/usage/concepts.rst @@ -20,6 +20,9 @@ A record is an data set with common properties, e.g. the electric field :math:`\ A density field could be another record - which is scalar as it only has one component. In general, openPMD allows records with arbitrary number of components (tensors), as well as vector records and scalar records. +In the case of vector records, the single components are stored as datasets within the record. +In the case of scalar records, the record and component are equivalent. +In the API, the record can be directly used as a component, and in the standard a scalar record is represented by the scalar dataset with attributes. Meshes and Particles -------------------- diff --git a/include/openPMD/backend/BaseRecord.hpp b/include/openPMD/backend/BaseRecord.hpp index 25606f9f76..88bd73b247 100644 --- a/include/openPMD/backend/BaseRecord.hpp +++ b/include/openPMD/backend/BaseRecord.hpp @@ -207,6 +207,16 @@ namespace internal }; } // namespace internal +/** + * @brief Base class for any type of record (e.g. mesh or particle record). + * + * If the record is a vector record, the single components are accessed via the + * container interface (base class 1). + * If the record is a scalar record, it directly acts as a record component + * (base class 2) and the Container API needs not be used. + * + * @tparam T_elem + */ template class BaseRecord : public Container From 8b9239abc9f58743680e9168ef54e3d85d1e49dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 25 Oct 2023 12:24:04 +0200 Subject: [PATCH 22/22] Adapt to refactored Python bindings after rebasing --- CMakeLists.txt | 1 - include/openPMD/binding/python/Common.hpp | 6 + include/openPMD/binding/python/Container.H | 29 ++-- include/openPMD/binding/python/Container.hpp | 152 ------------------ .../binding/python/RecordComponent.hpp | 7 + src/binding/python/BaseRecord.cpp | 59 ------- src/binding/python/Iteration.cpp | 5 +- src/binding/python/Mesh.cpp | 4 +- src/binding/python/MeshRecordComponent.cpp | 17 +- src/binding/python/ParticlePatches.cpp | 5 +- src/binding/python/ParticleSpecies.cpp | 5 +- src/binding/python/PatchRecord.cpp | 5 +- src/binding/python/PatchRecordComponent.cpp | 17 +- src/binding/python/Record.cpp | 4 +- src/binding/python/RecordComponent.cpp | 18 ++- src/binding/python/openPMD.cpp | 1 - 16 files changed, 84 insertions(+), 251 deletions(-) delete mode 100644 include/openPMD/binding/python/Container.hpp delete mode 100644 src/binding/python/BaseRecord.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 253b7b8bbe..20673b1c50 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -604,7 +604,6 @@ if(openPMD_HAVE_PYTHON) src/binding/python/openPMD.cpp src/binding/python/Access.cpp src/binding/python/Attributable.cpp - src/binding/python/BaseRecord.cpp src/binding/python/BaseRecordComponent.cpp src/binding/python/ChunkInfo.cpp src/binding/python/Dataset.cpp diff --git a/include/openPMD/binding/python/Common.hpp b/include/openPMD/binding/python/Common.hpp index ea6fff2832..7b42b919d8 100644 --- a/include/openPMD/binding/python/Common.hpp +++ b/include/openPMD/binding/python/Common.hpp @@ -13,6 +13,7 @@ #include "openPMD/ParticlePatches.hpp" #include "openPMD/ParticleSpecies.hpp" #include "openPMD/Record.hpp" +#include "openPMD/RecordComponent.hpp" #include "openPMD/Series.hpp" #include "openPMD/backend/BaseRecord.hpp" #include "openPMD/backend/BaseRecordComponent.hpp" @@ -42,6 +43,9 @@ using PyPatchRecordContainer = Container; using PyRecordComponentContainer = Container; using PyMeshRecordComponentContainer = Container; using PyPatchRecordComponentContainer = Container; +using PyBaseRecordRecordComponent = BaseRecord; +using PyBaseRecordMeshRecordComponent = BaseRecord; +using PyBaseRecordPatchRecordComponent = BaseRecord; PYBIND11_MAKE_OPAQUE(PyIterationContainer) PYBIND11_MAKE_OPAQUE(PyMeshContainer) PYBIND11_MAKE_OPAQUE(PyPartContainer) @@ -51,3 +55,5 @@ PYBIND11_MAKE_OPAQUE(PyPatchRecordContainer) PYBIND11_MAKE_OPAQUE(PyRecordComponentContainer) PYBIND11_MAKE_OPAQUE(PyMeshRecordComponentContainer) PYBIND11_MAKE_OPAQUE(PyPatchRecordComponentContainer) +PYBIND11_MAKE_OPAQUE(PyBaseRecordRecordComponent) +PYBIND11_MAKE_OPAQUE(PyBaseRecordPatchRecordComponent) diff --git a/include/openPMD/binding/python/Container.H b/include/openPMD/binding/python/Container.H index 28d0abedd1..350eaed823 100644 --- a/include/openPMD/binding/python/Container.H +++ b/include/openPMD/binding/python/Container.H @@ -29,7 +29,6 @@ #include "openPMD/backend/MeshRecordComponent.hpp" #include "openPMD/backend/PatchRecord.hpp" #include "openPMD/backend/PatchRecordComponent.hpp" -#include "openPMD/binding/python/Container.hpp" #include "openPMD/binding/python/Common.hpp" @@ -47,16 +46,14 @@ namespace openPMD * * BSD-style license, see pybind11 LICENSE file. */ -template < - typename Map, - typename holder_type = std::unique_ptr, - typename... Args> -py::class_ -declare_container(py::handle scope, std::string const &name, Args &&...args) +template +py::class_, Args...> +declare_container(py::handle scope, std::string const &name) { + using holder_type = std::unique_ptr; using KeyType = typename Map::key_type; using MappedType = typename Map::mapped_type; - using Class_ = py::class_; + using Class_ = py::class_; // If either type is a non-module-local bound type then make the map // binding non-local as well; otherwise (e.g. both types are either @@ -69,13 +66,9 @@ declare_container(py::handle scope, std::string const &name, Args &&...args) local = !tinfo || tinfo->module_local; } - Class_ cl( - scope, - name.c_str(), - py::module_local(local), - std::forward(args)...); + Class_ cl(scope, name.c_str(), py::module_local(local)); - cl.def(py::init()); + // cl.def(py::init()); // Register stream insertion operator (if possible) py::detail::map_if_insertion_operator(cl, name); @@ -110,15 +103,11 @@ declare_container(py::handle scope, std::string const &name, Args &&...args) return cl; } -template < - typename Map, - typename holder_type = std::unique_ptr> -py::class_ -finalize_container(py::class_ cl) +template +Class_ finalize_container(Class_ cl) { using KeyType = typename Map::key_type; using MappedType = typename Map::mapped_type; - using Class_ = py::class_; cl.def( "items", diff --git a/include/openPMD/binding/python/Container.hpp b/include/openPMD/binding/python/Container.hpp deleted file mode 100644 index 0b24951108..0000000000 --- a/include/openPMD/binding/python/Container.hpp +++ /dev/null @@ -1,152 +0,0 @@ -/* Copyright 2018-2022 Axel Huebl and 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 . - * - * The function `bind_container` is based on std_bind.h in pybind11 - * Copyright (c) 2016 Sergey Lyskov and Wenzel Jakob - * - * BSD-style license, see pybind11 LICENSE file. - */ - -#pragma once - -#include "openPMD/backend/Attributable.hpp" - -#include -#include -#include - -#include -#include -#include -#include - -namespace py = pybind11; - -namespace openPMD::detail -{ -/* based on std_bind.h in pybind11 - * - * Copyright (c) 2016 Sergey Lyskov and Wenzel Jakob - * - * BSD-style license, see pybind11 LICENSE file. - */ -template -Class_ bind_container(Class_ &cl, std::string const &name) -{ - using KeyType = typename Map::key_type; - using MappedType = typename Map::mapped_type; - - // Register stream insertion operator (if possible) - py::detail::map_if_insertion_operator(cl, name); - - cl.def( - "__bool__", - [](const Map &m) -> bool { return !m.empty(); }, - "Check whether the container is nonempty"); - - cl.def( - "__iter__", - [](Map &m) { return py::make_key_iterator(m.begin(), m.end()); }, - // keep container alive while iterator exists - py::keep_alive<0, 1>()); - - cl.def("__repr__", [name](Map const &m) { - std::stringstream stream; - stream << ""; - return stream.str(); - }); - - cl.def( - "items", - [](Map &m) { return py::make_iterator(m.begin(), m.end()); }, - // keep container alive while iterator exists - py::keep_alive<0, 1>()); - - // keep same policy as Container class: missing keys are created - cl.def( - "__getitem__", - [](Map &m, KeyType const &k) -> MappedType & { return m[k]; }, - // copy + keepalive - // All objects in the openPMD object model are handles, so using a copy - // is safer and still performant. - py::return_value_policy::copy, - py::keep_alive<0, 1>()); - - // Assignment provided only if the type is copyable - py::detail::map_assignment(cl); - - cl.def("__delitem__", [](Map &m, KeyType const &k) { - auto it = m.find(k); - if (it == m.end()) - throw py::key_error(); - m.erase(it); - }); - - cl.def("__len__", &Map::size); - - cl.def("_ipython_key_completions_", [](Map &m) { - auto l = py::list(); - for (const auto &myPair : m) - l.append(myPair.first); - return l; - }); - - return cl; -} - -template -py::class_, Args...> -create_and_bind_container(py::handle scope, std::string const &name) -{ - using holder_type = std::unique_ptr; - using KeyType = typename Map::key_type; - using MappedType = typename Map::mapped_type; - using Class_ = py::class_; - - // If either type is a non-module-local bound type then make the map - // binding non-local as well; otherwise (e.g. both types are either - // module-local or converting) the map will be module-local. - auto tinfo = py::detail::get_type_info(typeid(MappedType)); - bool local = !tinfo || tinfo->module_local; - if (local) - { - tinfo = py::detail::get_type_info(typeid(KeyType)); - local = !tinfo || tinfo->module_local; - } - - Class_ cl( - scope, - name.c_str(), - py::module_local(local), - py::multiple_inheritance()); - - return bind_container(cl, name); -} -} // namespace openPMD::detail diff --git a/include/openPMD/binding/python/RecordComponent.hpp b/include/openPMD/binding/python/RecordComponent.hpp index 70a957791a..80a349bfd4 100644 --- a/include/openPMD/binding/python/RecordComponent.hpp +++ b/include/openPMD/binding/python/RecordComponent.hpp @@ -46,6 +46,13 @@ py::array load_chunk(RecordComponent &r, py::tuple const &slices); void store_chunk(RecordComponent &r, py::array &a, py::tuple const &slices); +namespace docstring +{ +constexpr static char const *is_scalar = R"docstr( +Returns true if this record only contains a single component. +)docstr"; +} + template Class &&addRecordComponentSetGet(Class &&class_) { diff --git a/src/binding/python/BaseRecord.cpp b/src/binding/python/BaseRecord.cpp deleted file mode 100644 index dd90006c23..0000000000 --- a/src/binding/python/BaseRecord.cpp +++ /dev/null @@ -1,59 +0,0 @@ -/* Copyright 2018-2021 Axel Huebl - * - * This file is part of openPMD-api. - * - * openPMD-api is free software: you can redistribute it and/or modify - * it under the terms of of either the GNU General Public License or - * the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * openPMD-api is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License and the GNU Lesser General Public License - * for more details. - * - * You should have received a copy of the GNU General Public License - * and the GNU Lesser General Public License along with openPMD-api. - * If not, see . - */ -#include "openPMD/backend/BaseRecord.hpp" -#include "openPMD/backend/BaseRecordComponent.hpp" -#include "openPMD/backend/Container.hpp" -#include "openPMD/backend/MeshRecordComponent.hpp" -#include "openPMD/backend/PatchRecordComponent.hpp" - -#include "openPMD/binding/python/Common.hpp" -#include "openPMD/binding/python/Container.hpp" -#include "openPMD/binding/python/RecordComponent.hpp" -#include "openPMD/binding/python/UnitDimension.hpp" - -void init_BaseRecord(py::module &m) -{ - constexpr auto doc_scalar = R"docstr( -Returns true if this record only contains a single component. -)docstr"; - - addRecordComponentSetGet( - detail::create_and_bind_container< - BaseRecord, - Container, - RecordComponent>(m, "Base_Record_Record_Component")) - .def_property_readonly( - "scalar", &BaseRecord::scalar, doc_scalar); - addRecordComponentSetGet( - detail::create_and_bind_container< - BaseRecord, - Container, - MeshRecordComponent>(m, "Base_Record_Mesh_Record_Component")) - .def_property_readonly( - "scalar", &BaseRecord::scalar, doc_scalar); - addRecordComponentSetGet( - detail::create_and_bind_container< - BaseRecord, - Container, - PatchRecordComponent>(m, "Base_Record_Patch_Record_Component")) - .def_property_readonly( - "scalar", &BaseRecord::scalar, doc_scalar); -} diff --git a/src/binding/python/Iteration.cpp b/src/binding/python/Iteration.cpp index a9dc2c23c8..df017114e6 100644 --- a/src/binding/python/Iteration.cpp +++ b/src/binding/python/Iteration.cpp @@ -20,6 +20,7 @@ */ #include "openPMD/Iteration.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/binding/python/Common.hpp" #include "openPMD/binding/python/Container.H" @@ -29,8 +30,8 @@ void init_Iteration(py::module &m) { - auto py_it_cont = - declare_container(m, "Iteration_Container"); + auto py_it_cont = declare_container( + m, "Iteration_Container"); py::class_(m, "Iteration") .def(py::init()) diff --git a/src/binding/python/Mesh.cpp b/src/binding/python/Mesh.cpp index 506e1bba70..55c6fd13a4 100644 --- a/src/binding/python/Mesh.cpp +++ b/src/binding/python/Mesh.cpp @@ -19,6 +19,7 @@ * If not, see . */ #include "openPMD/Mesh.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/BaseRecord.hpp" #include "openPMD/backend/MeshRecordComponent.hpp" @@ -32,7 +33,8 @@ void init_Mesh(py::module &m) { - auto py_m_cont = declare_container(m, "Mesh_Container"); + auto py_m_cont = + declare_container(m, "Mesh_Container"); py::class_ > cl(m, "Mesh"); diff --git a/src/binding/python/MeshRecordComponent.cpp b/src/binding/python/MeshRecordComponent.cpp index e2e5e7b42a..1a43f0e289 100644 --- a/src/binding/python/MeshRecordComponent.cpp +++ b/src/binding/python/MeshRecordComponent.cpp @@ -23,17 +23,20 @@ #include "openPMD/RecordComponent.hpp" #include "openPMD/Series.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/binding/python/Common.hpp" #include "openPMD/binding/python/Container.H" #include "openPMD/binding/python/Pickle.hpp" +#include "openPMD/binding/python/RecordComponent.hpp" #include #include void init_MeshRecordComponent(py::module &m) { - auto py_mrc_cnt = declare_container( - m, "Mesh_Record_Component_Container"); + auto py_mrc_cnt = + declare_container( + m, "Mesh_Record_Component_Container"); py::class_ cl( m, "Mesh_Record_Component"); @@ -85,4 +88,14 @@ void init_MeshRecordComponent(py::module &m) }); finalize_container(py_mrc_cnt); + addRecordComponentSetGet( + finalize_container( + declare_container< + PyBaseRecordMeshRecordComponent, + PyMeshRecordComponentContainer, + MeshRecordComponent>(m, "Base_Record_Mesh_Record_Component"))) + .def_property_readonly( + "scalar", + &BaseRecord::scalar, + &docstring::is_scalar[1]); } diff --git a/src/binding/python/ParticlePatches.cpp b/src/binding/python/ParticlePatches.cpp index f04f36bf09..dbe4546514 100644 --- a/src/binding/python/ParticlePatches.cpp +++ b/src/binding/python/ParticlePatches.cpp @@ -19,6 +19,7 @@ * If not, see . */ #include "openPMD/ParticlePatches.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/Container.hpp" #include "openPMD/backend/PatchRecord.hpp" @@ -29,8 +30,8 @@ void init_ParticlePatches(py::module &m) { - auto py_pp_cnt = - declare_container(m, "Particle_Patches_Container"); + auto py_pp_cnt = declare_container( + m, "Particle_Patches_Container"); py::class_ >(m, "Particle_Patches") .def( diff --git a/src/binding/python/ParticleSpecies.cpp b/src/binding/python/ParticleSpecies.cpp index ae8f08b711..55fe0aaef0 100644 --- a/src/binding/python/ParticleSpecies.cpp +++ b/src/binding/python/ParticleSpecies.cpp @@ -21,6 +21,7 @@ #include "openPMD/ParticleSpecies.hpp" #include "openPMD/Record.hpp" #include "openPMD/Series.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/Container.hpp" #include "openPMD/binding/python/Common.hpp" @@ -33,8 +34,8 @@ void init_ParticleSpecies(py::module &m) { - auto py_ps_cnt = - declare_container(m, "Particle_Container"); + auto py_ps_cnt = declare_container( + m, "Particle_Container"); py::class_ > cl(m, "ParticleSpecies"); cl.def( diff --git a/src/binding/python/PatchRecord.cpp b/src/binding/python/PatchRecord.cpp index efeea08a97..72a0eec012 100644 --- a/src/binding/python/PatchRecord.cpp +++ b/src/binding/python/PatchRecord.cpp @@ -20,6 +20,7 @@ */ #include "openPMD/backend/PatchRecord.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/BaseRecord.hpp" #include "openPMD/backend/PatchRecordComponent.hpp" @@ -29,8 +30,8 @@ void init_PatchRecord(py::module &m) { - auto py_pr_cnt = - declare_container(m, "Patch_Record_Container"); + auto py_pr_cnt = declare_container( + m, "Patch_Record_Container"); py::class_ >( m, "Patch_Record") diff --git a/src/binding/python/PatchRecordComponent.cpp b/src/binding/python/PatchRecordComponent.cpp index 33b04e733c..8ad51d6e4f 100644 --- a/src/binding/python/PatchRecordComponent.cpp +++ b/src/binding/python/PatchRecordComponent.cpp @@ -21,11 +21,13 @@ #include "openPMD/backend/PatchRecordComponent.hpp" #include "openPMD/DatatypeHelpers.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/BaseRecordComponent.hpp" #include "openPMD/binding/python/Common.hpp" #include "openPMD/binding/python/Container.H" #include "openPMD/binding/python/Numpy.hpp" +#include "openPMD/binding/python/RecordComponent.hpp" namespace { @@ -43,8 +45,9 @@ struct Prc_Load void init_PatchRecordComponent(py::module &m) { - auto py_prc_cnt = declare_container( - m, "Patch_Record_Component_Container"); + auto py_prc_cnt = + declare_container( + m, "Patch_Record_Component_Container"); py::class_( m, "Patch_Record_Component") @@ -201,4 +204,14 @@ void init_PatchRecordComponent(py::module &m) .def("set_unit_SI", &PatchRecordComponent::setUnitSI); finalize_container(py_prc_cnt); + addRecordComponentSetGet( + finalize_container( + declare_container< + PyBaseRecordPatchRecordComponent, + PyPatchRecordComponentContainer, + PatchRecordComponent>(m, "Base_Record_Patch_Record_Component"))) + .def_property_readonly( + "scalar", + &BaseRecord::scalar, + &docstring::is_scalar[1]); } diff --git a/src/binding/python/Record.cpp b/src/binding/python/Record.cpp index 4b16088268..9cad75d03a 100644 --- a/src/binding/python/Record.cpp +++ b/src/binding/python/Record.cpp @@ -20,6 +20,7 @@ */ #include "openPMD/Record.hpp" #include "openPMD/RecordComponent.hpp" +#include "openPMD/backend/Attributable.hpp" #include "openPMD/backend/BaseRecord.hpp" #include "openPMD/binding/python/Common.hpp" @@ -32,7 +33,8 @@ void init_Record(py::module &m) { - auto py_r_cnt = declare_container(m, "Record_Container"); + auto py_r_cnt = declare_container( + m, "Record_Container"); py::class_ > cl(m, "Record"); cl.def(py::init()) diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index b39fd4c01f..37ad9a7cff 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -22,8 +22,6 @@ #include #include -#include "openPMD/binding/python/RecordComponent.hpp" - #include "openPMD/DatatypeHelpers.hpp" #include "openPMD/Error.hpp" #include "openPMD/Series.hpp" @@ -33,6 +31,7 @@ #include "openPMD/binding/python/Container.H" #include "openPMD/binding/python/Numpy.hpp" #include "openPMD/binding/python/Pickle.hpp" +#include "openPMD/binding/python/RecordComponent.hpp" #include #include @@ -763,8 +762,9 @@ void init_RecordComponent(py::module &m) return view.currentView(); }); - auto py_rc_cnt = declare_container( - m, "Record_Component_Container"); + auto py_rc_cnt = + declare_container( + m, "Record_Component_Container"); py::class_ cl(m, "Record_Component"); cl.def( @@ -1085,6 +1085,16 @@ void init_RecordComponent(py::module &m) addRecordComponentSetGet(cl); finalize_container(py_rc_cnt); + addRecordComponentSetGet( + finalize_container( + declare_container< + PyBaseRecordRecordComponent, + PyRecordComponentContainer, + RecordComponent>(m, "Base_Record_Record_Component"))) + .def_property_readonly( + "scalar", + &BaseRecord::scalar, + &docstring::is_scalar[1]); py::enum_(m, "Allocation") .value("USER", RecordComponent::Allocation::USER) diff --git a/src/binding/python/openPMD.cpp b/src/binding/python/openPMD.cpp index da6ebb0ebd..fe26bfced8 100644 --- a/src/binding/python/openPMD.cpp +++ b/src/binding/python/openPMD.cpp @@ -95,7 +95,6 @@ PYBIND11_MODULE(openpmd_api_cxx, m) init_MeshRecordComponent(m); init_PatchRecordComponent(m); - init_BaseRecord(m); init_Record(m); init_PatchRecord(m); init_ParticlePatches(m);