From 5ebd5dc30045b16fb5893a5410baee44d94c36e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 19 Jun 2019 15:32:39 +0200 Subject: [PATCH 1/9] Implement makeEmpty for record components --- include/openPMD/RecordComponent.hpp | 13 +++ .../openPMD/backend/BaseRecordComponent.hpp | 21 +++++ src/RecordComponent.cpp | 36 +++++++-- test/SerialIOTest.cpp | 80 +++++++++++++++++++ 4 files changed, 142 insertions(+), 8 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 295baa82a3..c55b8cfbfc 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -107,6 +107,9 @@ class RecordComponent : public BaseRecordComponent template< typename T > RecordComponent& makeConstant(T); + + template< typename T > + RecordComponent& makeEmpty( uint8_t dimensions ); /** Load and allocate a chunk of data * @@ -159,6 +162,7 @@ class RecordComponent : public BaseRecordComponent private: void flush(std::string const&); virtual void read(); + RecordComponent& makeEmpty( Dataset ); }; // RecordComponent @@ -174,6 +178,15 @@ RecordComponent::makeConstant(T value) return *this; } +template< typename T > +inline RecordComponent& +RecordComponent::makeEmpty( uint8_t dimensions ) +{ + return makeEmpty( Dataset( + determineDatatype< T >(), + Extent( dimensions, 0 ) ) ); +} + template< typename T > inline std::shared_ptr< T > RecordComponent::loadChunk(Offset o, Extent e, double targetUnitSI) diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index 8fdfdc2e95..41c8784415 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -61,4 +61,25 @@ class BaseRecordComponent : public Attributable std::shared_ptr< Dataset > m_dataset; std::shared_ptr< bool > m_isConstant; }; // BaseRecordComponent +namespace detail +{ +template< typename RecordComponent_T > +struct DefaultValue +{ + template< typename T > + void + operator()( RecordComponent_T & rc ) + { + rc.makeConstant( T() ); + } + + template< unsigned n, typename... Args > + void + operator()( Args &&... ) + { + throw std::runtime_error( + "makeEmpty: Datatype not supported by openPMD." ); + } +}; +} // namespace detail } // namespace openPMD diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 8e5282f2eb..ea1df14013 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -46,16 +46,18 @@ RecordComponent::setUnitSI(double usi) return *this; } -RecordComponent& -RecordComponent::resetDataset(Dataset d) +RecordComponent & +RecordComponent::resetDataset( Dataset d ) { if( written ) - throw std::runtime_error("A Records Dataset can not (yet) be changed after it has been written."); - if( d.extent.empty() ) - throw std::runtime_error("Dataset extent must be at least 1D."); - if( std::any_of(d.extent.begin(), d.extent.end(), - [](Extent::value_type const& i) { return i == 0u; }) ) - throw std::runtime_error("Dataset extent must not be zero in any dimension."); + throw std::runtime_error( "A Records Dataset can not (yet) be changed " + "after it has been written." ); + if( d.extent.empty() || + std::any_of( + d.extent.begin(), + d.extent.end(), + []( Extent::value_type const & i ) { return i == 0u; } ) ) + return makeEmpty( d ); *m_dataset = d; dirty = true; @@ -74,6 +76,24 @@ RecordComponent::getExtent() const return m_dataset->extent; } +RecordComponent& +RecordComponent::makeEmpty( Dataset d ) +{ + if( written ) + throw std::runtime_error( + "A recordComponent can not (yet) be made" + " empty after it has been written."); + + *m_dataset = d; + dirty = true; + static detail::DefaultValue< RecordComponent > dv; + switchType( + d.dtype, + dv, + *this ); + return *this; +} + void RecordComponent::flush(std::string const& name) { diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index efbb81fead..c1244f16a3 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -37,6 +37,86 @@ std::vector> getBackends() { auto const backends = getBackends(); +inline +void +empty_dataset_test( std::string file_ending ) +{ + { + Series series( + "../samples/empty_datasets." + file_ending, AccessType::CREATE ); + + // The ADIOS1 backend prefers to have actual data written to it + // as soon as possible + // related: https://github.com/openPMD/openPMD-api/issues/520 + RecordComponent circumvent_ADIOS1_bug = + series.iterations[ 1 ] + .meshes[ "rho" ][ "circumvent_ADIOS1_bug"]; + circumvent_ADIOS1_bug.resetDataset( + Dataset(Datatype::INT, Extent{1})); + std::vector< int > data{ 1 }; + circumvent_ADIOS1_bug.storeChunk( + data, + Offset{0}, + Extent{1} ); + series.flush(); + + RecordComponent makeEmpty_dim_7_int = + series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_int" ]; + RecordComponent makeEmpty_dim_7_long = + series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_bool" ]; + RecordComponent makeEmpty_resetDataset_dim3 = + series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_resetDataset_dim3" ]; + RecordComponent makeEmpty_resetDataset_dim3_notallzero = + series.iterations[ 1 ] + .meshes[ "rho" ][ "makeEmpty_resetDataset_dim3_notallzero" ]; + makeEmpty_dim_7_int.makeEmpty< int >( 7 ); + makeEmpty_dim_7_long.makeEmpty< long >( 7 ); + makeEmpty_resetDataset_dim3.resetDataset( + Dataset( Datatype::LONG, Extent( 3, 0 ) ) ); + makeEmpty_resetDataset_dim3_notallzero.resetDataset( + Dataset( Datatype::LONG_DOUBLE, Extent{ 1, 2, 0 } ) ); + series.flush(); + + } + { + Series series( + "../samples/empty_datasets." + file_ending, AccessType::READ_ONLY ); + RecordComponent makeEmpty_dim_7_int = + series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_int" ]; + RecordComponent makeEmpty_dim_7_long = + series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_bool" ]; + RecordComponent makeEmpty_resetDataset_dim3 = + series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_resetDataset_dim3" ]; + RecordComponent makeEmpty_resetDataset_dim3_notallzero = + series.iterations[ 1 ] + .meshes[ "rho" ][ "makeEmpty_resetDataset_dim3_notallzero" ]; + REQUIRE(makeEmpty_dim_7_int.getDimensionality() == 7); + REQUIRE(makeEmpty_dim_7_int.getExtent() == Extent(7, 0)); + REQUIRE(makeEmpty_dim_7_int.getDatatype() == determineDatatype< int >()); + + REQUIRE(makeEmpty_dim_7_long.getDimensionality() == 7); + REQUIRE(makeEmpty_dim_7_long.getExtent() == Extent(7, 0)); + REQUIRE(makeEmpty_dim_7_long.getDatatype() == determineDatatype< long >()); + + REQUIRE(makeEmpty_resetDataset_dim3.getDimensionality() == 3); + REQUIRE(makeEmpty_resetDataset_dim3.getExtent() == Extent(3, 0)); + REQUIRE(makeEmpty_resetDataset_dim3.getDatatype() == Datatype::LONG); + + REQUIRE(makeEmpty_resetDataset_dim3_notallzero.getDimensionality() == 3); + REQUIRE(makeEmpty_resetDataset_dim3_notallzero.getExtent() == Extent{1,2,0}); + REQUIRE(makeEmpty_resetDataset_dim3_notallzero.getDatatype() == Datatype::LONG_DOUBLE); + } +} + +TEST_CASE( "empty_dataset_test", "[serial]" ) +{ + for (auto const & t: backends) + { + //if (std::get<0>(t) == "json") + empty_dataset_test(std::get<0>(t)); + } +} + inline void constant_scalar(std::string file_ending) { From 77c568e0ab9afd27f2c651152c94a931fdecd0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 19 Jun 2019 16:16:21 +0200 Subject: [PATCH 2/9] Add empty_dataset_test --- include/openPMD/RecordComponent.hpp | 2 +- test/CoreTest.cpp | 18 ------------------ test/SerialIOTest.cpp | 10 +++++----- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index c55b8cfbfc..575c5c3205 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -107,7 +107,7 @@ class RecordComponent : public BaseRecordComponent template< typename T > RecordComponent& makeConstant(T); - + template< typename T > RecordComponent& makeEmpty( uint8_t dimensions ); diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index ed9a17959b..fb2a111069 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -671,21 +671,3 @@ TEST_CASE( "empty_record_test", "[core]" ) o.iterations[1].meshes["E"][RecordComponent::SCALAR].resetDataset(Dataset(Datatype::DOUBLE, {1})); o.flush(); } - -TEST_CASE( "zero_extent_component", "[core]" ) -{ - Series o = Series("./new_openpmd_output", AccessType::CREATE); - - auto E_x = o.iterations[1].meshes["E"]["x"]; - E_x.setComment("Datasets with zero extent in any dimension are not allowed."); - REQUIRE_THROWS_WITH(E_x.resetDataset(Dataset(Datatype::LONG, {0})), - Catch::Equals("Dataset extent must not be zero in any dimension.")); - REQUIRE_THROWS_WITH(E_x.resetDataset(Dataset(Datatype::LONG, {1, 0})), - Catch::Equals("Dataset extent must not be zero in any dimension.")); - REQUIRE_THROWS_WITH(E_x.resetDataset(Dataset(Datatype::LONG, {0, 1})), - Catch::Equals("Dataset extent must not be zero in any dimension.")); - E_x.setComment("Datasets must contain dimensions."); - REQUIRE_THROWS_WITH(E_x.resetDataset(Dataset(Datatype::LONG, {})), - Catch::Equals("Dataset extent must be at least 1D.")); - E_x.resetDataset(Dataset(Datatype::DOUBLE, {1})); -} diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index c1244f16a3..92f3fb45b8 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -92,19 +92,19 @@ empty_dataset_test( std::string file_ending ) .meshes[ "rho" ][ "makeEmpty_resetDataset_dim3_notallzero" ]; REQUIRE(makeEmpty_dim_7_int.getDimensionality() == 7); REQUIRE(makeEmpty_dim_7_int.getExtent() == Extent(7, 0)); - REQUIRE(makeEmpty_dim_7_int.getDatatype() == determineDatatype< int >()); + REQUIRE(isSame(makeEmpty_dim_7_int.getDatatype(), determineDatatype< int >())); REQUIRE(makeEmpty_dim_7_long.getDimensionality() == 7); REQUIRE(makeEmpty_dim_7_long.getExtent() == Extent(7, 0)); - REQUIRE(makeEmpty_dim_7_long.getDatatype() == determineDatatype< long >()); + REQUIRE(isSame(makeEmpty_dim_7_long.getDatatype(), determineDatatype< long >())); REQUIRE(makeEmpty_resetDataset_dim3.getDimensionality() == 3); REQUIRE(makeEmpty_resetDataset_dim3.getExtent() == Extent(3, 0)); - REQUIRE(makeEmpty_resetDataset_dim3.getDatatype() == Datatype::LONG); - + REQUIRE(isSame(makeEmpty_resetDataset_dim3.getDatatype(), Datatype::LONG)); + REQUIRE(makeEmpty_resetDataset_dim3_notallzero.getDimensionality() == 3); REQUIRE(makeEmpty_resetDataset_dim3_notallzero.getExtent() == Extent{1,2,0}); - REQUIRE(makeEmpty_resetDataset_dim3_notallzero.getDatatype() == Datatype::LONG_DOUBLE); + REQUIRE(isSame(makeEmpty_resetDataset_dim3_notallzero.getDatatype(), Datatype::LONG_DOUBLE)); } } From 17c9ebc08becf48757c965eb4218ec4e4a209649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 20 Jun 2019 12:47:19 +0200 Subject: [PATCH 3/9] Disallow zero-dimensional datasets again --- src/RecordComponent.cpp | 7 ++++--- test/CoreTest.cpp | 11 +++++++++++ test/SerialIOTest.cpp | 21 +++------------------ 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index ea1df14013..bf5d40faa7 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -52,8 +52,9 @@ RecordComponent::resetDataset( Dataset d ) if( written ) throw std::runtime_error( "A Records Dataset can not (yet) be changed " "after it has been written." ); - if( d.extent.empty() || - std::any_of( + if( d.extent.empty() ) + throw std::runtime_error("Dataset extent must be at least 1D."); + if( std::any_of( d.extent.begin(), d.extent.end(), []( Extent::value_type const & i ) { return i == 0u; } ) ) @@ -83,7 +84,7 @@ RecordComponent::makeEmpty( Dataset d ) throw std::runtime_error( "A recordComponent can not (yet) be made" " empty after it has been written."); - + *m_dataset = d; dirty = true; static detail::DefaultValue< RecordComponent > dv; diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index fb2a111069..fe9d00058a 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -671,3 +671,14 @@ TEST_CASE( "empty_record_test", "[core]" ) o.iterations[1].meshes["E"][RecordComponent::SCALAR].resetDataset(Dataset(Datatype::DOUBLE, {1})); o.flush(); } + +TEST_CASE( "zero_extent_component", "[core]" ) +{ + Series o = Series("./new_openpmd_output", AccessType::CREATE); + + auto E_x = o.iterations[1].meshes["E"]["x"]; + E_x.setComment("Datasets must contain dimensions."); + REQUIRE_THROWS_WITH(E_x.resetDataset(Dataset(Datatype::LONG, {})), + Catch::Equals("Dataset extent must be at least 1D.")); + E_x.resetDataset(Dataset(Datatype::DOUBLE, {1})); +} \ No newline at end of file diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 92f3fb45b8..6b4306759e 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -44,22 +44,7 @@ empty_dataset_test( std::string file_ending ) { Series series( "../samples/empty_datasets." + file_ending, AccessType::CREATE ); - - // The ADIOS1 backend prefers to have actual data written to it - // as soon as possible - // related: https://github.com/openPMD/openPMD-api/issues/520 - RecordComponent circumvent_ADIOS1_bug = - series.iterations[ 1 ] - .meshes[ "rho" ][ "circumvent_ADIOS1_bug"]; - circumvent_ADIOS1_bug.resetDataset( - Dataset(Datatype::INT, Extent{1})); - std::vector< int > data{ 1 }; - circumvent_ADIOS1_bug.storeChunk( - data, - Offset{0}, - Extent{1} ); - series.flush(); - + RecordComponent makeEmpty_dim_7_int = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_int" ]; RecordComponent makeEmpty_dim_7_long = @@ -267,9 +252,9 @@ TEST_CASE( "flush_without_position_positionOffset", "[serial]" ) []( float * ptr ) { delete[] ptr; } ), { 0, 0 }, { 2, 2 } ); - + s.flush(); - + for( auto const & key : { "position", "positionOffset" } ) { for( auto const & dim : { "x", "y", "z" } ) From 848884903fcbafa87296bcced8a45102d70cce1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 20 Jun 2019 15:19:43 +0200 Subject: [PATCH 4/9] Allow zero-dimensional calls to RecordComponent::makeEmpty() --- include/openPMD/RecordComponent.hpp | 2 ++ test/CoreTest.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 575c5c3205..e5d02fbf7c 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -182,6 +182,8 @@ template< typename T > inline RecordComponent& RecordComponent::makeEmpty( uint8_t dimensions ) { + if ( dimensions == 0 ) + throw std::runtime_error("Dataset extent must be at least 1D."); return makeEmpty( Dataset( determineDatatype< T >(), Extent( dimensions, 0 ) ) ); diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index fe9d00058a..e9ac792090 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -680,5 +680,7 @@ TEST_CASE( "zero_extent_component", "[core]" ) E_x.setComment("Datasets must contain dimensions."); REQUIRE_THROWS_WITH(E_x.resetDataset(Dataset(Datatype::LONG, {})), Catch::Equals("Dataset extent must be at least 1D.")); + REQUIRE_THROWS_WITH(E_x.makeEmpty(0), + Catch::Equals("Dataset extent must be at least 1D.")); E_x.resetDataset(Dataset(Datatype::DOUBLE, {1})); } \ No newline at end of file From a9f8737f0d8814a5539eb1db9bb21c1f57c33c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 20 Jun 2019 15:21:47 +0200 Subject: [PATCH 5/9] Fix object slicing in tests --- test/SerialIOTest.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 6b4306759e..cb4822a1e9 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -45,13 +45,13 @@ empty_dataset_test( std::string file_ending ) Series series( "../samples/empty_datasets." + file_ending, AccessType::CREATE ); - RecordComponent makeEmpty_dim_7_int = + auto makeEmpty_dim_7_int = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_int" ]; - RecordComponent makeEmpty_dim_7_long = + auto makeEmpty_dim_7_long = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_bool" ]; - RecordComponent makeEmpty_resetDataset_dim3 = + auto makeEmpty_resetDataset_dim3 = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_resetDataset_dim3" ]; - RecordComponent makeEmpty_resetDataset_dim3_notallzero = + auto makeEmpty_resetDataset_dim3_notallzero = series.iterations[ 1 ] .meshes[ "rho" ][ "makeEmpty_resetDataset_dim3_notallzero" ]; makeEmpty_dim_7_int.makeEmpty< int >( 7 ); @@ -66,13 +66,13 @@ empty_dataset_test( std::string file_ending ) { Series series( "../samples/empty_datasets." + file_ending, AccessType::READ_ONLY ); - RecordComponent makeEmpty_dim_7_int = + auto makeEmpty_dim_7_int = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_int" ]; - RecordComponent makeEmpty_dim_7_long = + auto makeEmpty_dim_7_long = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_dim_7_bool" ]; - RecordComponent makeEmpty_resetDataset_dim3 = + auto makeEmpty_resetDataset_dim3 = series.iterations[ 1 ].meshes[ "rho" ][ "makeEmpty_resetDataset_dim3" ]; - RecordComponent makeEmpty_resetDataset_dim3_notallzero = + auto makeEmpty_resetDataset_dim3_notallzero = series.iterations[ 1 ] .meshes[ "rho" ][ "makeEmpty_resetDataset_dim3_notallzero" ]; REQUIRE(makeEmpty_dim_7_int.getDimensionality() == 7); From 3de8a3cdae6f141e9567c51e550f7cecd85e0cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Fri, 21 Jun 2019 10:59:36 +0200 Subject: [PATCH 6/9] Allow to storeChunk on an empty dataset The arguments are sanity-checked, but no operation is passed down to the backend. --- include/openPMD/RecordComponent.hpp | 7 ++++++- src/RecordComponent.cpp | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index e5d02fbf7c..f43604cdc3 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -158,6 +158,9 @@ class RecordComponent : public BaseRecordComponent std::shared_ptr< std::queue< IOTask > > m_chunks; std::shared_ptr< Attribute > m_constantValue; + std::shared_ptr< bool > m_isEmpty{ + std::make_shared< bool >( false ) + }; private: void flush(std::string const&); @@ -297,7 +300,7 @@ template< typename T > inline void RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) { - if( *m_isConstant ) + if( *m_isConstant && !*m_isEmpty ) throw std::runtime_error("Chunks can not be written for a constant RecordComponent."); if( !data ) throw std::runtime_error("Unallocated pointer passed during chunk store."); @@ -332,6 +335,8 @@ RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) + " - Chunk: " + std::to_string(o[i] + e[i]) + ")"); + if (*m_isEmpty) + return; Parameter< Operation::WRITE_DATASET > dWrite; dWrite.offset = o; dWrite.extent = e; diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index bf5d40faa7..6f027bca01 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -85,6 +85,7 @@ RecordComponent::makeEmpty( Dataset d ) "A recordComponent can not (yet) be made" " empty after it has been written."); + *m_isEmpty = true; *m_dataset = d; dirty = true; static detail::DefaultValue< RecordComponent > dv; From 2aa07653fd64ecfd48a7dfd5bb24588c38e1af2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 27 Jun 2019 11:21:12 +0200 Subject: [PATCH 7/9] Cleanup Add documentation, change some typos in error messages, clean dead code --- include/openPMD/RecordComponent.hpp | 21 ++++++++++++++----- .../openPMD/backend/BaseRecordComponent.hpp | 9 ++++++++ src/RecordComponent.cpp | 6 ++++-- test/SerialIOTest.cpp | 1 - 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index f43604cdc3..f96ccaafaf 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -108,6 +108,14 @@ class RecordComponent : public BaseRecordComponent template< typename T > RecordComponent& makeConstant(T); + /** + * Create a dataset with zero extent in each dimension. + * Implemented by creating a constant record component with + * a dummy value (default constructor of the respective datatype). + * @param dimensions The number of dimensions. Must be greater than + * zero. + * @return A reference to this RecordComponent. + */ template< typename T > RecordComponent& makeEmpty( uint8_t dimensions ); @@ -158,13 +166,18 @@ class RecordComponent : public BaseRecordComponent std::shared_ptr< std::queue< IOTask > > m_chunks; std::shared_ptr< Attribute > m_constantValue; - std::shared_ptr< bool > m_isEmpty{ - std::make_shared< bool >( false ) - }; + std::shared_ptr< bool > m_isEmpty = std::make_shared< bool >( false ); private: void flush(std::string const&); virtual void read(); + + /** + * Internal method to be called by all methods that create an empty dataset. + * + * @param The dataset description. Must have nonzero dimensions. + * @return Reference to this RecordComponent instance. + */ RecordComponent& makeEmpty( Dataset ); }; // RecordComponent @@ -185,8 +198,6 @@ template< typename T > inline RecordComponent& RecordComponent::makeEmpty( uint8_t dimensions ) { - if ( dimensions == 0 ) - throw std::runtime_error("Dataset extent must be at least 1D."); return makeEmpty( Dataset( determineDatatype< T >(), Extent( dimensions, 0 ) ) ); diff --git a/include/openPMD/backend/BaseRecordComponent.hpp b/include/openPMD/backend/BaseRecordComponent.hpp index 41c8784415..2c7777284c 100644 --- a/include/openPMD/backend/BaseRecordComponent.hpp +++ b/include/openPMD/backend/BaseRecordComponent.hpp @@ -61,8 +61,17 @@ class BaseRecordComponent : public Attributable std::shared_ptr< Dataset > m_dataset; std::shared_ptr< bool > m_isConstant; }; // BaseRecordComponent + namespace detail { +/** + * Functor template to be used in combination with "()" + * to set a default value for constant record components via the + * respective type's default constructor. + * Used to implement empty datasets in subclasses of BaseRecordComponent + * (currently RecordComponent). + * @param rc + */ template< typename RecordComponent_T > struct DefaultValue { diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index 6f027bca01..d180e83901 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -50,7 +50,7 @@ RecordComponent & RecordComponent::resetDataset( Dataset d ) { if( written ) - throw std::runtime_error( "A Records Dataset can not (yet) be changed " + throw std::runtime_error( "A record's Dataset cannot (yet) be changed " "after it has been written." ); if( d.extent.empty() ) throw std::runtime_error("Dataset extent must be at least 1D."); @@ -82,8 +82,10 @@ RecordComponent::makeEmpty( Dataset d ) { if( written ) throw std::runtime_error( - "A recordComponent can not (yet) be made" + "A RecordComponent cannot (yet) be made" " empty after it has been written."); + if( d.extent.size() == 0 ) + throw std::runtime_error("Dataset extent must be at least 1D."); *m_isEmpty = true; *m_dataset = d; diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index cb4822a1e9..b681fd0364 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -97,7 +97,6 @@ TEST_CASE( "empty_dataset_test", "[serial]" ) { for (auto const & t: backends) { - //if (std::get<0>(t) == "json") empty_dataset_test(std::get<0>(t)); } } From 34e2aa72c63582b061a7c0e5d25f30da499b1362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 27 Jun 2019 14:56:45 +0200 Subject: [PATCH 8/9] Require nullpointer on chunk store of an empty dataset --- include/openPMD/RecordComponent.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index f96ccaafaf..199e58b86f 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -313,8 +313,10 @@ RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) { if( *m_isConstant && !*m_isEmpty ) throw std::runtime_error("Chunks can not be written for a constant RecordComponent."); - if( !data ) + if( !data && !*m_isEmpty ) throw std::runtime_error("Unallocated pointer passed during chunk store."); + if( *m_isEmpty && data ) + throw std::runtime_error("Non-null pointer passed during chunk store of an empty RecordComponent."); Datatype dtype = determineDatatype(data); if( dtype != getDatatype() ) { From 3adee766230da0fd4c40494b1b40e3bd7b032539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 27 Jun 2019 16:23:52 +0200 Subject: [PATCH 9/9] Disallow the usage of storeChunk on empty record components --- include/openPMD/RecordComponent.hpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/include/openPMD/RecordComponent.hpp b/include/openPMD/RecordComponent.hpp index 199e58b86f..0337d4ee01 100644 --- a/include/openPMD/RecordComponent.hpp +++ b/include/openPMD/RecordComponent.hpp @@ -311,12 +311,12 @@ template< typename T > inline void RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) { - if( *m_isConstant && !*m_isEmpty ) - throw std::runtime_error("Chunks can not be written for a constant RecordComponent."); - if( !data && !*m_isEmpty ) - throw std::runtime_error("Unallocated pointer passed during chunk store."); - if( *m_isEmpty && data ) - throw std::runtime_error("Non-null pointer passed during chunk store of an empty RecordComponent."); + if( *m_isConstant ) + throw std::runtime_error("Chunks cannot be written for a constant RecordComponent."); + if( *m_isEmpty ) + throw std::runtime_error("Chunks cannot be written for an empty RecordComponent."); + if( !data ) +throw std::runtime_error("Unallocated pointer passed during chunk store."); Datatype dtype = determineDatatype(data); if( dtype != getDatatype() ) { @@ -348,8 +348,6 @@ RecordComponent::storeChunk(std::shared_ptr data, Offset o, Extent e) + " - Chunk: " + std::to_string(o[i] + e[i]) + ")"); - if (*m_isEmpty) - return; Parameter< Operation::WRITE_DATASET > dWrite; dWrite.offset = o; dWrite.extent = e;