diff --git a/examples/7_extended_write_serial.cpp b/examples/7_extended_write_serial.cpp index bfb64e1fff..580894a34f 100644 --- a/examples/7_extended_write_serial.cpp +++ b/examples/7_extended_write_serial.cpp @@ -83,8 +83,9 @@ int main() {{io::UnitDimension::M, 1}}); electrons["displacement"]["x"].setUnitSI(1e-6); electrons.erase("displacement"); - electrons["weighting"][io::RecordComponent::SCALAR].makeConstant( - 1.e-5); + electrons["weighting"][io::RecordComponent::SCALAR] + .resetDataset({io::Datatype::FLOAT, {1}}) + .makeConstant(1.e-5); } io::Mesh mesh = cur_it.meshes["lowRez_2D_field"]; diff --git a/examples/7_extended_write_serial.py b/examples/7_extended_write_serial.py index 84ca5002db..e16b4b993a 100755 --- a/examples/7_extended_write_serial.py +++ b/examples/7_extended_write_serial.py @@ -90,7 +90,9 @@ electrons["displacement"].unit_dimension = {Unit_Dimension.M: 1} electrons["displacement"]["x"].unit_SI = 1.e-6 del electrons["displacement"] - electrons["weighting"][SCALAR].make_constant(1.e-5) + electrons["weighting"][SCALAR] \ + .reset_dataset(Dataset(np.dtype("float32"), extent=[1])) \ + .make_constant(1.e-5) mesh = cur_it.meshes["lowRez_2D_field"] mesh.axis_labels = ["x", "y"] diff --git a/examples/9_particle_write_serial.py b/examples/9_particle_write_serial.py index aebd266528..0109a48a57 100644 --- a/examples/9_particle_write_serial.py +++ b/examples/9_particle_write_serial.py @@ -44,7 +44,9 @@ # don't like it anymore? remove it with: # del electrons["displacement"] - electrons["weighting"][SCALAR].make_constant(1.e-5) + electrons["weighting"][SCALAR] \ + .reset_dataset(Dataset(np.dtype("float32"), extent=[1])) \ + .make_constant(1.e-5) particlePos_x = np.random.rand(234).astype(np.float32) particlePos_y = np.random.rand(234).astype(np.float32) diff --git a/include/openPMD/RecordComponent.tpp b/include/openPMD/RecordComponent.tpp index 4256fc6fa8..00c67dcf2a 100644 --- a/include/openPMD/RecordComponent.tpp +++ b/include/openPMD/RecordComponent.tpp @@ -332,7 +332,13 @@ RecordComponent::storeChunk(Offset o, Extent e, F &&createBuffer) dCreate.name = rc.m_name; dCreate.extent = getExtent(); dCreate.dtype = getDatatype(); - dCreate.options = rc.m_dataset.options; + if (!rc.m_dataset.has_value()) + { + throw error::WrongAPIUsage( + "[RecordComponent] Must specify dataset type and extent before " + "using storeChunk() (see RecordComponent::resetDataset())."); + } + dCreate.options = rc.m_dataset.value().options; IOHandler()->enqueue(IOTask(this, dCreate)); } Parameter getBufferView; diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index a2ea09e23e..fd8279eb05 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -24,6 +24,8 @@ #include "openPMD/Error.hpp" #include "openPMD/backend/Attributable.hpp" +#include + // expose private and protected members for invasive testing #ifndef OPENPMD_protected #define OPENPMD_protected protected: @@ -39,7 +41,7 @@ namespace internal /** * The type and extent of the dataset defined by this component. */ - Dataset m_dataset{Datatype::UNDEFINED, {}}; + std::optional m_dataset; /** * True if this is defined as a constant record component as specified * in the openPMD standard. diff --git a/src/IO/JSON/JSONIOHandlerImpl.cpp b/src/IO/JSON/JSONIOHandlerImpl.cpp index a9e86f0cde..e17e4929eb 100644 --- a/src/IO/JSON/JSONIOHandlerImpl.cpp +++ b/src/IO/JSON/JSONIOHandlerImpl.cpp @@ -58,23 +58,7 @@ JSONIOHandlerImpl::JSONIOHandlerImpl(AbstractIOHandler *handler) : AbstractIOHandlerImpl(handler) {} -JSONIOHandlerImpl::~JSONIOHandlerImpl() -{ - // we must not throw in a destructor - try - { - flush(); - } - catch (std::exception const &ex) - { - std::cerr << "[~JSONIOHandlerImpl] An error occurred: " << ex.what() - << std::endl; - } - catch (...) - { - std::cerr << "[~JSONIOHandlerImpl] An error occurred." << std::endl; - } -} +JSONIOHandlerImpl::~JSONIOHandlerImpl() = default; std::future JSONIOHandlerImpl::flush() { diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 164b38d127..a54373be59 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -42,7 +42,6 @@ namespace internal RecordComponent impl{ std::shared_ptr{this, [](auto const *) {}}}; impl.setUnitSI(1); - impl.resetDataset(Dataset(Datatype::CHAR, {1})); } } // namespace internal @@ -71,11 +70,17 @@ RecordComponent &RecordComponent::resetDataset(Dataset d) auto &rc = get(); if (written()) { + if (!rc.m_dataset.has_value()) + { + throw error::Internal( + "Internal control flow error: Written record component must " + "have defined datatype and extent."); + } if (d.dtype == Datatype::UNDEFINED) { - d.dtype = rc.m_dataset.dtype; + d.dtype = rc.m_dataset.value().dtype; } - else if (d.dtype != rc.m_dataset.dtype) + else if (d.dtype != rc.m_dataset.value().dtype) { throw std::runtime_error( "Cannot change the datatype of a dataset."); @@ -99,7 +104,7 @@ RecordComponent &RecordComponent::resetDataset(Dataset d) rc.m_isEmpty = false; if (written()) { - rc.m_dataset.extend(std::move(d.extent)); + rc.m_dataset.value().extend(std::move(d.extent)); } else { @@ -112,12 +117,28 @@ RecordComponent &RecordComponent::resetDataset(Dataset d) uint8_t RecordComponent::getDimensionality() const { - return get().m_dataset.rank; + auto &rc = get(); + if (rc.m_dataset.has_value()) + { + return rc.m_dataset.value().rank; + } + else + { + return 1; + } } Extent RecordComponent::getExtent() const { - return get().m_dataset.extent; + auto &rc = get(); + if (rc.m_dataset.has_value()) + { + return rc.m_dataset.value().extent; + } + else + { + return {1}; + } } namespace detail @@ -149,6 +170,12 @@ RecordComponent &RecordComponent::makeEmpty(Dataset d) auto &rc = get(); if (written()) { + if (!rc.m_dataset.has_value()) + { + throw error::Internal( + "Internal control flow error: Written record component must " + "have defined datatype and extent."); + } if (!constant()) { throw std::runtime_error( @@ -158,14 +185,14 @@ RecordComponent &RecordComponent::makeEmpty(Dataset d) } if (d.dtype == Datatype::UNDEFINED) { - d.dtype = rc.m_dataset.dtype; + d.dtype = rc.m_dataset.value().dtype; } - else if (d.dtype != rc.m_dataset.dtype) + else if (d.dtype != rc.m_dataset.value().dtype) { throw std::runtime_error( "Cannot change the datatype of a dataset."); } - rc.m_dataset.extend(std::move(d.extent)); + rc.m_dataset.value().extend(std::move(d.extent)); rc.m_hasBeenExtended = true; } else @@ -173,7 +200,7 @@ RecordComponent &RecordComponent::makeEmpty(Dataset d) rc.m_dataset = std::move(d); } - if (rc.m_dataset.extent.size() == 0) + if (rc.m_dataset.value().extent.size() == 0) throw std::runtime_error("Dataset extent must be at least 1D."); rc.m_isEmpty = true; @@ -181,7 +208,7 @@ RecordComponent &RecordComponent::makeEmpty(Dataset d) if (!written()) { switchType >( - rc.m_dataset.dtype, *this); + rc.m_dataset.value().dtype, *this); } return *this; } @@ -213,11 +240,23 @@ void RecordComponent::flush( /* * This catches when a user forgets to use resetDataset. */ - if (rc.m_dataset.dtype == Datatype::UNDEFINED) + if (!rc.m_dataset.has_value()) { - throw error::WrongAPIUsage( - "[RecordComponent] Must set specific datatype (Use " - "resetDataset call)."); + // The check for !written() is technically not needed, just + // defensive programming against internal bugs that go on us. + if (!written() && rc.m_chunks.empty()) + { + // No data written yet, just accessed the object so far without + // doing anything + // Just do nothing and skip this record component. + return; + } + else + { + throw error::WrongAPIUsage( + "[RecordComponent] Must specify dataset type and extent " + "before flushing (see RecordComponent::resetDataset())."); + } } if (!written()) { @@ -243,7 +282,7 @@ void RecordComponent::flush( dCreate.name = name; dCreate.extent = getExtent(); dCreate.dtype = getDatatype(); - dCreate.options = rc.m_dataset.options; + dCreate.options = rc.m_dataset.value().options; IOHandler()->enqueue(IOTask(this, dCreate)); } } @@ -262,7 +301,7 @@ void RecordComponent::flush( else { Parameter pExtend; - pExtend.extent = rc.m_dataset.extent; + pExtend.extent = rc.m_dataset.value().extent; IOHandler()->enqueue(IOTask(this, std::move(pExtend))); rc.m_hasBeenExtended = false; } diff --git a/src/backend/BaseRecordComponent.cpp b/src/backend/BaseRecordComponent.cpp index 4460b7ede0..79890b95ab 100644 --- a/src/backend/BaseRecordComponent.cpp +++ b/src/backend/BaseRecordComponent.cpp @@ -35,13 +35,29 @@ BaseRecordComponent &BaseRecordComponent::resetDatatype(Datatype d) "A Records Datatype can not (yet) be changed after it has been " "written."); - get().m_dataset.dtype = d; + auto &rc = get(); + if (rc.m_dataset.has_value()) + { + rc.m_dataset.value().dtype = d; + } + else + { + rc.m_dataset = Dataset{d, {1}}; + } return *this; } Datatype BaseRecordComponent::getDatatype() const { - return get().m_dataset.dtype; + auto &rc = get(); + if (rc.m_dataset.has_value()) + { + return rc.m_dataset.value().dtype; + } + else + { + return Datatype::UNDEFINED; + } } bool BaseRecordComponent::constant() const @@ -54,8 +70,12 @@ ChunkTable BaseRecordComponent::availableChunks() auto &rc = get(); if (rc.m_isConstant) { - Offset offset(rc.m_dataset.extent.size(), 0); - return ChunkTable{{std::move(offset), rc.m_dataset.extent}}; + if (!rc.m_dataset.has_value()) + { + return ChunkTable{}; + } + Offset offset(rc.m_dataset.value().extent.size(), 0); + return ChunkTable{{std::move(offset), rc.m_dataset.value().extent}}; } containingIteration().open(); Parameter param; diff --git a/src/backend/PatchRecordComponent.cpp b/src/backend/PatchRecordComponent.cpp index e1477ef7bd..c418eb5fed 100644 --- a/src/backend/PatchRecordComponent.cpp +++ b/src/backend/PatchRecordComponent.cpp @@ -67,7 +67,15 @@ uint8_t PatchRecordComponent::getDimensionality() const Extent PatchRecordComponent::getExtent() const { - return get().m_dataset.extent; + auto &rc = get(); + if (rc.m_dataset.has_value()) + { + return rc.m_dataset.value().extent; + } + else + { + return {1}; + } } PatchRecordComponent::PatchRecordComponent() : BaseRecordComponent{nullptr} @@ -94,13 +102,32 @@ void PatchRecordComponent::flush( } else { + if (!rc.m_dataset.has_value()) + { + // The check for !written() is technically not needed, just + // defensive programming against internal bugs that go on us. + if (!written() && rc.m_chunks.empty()) + { + // No data written yet, just accessed the object so far without + // doing anything + // Just do nothing and skip this record component. + return; + } + else + { + throw error::WrongAPIUsage( + "[PatchRecordComponent] Must specify dataset type and " + "extent before flushing (see " + "RecordComponent::resetDataset())."); + } + } if (!written()) { Parameter dCreate; dCreate.name = name; dCreate.extent = getExtent(); dCreate.dtype = getDatatype(); - dCreate.options = rc.m_dataset.options; + dCreate.options = rc.m_dataset.value().options; IOHandler()->enqueue(IOTask(this, dCreate)); } diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index f4ad2ce110..8c34faf29c 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -25,6 +25,8 @@ using namespace openPMD; +Dataset globalDataset(Datatype::CHAR, {1}); + TEST_CASE("versions_test", "[core]") { auto const apiVersion = getVersion(); @@ -439,11 +441,11 @@ TEST_CASE("record_constructor_test", "[core]") ps["position"][RecordComponent::SCALAR].resetDataset(dset); ps["positionOffset"][RecordComponent::SCALAR].resetDataset(dset); - REQUIRE(r["x"].unitSI() == 1); + REQUIRE(r["x"].resetDataset(dset).unitSI() == 1); REQUIRE(r["x"].numAttributes() == 1); /* unitSI */ - REQUIRE(r["y"].unitSI() == 1); + REQUIRE(r["y"].resetDataset(dset).unitSI() == 1); REQUIRE(r["y"].numAttributes() == 1); /* unitSI */ - REQUIRE(r["z"].unitSI() == 1); + REQUIRE(r["z"].resetDataset(dset).unitSI() == 1); REQUIRE(r["z"].numAttributes() == 1); /* unitSI */ std::array zeros{{0., 0., 0., 0., 0., 0., 0.}}; REQUIRE(r.unitDimension() == zeros); @@ -488,13 +490,15 @@ TEST_CASE("recordComponent_modification_test", "[core]") r["x"].setUnitSI(2.55999e-7); r["y"].setUnitSI(4.42999e-8); - REQUIRE(r["x"].unitSI() == static_cast(2.55999e-7)); + REQUIRE( + r["x"].resetDataset(dset).unitSI() == static_cast(2.55999e-7)); REQUIRE(r["x"].numAttributes() == 1); /* unitSI */ - REQUIRE(r["y"].unitSI() == static_cast(4.42999e-8)); + REQUIRE( + r["y"].resetDataset(dset).unitSI() == static_cast(4.42999e-8)); REQUIRE(r["y"].numAttributes() == 1); /* unitSI */ r["z"].setUnitSI(1); - REQUIRE(r["z"].unitSI() == static_cast(1)); + REQUIRE(r["z"].resetDataset(dset).unitSI() == static_cast(1)); REQUIRE(r["z"].numAttributes() == 1); /* unitSI */ } @@ -505,13 +509,13 @@ TEST_CASE("mesh_constructor_test", "[core]") Mesh &m = o.iterations[42].meshes["E"]; std::vector pos{0}; - REQUIRE(m["x"].unitSI() == 1); + REQUIRE(m["x"].resetDataset(globalDataset).unitSI() == 1); REQUIRE(m["x"].numAttributes() == 2); /* unitSI, position */ REQUIRE(m["x"].position() == pos); - REQUIRE(m["y"].unitSI() == 1); + REQUIRE(m["y"].resetDataset(globalDataset).unitSI() == 1); REQUIRE(m["y"].numAttributes() == 2); /* unitSI, position */ REQUIRE(m["y"].position() == pos); - REQUIRE(m["z"].unitSI() == 1); + REQUIRE(m["z"].resetDataset(globalDataset).unitSI() == 1); REQUIRE(m["z"].numAttributes() == 2); /* unitSI, position */ REQUIRE(m["z"].position() == pos); REQUIRE(m.geometry() == Mesh::Geometry::cartesian); @@ -534,9 +538,9 @@ TEST_CASE("mesh_modification_test", "[core]") Series o = Series("./MyOutput_%T.json", Access::CREATE); Mesh &m = o.iterations[42].meshes["E"]; - m["x"]; - m["y"]; - m["z"]; + m["x"].resetDataset(globalDataset); + m["y"].resetDataset(globalDataset); + m["z"].resetDataset(globalDataset); m.setGeometry(Mesh::Geometry::spherical); REQUIRE(m.geometry() == Mesh::Geometry::spherical); diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 0607a19a4a..ce37e4f352 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -6430,11 +6430,19 @@ void unfinished_iteration_test( */ it5.setAttribute("__openPMD_internal_fail", "asking for trouble"); auto it10 = write.writeIterations()[10]; + Dataset ds(Datatype::INT, {10}); auto E_x = it10.meshes["E"]["x"]; auto e_density = it10.meshes["e_density"][RecordComponent::SCALAR]; auto electron_x = it10.particles["e"]["position"]["x"]; auto electron_mass = it10.particles["e"]["mass"][RecordComponent::SCALAR]; + + RecordComponent *resetThese[] = { + &E_x, &e_density, &electron_x, &electron_mass}; + for (RecordComponent *rc : resetThese) + { + rc->resetDataset(ds); + } } auto tryReading = [&config, file, encoding]( Access access,