Adhere to the standard changes more closely

franzpoeschel · Oct 11, 2024 · 11425ce · 11425ce
1 parent 2a03187
commit 11425ce
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,20 +249,23 @@ 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);
                 }
             }
         }
         else
         {
             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
@@ -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
@@ -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
@@ -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
@@ -68,14 +68,16 @@ void MeshRecordComponent::read()
 }
 
 void MeshRecordComponent::flush(
-    std::string const &name, internal::FlushParams const &params)
+    std::string const &name,
+    internal::FlushParams const &params,
+    bool is_scalar)
 {
     if (access::write(IOHandler()->m_frontendAccess) &&
         !containsAttribute("position"))
     {
         setPosition(std::vector<double>{0});
     }
-    RecordComponent::flush(name, params);
+    RecordComponent::flush(name, params, is_scalar);
 }
 
 template <typename T>

diff --git a/src/backend/PatchRecord.cpp b/src/backend/PatchRecord.cpp
@@ -41,17 +41,17 @@ PatchRecord::setUnitDimension(std::map<UnitDimension, double> 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<PatchRecordComponent>::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);