Adhere to the standard changes more closely

franzpoeschel · franzpoeschel · commit f1889d15d800 · 2024-12-11T10:13:47.000+01:00
diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp
@@ -487,7 +487,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
@@ -623,10 +623,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
@@ -371,12 +371,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
@@ -386,7 +388,7 @@ void Mesh::flush_impl(
             if (scalar())
             {
                 MeshRecordComponent &mrc = *this;
-                mrc.flush(name, flushParams);
+                mrc.flush(name, flushParams, /* is_scalar = */ true);
             }
             else
             {
@@ -396,20 +398,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);
             }
         }
         if (!containsAttribute("gridUnitSI"))
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
@@ -56,12 +56,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
@@ -71,7 +73,7 @@ void Record::flush_impl(
             if (scalar())
             {
                 RecordComponent &rc = *this;
-                rc.flush(name, flushParams);
+                rc.flush(name, flushParams, /* is_scalar = */ true);
             }
             else
             {
@@ -81,7 +83,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);
                 }
             }
         }
@@ -90,12 +93,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
`@@ -371,12 +371,14 @@ void Mesh::flush_impl(`
`371`	`371`	`auto &m = get();`
`372`	`372`	`if (m.m_datasetDefined)`
`373`	`373`	`{`
`374`		`- T_RecordComponent::flush(SCALAR, flushParams);`
	`374`	`+ T_RecordComponent::flush(`
	`375`	`+ SCALAR, flushParams, /* is_scalar = */ true);`
`375`	`376`	`}`
`376`	`377`	`else`
`377`	`378`	`{`
`378`	`379`	`for (auto &comp : *this)`
`379`		`- comp.second.flush(comp.first, flushParams);`
	`380`	`+ comp.second.flush(`
	`381`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`380`	`382`	`}`
`381`	`383`	`}`
`382`	`384`	`else`
`@@ -386,7 +388,7 @@ void Mesh::flush_impl(`
`386`	`388`	`if (scalar())`
`387`	`389`	`{`
`388`	`390`	`MeshRecordComponent &mrc = *this;`
`389`		`- mrc.flush(name, flushParams);`
	`391`	`+ mrc.flush(name, flushParams, /* is_scalar = */ true);`
`390`	`392`	`}`
`391`	`393`	`else`
`392`	`394`	`{`
`@@ -396,20 +398,23 @@ void Mesh::flush_impl(`
`396`	`398`	`for (auto &comp : *this)`
`397`	`399`	`{`
`398`	`400`	`comp.second.parent() = &this->writable();`
`399`		`- comp.second.flush(comp.first, flushParams);`
	`401`	`+ comp.second.flush(`
	`402`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`400`	`403`	`}`
`401`	`404`	`}`
`402`	`405`	`}`
`403`	`406`	`else`
`404`	`407`	`{`
`405`	`408`	`if (scalar())`
`406`	`409`	`{`
`407`		`- T_RecordComponent::flush(name, flushParams);`
	`410`	`+ T_RecordComponent::flush(`
	`411`	`+ name, flushParams, /* is_scalar = */ true);`
`408`	`412`	`}`
`409`	`413`	`else`
`410`	`414`	`{`
`411`	`415`	`for (auto &comp : *this)`
`412`		`- comp.second.flush(comp.first, flushParams);`
	`416`	`+ comp.second.flush(`
	`417`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`413`	`418`	`}`
`414`	`419`	`}`
`415`	`420`	`if (!containsAttribute("gridUnitSI"))`
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
`@@ -56,12 +56,14 @@ void Record::flush_impl(`
`56`	`56`	`{`
`57`	`57`	`if (scalar())`
`58`	`58`	`{`
`59`		`- T_RecordComponent::flush(SCALAR, flushParams);`
	`59`	`+ T_RecordComponent::flush(`
	`60`	`+ SCALAR, flushParams, /* is_scalar = */ true);`
`60`	`61`	`}`
`61`	`62`	`else`
`62`	`63`	`{`
`63`	`64`	`for (auto &comp : *this)`
`64`		`- comp.second.flush(comp.first, flushParams);`
	`65`	`+ comp.second.flush(`
	`66`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`65`	`67`	`}`
`66`	`68`	`}`
`67`	`69`	`else`
`@@ -71,7 +73,7 @@ void Record::flush_impl(`
`71`	`73`	`if (scalar())`
`72`	`74`	`{`
`73`	`75`	`RecordComponent &rc = *this;`
`74`		`- rc.flush(name, flushParams);`
	`76`	`+ rc.flush(name, flushParams, /* is_scalar = */ true);`
`75`	`77`	`}`
`76`	`78`	`else`
`77`	`79`	`{`
`@@ -81,7 +83,8 @@ void Record::flush_impl(`
`81`	`83`	`for (auto &comp : *this)`
`82`	`84`	`{`
`83`	`85`	`comp.second.parent() = getWritable(this);`
`84`		`- comp.second.flush(comp.first, flushParams);`
	`86`	`+ comp.second.flush(`
	`87`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`85`	`88`	`}`
`86`	`89`	`}`
`87`	`90`	`}`
`@@ -90,12 +93,14 @@ void Record::flush_impl(`
`90`	`93`
`91`	`94`	`if (scalar())`
`92`	`95`	`{`
`93`		`- T_RecordComponent::flush(name, flushParams);`
	`96`	`+ T_RecordComponent::flush(`
	`97`	`+ name, flushParams, /* is_scalar = */ true);`
`94`	`98`	`}`
`95`	`99`	`else`
`96`	`100`	`{`
`97`	`101`	`for (auto &comp : *this)`
`98`		`- comp.second.flush(comp.first, flushParams);`
	`102`	`+ comp.second.flush(`
	`103`	`+ comp.first, flushParams, /* is_scalar = */ false);`
`99`	`104`	`}`
`100`	`105`	`}`
`101`	`106`
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);`