From 11425ce9ff9b125627d5ad291b7da600df57a7b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 11 Oct 2024 13:34:05 +0200 Subject: [PATCH] Adhere to the standard changes more closely --- include/openPMD/RecordComponent.hpp | 3 ++- .../openPMD/backend/MeshRecordComponent.hpp | 3 ++- src/Iteration.cpp | 6 +++++- src/Mesh.cpp | 17 +++++++++++------ src/ParticleSpecies.cpp | 2 ++ src/Record.cpp | 17 +++++++++++------ src/RecordComponent.cpp | 19 ++++++++++++++++++- src/backend/MeshRecordComponent.cpp | 6 ++++-- src/backend/PatchRecord.cpp | 6 +++--- 9 files changed, 58 insertions(+), 21 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index ebb5a80ca8..253194d2cc 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -488,7 +488,8 @@ class RecordComponent : public BaseRecordComponent static constexpr char const *const SCALAR = "\vScalar"; protected: - void flush(std::string const &, internal::FlushParams const &); + void + flush(std::string const &, internal::FlushParams const &, bool is_scalar); void read(bool require_unit_si); private: diff --git a/include/openPMD/backend/MeshRecordComponent.hpp b/include/openPMD/backend/MeshRecordComponent.hpp index d05163d754..f8229635ba 100644 --- a/include/openPMD/backend/MeshRecordComponent.hpp +++ b/include/openPMD/backend/MeshRecordComponent.hpp @@ -47,7 +47,8 @@ class MeshRecordComponent : public RecordComponent MeshRecordComponent(); MeshRecordComponent(NoInit); void read(); - void flush(std::string const &, internal::FlushParams const &); + void + flush(std::string const &, internal::FlushParams const &, bool is_scalar); public: ~MeshRecordComponent() override = default; diff --git a/src/Iteration.cpp b/src/Iteration.cpp index 1afc97c3bf..9ef4014b5d 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -592,10 +592,14 @@ void Iteration::readMeshes(std::string const &meshesPath) IOHandler()->enqueue(IOTask(&m, aList)); IOHandler()->flush(internal::defaultFlushParams); + // Find constant scalar meshes. shape generally required for meshes, + // shape also required for scalars. + // https://github.com/openPMD/openPMD-standard/pull/289 auto att_begin = aList.attributes->begin(); auto att_end = aList.attributes->end(); auto value = std::find(att_begin, att_end, "value"); - if (value != att_end) + auto shape = std::find(att_begin, att_end, "shape"); + if (value != att_end && shape != att_end) { MeshRecordComponent &mrc = m; IOHandler()->enqueue(IOTask(&mrc, pOpen)); diff --git a/src/Mesh.cpp b/src/Mesh.cpp index f977bbe905..5c288001ea 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -222,12 +222,14 @@ void Mesh::flush_impl( auto &m = get(); if (m.m_datasetDefined) { - T_RecordComponent::flush(SCALAR, flushParams); + T_RecordComponent::flush( + SCALAR, flushParams, /* is_scalar = */ true); } else { for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + comp.second.flush( + comp.first, flushParams, /* is_scalar = */ false); } } else @@ -237,7 +239,7 @@ void Mesh::flush_impl( if (scalar()) { MeshRecordComponent &mrc = *this; - mrc.flush(name, flushParams); + mrc.flush(name, flushParams, /* is_scalar = */ true); } else { @@ -247,7 +249,8 @@ void Mesh::flush_impl( for (auto &comp : *this) { comp.second.parent() = &this->writable(); - comp.second.flush(comp.first, flushParams); + comp.second.flush( + comp.first, flushParams, /* is_scalar = */ false); } } } @@ -255,12 +258,14 @@ void Mesh::flush_impl( { if (scalar()) { - T_RecordComponent::flush(name, flushParams); + T_RecordComponent::flush( + name, flushParams, /* is_scalar = */ true); } else { for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + comp.second.flush( + comp.first, flushParams, /* is_scalar = */ false); } } flushAttributes(flushParams); diff --git a/src/ParticleSpecies.cpp b/src/ParticleSpecies.cpp index 81e7cdcd8c..1c3da720ce 100644 --- a/src/ParticleSpecies.cpp +++ b/src/ParticleSpecies.cpp @@ -76,6 +76,8 @@ void ParticleSpecies::read() auto att_begin = aList.attributes->begin(); auto att_end = aList.attributes->end(); auto value = std::find(att_begin, att_end, "value"); + // @todo see this comment: + // https://github.com/openPMD/openPMD-standard/pull/289#issuecomment-2407263974 if (value != att_end) { RecordComponent &rc = r; diff --git a/src/Record.cpp b/src/Record.cpp index 3bcac4d7e1..c271f03316 100644 --- a/src/Record.cpp +++ b/src/Record.cpp @@ -50,12 +50,14 @@ void Record::flush_impl( { if (scalar()) { - T_RecordComponent::flush(SCALAR, flushParams); + T_RecordComponent::flush( + SCALAR, flushParams, /* is_scalar = */ true); } else { for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + comp.second.flush( + comp.first, flushParams, /* is_scalar = */ false); } } else @@ -65,7 +67,7 @@ void Record::flush_impl( if (scalar()) { RecordComponent &rc = *this; - rc.flush(name, flushParams); + rc.flush(name, flushParams, /* is_scalar = */ true); } else { @@ -75,7 +77,8 @@ void Record::flush_impl( for (auto &comp : *this) { comp.second.parent() = getWritable(this); - comp.second.flush(comp.first, flushParams); + comp.second.flush( + comp.first, flushParams, /* is_scalar = */ false); } } } @@ -84,12 +87,14 @@ void Record::flush_impl( if (scalar()) { - T_RecordComponent::flush(name, flushParams); + T_RecordComponent::flush( + name, flushParams, /* is_scalar = */ true); } else { for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + comp.second.flush( + comp.first, flushParams, /* is_scalar = */ false); } } diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index aef392a2fe..5b060fab33 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -241,7 +241,9 @@ bool RecordComponent::empty() const } void RecordComponent::flush( - std::string const &name, internal::FlushParams const &flushParams) + std::string const &name, + internal::FlushParams const &flushParams, + bool is_scalar) { auto &rc = get(); if (flushParams.flushLevel == FlushLevel::SkeletonOnly) @@ -285,6 +287,21 @@ void RecordComponent::flush( setUnitSI(1); } auto constant_component_write_shape = [&]() { + if (is_scalar) + { + // Must write shape in any case: + // 1. Non-scalar constant components can be distinguished from + // normal components by checking if the backend reports a + // group or a dataset. This does not work for scalar constant + // components, so the parser needs to check if the attributes + // value and shape are there. If they're not, the group is + // not considered as a constant component. + // 2. Scalar constant components are required to write the shape + // by standard anyway since the standard requires that at + // least one component in a record have a shape. For scalars, + // there is only one component, so it must have a shape. + return true; + } auto extent = getExtent(); return !extent.empty() && std::none_of(extent.begin(), extent.end(), [](auto val) { diff --git a/src/backend/MeshRecordComponent.cpp b/src/backend/MeshRecordComponent.cpp index ed50080757..af2e683eb5 100644 --- a/src/backend/MeshRecordComponent.cpp +++ b/src/backend/MeshRecordComponent.cpp @@ -68,14 +68,16 @@ void MeshRecordComponent::read() } void MeshRecordComponent::flush( - std::string const &name, internal::FlushParams const ¶ms) + std::string const &name, + internal::FlushParams const ¶ms, + bool is_scalar) { if (access::write(IOHandler()->m_frontendAccess) && !containsAttribute("position")) { setPosition(std::vector{0}); } - RecordComponent::flush(name, params); + RecordComponent::flush(name, params, is_scalar); } template diff --git a/src/backend/PatchRecord.cpp b/src/backend/PatchRecord.cpp index 5d2b38d50f..2b0baf6db1 100644 --- a/src/backend/PatchRecord.cpp +++ b/src/backend/PatchRecord.cpp @@ -41,17 +41,17 @@ PatchRecord::setUnitDimension(std::map const &udim) void PatchRecord::flush_impl( std::string const &path, internal::FlushParams const &flushParams) { - if (!this->datasetDefined()) + if (!this->scalar()) { if (IOHandler()->m_frontendAccess != Access::READ_ONLY) Container::flush( path, flushParams); // warning (clang-tidy-10): // bugprone-parent-virtual-call for (auto &comp : *this) - comp.second.flush(comp.first, flushParams); + comp.second.flush(comp.first, flushParams, /* is_scalar = */ false); } else - T_RecordComponent::flush(path, flushParams); + T_RecordComponent::flush(path, flushParams, /* is_scalar = */ true); if (flushParams.flushLevel != FlushLevel::SkeletonOnly) { setDirty(false);