Make shape an optional attribute for constant components

Author: franzpoeschel

include/openPMD/backend/Attributable.hpp

@@ -20,6 +20,7 @@
  */
 #pragma once
 
+#include "openPMD/Error.hpp"
 #include "openPMD/IO/AbstractIOHandler.hpp"
 #include "openPMD/ThrowError.hpp"
 #include "openPMD/auxiliary/OutOfRangeMsg.hpp"
@@ -30,6 +31,7 @@
 #include <exception>
 #include <map>
 #include <memory>
+#include <optional>
 #include <string>
 #include <type_traits>
 #include <vector>
@@ -106,6 +108,7 @@ namespace internal
         friend class openPMD::Attributable;
 
         using SharedData_t = std::shared_ptr<SharedAttributableData>;
+        using A_MAP = SharedData_t::element_type::A_MAP;
 
     public:
         AttributableData();
@@ -152,6 +155,32 @@ namespace internal
                 std::shared_ptr<typename T::Data_t>(self, [](auto const *) {}));
             return res;
         }
+
+        inline auto attributes() -> A_MAP &
+        {
+            return operator*().m_attributes;
+        }
+        [[nodiscard]] inline auto attributes() const -> A_MAP const &
+        {
+            return operator*().m_attributes;
+        }
+        [[nodiscard]] inline auto
+        readAttribute(std::string const &name) const -> Attribute const &
+        {
+            auto const & attr = attributes();
+            if (auto it = attr.find(name); it != attr.end())
+            {
+                return it->second;
+            }
+            else
+            {
+                throw error::ReadError(
+                    error::AffectedObject::Attribute,
+                    error::Reason::NotFound,
+                    std::nullopt,
+                    "Not found: '" + name + "'.");
+            }
+        }
     };
 
     template <typename, typename>

src/Iteration.cpp

@@ -641,8 +641,7 @@ void Iteration::readMeshes(std::string const &meshesPath)
         auto att_begin = aList.attributes->begin();
         auto att_end = aList.attributes->end();
         auto value = std::find(att_begin, att_end, "value");
