some small fixed, and test coverage

corsika/detail/modules/writers/ObservationPlaneWriterParquet.inl

@@ -10,10 +10,10 @@
 
 namespace corsika {
 
-  ObservationPlaneWriterParquet::ObservationPlaneWriterParquet()
+  inline ObservationPlaneWriterParquet::ObservationPlaneWriterParquet()
       : output_() {}
 
-  void ObservationPlaneWriterParquet::startOfLibrary(
+  inline void ObservationPlaneWriterParquet::startOfLibrary(
       boost::filesystem::path const& directory) {
 
     // setup the streamer
@@ -34,24 +34,16 @@ namespace corsika {
 
     // and build the streamer
     output_.buildStreamer();
-
-    setInit(true);
   }
 
-  void ObservationPlaneWriterParquet::endOfShower() { ++shower_; }
-
-  void ObservationPlaneWriterParquet::endOfLibrary() { output_.closeStreamer(); }
+  inline void ObservationPlaneWriterParquet::endOfShower() { ++shower_; }
 
-  void ObservationPlaneWriterParquet::write(Code const& pid, HEPEnergyType const& energy,
-                                            LengthType const& x, LengthType const& y) {
-    if (!isInit()) {
-      std::runtime_error(
-          "ObservationPlaneWriterParquet not initialized. Either 1) add the "
-          "corresponding module to "
-          "the OutputManager, or 2) declare the module to write no output using "
-          "NoOutput.");
-    }
+  inline void ObservationPlaneWriterParquet::endOfLibrary() { output_.closeStreamer(); }
 
+  inline void ObservationPlaneWriterParquet::write(Code const& pid,
+                                                   HEPEnergyType const& energy,
+                                                   LengthType const& x,
+                                                   LengthType const& y) {
     // write the next row - we must write `shower_` first.
     *(output_.getWriter()) << shower_ << static_cast<int>(get_PDG(pid))
                            << static_cast<float>(energy / 1_GeV)

corsika/detail/modules/writers/TrackWriterParquet.inl

@@ -10,10 +10,11 @@
 
 namespace corsika {
 
-  TrackWriterParquet::TrackWriterParquet()
+  inline TrackWriterParquet::TrackWriterParquet()
       : output_() {}
 
-  void TrackWriterParquet::startOfLibrary(boost::filesystem::path const& directory) {
+  inline void TrackWriterParquet::startOfLibrary(
+      boost::filesystem::path const& directory) {
 
     // setup the streamer
     output_.initStreamer((directory / "tracks.parquet").string());
@@ -38,24 +39,15 @@ namespace corsika {
 
     // and build the streamer
     output_.buildStreamer();
-
-    setInit(true);
   }
 
-  void TrackWriterParquet::endOfShower() { ++shower_; }
-
-  void TrackWriterParquet::endOfLibrary() { output_.closeStreamer(); }
+  inline void TrackWriterParquet::endOfShower() { ++shower_; }
 
-  void TrackWriterParquet::write(Code const& pid, HEPEnergyType const& energy,
-                                 QuantityVector<length_d> const& start,
-                                 QuantityVector<length_d> const& end) {
+  inline void TrackWriterParquet::endOfLibrary() { output_.closeStreamer(); }
 
-    if (!isInit()) {
-      std::runtime_error(
-          "TrackWriterParquet not initialized. Either 1) add the corresponding module to "
-          "the OutputManager, or 2) declare the module to write no output using "
-          "NoOutput.");
-    }
+  inline void TrackWriterParquet::write(Code const& pid, HEPEnergyType const& energy,
+                                        QuantityVector<length_d> const& start,
+                                        QuantityVector<length_d> const& end) {
 
     // write the next row - we must write `shower_` first.
     // clang-format off

corsika/detail/output/BaseOutput.inl

@@ -9,7 +9,7 @@
 
 namespace corsika {
 
-  BaseOutput::BaseOutput()
+  inline BaseOutput::BaseOutput()
       : shower_(0) {}
 
 } // namespace corsika

corsika/detail/output/DummyOutputManager.inl

@@ -9,19 +9,19 @@
 
 namespace corsika {
 
-  DummyOutputManager::DummyOutputManager() {}
+  inline DummyOutputManager::DummyOutputManager() {}
 
-  DummyOutputManager::~DummyOutputManager() {}
+  inline DummyOutputManager::~DummyOutputManager() {}
 
   template <typename TOutput>
-  void DummyOutputManager::add(std::string const& name, TOutput& output) {}
+  inline void DummyOutputManager::add(std::string const&, TOutput&) {}
 
-  void DummyOutputManager::startOfLibrary() {}
+  inline void DummyOutputManager::startOfLibrary() {}
 
-  void DummyOutputManager::startOfShower() {}
+  inline void DummyOutputManager::startOfShower() {}
 
-  void DummyOutputManager::endOfShower() {}
+  inline void DummyOutputManager::endOfShower() {}
 
-  void DummyOutputManager::endOfLibrary() {}
+  inline void DummyOutputManager::endOfLibrary() {}
 
 } // namespace corsika

corsika/detail/output/OutputManager.inl

@@ -22,8 +22,8 @@
 
 namespace corsika {
 
-  void OutputManager::writeNode(YAML::Node const& node,
-                                boost::filesystem::path const& path) const {
+  inline void OutputManager::writeNode(YAML::Node const& node,
+                                       boost::filesystem::path const& path) const {
 
     // construct a YAML emitter for this config file
     YAML::Emitter out;
@@ -38,7 +38,7 @@ namespace corsika {
     file << out.c_str() << std::endl;
   }
 
-  void OutputManager::writeTopLevelConfig() const {
+  inline void OutputManager::writeTopLevelConfig() const {
 
     YAML::Node config;
 
@@ -51,7 +51,7 @@ namespace corsika {
     writeNode(config, root_ / ("config.yaml"));
   }
 
-  void OutputManager::writeTopLevelSummary() const {
+  inline void OutputManager::writeTopLevelSummary() const {
 
     YAML::Node config;
 
@@ -89,7 +89,7 @@ namespace corsika {
     writeNode(config, root_ / ("summary.yaml"));
   }
 
-  void OutputManager::initOutput(std::string const& name) const {
+  inline void OutputManager::initOutput(std::string const& name) const {
     // construct the path to this directory
     auto const path{root_ / name};
 
@@ -106,7 +106,7 @@ namespace corsika {
     writeNode(config, path / "config.yaml");
   }
 
-  OutputManager::OutputManager(
+  inline OutputManager::OutputManager(
       std::string const& name,
       boost::filesystem::path const& dir = boost::filesystem::current_path())
       : name_(name)
@@ -114,20 +114,19 @@ namespace corsika {
 
     // check if this directory already exists
     if (boost::filesystem::exists(root_)) {
-      logger->warn(
-          "Output directory '{}' already exists! This is currenty not supported.",
-          root_.string());
+      logger->error("Output directory '{}' already exists! Do not overwrite!.",
+                    root_.string());
       throw std::runtime_error("Output directory already exists.");
     }
 
     // construct the directory for this library
-    boost::filesystem::create_directory(root_);
+    boost::filesystem::create_directories(root_);
 
     // write the top level config file
     writeTopLevelConfig();
   }
 
-  OutputManager::~OutputManager() {
+  inline OutputManager::~OutputManager() {
 
     if (state_ == OutputState::ShowerInProgress) {
       // if this the destructor is called before the shower has been explicitly
@@ -148,13 +147,13 @@ namespace corsika {
   }
 
   template <typename TOutput>
-  void OutputManager::add(std::string const& name, TOutput& output) {
+  inline void OutputManager::add(std::string const& name, TOutput& output) {
 
     // check if that name is already in the map
     if (outputs_.count(name) > 0) {
-      logger->warn("'{}' is already registered. All outputs must have unique names.",
-                   name);
-      return;
+      logger->error("'{}' is already registered. All outputs must have unique names.",
+                    name);
+      throw std::runtime_error("Output already exists. Do not overwrite!");
     }
 
     // if we get here, the name is not already in the map
@@ -165,7 +164,7 @@ namespace corsika {
     initOutput(name);
   }
 
-  void OutputManager::startOfLibrary() {
+  inline void OutputManager::startOfLibrary() {
 
     // this is only valid when we haven't started a library
     // or have already finished a library
@@ -188,7 +187,7 @@ namespace corsika {
     state_ = OutputState::LibraryReady;
   }
 
-  void OutputManager::startOfShower() {
+  inline void OutputManager::startOfShower() {
 
     // if this is called and we still in the "no init" state, then
     // this is the first shower in the library so make sure we start it
@@ -204,7 +203,7 @@ namespace corsika {
     state_ = OutputState::ShowerInProgress;
   }
 
-  void OutputManager::endOfShower() {
+  inline void OutputManager::endOfShower() {
 
     for (auto& [name, output] : outputs_) { output.get().endOfShower(); }
 
@@ -212,7 +211,7 @@ namespace corsika {
     state_ = OutputState::LibraryReady;
   }
 
-  void OutputManager::endOfLibrary() {
+  inline void OutputManager::endOfLibrary() {
 
     // we can only call endOfLibrary when we have already started
     if (state_ == OutputState::NoInit) {

corsika/detail/output/ParquetStreamer.inl

@@ -10,9 +10,10 @@
 
 namespace corsika {
 
-  ParquetStreamer::ParquetStreamer() {}
+  inline ParquetStreamer::ParquetStreamer()
+      : isInit_(false) {}
 
-  void ParquetStreamer::initStreamer(std::string const& filepath) {
+  inline void ParquetStreamer::initStreamer(std::string const& filepath) {
 
     // open the file and connect it to our pointer
     PARQUET_ASSIGN_OR_THROW(outfile_, arrow::io::FileOutputStream::Open(filepath));
@@ -26,16 +27,16 @@ namespace corsika {
   }
 
   template <typename... TArgs>
-  void ParquetStreamer::addField(TArgs&&... args) {
+  inline void ParquetStreamer::addField(TArgs&&... args) {
     fields_.push_back(parquet::schema::PrimitiveNode::Make(args...));
   }
 
-  void ParquetStreamer::enableCompression(int const /*level*/) {
+  inline void ParquetStreamer::enableCompression(int const /*level*/) {
     // builder_.compression(parquet::Compression::ZSTD);
     // builder_.compression_level(level);
   }
 
-  void ParquetStreamer::buildStreamer() {
+  inline void ParquetStreamer::buildStreamer() {
 
     // build the top level schema
     schema_ = std::static_pointer_cast<parquet::schema::GroupNode>(
@@ -45,13 +46,26 @@ namespace corsika {
     // and build the writer
     writer_ = std::make_shared<parquet::StreamWriter>(
         parquet::ParquetFileWriter::Open(outfile_, schema_, builder_.build()));
+
+    //  only now this object is ready to stream
+    isInit_ = true;
   }
 
-  void ParquetStreamer::closeStreamer() {
+  inline void ParquetStreamer::closeStreamer() {
     writer_.reset();
     [[maybe_unused]] auto status = outfile_->Close();
+    isInit_ = false;
   }
 
-  std::shared_ptr<parquet::StreamWriter> ParquetStreamer::getWriter() { return writer_; }
+  inline std::shared_ptr<parquet::StreamWriter> ParquetStreamer::getWriter() {
+    if (!isInit()) {
+      throw std::runtime_error(
+          "ParquetStreamer not initialized. Either 1) add the "
+          "corresponding module to "
+          "the OutputManager, or 2) declare the module to write no output using "
+          "NoOutput.");
+    }
+    return writer_;
+  }
 
 } // namespace corsika

corsika/output/BaseOutput.hpp

@@ -51,20 +51,21 @@ namespace corsika {
     /**
      * Get any summary information for the entire library.
      */
-    virtual YAML::Node getSummary() { return YAML::Node(); };
+    virtual YAML::Node getSummary() { return YAML::Node(); }
 
     /**
      * Flag to indicate readiness.
      */
     bool isInit() const { return is_init_; }
 
+  protected:
     /**
      * Set init flag.
      */
     void setInit(bool const v) { is_init_ = v; }
 
-  protected:
     int shower_{0}; ///< The current event number.
+
   private:
     bool is_init_{false}; ///< flag to indicate readiness
   };

corsika/output/ParquetStreamer.hpp

@@ -27,13 +27,6 @@ namespace corsika {
    */
   class ParquetStreamer {
 
-  protected:
-    parquet::WriterProperties::Builder builder_; ///< The writer properties builder.
-    parquet::schema::NodeVector fields_;         ///< The fields in this file.
-    std::shared_ptr<parquet::schema::GroupNode> schema_;   ///< The schema for this file.
-    std::shared_ptr<arrow::io::FileOutputStream> outfile_; ///< The output file.
-    std::shared_ptr<parquet::StreamWriter> writer_; ///< The stream writer to 'outfile'
-
   public:
     /**
      * ParquetStreamer's take no constructor arguments.
@@ -71,7 +64,20 @@ namespace corsika {
      */
     std::shared_ptr<parquet::StreamWriter> getWriter();
 
-  }; // class ParquetHelper
+    /**
+     * @return status of streamer
+     */
+    bool isInit() const { return isInit_; }
+
+  private:
+    bool isInit_ = false;                        ///< flag to handle state of writer
+    parquet::WriterProperties::Builder builder_; ///< The writer properties builder.
+    parquet::schema::NodeVector fields_;         ///< The fields in this file.
+    std::shared_ptr<parquet::schema::GroupNode> schema_;   ///< The schema for this file.
+    std::shared_ptr<arrow::io::FileOutputStream> outfile_; ///< The output file.
+    std::shared_ptr<parquet::StreamWriter> writer_; ///< The stream writer to 'outfile'
+
+  }; // class ParquetStreamer
 } // namespace corsika
 
 #include <corsika/detail/output/ParquetStreamer.inl>

tests/CMakeLists.txt

@@ -3,3 +3,4 @@ add_subdirectory (framework)
 add_subdirectory (media)
 add_subdirectory (stack)
 add_subdirectory (modules)
+add_subdirectory (output)

tests/modules/testObservationPlane.cpp

@@ -28,7 +28,6 @@ using namespace corsika;
 TEST_CASE("ObservationPlane", "[proccesses][observation_plane]") {
 
   logging::set_level(logging::level::trace);
-  corsika_logger->set_pattern("[%n:%^%-8l%$]: %v");
 
   auto [env, csPtr, nodePtr] = setup::testing::setup_environment(Code::Oxygen);
   auto const& cs = *csPtr;

tests/output/CMakeLists.txt

+set (test_output_sources
+  TestMain.cpp
+  testOutputManager.cpp
+  testDummyOutputManager.cpp
+  testParquetStreamer.cpp
+  #testWriterObservationPlane.cpp
+  #testWriterTrack.cpp
+  )
+
+CORSIKA_ADD_TEST (testOutput SOURCES ${test_output_sources})
+
+target_compile_definitions (
+  testOutput
+  PRIVATE
+  REFDATADIR="${CMAKE_CURRENT_SOURCE_DIR}"
+)

tests/output/TestMain.cpp

+/*
+ * (c) Copyright 2020 CORSIKA Project, corsika-project@lists.kit.edu
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * Licence version 3 (GPL Version 3). See file LICENSE for a full version of
+ * the license.
+ */
+
+#define CATCH_CONFIG_MAIN // This tells Catch to provide a main() - only do this in one
+                          // cpp file
+#include <catch2/catch.hpp>

tests/output/testDummyOutputManager.cpp

+/*
+ * (c) Copyright 2020 CORSIKA Project, corsika-project@lists.kit.edu
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * Licence version 3 (GPL Version 3). See file LICENSE for a full version of
+ * the license.
+ */
+
+#include <catch2/catch.hpp>
+
+#include <corsika/output/DummyOutputManager.hpp>
+#include <corsika/framework/core/Logging.hpp>
+
+using namespace corsika;
+
+class DummyOutput {};
+
+TEST_CASE("DummyOutputManager", "interface") {
+
+  logging::set_level(logging::level::info);
+
+  // output manager performs nothing, no action, just interface
+  DummyOutputManager output;
+
+  DummyOutput test;
+  output.add("test", test);
+
+  output.startOfLibrary();
+  output.startOfShower();
+  output.endOfShower();
+  output.endOfLibrary();
+}

tests/output/testOutputManager.cpp

+/*
+ * (c) Copyright 2020 CORSIKA Project, corsika-project@lists.kit.edu
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * Licence version 3 (GPL Version 3). See file LICENSE for a full version of
+ * the license.
+ */
+
+#include <catch2/catch.hpp>
+
+#include <boost/filesystem.hpp>
+
+#include <corsika/output/OutputManager.hpp>
+#include <corsika/framework/core/Logging.hpp>
+
+using namespace corsika;
+
+struct DummyOutput : public BaseOutput {
+
+  mutable bool isConfig_ = false;
+  mutable bool isSummary_ = false;
+  bool startLibrary_ = false;
+  bool startShower_ = false;
+  bool endLibrary_ = false;
+  bool endShower_ = false;
+
+  void startOfLibrary(boost::filesystem::path const&) { startLibrary_ = true; }
+
+  void startOfShower() { startShower_ = true; }
+
+  void endOfShower() { endShower_ = true; }
+
+  void endOfLibrary() { endLibrary_ = true; }
+
+  YAML::Node getConfig() const {
+    isConfig_ = true;
+    return YAML::Node();
+  }
+
+  YAML::Node getSummary() {
+    isSummary_ = true;
+    return YAML::Node();
+  }
+};
+
+TEST_CASE("OutputManager") {
+
+  logging::set_level(logging::level::info);
+
+  SECTION("standard") {
+
+    // preparation
+    if (boost::filesystem::exists("./out_test")) {
+      boost::filesystem::remove_all("./out_test");
+    }
+
+    // output manager performs nothing, no action, just interface
+    OutputManager output("check", "./out_test");
+
+    CHECK(boost::filesystem::is_directory("./out_test/check"));
+
+    DummyOutput test;
+    output.add("test", test);
+
+    CHECK_THROWS(output.add(
+        "test",
+        test)); // should emit warning which cannot be catched, but no action or failure
+
+    CHECK(test.isConfig_);
+    test.isConfig_ = false;
+
+    output.startOfLibrary();
+    CHECK(test.startLibrary_);
+    test.startLibrary_ = false;
+
+    output.startOfShower();
+    CHECK(test.startShower_);
+    test.startShower_ = false;
+
+    output.endOfShower();
+    CHECK(test.endShower_);
+    test.endShower_ = false;
+
+    output.endOfLibrary();
+    CHECK(test.endLibrary_);
+    CHECK(test.isSummary_);
+    test.isSummary_ = false;
+    test.endLibrary_ = false;
+  }
+
+  SECTION("failures") {
+
+    logging::set_level(logging::level::info);
+
+    // preparation
+    if (boost::filesystem::exists("./out_test")) {
+      boost::filesystem::remove_all("./out_test");
+    }
+
+    // output manager performs nothing, no action, just interface
+    OutputManager output("check", "./out_test");
+    CHECK_THROWS(new OutputManager("check", "./out_test"));
+
+    // CHECK_THROWS(output.startOfShower());
+    // CHECK_THROWS(output.endOfShower());
+    CHECK_THROWS(output.endOfLibrary());
+
+    output.startOfLibrary();
+
+    CHECK_THROWS(output.startOfLibrary());
+    // CHECK_THROWS(output.endOfShower());
+    // CHECK_THROWS(output.endOfLibrary());
+
+    output.startOfShower();
+
+    CHECK_THROWS(output.startOfLibrary());
+    // CHECK_THROWS(output.startOfShower());
+    // CHECK_THROWS(output.endOfLibrary());
+
+    output.endOfShower();
+
+    CHECK_THROWS(output.startOfLibrary());
+    // CHECK_THROWS(output.startOfShower());
+    // CHECK_THROWS(output.endOfShower());
+
+    output.endOfLibrary();
+
+    // CHECK_THROWS(output.endOfShower());
+    // CHECK_THROWS(output.startOfShower());
+    // CHECK_THROWS(output.endOfLibrary());
+  }
+}
\ No newline at end of file

tests/output/testParquetStreamer.cpp

+/*
+ * (c) Copyright 2020 CORSIKA Project, corsika-project@lists.kit.edu
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * Licence version 3 (GPL Version 3). See file LICENSE for a full version of
+ * the license.
+ */
+
+#include <catch2/catch.hpp>
+
+#include <boost/filesystem.hpp>
+
+#include <corsika/output/ParquetStreamer.hpp>
+#include <corsika/framework/core/Logging.hpp>
+
+using namespace corsika;
+
+TEST_CASE("ParquetStreamer") {
+
+  logging::set_level(logging::level::info);
+
+  SECTION("standard") {
+
+    // preparation
+    if (boost::filesystem::exists("./parquet_test.parquet")) {
+      boost::filesystem::remove_all("./parquet_test.parquet");
+    }
+
+    ParquetStreamer test;
+    CHECK_FALSE(test.isInit());
+    CHECK_THROWS(test.getWriter());
+    test.initStreamer("./parquet_test.parquet");
+
+    test.addField("testint", parquet::Repetition::REQUIRED, parquet::Type::INT32,
+                  parquet::ConvertedType::INT_32);
+    test.addField("testfloat", parquet::Repetition::REQUIRED, parquet::Type::FLOAT,
+                  parquet::ConvertedType::NONE);
+
+    test.enableCompression(1);
+
+    test.buildStreamer();
+    CHECK(test.isInit());
+
+    int testint = 1;
+    double testfloat = 2.0;
+
+    std::shared_ptr<parquet::StreamWriter> writer = test.getWriter();
+    (*writer) << static_cast<int>(testint) << static_cast<int>(testint)
+              << static_cast<float>(testfloat) << parquet::EndRow;
+
+    test.closeStreamer();
+    CHECK_THROWS(test.getWriter());
+    CHECK_FALSE(test.isInit());
+    CHECK(boost::filesystem::exists("./parquet_test.parquet"));
+  }
+}
\ No newline at end of file

tests/output/testParquetStreamercpp

+/*
+ * (c) Copyright 2020 CORSIKA Project, corsika-project@lists.kit.edu
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * Licence version 3 (GPL Version 3). See file LICENSE for a full version of
+ * the license.
+ */
+
+#include <catch2/catch.hpp>
+
+#include <boost/filesystem.hpp>
+
+#include <corsika/output/ParquetStreamer.hpp>
+#include <corsika/framework/core/Logging.hpp>
+
+using namespace corsika;
+
+TEST_CASE("ParquetStreamer") {
+
+  logging::set_level(logging::level::info);
+
+  SECTION("standard") {
+
+    // preparation
+    if (boost::filesystem::exists("./parquet_test.parquet")) {
+      boost::filesystem::remove_all("./parquet_test.parquet");
+    }
+
+    ParquetStreamer test;
+    test.initStreamer("./parquet_test.parquet");
+
+    test.addField();
+
+    test.enableCompression(5);
+
+    test.buildStreamer();
+    test.closeStreamer();
+
+    std::shared_ptr<parquet::StreamWriter> writer = test.getWriter();
+  }
+}
\ No newline at end of file