Derive PatchComponent from RecordComponent (#1594)

* Derive PatchRecordComponent from RecordComponent This way, we get the RecordComponent API too * Use generic read()/flush() logic for patch components
openPMD · Feb 13, 2024 · 283b2fe · 283b2fe
1 parent d9fce66
commit 283b2fe
Show file tree

Hide file tree

Showing 13 changed files with 52 additions and 202 deletions.
diff --git a/include/openPMD/Mesh.hpp b/include/openPMD/Mesh.hpp
@@ -230,7 +230,7 @@ class Mesh : public BaseRecord<MeshRecordComponent>
 
     void
     flush_impl(std::string const &, internal::FlushParams const &) override;
-    void read() override;
+    void read();
 }; // Mesh
 
 template <typename T>

diff --git a/include/openPMD/Record.hpp b/include/openPMD/Record.hpp
@@ -52,7 +52,7 @@ class Record : public BaseRecord<RecordComponent>
 
     void
     flush_impl(std::string const &, internal::FlushParams const &) override;
-    void read() override;
+    void read();
 }; // Record
 
 template <typename T>

diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp
@@ -481,10 +481,11 @@ class RecordComponent : public BaseRecordComponent
 
     static constexpr char const *const SCALAR = "\vScalar";
 
-private:
+protected:
     void flush(std::string const &, internal::FlushParams const &);
-    virtual void read();
+    void read(bool require_unit_si);
 
+private:
     /**
      * Internal method to be called by all methods that create an empty dataset.
      *
@@ -535,7 +536,7 @@ OPENPMD_protected
         BaseRecordComponent::setData(m_recordComponentData);
     }
 
-    void readBase();
+    void readBase(bool require_unit_si);
 }; // RecordComponent
 
 } // namespace openPMD

diff --git a/include/openPMD/backend/MeshRecordComponent.hpp b/include/openPMD/backend/MeshRecordComponent.hpp
@@ -46,7 +46,7 @@ class MeshRecordComponent : public RecordComponent
 private:
     MeshRecordComponent();
     MeshRecordComponent(NoInit);
-    void read() override;
+    void read();
     void flush(std::string const &, internal::FlushParams const &);
 
 public:

diff --git a/include/openPMD/backend/PatchRecord.hpp b/include/openPMD/backend/PatchRecord.hpp
@@ -43,6 +43,6 @@ class PatchRecord : public BaseRecord<PatchRecordComponent>
 
     void
     flush_impl(std::string const &, internal::FlushParams const &) override;
-    void read() override;
+    void read();
 }; // PatchRecord
 } // namespace openPMD
diff --git a/include/openPMD/backend/PatchRecordComponent.hpp b/include/openPMD/backend/PatchRecordComponent.hpp
@@ -20,6 +20,7 @@
  */
 #pragma once
 
+#include "openPMD/RecordComponent.hpp"
 #include "openPMD/auxiliary/ShareRawInternal.hpp"
 #include "openPMD/backend/BaseRecordComponent.hpp"
 
