Adhere to the standard changes more closely

franzpoeschel · franzpoeschel · commit 11425ce9ff9b · 2024-10-11T17:08:01.000+02:00
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);

Original file line number	Diff line number	Diff line change
`@@ -222,12 +222,14 @@ void Mesh::flush_impl(`
`222`	`222`	`auto &m = get();`
`223`	`223`	`if (m.m_datasetDefined)`
`224`	`224`	`{`
`225`		`- T_RecordComponent::flush(SCALAR, flushParams);`
	`225`	`+ T_RecordComponent::flush(`
	`226`	`+ SCALAR, flushParams, /* is_scalar = */ true);`
`226`	`227`	`}`
`227`	`228`	`else`
`228`	`229`	`{`
`229`	`230`	`for (auto &comp : *this)`
`230`		`- comp.second.flush(comp.first, flushParams);`
	`231`	`+ comp.second.flush(`
	`232`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`231`	`233`	`}`
`232`	`234`	`}`
`233`	`235`	`else`
`@@ -237,7 +239,7 @@ void Mesh::flush_impl(`
`237`	`239`	`if (scalar())`
`238`	`240`	`{`
`239`	`241`	`MeshRecordComponent &mrc = *this;`
`240`		`- mrc.flush(name, flushParams);`
	`242`	`+ mrc.flush(name, flushParams, /* is_scalar = */ true);`
`241`	`243`	`}`
`242`	`244`	`else`
`243`	`245`	`{`
`@@ -247,20 +249,23 @@ void Mesh::flush_impl(`
`247`	`249`	`for (auto &comp : *this)`
`248`	`250`	`{`
`249`	`251`	`comp.second.parent() = &this->writable();`
`250`		`- comp.second.flush(comp.first, flushParams);`
	`252`	`+ comp.second.flush(`
	`253`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`251`	`254`	`}`
`252`	`255`	`}`
`253`	`256`	`}`
`254`	`257`	`else`
`255`	`258`	`{`
`256`	`259`	`if (scalar())`
`257`	`260`	`{`
`258`		`- T_RecordComponent::flush(name, flushParams);`
	`261`	`+ T_RecordComponent::flush(`
	`262`	`+ name, flushParams, /* is_scalar = */ true);`
`259`	`263`	`}`
`260`	`264`	`else`
`261`	`265`	`{`
`262`	`266`	`for (auto &comp : *this)`
`263`		`- comp.second.flush(comp.first, flushParams);`
	`267`	`+ comp.second.flush(`
	`268`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`264`	`269`	`}`
`265`	`270`	`}`
`266`	`271`	`flushAttributes(flushParams);`
Original file line number	Diff line number	Diff line change
`@@ -76,6 +76,8 @@ void ParticleSpecies::read()`
`76`	`76`	`auto att_begin = aList.attributes->begin();`
`77`	`77`	`auto att_end = aList.attributes->end();`
`78`	`78`	`auto value = std::find(att_begin, att_end, "value");`
	`79`	`+ // @todo see this comment:`
	`80`	`+ // https://github.com/openPMD/openPMD-standard/pull/289#issuecomment-2407263974`
`79`	`81`	`if (value != att_end)`
`80`	`82`	`{`
`81`	`83`	`RecordComponent &rc = r;`
Original file line number	Diff line number	Diff line change
`@@ -50,12 +50,14 @@ void Record::flush_impl(`
`50`	`50`	`{`
`51`	`51`	`if (scalar())`
`52`	`52`	`{`
`53`		`- T_RecordComponent::flush(SCALAR, flushParams);`
	`53`	`+ T_RecordComponent::flush(`
	`54`	`+ SCALAR, flushParams, /* is_scalar = */ true);`
`54`	`55`	`}`
`55`	`56`	`else`
`56`	`57`	`{`
`57`	`58`	`for (auto &comp : *this)`
`58`		`- comp.second.flush(comp.first, flushParams);`
	`59`	`+ comp.second.flush(`
	`60`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`59`	`61`	`}`