-        auto shape = std::find(att_begin, att_end, "shape");
-        if (value != att_end && shape != att_end)
+        if (value != att_end)
         {
             MeshRecordComponent &mrc = m;
             IOHandler()->enqueue(IOTask(&mrc, pOpen));

src/ParticleSpecies.cpp

@@ -76,8 +76,7 @@ void ParticleSpecies::read()
             auto att_begin = aList.attributes->begin();
             auto att_end = aList.attributes->end();
             auto value = std::find(att_begin, att_end, "value");
-            auto shape = std::find(att_begin, att_end, "shape");
-            if (value != att_end && shape != att_end)
+            if (value != att_end)
             {
                 RecordComponent &rc = r;
                 IOHandler()->enqueue(IOTask(&rc, pOpen));

src/RecordComponent.cpp

@@ -108,7 +108,14 @@ RecordComponent &RecordComponent::resetDataset(Dataset d)
         throw std::runtime_error("Dataset extent must be at least 1D.");
     if (d.empty())
     {
-        if (d.dtype != Datatype::UNDEFINED)
+        if (d.extent.empty())
+        {
+            throw error::Internal(
+                "A zero-dimensional dataset is not to be considered empty, but "
+                "undefined. This is an internal safeguard against future "
+                "changes that might not consider this.");
+        }
+        else if (d.dtype != Datatype::UNDEFINED)
         {
             return makeEmpty(std::move(d));
         }
@@ -280,6 +287,13 @@ void RecordComponent::flush(
         {
             setUnitSI(1);
         }
+        auto constant_component_write_shape = [&]() {
+            auto extent = getExtent();
+            return !extent.empty() &&
+                std::none_of(extent.begin(), extent.end(), [](auto val) {
+                    return val == Dataset::JOINED_DIMENSION;
+                });
+        };
         if (!written())
         {
             if (constant())
@@ -299,16 +313,20 @@ void RecordComponent::flush(
                         Operation::WRITE_ATT>::ChangesOverSteps::IfPossible;
                 }
                 IOHandler()->enqueue(IOTask(this, aWrite));
-                aWrite.name = "shape";
-                Attribute a(getExtent());
-                aWrite.dtype = a.dtype;
-                aWrite.resource = a.getResource();
-                if (isVBased)
+                if (constant_component_write_shape())
                 {
-                    aWrite.changesOverSteps = Parameter<
-                        Operation::WRITE_ATT>::ChangesOverSteps::IfPossible;
+
+                    aWrite.name = "shape";
+                    Attribute a(getExtent());
+                    aWrite.dtype = a.dtype;
+                    aWrite.resource = a.getResource();
+                    if (isVBased)
+                    {
+                        aWrite.changesOverSteps = Parameter<
+                            Operation::WRITE_ATT>::ChangesOverSteps::IfPossible;
+                    }
+                    IOHandler()->enqueue(IOTask(this, aWrite));
                 }
-                IOHandler()->enqueue(IOTask(this, aWrite));
             }
             else
             {
@@ -323,6 +341,13 @@ void RecordComponent::flush(
         {
             if (constant())
             {
+                if (!constant_component_write_shape())
+                {
+                    throw error::WrongAPIUsage(
+                        "Extended constant component from a previous shape to "
+                        "one that cannot be written (empty or with joined "
+                        "dimension).");
+                }
                 bool isVBased = retrieveSeries().iterationEncoding() ==
                     IterationEncoding::variableBased;
                 Parameter<Operation::WRITE_ATT> aWrite;
@@ -387,28 +412,35 @@ namespace
     };
 } // namespace
 
+inline void breakpoint()
+{}
+
 void RecordComponent::readBase(bool require_unit_si)
 {
     using DT = Datatype;
-    // auto & rc = get();
-    Parameter<Operation::READ_ATT> aRead;
+    auto &rc = get();
 
-    if (constant() && !empty())
-    {
-        aRead.name = "value";
-        IOHandler()->enqueue(IOTask(this, aRead));
-        IOHandler()->flush(internal::defaultFlushParams);
+    readAttributes(ReadMode::FullyReread);
 
-        Attribute a(*aRead.resource);
-        DT dtype = *aRead.dtype;
+    auto read_constant =
+        [&]() // comment for forcing clang-format into putting a newline here
+    {
+        Attribute a = rc.readAttribute("value");
+        DT dtype = a.dtype;
         setWritten(false, Attributable::EnqueueAsynchronously::No);
         switchNonVectorType<MakeConstant>(dtype, *this, a);
         setWritten(true, Attributable::EnqueueAsynchronously::No);
 
-        aRead.name = "shape";
-        IOHandler()->enqueue(IOTask(this, aRead));
-        IOHandler()->flush(internal::defaultFlushParams);
-        a = Attribute(*aRead.resource);
+        if (!containsAttribute("shape"))
+        {
+            setWritten(false, Attributable::EnqueueAsynchronously::No);
+            resetDataset(Dataset(dtype, {}));
+            setWritten(true, Attributable::EnqueueAsynchronously::No);
+
+            return;
+        }
+
+        a = rc.attributes().at("shape");
         Extent e;
 
         // uint64_t check
@@ -418,7 +450,7 @@ void RecordComponent::readBase(bool require_unit_si)
         else
         {
             std::ostringstream oss;
-            oss << "Unexpected datatype (" << *aRead.dtype
+            oss << "Unexpected datatype (" << a.dtype
                 << ") for attribute 'shape' (" << determineDatatype<uint64_t>()
                 << " aka uint64_t)";
             throw error::ReadError(
@@ -431,9 +463,13 @@ void RecordComponent::readBase(bool require_unit_si)
         setWritten(false, Attributable::EnqueueAsynchronously::No);
         resetDataset(Dataset(dtype, e));
         setWritten(true, Attributable::EnqueueAsynchronously::No);
-    }
+    };
 
-    readAttributes(ReadMode::FullyReread);
+    if (constant() && !empty())
+    {
+        breakpoint();
+        read_constant();
+    }
 
     if (require_unit_si)
     {
@@ -447,16 +483,17 @@ void RecordComponent::readBase(bool require_unit_si)
                 "'" +
                     myPath().openPMDPath() + "'.");
         }
-        if (!getAttribute("unitSI").getOptional<double>().has_value())
+        if (auto attr = getAttribute("unitSI");
+            !attr.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() + "'.");
+                    datatypeToString(attr.dtype) + ") in '" +
+                    myPath().openPMDPath() + "'.");
         }
     }
 }