@@ -36,41 +37,11 @@
 
 namespace openPMD
 {
-namespace internal
-{
-    class PatchRecordComponentData : public BaseRecordComponentData
-    {
-    public:
-        /**
-         * Chunk reading/writing requests on the contained dataset.
-         */
-        std::queue<IOTask> m_chunks;
-
-        PatchRecordComponentData(PatchRecordComponentData const &) = delete;
-        PatchRecordComponentData(PatchRecordComponentData &&) = delete;
-
-        PatchRecordComponentData &
-        operator=(PatchRecordComponentData const &) = delete;
-        PatchRecordComponentData &
-        operator=(PatchRecordComponentData &&) = delete;
-
-        PatchRecordComponentData();
-
-        void reset() override
-        {
-            BaseRecordComponentData::reset();
-            m_chunks = std::queue<IOTask>();
-        }
-    };
-
-    template <typename, typename>
-    class BaseRecordData;
-} // namespace internal
 
 /**
  * @todo add support for constant patch record components
  */
-class PatchRecordComponent : public BaseRecordComponent
+class PatchRecordComponent : public RecordComponent
 {
     template <typename T, typename T_key, typename T_container>
     friend class Container;
@@ -81,7 +52,6 @@ class PatchRecordComponent : public BaseRecordComponent
     friend class ParticlePatches;
     friend class PatchRecord;
     friend class ParticleSpecies;
-    friend class internal::PatchRecordComponentData;
 
 public:
     /**
@@ -95,7 +65,7 @@ class PatchRecordComponent : public BaseRecordComponent
 
     PatchRecordComponent &setUnitSI(double);
 
-    virtual PatchRecordComponent &resetDataset(Dataset);
+    PatchRecordComponent &resetDataset(Dataset) override;
 
     uint8_t getDimensionality() const;
     Extent getExtent() const;
@@ -119,48 +89,14 @@ class PatchRecordComponent : public BaseRecordComponent
 OPENPMD_private
     // clang-format on
 
-    void flush(std::string const &, internal::FlushParams const &);
-    virtual void read();
-
-    /**
-     * @brief Check recursively whether this RecordComponent is dirty.
-     *        It is dirty if any attribute or dataset is read from or written to
-     *        the backend.
-     *
-     * @return true If dirty.
-     * @return false Otherwise.
-     */
-    bool dirtyRecursive() const;
+    using RecordComponent::flush;
 
     // clang-format off
 OPENPMD_protected
     // clang-format on
 
-    using Data_t = internal::PatchRecordComponentData;
-
-    std::shared_ptr<Data_t> m_patchRecordComponentData;
-
     PatchRecordComponent();
     PatchRecordComponent(NoInit);
-
-    inline Data_t const &get() const
-    {
-        // cannot call this in the const overload
-        // setDatasetDefined(*m_recordComponentData);
-        return *m_patchRecordComponentData;
-    }
-
-    inline Data_t &get()
-    {
-        setDatasetDefined(*m_patchRecordComponentData);
-        return *m_patchRecordComponentData;
-    }
-
-    inline void setData(std::shared_ptr<Data_t> data)
-    {
-        m_patchRecordComponentData = std::move(data);
-        BaseRecordComponent::setData(m_patchRecordComponentData);
-    }
 }; // PatchRecordComponent
 
 template <typename T>

diff --git a/src/ParticlePatches.cpp b/src/ParticlePatches.cpp
@@ -100,7 +100,7 @@ void ParticlePatches::read()
         pr.dirty() = false;
         try
         {
-            prc.PatchRecordComponent::read();
+            prc.PatchRecordComponent::read(/* require_unit_si = */ false);
         }
         catch (error::ReadError const &err)
         {

diff --git a/src/Record.cpp b/src/Record.cpp
@@ -104,7 +104,7 @@ void Record::read()
         /* using operator[] will incorrectly update parent */
         try
         {
-            T_RecordComponent::read();
+            T_RecordComponent::read(/* require_unit_si = */ true);
         }
         catch (error::ReadError const &err)
         {
@@ -128,7 +128,7 @@ void Record::read()
             rc.get().m_isConstant = true;
             try
             {
-                rc.read();
+                rc.read(/* require_unit_si = */ true);
             }
             catch (error::ReadError const &err)
             {
@@ -155,7 +155,7 @@ void Record::read()
             rc.written() = true;
             try
             {
-                rc.read();
+                rc.read(/* require_unit_si = */ true);
             }
             catch (error::ReadError const &err)
             {

diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp
@@ -340,9 +340,9 @@ void RecordComponent::flush(
     }
 }
 
-void RecordComponent::read()
+void RecordComponent::read(bool require_unit_si)
 {
-    readBase();
+    readBase(require_unit_si);
 }
 
 namespace
@@ -367,7 +367,7 @@ namespace
     };
 } // namespace
 
-void RecordComponent::readBase()
+void RecordComponent::readBase(bool require_unit_si)
 {
     using DT = Datatype;
     // auto & rc = get();
@@ -413,22 +413,32 @@ void RecordComponent::readBase()
         written() = true;
     }
 
-    aRead.name = "unitSI";
-    IOHandler()->enqueue(IOTask(this, aRead));
-    IOHandler()->flush(internal::defaultFlushParams);
-    if (auto val = Attribute(*aRead.resource).getOptional<double>();
-        val.has_value())
-        setUnitSI(val.value());
-    else
-        throw error::ReadError(
-            error::AffectedObject::Attribute,
-            error::Reason::UnexpectedContent,
-            {},
-            "Unexpected Attribute datatype for 'unitSI' (expected double, "
-            "found " +
-                datatypeToString(Attribute(*aRead.resource).dtype) + ")");
-
     readAttributes(ReadMode::FullyReread);
+
+    if (require_unit_si)
+    {
+        if (!containsAttribute("unitSI"))
+        {
+            throw error::ReadError(
+                error::AffectedObject::Attribute,
+                error::Reason::NotFound,
+                {},
+                "Attribute unitSI required for record components, not found in "
+                "'" +
+                    myPath().openPMDPath() + "'.");
+        }
+        if (!getAttribute("unitSI").getOptional<double>().has_value())
+        {
+            throw error::ReadError(
+                error::AffectedObject::Attribute,
+                error::Reason::UnexpectedContent,
+                {},
+                "Unexpected Attribute datatype for 'unitSI' (expected double, "
+                "found " +
+                    datatypeToString(Attribute(*aRead.resource).dtype) +
+                    ") in '" + myPath().openPMDPath() + "'.");
+        }
+    }
 }
 
 bool RecordComponent::dirtyRecursive() const

diff --git a/src/backend/MeshRecordComponent.cpp b/src/backend/MeshRecordComponent.cpp
@@ -64,7 +64,7 @@ void MeshRecordComponent::read()
             "of any floating point type, found " +
                 datatypeToString(Attribute(*aRead.resource).dtype) + ")");
 
-    readBase();
+    readBase(/* require_unit_si = */ true);
 }
 
 void MeshRecordComponent::flush(

diff --git a/src/backend/PatchRecord.cpp b/src/backend/PatchRecord.cpp
@@ -95,7 +95,7 @@ void PatchRecord::read()
         prc.written() = true;
         try
         {
-            prc.read();
+            prc.read(/* require_unit_si = */ false);
         }
         catch (error::ReadError const &err)
         {