`60`	`62`	`}`
`61`	`63`	`else`
`@@ -65,7 +67,7 @@ void Record::flush_impl(`
`65`	`67`	`if (scalar())`
`66`	`68`	`{`
`67`	`69`	`RecordComponent &rc = *this;`
`68`		`- rc.flush(name, flushParams);`
	`70`	`+ rc.flush(name, flushParams, /* is_scalar = */ true);`
`69`	`71`	`}`
`70`	`72`	`else`
`71`	`73`	`{`
`@@ -75,7 +77,8 @@ void Record::flush_impl(`
`75`	`77`	`for (auto &comp : *this)`
`76`	`78`	`{`
`77`	`79`	`comp.second.parent() = getWritable(this);`
`78`		`- comp.second.flush(comp.first, flushParams);`
	`80`	`+ comp.second.flush(`
	`81`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`79`	`82`	`}`
`80`	`83`	`}`
`81`	`84`	`}`
`@@ -84,12 +87,14 @@ void Record::flush_impl(`
`84`	`87`
`85`	`88`	`if (scalar())`
`86`	`89`	`{`
`87`		`- T_RecordComponent::flush(name, flushParams);`
	`90`	`+ T_RecordComponent::flush(`
	`91`	`+ name, flushParams, /* is_scalar = */ true);`
`88`	`92`	`}`
`89`	`93`	`else`
`90`	`94`	`{`
`91`	`95`	`for (auto &comp : *this)`
`92`		`- comp.second.flush(comp.first, flushParams);`
	`96`	`+ comp.second.flush(`
	`97`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`93`	`98`	`}`
`94`	`99`	`}`
`95`	`100`
Original file line number	Diff line number	Diff line change
`@@ -68,14 +68,16 @@ void MeshRecordComponent::read()`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`void MeshRecordComponent::flush(`
`71`		`- std::string const &name, internal::FlushParams const &params)`
	`71`	`+ std::string const &name,`
	`72`	`+ internal::FlushParams const &params,`
	`73`	`+ bool is_scalar)`
`72`	`74`	`{`
`73`	`75`	`if (access::write(IOHandler()->m_frontendAccess) &&`
`74`	`76`	`!containsAttribute("position"))`
`75`	`77`	`{`
`76`	`78`	`setPosition(std::vector<double>{0});`
`77`	`79`	`}`
`78`		`- RecordComponent::flush(name, params);`
	`80`	`+ RecordComponent::flush(name, params, is_scalar);`
`79`	`81`	`}`
`80`	`82`
`81`	`83`	`template <typename T>`
Original file line number	Diff line number	Diff line change
`@@ -41,17 +41,17 @@ PatchRecord::setUnitDimension(std::map<UnitDimension, double> const &udim)`
`41`	`41`	`void PatchRecord::flush_impl(`
`42`	`42`	`std::string const &path, internal::FlushParams const &flushParams)`
`43`	`43`	`{`
`44`		`- if (!this->datasetDefined())`
	`44`	`+ if (!this->scalar())`
`45`	`45`	`{`
`46`	`46`	`if (IOHandler()->m_frontendAccess != Access::READ_ONLY)`
`47`	`47`	`Container<PatchRecordComponent>::flush(`
`48`	`48`	`path, flushParams); // warning (clang-tidy-10):`
`49`	`49`	`// bugprone-parent-virtual-call`
`50`	`50`	`for (auto &comp : *this)`
`51`		`- comp.second.flush(comp.first, flushParams);`
	`51`	`+ comp.second.flush(comp.first, flushParams, /* is_scalar = */ false);`
`52`	`52`	`}`
`53`	`53`	`else`
`54`		`- T_RecordComponent::flush(path, flushParams);`
	`54`	`+ T_RecordComponent::flush(path, flushParams, /* is_scalar = */ true);`
`55`	`55`	`if (flushParams.flushLevel != FlushLevel::SkeletonOnly)`
`56`	`56`	`{`
`57`	`57`	`setDirty(false);`