From 137982fe50728c6937c2c681f11fe14992764406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 26 Oct 2023 15:34:15 +0200 Subject: [PATCH] More symmetric design for container types --- include/openPMD/CustomHierarchy.hpp | 111 ++++++++++++++++++++----- src/CustomHierarchy.cpp | 81 ++++++------------ src/Iteration.cpp | 4 +- src/binding/python/CustomHierarchy.cpp | 40 +++++++-- src/binding/python/openPMD.cpp | 2 +- test/SerialIOTest.cpp | 4 +- 6 files changed, 154 insertions(+), 88 deletions(-) diff --git a/include/openPMD/CustomHierarchy.hpp b/include/openPMD/CustomHierarchy.hpp index fb3402cf23..ee585ed6b3 100644 --- a/include/openPMD/CustomHierarchy.hpp +++ b/include/openPMD/CustomHierarchy.hpp @@ -23,6 +23,7 @@ #include "openPMD/IO/AbstractIOHandler.hpp" #include "openPMD/Mesh.hpp" #include "openPMD/ParticleSpecies.hpp" +#include "openPMD/RecordComponent.hpp" #include "openPMD/backend/Container.hpp" #include @@ -74,27 +75,67 @@ namespace internal isMeshContainer(std::vector const &path) const; }; - struct CustomHierarchyData : ContainerData + struct CustomHierarchyData + : ContainerData + , ContainerData + , ContainerData + , ContainerData { explicit CustomHierarchyData(); void syncAttributables(); - Container m_embeddedDatasets; - Container m_embeddedMeshes; - Container m_embeddedParticles; + Container customHierarchies() + { + Container res; + res.setData( + {static_cast *>(this), + [](auto const *) {}}); + return res; + } + Container embeddedDatasets() + { + Container res; + res.setData( + {static_cast *>(this), + [](auto const *) {}}); + return res; + } + Container embeddedMeshes() + { + Container res; + res.setData( + {static_cast *>(this), + [](auto const *) {}}); + return res; + } + + Container embeddedParticles() + { + Container res; + res.setData( + {static_cast *>(this), + [](auto const *) {}}); + return res; + } }; } // namespace internal -class CustomHierarchy : public Container +template +class ConversibleContainer : public Container { - friend class Iteration; - friend class Container; + template + friend class ConversibleContainer; -private: - using Container_t = Container; +protected: + using Container_t = Container; using Data_t = internal::CustomHierarchyData; - static_assert(std::is_base_of_v); + static_assert( + std::is_base_of_v); + + ConversibleContainer(Attributable::NoInit) + : Container_t(Attributable::NoInit{}) + {} std::shared_ptr m_customHierarchyData; @@ -107,6 +148,47 @@ class CustomHierarchy : public Container return *m_customHierarchyData; } + inline void setData(std::shared_ptr data) + { + m_customHierarchyData = data; + Container_t::setData(std::move(data)); + } + +public: + template + auto asContainerOf() -> ConversibleContainer + { + if constexpr ( + std::is_same_v || + std::is_same_v || + std::is_same_v || + std::is_same_v) + { + ConversibleContainer res(Attributable::NoInit{}); + res.setData(m_customHierarchyData); + return res; + } + else + { + static_assert( + auxiliary::dependent_false_v, + "[CustomHierarchy::asContainerOf] Type parameter must be " + "one of: CustomHierarchy, RecordComponent, Mesh, " + "ParticleSpecies."); + } + } +}; + +class CustomHierarchy : public ConversibleContainer +{ + friend class Iteration; + friend class Container; + +private: + using Container_t = Container; + using Parent_t = ConversibleContainer; + using Data_t = typename Parent_t::Data_t; + using EraseStaleMeshes = internal::EraseStaleEntries>; using EraseStaleParticles = internal::EraseStaleEntries>; @@ -118,12 +200,6 @@ class CustomHierarchy : public Container CustomHierarchy(); CustomHierarchy(NoInit); - inline void setData(std::shared_ptr data) - { - m_customHierarchyData = data; - Container_t::setData(std::move(data)); - } - void read(internal::MeshesParticlesPath const &); void read( internal::MeshesParticlesPath const &, @@ -158,8 +234,5 @@ class CustomHierarchy : public Container CustomHierarchy &operator=(CustomHierarchy const &) = default; CustomHierarchy &operator=(CustomHierarchy &&) = default; - - template - auto asContainerOf() -> Container &; }; } // namespace openPMD diff --git a/src/CustomHierarchy.cpp b/src/CustomHierarchy.cpp index 845cb52820..c65fa3c7b9 100644 --- a/src/CustomHierarchy.cpp +++ b/src/CustomHierarchy.cpp @@ -262,21 +262,29 @@ namespace internal /* * m_embeddeddatasets and its friends should point to the same instance * of Attributable. + * Not strictly necessary to do this explicitly due to virtual + * inheritance (all Attributable instances are the same anyway), + * but let's be explicit about this. */ - for (auto p : std::initializer_list{ - &m_embeddedDatasets, &m_embeddedMeshes, &m_embeddedParticles}) + for (auto p : std::initializer_list{ + static_cast *>(this), + static_cast *>(this), + static_cast *>(this), + static_cast *>(this)}) { - p->m_attri->asSharedPtrOfAttributable() = - this->asSharedPtrOfAttributable(); + p->asSharedPtrOfAttributable() = this->asSharedPtrOfAttributable(); } } } // namespace internal -CustomHierarchy::CustomHierarchy() +// template +// class ConversibleContainer; + +CustomHierarchy::CustomHierarchy() : ConversibleContainer(NoInit{}) { setData(std::make_shared()); } -CustomHierarchy::CustomHierarchy(NoInit) : Container_t(NoInit()) +CustomHierarchy::CustomHierarchy(NoInit) : ConversibleContainer(NoInit{}) {} void CustomHierarchy::readNonscalarMesh( @@ -394,8 +402,8 @@ void CustomHierarchy::read( std::deque constantComponentsPushback; auto &data = get(); - EraseStaleMeshes meshesMap(data.m_embeddedMeshes); - EraseStaleParticles particlesMap(data.m_embeddedParticles); + EraseStaleMeshes meshesMap(data.embeddedMeshes()); + EraseStaleParticles particlesMap(data.embeddedParticles()); for (auto const &path : *pList.paths) { switch (mpp.determineType(currentPath)) @@ -469,7 +477,7 @@ void CustomHierarchy::read( // Group is a bit of an internal misnomer here, it just means that // it matches neither meshes nor particles path case internal::ContainedType::Group: { - auto &rc = data.m_embeddedDatasets[path]; + auto &rc = data.embeddedDatasets()[path]; Parameter dOpen; dOpen.name = path; IOHandler()->enqueue(IOTask(&rc, dOpen)); @@ -487,7 +495,7 @@ void CustomHierarchy::read( << "' at path '" << myPath().printGroup() << "' and will skip it due to read error:\n" << err.what() << std::endl; - data.m_embeddedDatasets.container().erase(path); + data.embeddedDatasets().container().erase(path); } break; } @@ -518,7 +526,7 @@ void CustomHierarchy::read( for (auto const &path : constantComponentsPushback) { - auto &rc = data.m_embeddedDatasets[path]; + auto &rc = data.embeddedDatasets()[path]; try { Parameter pOpen; @@ -533,7 +541,7 @@ void CustomHierarchy::read( << myPath().printGroup() << "/" << path << "' and will skip it due to read error:\n" << err.what() << std::endl; - data.m_embeddedDatasets.container().erase(path); + data.embeddedDatasets().container().erase(path); } } } @@ -569,7 +577,7 @@ void CustomHierarchy::flush_internal( subpath.flush_internal(flushParams, mpp, currentPath); currentPath.pop_back(); } - for (auto &[name, mesh] : data.m_embeddedMeshes) + for (auto &[name, mesh] : data.embeddedMeshes()) { if (!mpp.isMeshContainer(currentPath)) { @@ -593,7 +601,7 @@ void CustomHierarchy::flush_internal( } mesh.flush(name, flushParams); } - for (auto &[name, particleSpecies] : data.m_embeddedParticles) + for (auto &[name, particleSpecies] : data.embeddedParticles()) { if (!mpp.isParticleContainer(currentPath)) { @@ -619,7 +627,7 @@ void CustomHierarchy::flush_internal( } particleSpecies.flush(name, flushParams); } - for (auto &[name, dataset] : get().m_embeddedDatasets) + for (auto &[name, dataset] : get().embeddedDatasets()) { dataset.flush(name, flushParams); } @@ -654,47 +662,10 @@ bool CustomHierarchy::dirtyRecursive() const } return false; }; - auto &data = get(); - return check(data.m_embeddedMeshes) || check(data.m_embeddedParticles) || - check(data.m_embeddedDatasets) || check(*this); -} - -template -auto CustomHierarchy::asContainerOf() -> Container & -{ - if constexpr (std::is_same_v) - { - return *static_cast *>(this); - } - else if constexpr (std::is_same_v) - { - return get().m_embeddedMeshes; - } - else if constexpr (std::is_same_v) - { - return get().m_embeddedParticles; - } - else if constexpr (std::is_same_v) - { - return get().m_embeddedDatasets; - } - else - { - static_assert( - auxiliary::dependent_false_v, - "[CustomHierarchy::asContainerOf] Type parameter must be " - "one of: CustomHierarchy, RecordComponent, Mesh, " - "ParticleSpecies."); - } + auto &data = const_cast(get()); // @todo do this better + return check(data.embeddedMeshes()) || check(data.embeddedParticles()) || + check(data.embeddedDatasets()) || check(data.customHierarchies()); } - -template auto CustomHierarchy::asContainerOf() - -> Container &; -template auto CustomHierarchy::asContainerOf() - -> Container &; -template auto CustomHierarchy::asContainerOf() -> Container &; -template auto CustomHierarchy::asContainerOf() - -> Container &; } // namespace openPMD #undef OPENPMD_LEGAL_IDENTIFIER_CHARS diff --git a/src/Iteration.cpp b/src/Iteration.cpp index c8a76a907d..9cb36f7a60 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -369,7 +369,7 @@ void Iteration::sync_meshes_and_particles_from_alias_to_subgroups( { return; } - auto &container = (*this)[defaultPath].asContainerOf(); + auto container = (*this)[defaultPath].asContainerOf(); for (auto &[name, entry] : m_or_p) { @@ -415,7 +415,7 @@ void Iteration::sync_meshes_and_particles_from_subgroups_to_alias( { return; } - auto &container = it->second.asContainerOf(); + auto container = it->second.asContainerOf(); for (auto &[name, entry] : container) { m_or_p.emplace(name, entry); diff --git a/src/binding/python/CustomHierarchy.cpp b/src/binding/python/CustomHierarchy.cpp index d618226d15..cd4b893909 100644 --- a/src/binding/python/CustomHierarchy.cpp +++ b/src/binding/python/CustomHierarchy.cpp @@ -11,21 +11,43 @@ namespace py = pybind11; using namespace openPMD; +template +void define_conversible_container(py::module &m, std::string const &name) +{ + using CC = ConversibleContainer; + py::class_, Attributable>(m, name.c_str()) + .def( + "as_container_of_datasets", + &CC::template asContainerOf) + .def("as_container_of_meshes", &CC::template asContainerOf) + .def( + "as_container_of_particles", + &CC::template asContainerOf) + .def( + "as_container_of_custom_hierarchy", + &CC::template asContainerOf); +} + void init_CustomHierarchy(py::module &m) { auto py_ch_cont = declare_container( m, "Container_CustomHierarchy"); - py::class_, Attributable>( - m, "CustomHierarchy") - .def( - "as_container_of_datasets", - &CustomHierarchy::asContainerOf) - .def("as_container_of_meshes", &CustomHierarchy::asContainerOf) - .def( - "as_container_of_particles", - &CustomHierarchy::asContainerOf); + define_conversible_container( + m, "ConversibleContainer_CustomHierarchy"); + define_conversible_container( + m, "ConversibleContainer_ParticleSpecies"); + define_conversible_container( + m, "ConversibleContainer_RecordComponent"); + define_conversible_container(m, "ConversibleContainer_Mesh"); + + [[maybe_unused]] py::class_< + CustomHierarchy, + ConversibleContainer, + Container, + Attributable> + custom_hierarchy(m, "CustomHierarchy"); finalize_container(py_ch_cont); } diff --git a/src/binding/python/openPMD.cpp b/src/binding/python/openPMD.cpp index 4e2b74b892..996208bd55 100644 --- a/src/binding/python/openPMD.cpp +++ b/src/binding/python/openPMD.cpp @@ -91,7 +91,6 @@ PYBIND11_MODULE(openpmd_api_cxx, m) init_Datatype(m); init_Dataset(m); - init_CustomHierarchy(m); init_BaseRecordComponent(m); init_RecordComponent(m); init_MeshRecordComponent(m); @@ -103,6 +102,7 @@ PYBIND11_MODULE(openpmd_api_cxx, m) init_ParticleSpecies(m); init_Mesh(m); + init_CustomHierarchy(m); init_Iteration(m); init_IterationEncoding(m); init_Series(m); diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index c4cc50eb3a..6476d5475d 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -2617,11 +2617,11 @@ TEST_CASE("git_hdf5_sample_structure_test", "[serial][hdf5]") getWritable(&o.iterations[100].meshes["E"])); REQUIRE( o.iterations[100]["fields"].asContainerOf()["E"].parent() == - getWritable(&o.iterations[100]["fields"].asContainerOf())); + &o.iterations[100]["fields"].asContainerOf().writable()); REQUIRE( o.iterations[100].meshes["E"].parent() == // Iteration::meshes is only an alias, this is the actual parent - getWritable(&o.iterations[100]["fields"].asContainerOf())); + &o.iterations[100]["fields"].asContainerOf().writable()); REQUIRE( o.iterations[100].meshes["E"]["x"].parent() == getWritable(&o.iterations[100].meshes["E"]));