From af26e1b228e4dd4eb6f4ddd187bbd37023c434f8 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Thu, 25 Jan 2024 09:48:59 +0100 Subject: [PATCH 01/37] Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer} --- include/podio/GenericParameters.h | 8 +-- include/podio/RNTupleReader.h | 96 ++++++++++----------------- include/podio/RNTupleWriter.h | 107 ++++++++++-------------------- src/RNTupleReader.cc | 40 +++++++++++ src/RNTupleWriter.cc | 45 +++++++++++++ 5 files changed, 158 insertions(+), 138 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 3e1700549..9e4ef9553 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -20,8 +20,8 @@ using version_type = uint32_t; // from sio/definitions #if PODIO_ENABLE_RNTUPLE namespace podio { -class RNTupleReader; -class RNTupleWriter; +class ROOTRNTupleReader; +class ROOTRNTupleWriter; } // namespace podio #endif @@ -149,8 +149,8 @@ class GenericParameters { friend void readGenericParameters(sio::read_device& device, GenericParameters& parameters, sio::version_type version); #if PODIO_ENABLE_RNTUPLE - friend RNTupleReader; - friend RNTupleWriter; + friend ROOTRNTupleReader; + friend ROOTRNTupleWriter; #endif /// Get a reference to the internal map for a given type diff --git a/include/podio/RNTupleReader.h b/include/podio/RNTupleReader.h index 1cffa149f..05cc0b319 100644 --- a/include/podio/RNTupleReader.h +++ b/include/podio/RNTupleReader.h @@ -1,5 +1,10 @@ +<<<<<<<< HEAD:include/podio/RNTupleReader.h #ifndef PODIO_RNTUPLEREADER_H #define PODIO_RNTUPLEREADER_H +======== +#ifndef PODIO_ROOTRNTUPLEREADER_H +#define PODIO_ROOTRNTUPLEREADER_H +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleReader.h #include "podio/CollectionBranches.h" #include "podio/ICollectionProvider.h" @@ -15,10 +20,6 @@ #include #include -#include -#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) - #include -#endif namespace podio { @@ -26,95 +27,66 @@ namespace podio { This class has the function to read available data from disk and to prepare collections and buffers. **/ -/// The RNTupleReader can be used to read files that have been written with the -/// RNTuple backend. -/// -/// The RNTupleReader provides the data as ROOTFrameData from which a podio::Frame -/// can be constructed. It can be used to read files written by the RNTupleWriter. +<<<<<<<< HEAD:include/podio/RNTupleReader.h class RNTupleReader { public: - /// Create a RNTupleReader RNTupleReader() = default; - /// Destructor ~RNTupleReader() = default; - /// The RNTupleReader is not copy-able + RNTupleReader(const RNTupleReader&) = delete; - /// The RNTupleReader is not copy-able RNTupleReader& operator=(const RNTupleReader&) = delete; +======== +class ROOTRNTupleReader { + +public: + ROOTRNTupleReader() = default; + ~ROOTRNTupleReader() = default; + + ROOTRNTupleReader(const ROOTRNTupleReader&) = delete; + ROOTRNTupleReader& operator=(const ROOTRNTupleReader&) = delete; +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleReader.h - /// Open a single file for reading. - /// - /// @param filename The name of the input file void openFile(const std::string& filename); + void openFiles(const std::vector& filename); - /// Open multiple files for reading and then treat them as if they are one file - /// - /// @note All of the files are assumed to have the same structure. Specifically - /// this means: - /// - The same categories are available from all files - /// - The collections that are contained in the individual categories are the - /// same across all files - /// - This usually boils down to "the files have been written with the same - /// "settings", e.g. they are outputs of a batched process. - /// - /// @param filenames The filenames of all input files that should be read - void openFiles(const std::vector& filenames); - - /// Read the next data entry for a given category. - /// - /// @param name The category name for which to read the next entry - /// - /// @returns FrameData from which a podio::Frame can be constructed if the - /// category exists and if there are still entries left to read. - /// Otherwise a nullptr + /** + * Read the next data entry from which a Frame can be constructed for the + * given name. In case there are no more entries left for this name or in + * case there is no data for this name, this returns a nullptr. + */ std::unique_ptr readNextEntry(const std::string& name); - /// Read the desired data entry for a given category. - /// - /// @param name The category name for which to read the next entry - /// @param entry The entry number to read - /// - /// @returns FrameData from which a podio::Frame can be constructed if the - /// category and the desired entry exist. Otherwise a nullptr + /** + * Read the specified data entry from which a Frame can be constructed for + * the given name. In case the entry does not exist for this name or in case + * there is no data for this name, this returns a nullptr. + */ std::unique_ptr readEntry(const std::string& name, const unsigned entry); - /// Get the names of all the available Frame categories in the current file(s). - /// - /// @returns The names of the available categores from the file + /// Get the names of all the available Frame categories in the current file(s) std::vector getAvailableCategories() const; - /// Get the number of entries for the given name - /// - /// @param name The name of the category - /// - /// @returns The number of entries that are available for the category + /// Returns number of entries for the given name unsigned getEntries(const std::string& name); - /// Get the build version of podio that has been used to write the current - /// file - /// - /// @returns The podio build version + /// Get the build version of podio that has been used to write the current file podio::version::Version currentFileVersion() const { return m_fileVersion; } /// Get the datamodel definition for the given name - /// - /// @param name The name of the datamodel - /// - /// @returns The high level definition of the datamodel in JSON format const std::string_view getDatamodelDefinition(const std::string& name) const { return m_datamodelHolder.getDatamodelDefinition(name); } - /// Get all names of the datamodels that are available from this reader - /// - /// @returns The names of the datamodels + /// Get all names of the datamodels that ara available from this reader std::vector getAvailableDatamodels() const { return m_datamodelHolder.getAvailableDatamodels(); } + void closeFile(); + private: /** * Initialize the given category by filling the maps with metadata information diff --git a/include/podio/RNTupleWriter.h b/include/podio/RNTupleWriter.h index ed0f3012c..8820ba4d9 100644 --- a/include/podio/RNTupleWriter.h +++ b/include/podio/RNTupleWriter.h @@ -1,5 +1,10 @@ +<<<<<<<< HEAD:include/podio/RNTupleWriter.h #ifndef PODIO_RNTUPLEWRITER_H #define PODIO_RNTUPLEWRITER_H +======== +#ifndef PODIO_ROOTRNTUPLEWRITER_H +#define PODIO_ROOTRNTUPLEWRITER_H +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleWriter.h #include "podio/CollectionBase.h" #include "podio/Frame.h" @@ -10,10 +15,6 @@ #include "TFile.h" #include #include -#include -#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) - #include -#endif #include #include @@ -21,90 +22,48 @@ namespace podio { -/// The RNTupleWriter writes podio files into ROOT files using the new RNTuple -/// format. -/// -/// Each category gets its own RNTuple. Additionally, there is a podio_metadata -/// RNTuple that contains metadata that is necessary for interpreting the files -/// for reading. -/// -/// Files written with the RNTupleWriter can be read with the RNTupleReader. +<<<<<<<< HEAD:include/podio/RNTupleWriter.h class RNTupleWriter { public: - /// Create a RNTupleWriter to write to a file. - /// - /// @note Existing files will be overwritten without warning. - /// - /// @param filename The path to the file that will be created. RNTupleWriter(const std::string& filename); - - /// RNTupleWriter destructor - /// - /// This also takes care of writing all the necessary metadata in order to be - /// able to read files back again. ~RNTupleWriter(); - /// The RNTupleWriter is not copy-able RNTupleWriter(const RNTupleWriter&) = delete; - /// The RNTupleWriter is not copy-able RNTupleWriter& operator=(const RNTupleWriter&) = delete; +======== +class ROOTRNTupleWriter { +public: + ROOTRNTupleWriter(const std::string& filename); + ~ROOTRNTupleWriter(); - /// Store the given frame with the given category. - /// - /// This stores all available collections from the Frame. - /// - /// @note The contents of the first Frame that is written in this way - /// determines the contents that will be written for all subsequent Frames. - /// - /// @param frame The Frame to store - /// @param category The category name under which this Frame should be stored - void writeFrame(const podio::Frame& frame, const std::string& category); + ROOTRNTupleWriter(const ROOTRNTupleWriter&) = delete; + ROOTRNTupleWriter& operator=(const ROOTRNTupleWriter&) = delete; +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleWriter.h - /// Store the given Frame with the given category. - /// - /// This stores only the desired collections and not the complete frame. - /// - /// @note The contents of the first Frame that is written in this way - /// determines the contents that will be written for all subsequent Frames. - /// - /// @param frame The Frame to store - /// @param category The category name under which this Frame should be - /// stored - /// @param collsToWrite The collection names that should be written - void writeFrame(const podio::Frame& frame, const std::string& category, const std::vector& collsToWrite); + template + void fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry); - /// Write the current file, including all the necessary metadata to read it - /// again. - /// - /// @note The destructor will also call this, so letting a RNTupleWriter go out - /// of scope is also a viable way to write a readable file + void writeFrame(const podio::Frame& frame, const std::string& category); + void writeFrame(const podio::Frame& frame, const std::string& category, const std::vector& collsToWrite); void finish(); - /// Check whether the collsToWrite are consistent with the state of the passed - /// category. - /// - /// @note This will only be a meaningful check if the first Frame of the passed - /// category has already been written. Also, this check is rather expensive as - /// it has to effectively do two set differences. - /// - /// - /// @param collsToWrite The collection names that should be checked for - /// consistency - /// @param category The category name for which consistency should be - /// checked - /// - /// @returns two vectors of collection names. The first one contains all the - /// names that were missing from the collsToWrite but were present in the - /// category. The second one contains the names that are present in the - /// collsToWrite only. If both vectors are empty the category and the passed - /// collsToWrite are consistent. + /** Check whether the collsToWrite are consistent with the state of the passed + * category. + * + * Return two vectors of collection names. The first one contains all the + * names that were missing from the collsToWrite but were present in the + * category. The second one contains the names that are present in the + * collsToWrite only. If both vectors are empty the category and the passed + * collsToWrite are consistent. + * + * NOTE: This will only be a meaningful check if the first Frame of the passed + * category has already been written. Also, this check is rather expensive as + * it has to effectively do two set differences. + */ std::tuple, std::vector> checkConsistency(const std::vector& collsToWrite, const std::string& category) const; private: - template - void fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry); - using StoreCollection = std::pair; std::unique_ptr createModels(const std::vector& collections); @@ -142,4 +101,8 @@ class RNTupleWriter { } // namespace podio +<<<<<<<< HEAD:include/podio/RNTupleWriter.h #endif // PODIO_RNTUPLEWRITER_H +======== +#endif // PODIO_ROOTRNTUPLEWRITER_H +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleWriter.h diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index a372f6c2f..706e3798d 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -1,4 +1,8 @@ +<<<<<<<< HEAD:src/RNTupleReader.cc #include "podio/RNTupleReader.h" +======== +#include "podio/ROOTRNTupleReader.h" +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc #include "podio/CollectionBase.h" #include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" @@ -14,7 +18,11 @@ namespace podio { template +<<<<<<<< HEAD:src/RNTupleReader.cc void RNTupleReader::readParams(const std::string& name, unsigned entNum, GenericParameters& params) { +======== +void ROOTRNTupleReader::readParams(const std::string& name, unsigned entNum, GenericParameters& params) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc auto keyView = m_readers[name][0]->GetView>(root_utils::getGPKeyName()); auto valueView = m_readers[name][0]->GetView>>(root_utils::getGPValueName()); @@ -23,7 +31,11 @@ void RNTupleReader::readParams(const std::string& name, unsigned entNum, Generic } } +<<<<<<<< HEAD:src/RNTupleReader.cc GenericParameters RNTupleReader::readEventMetaData(const std::string& name, unsigned entNum) { +======== +GenericParameters ROOTRNTupleReader::readEventMetaData(const std::string& name, unsigned entNum) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc GenericParameters params; readParams(name, entNum, params); @@ -34,7 +46,11 @@ GenericParameters RNTupleReader::readEventMetaData(const std::string& name, unsi return params; } +<<<<<<<< HEAD:src/RNTupleReader.cc bool RNTupleReader::initCategory(const std::string& category) { +======== +bool ROOTRNTupleReader::initCategory(const std::string& category) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc if (std::find(m_availableCategories.begin(), m_availableCategories.end(), category) == m_availableCategories.end()) { return false; } @@ -65,11 +81,19 @@ bool RNTupleReader::initCategory(const std::string& category) { return true; } +<<<<<<<< HEAD:src/RNTupleReader.cc void RNTupleReader::openFile(const std::string& filename) { openFiles({filename}); } void RNTupleReader::openFiles(const std::vector& filenames) { +======== +void ROOTRNTupleReader::openFile(const std::string& filename) { + openFiles({filename}); +} + +void ROOTRNTupleReader::openFiles(const std::vector& filenames) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc m_filenames.insert(m_filenames.end(), filenames.begin(), filenames.end()); for (auto& filename : filenames) { @@ -93,7 +117,11 @@ void RNTupleReader::openFiles(const std::vector& filenames) { m_availableCategories = availableCategoriesField(0); } +<<<<<<<< HEAD:src/RNTupleReader.cc unsigned RNTupleReader::getEntries(const std::string& name) { +======== +unsigned ROOTRNTupleReader::getEntries(const std::string& name) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc if (m_readers.find(name) == m_readers.end()) { for (auto& filename : m_filenames) { try { @@ -108,7 +136,11 @@ unsigned RNTupleReader::getEntries(const std::string& name) { return m_totalEntries[name]; } +<<<<<<<< HEAD:src/RNTupleReader.cc std::vector RNTupleReader::getAvailableCategories() const { +======== +std::vector ROOTRNTupleReader::getAvailableCategories() const { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc std::vector cats; cats.reserve(m_availableCategories.size()); for (const auto& cat : m_availableCategories) { @@ -117,11 +149,19 @@ std::vector RNTupleReader::getAvailableCategories() const { return cats; } +<<<<<<<< HEAD:src/RNTupleReader.cc std::unique_ptr RNTupleReader::readNextEntry(const std::string& name) { return readEntry(name, m_entries[name]); } std::unique_ptr RNTupleReader::readEntry(const std::string& category, const unsigned entNum) { +======== +std::unique_ptr ROOTRNTupleReader::readNextEntry(const std::string& name) { + return readEntry(name, m_entries[name]); +} + +std::unique_ptr ROOTRNTupleReader::readEntry(const std::string& category, const unsigned entNum) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc if (m_totalEntries.find(category) == m_totalEntries.end()) { getEntries(category); } diff --git a/src/RNTupleWriter.cc b/src/RNTupleWriter.cc index 38231cc18..24272c6c2 100644 --- a/src/RNTupleWriter.cc +++ b/src/RNTupleWriter.cc @@ -1,4 +1,8 @@ +<<<<<<<< HEAD:src/RNTupleWriter.cc #include "podio/RNTupleWriter.h" +======== +#include "podio/ROOTRNTupleWriter.h" +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc #include "podio/CollectionBase.h" #include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" @@ -16,19 +20,31 @@ namespace podio { +<<<<<<<< HEAD:src/RNTupleWriter.cc RNTupleWriter::RNTupleWriter(const std::string& filename) : +======== +ROOTRNTupleWriter::ROOTRNTupleWriter(const std::string& filename) : +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc m_metadata(ROOT::Experimental::RNTupleModel::Create()), m_file(new TFile(filename.c_str(), "RECREATE", "data file")) { } +<<<<<<<< HEAD:src/RNTupleWriter.cc RNTupleWriter::~RNTupleWriter() { +======== +ROOTRNTupleWriter::~ROOTRNTupleWriter() { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if (!m_finished) { finish(); } } template +<<<<<<<< HEAD:src/RNTupleWriter.cc std::pair&, std::vector>&> RNTupleWriter::getKeyValueVectors() { +======== +std::pair&, std::vector>&> ROOTRNTupleWriter::getKeyValueVectors() { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if constexpr (std::is_same_v) { return {m_intkeys, m_intvalues}; } else if constexpr (std::is_same_v) { @@ -43,7 +59,11 @@ std::pair&, std::vector>&> RNTupleWriter } template +<<<<<<<< HEAD:src/RNTupleWriter.cc void RNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { +======== +void ROOTRNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto [key, value] = getKeyValueVectors(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) entry->BindRawPtr(root_utils::getGPKeyName(), &key); @@ -64,12 +84,21 @@ void RNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::RE } } +<<<<<<<< HEAD:src/RNTupleWriter.cc void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { writeFrame(frame, category, frame.getAvailableCollections()); } void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, const std::vector& collsToWrite) { +======== +void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { + writeFrame(frame, category, frame.getAvailableCollections()); +} + +void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, + const std::vector& collsToWrite) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto& catInfo = getCategoryInfo(category); // Use the writer as proxy to check whether this category has been initialized @@ -188,7 +217,11 @@ void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& cat } std::unique_ptr +<<<<<<<< HEAD:src/RNTupleWriter.cc RNTupleWriter::createModels(const std::vector& collections) { +======== +ROOTRNTupleWriter::createModels(const std::vector& collections) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto model = ROOT::Experimental::RNTupleModel::CreateBare(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) @@ -260,7 +293,11 @@ RNTupleWriter::createModels(const std::vector& collections) { return model; } +<<<<<<<< HEAD:src/RNTupleWriter.cc RNTupleWriter::CollectionInfo& RNTupleWriter::getCategoryInfo(const std::string& category) { +======== +ROOTRNTupleWriter::CollectionInfo& ROOTRNTupleWriter::getCategoryInfo(const std::string& category) { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if (auto it = m_categories.find(category); it != m_categories.end()) { return it->second; } @@ -269,7 +306,11 @@ RNTupleWriter::CollectionInfo& RNTupleWriter::getCategoryInfo(const std::string& return it->second; } +<<<<<<<< HEAD:src/RNTupleWriter.cc void RNTupleWriter::finish() { +======== +void ROOTRNTupleWriter::finish() { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto podioVersion = podio::version::build_version; auto versionField = m_metadata->MakeField>(root_utils::versionBranchName); @@ -317,7 +358,11 @@ void RNTupleWriter::finish() { } std::tuple, std::vector> +<<<<<<<< HEAD:src/RNTupleWriter.cc RNTupleWriter::checkConsistency(const std::vector& collsToWrite, const std::string& category) const { +======== +ROOTRNTupleWriter::checkConsistency(const std::vector& collsToWrite, const std::string& category) const { +>>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if (const auto it = m_categories.find(category); it != m_categories.end()) { return root_utils::getInconsistentColls(it->second.name, collsToWrite); } From 9d04697e42765b28f48f1be764488fbbe0bc948f Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Jan 2024 14:38:59 +0100 Subject: [PATCH 02/37] clang-format --- src/RNTupleWriter.cc | 47 +------------------------------------------- 1 file changed, 1 insertion(+), 46 deletions(-) diff --git a/src/RNTupleWriter.cc b/src/RNTupleWriter.cc index 24272c6c2..0ea5bc663 100644 --- a/src/RNTupleWriter.cc +++ b/src/RNTupleWriter.cc @@ -1,8 +1,4 @@ -<<<<<<<< HEAD:src/RNTupleWriter.cc -#include "podio/RNTupleWriter.h" -======== #include "podio/ROOTRNTupleWriter.h" ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc #include "podio/CollectionBase.h" #include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" @@ -20,31 +16,19 @@ namespace podio { -<<<<<<<< HEAD:src/RNTupleWriter.cc -RNTupleWriter::RNTupleWriter(const std::string& filename) : -======== ROOTRNTupleWriter::ROOTRNTupleWriter(const std::string& filename) : ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc m_metadata(ROOT::Experimental::RNTupleModel::Create()), m_file(new TFile(filename.c_str(), "RECREATE", "data file")) { } -<<<<<<<< HEAD:src/RNTupleWriter.cc -RNTupleWriter::~RNTupleWriter() { -======== ROOTRNTupleWriter::~ROOTRNTupleWriter() { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if (!m_finished) { finish(); } } template -<<<<<<<< HEAD:src/RNTupleWriter.cc -std::pair&, std::vector>&> RNTupleWriter::getKeyValueVectors() { -======== std::pair&, std::vector>&> ROOTRNTupleWriter::getKeyValueVectors() { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if constexpr (std::is_same_v) { return {m_intkeys, m_intvalues}; } else if constexpr (std::is_same_v) { @@ -59,11 +43,7 @@ std::pair&, std::vector>&> ROOTRNTupleWr } template -<<<<<<<< HEAD:src/RNTupleWriter.cc -void RNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { -======== void ROOTRNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto [key, value] = getKeyValueVectors(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) entry->BindRawPtr(root_utils::getGPKeyName(), &key); @@ -84,21 +64,12 @@ void ROOTRNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental } } -<<<<<<<< HEAD:src/RNTupleWriter.cc -void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { - writeFrame(frame, category, frame.getAvailableCollections()); -} - -void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, - const std::vector& collsToWrite) { -======== void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { writeFrame(frame, category, frame.getAvailableCollections()); } void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, - const std::vector& collsToWrite) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc + const std::vector& collsToWrite) { auto& catInfo = getCategoryInfo(category); // Use the writer as proxy to check whether this category has been initialized @@ -217,11 +188,7 @@ void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& } std::unique_ptr -<<<<<<<< HEAD:src/RNTupleWriter.cc -RNTupleWriter::createModels(const std::vector& collections) { -======== ROOTRNTupleWriter::createModels(const std::vector& collections) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto model = ROOT::Experimental::RNTupleModel::CreateBare(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) @@ -293,11 +260,7 @@ ROOTRNTupleWriter::createModels(const std::vector& collections) return model; } -<<<<<<<< HEAD:src/RNTupleWriter.cc -RNTupleWriter::CollectionInfo& RNTupleWriter::getCategoryInfo(const std::string& category) { -======== ROOTRNTupleWriter::CollectionInfo& ROOTRNTupleWriter::getCategoryInfo(const std::string& category) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if (auto it = m_categories.find(category); it != m_categories.end()) { return it->second; } @@ -306,11 +269,7 @@ ROOTRNTupleWriter::CollectionInfo& ROOTRNTupleWriter::getCategoryInfo(const std: return it->second; } -<<<<<<<< HEAD:src/RNTupleWriter.cc -void RNTupleWriter::finish() { -======== void ROOTRNTupleWriter::finish() { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc auto podioVersion = podio::version::build_version; auto versionField = m_metadata->MakeField>(root_utils::versionBranchName); @@ -358,11 +317,7 @@ void ROOTRNTupleWriter::finish() { } std::tuple, std::vector> -<<<<<<<< HEAD:src/RNTupleWriter.cc -RNTupleWriter::checkConsistency(const std::vector& collsToWrite, const std::string& category) const { -======== ROOTRNTupleWriter::checkConsistency(const std::vector& collsToWrite, const std::string& category) const { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleWriter.cc if (const auto it = m_categories.find(category); it != m_categories.end()) { return root_utils::getInconsistentColls(it->second.name, collsToWrite); } From 7c7d387db9e474c9ebab95b9ac9929089893192a Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Thu, 25 Jan 2024 09:49:52 +0100 Subject: [PATCH 03/37] Use RNTuple{Reader,Writer} --- include/podio/GenericParameters.h | 8 ++++---- src/RNTupleWriter.cc | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 9e4ef9553..3e1700549 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -20,8 +20,8 @@ using version_type = uint32_t; // from sio/definitions #if PODIO_ENABLE_RNTUPLE namespace podio { -class ROOTRNTupleReader; -class ROOTRNTupleWriter; +class RNTupleReader; +class RNTupleWriter; } // namespace podio #endif @@ -149,8 +149,8 @@ class GenericParameters { friend void readGenericParameters(sio::read_device& device, GenericParameters& parameters, sio::version_type version); #if PODIO_ENABLE_RNTUPLE - friend ROOTRNTupleReader; - friend ROOTRNTupleWriter; + friend RNTupleReader; + friend RNTupleWriter; #endif /// Get a reference to the internal map for a given type diff --git a/src/RNTupleWriter.cc b/src/RNTupleWriter.cc index 0ea5bc663..38231cc18 100644 --- a/src/RNTupleWriter.cc +++ b/src/RNTupleWriter.cc @@ -1,4 +1,4 @@ -#include "podio/ROOTRNTupleWriter.h" +#include "podio/RNTupleWriter.h" #include "podio/CollectionBase.h" #include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" @@ -16,19 +16,19 @@ namespace podio { -ROOTRNTupleWriter::ROOTRNTupleWriter(const std::string& filename) : +RNTupleWriter::RNTupleWriter(const std::string& filename) : m_metadata(ROOT::Experimental::RNTupleModel::Create()), m_file(new TFile(filename.c_str(), "RECREATE", "data file")) { } -ROOTRNTupleWriter::~ROOTRNTupleWriter() { +RNTupleWriter::~RNTupleWriter() { if (!m_finished) { finish(); } } template -std::pair&, std::vector>&> ROOTRNTupleWriter::getKeyValueVectors() { +std::pair&, std::vector>&> RNTupleWriter::getKeyValueVectors() { if constexpr (std::is_same_v) { return {m_intkeys, m_intvalues}; } else if constexpr (std::is_same_v) { @@ -43,7 +43,7 @@ std::pair&, std::vector>&> ROOTRNTupleWr } template -void ROOTRNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { +void RNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { auto [key, value] = getKeyValueVectors(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) entry->BindRawPtr(root_utils::getGPKeyName(), &key); @@ -64,12 +64,12 @@ void ROOTRNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental } } -void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { +void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { writeFrame(frame, category, frame.getAvailableCollections()); } -void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, - const std::vector& collsToWrite) { +void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, + const std::vector& collsToWrite) { auto& catInfo = getCategoryInfo(category); // Use the writer as proxy to check whether this category has been initialized @@ -188,7 +188,7 @@ void ROOTRNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& } std::unique_ptr -ROOTRNTupleWriter::createModels(const std::vector& collections) { +RNTupleWriter::createModels(const std::vector& collections) { auto model = ROOT::Experimental::RNTupleModel::CreateBare(); #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) @@ -260,7 +260,7 @@ ROOTRNTupleWriter::createModels(const std::vector& collections) return model; } -ROOTRNTupleWriter::CollectionInfo& ROOTRNTupleWriter::getCategoryInfo(const std::string& category) { +RNTupleWriter::CollectionInfo& RNTupleWriter::getCategoryInfo(const std::string& category) { if (auto it = m_categories.find(category); it != m_categories.end()) { return it->second; } @@ -269,7 +269,7 @@ ROOTRNTupleWriter::CollectionInfo& ROOTRNTupleWriter::getCategoryInfo(const std: return it->second; } -void ROOTRNTupleWriter::finish() { +void RNTupleWriter::finish() { auto podioVersion = podio::version::build_version; auto versionField = m_metadata->MakeField>(root_utils::versionBranchName); @@ -317,7 +317,7 @@ void ROOTRNTupleWriter::finish() { } std::tuple, std::vector> -ROOTRNTupleWriter::checkConsistency(const std::vector& collsToWrite, const std::string& category) const { +RNTupleWriter::checkConsistency(const std::vector& collsToWrite, const std::string& category) const { if (const auto it = m_categories.find(category); it != m_categories.end()) { return root_utils::getInconsistentColls(it->second.name, collsToWrite); } From 455cffc66330751c97bee5125f3e220a8f89c38d Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 6 Feb 2024 17:47:08 +0100 Subject: [PATCH 04/37] Add test, reader and writer --- include/podio/Reader.h | 96 ++++++++++++++++++++++++++ include/podio/Writer.h | 67 ++++++++++++++++++ src/CMakeLists.txt | 4 ++ src/Reader.cc | 77 +++++++++++++++++++++ src/Writer.cc | 52 ++++++++++++++ tests/root_io/CMakeLists.txt | 4 ++ tests/root_io/read_interface.cpp | 110 ++++++++++++++++++++++++++++++ tests/root_io/write_interface.cpp | 63 +++++++++++++++++ 8 files changed, 473 insertions(+) create mode 100644 include/podio/Reader.h create mode 100644 include/podio/Writer.h create mode 100644 src/Reader.cc create mode 100644 src/Writer.cc create mode 100644 tests/root_io/read_interface.cpp create mode 100644 tests/root_io/write_interface.cpp diff --git a/include/podio/Reader.h b/include/podio/Reader.h new file mode 100644 index 000000000..254a9f1ea --- /dev/null +++ b/include/podio/Reader.h @@ -0,0 +1,96 @@ +#ifndef PODIO_READER_H +#define PODIO_READER_H + +#include "podio/Frame.h" +#include "podio/podioVersion.h" + +namespace podio { + +class Reader { +public: + struct ReaderConcept { + virtual ~ReaderConcept() = default; + + virtual podio::Frame readNextFrame(const std::string& name) = 0; + virtual podio::Frame readFrame(const std::string& name, size_t index) = 0; + virtual size_t getEntries(const std::string& name) = 0; + virtual podio::Frame readNextEvent() = 0; + virtual podio::Frame readEvent(size_t index) = 0; + virtual size_t getEvents() = 0; + virtual podio::version::Version currentFileVersion() const = 0; + }; + + template + struct ReaderModel : public ReaderConcept { + ReaderModel(T* reader) : m_reader(reader) { + } + ReaderModel(const ReaderModel&) = delete; + ReaderModel& operator=(const ReaderModel&) = delete; + + podio::Frame readNextFrame(const std::string& name) override { + auto maybeFrame = m_reader->readNextEntry(name); + if (maybeFrame) { + return std::move(maybeFrame); + } + throw std::runtime_error("Could not read frame (reading beyond bounds?)"); + } + podio::Frame readNextEvent() override { + return readNextFrame(podio::Category::Event); + } + + podio::Frame readFrame(const std::string& name, size_t index) override { + auto maybeFrame = m_reader->readEntry(name, index); + if (maybeFrame) { + return std::move(maybeFrame); + } + throw std::runtime_error("Could not read frame (reading beyond bounds?)"); + } + podio::Frame readEvent(size_t index) override { + return readFrame(podio::Category::Event, index); + } + size_t getEntries(const std::string& name) override { + return m_reader->getEntries(name); + } + size_t getEvents() override { + return getEntries(podio::Category::Event); + } + podio::version::Version currentFileVersion() const override { + return m_reader->currentFileVersion(); + } + T* m_reader{nullptr}; + }; + + std::unique_ptr m_self{nullptr}; + + template + Reader(std::unique_ptr); + + podio::Frame readNextFrame(const std::string& name) { + return m_self->readNextFrame(name); + } + podio::Frame readNextEvent() { + return readNextFrame(podio::Category::Event); + } + podio::Frame readFrame(const std::string& name, size_t index) { + return m_self->readFrame(name, index); + } + podio::Frame readEvent(size_t index) { + return readFrame(podio::Category::Event, index); + } + size_t getEntries(const std::string& name) { + return m_self->getEntries(name); + } + size_t getEvents() { + return getEntries(podio::Category::Event); + } + podio::version::Version currentFileVersion() const { + return m_self->currentFileVersion(); + } +}; + +std::unique_ptr makeReader(const std::string& filename); +std::unique_ptr makeReader(const std::vector& filename); + +} // namespace podio + +#endif // PODIO_READER_H diff --git a/include/podio/Writer.h b/include/podio/Writer.h new file mode 100644 index 000000000..40a466dca --- /dev/null +++ b/include/podio/Writer.h @@ -0,0 +1,67 @@ +#ifndef PODIO_WRITER_H +#define PODIO_WRITER_H + +#include "podio/Frame.h" +#include "podio/podioVersion.h" + +namespace podio { + +class Writer { +public: + struct WriterConcept { + virtual ~WriterConcept() = default; + + virtual void writeFrame(const podio::Frame& frame, const std::string& category) = 0; + virtual void writeFrame(const podio::Frame& frame, const std::string& category, + const std::vector& collections) = 0; + virtual void writeEvent(const podio::Frame& frame) = 0; + virtual void writeEvent(const podio::Frame& frame, const std::vector& collections) = 0; + }; + + template + struct WriterModel : public WriterConcept { + WriterModel(T* writer) : m_writer(writer) { + } + WriterModel(const WriterModel&) = delete; + WriterModel& operator=(const WriterModel&) = delete; + + void writeFrame(const podio::Frame& frame, const std::string& category) override { + return m_writer->writeFrame(frame, category); + } + void writeFrame(const podio::Frame& frame, const std::string& category, + const std::vector& collections) override { + return m_writer->writeFrame(frame, category, collections); + } + void writeEvent(const podio::Frame& frame) override { + return writeFrame(frame, podio::Category::Event); + } + void writeEvent(const podio::Frame& frame, const std::vector& collections) override { + return writeFrame(frame, podio::Category::Event, collections); + } + std::unique_ptr m_writer{nullptr}; + }; + + std::unique_ptr m_self{nullptr}; + + template + Writer(std::unique_ptr); + + void writeFrame(const podio::Frame& frame, const std::string& category) { + return m_self->writeFrame(frame, category); + } + void writeFrame(const podio::Frame& frame, const std::string& category, const std::vector& collections) { + return m_self->writeFrame(frame, category, collections); + } + void writeEvent(const podio::Frame& frame) { + return writeFrame(frame, podio::Category::Event); + } + void writeEvent(const podio::Frame& frame, const std::vector& collections) { + return writeFrame(frame, podio::Category::Event, collections); + } +}; + +std::unique_ptr makeWriter(const std::string& filename, const std::string& type = "default"); + +} // namespace podio + +#endif // PODIO_WRITER_H diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1397f2e35..a2dc67b10 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -80,6 +80,8 @@ SET(root_sources ROOTWriter.cc ROOTReader.cc ROOTLegacyReader.cc + Reader.cc + Writer.cc ) if(ENABLE_RNTUPLE) list(APPEND root_sources @@ -92,6 +94,8 @@ SET(root_headers ${PROJECT_SOURCE_DIR}/include/podio/ROOTReader.h ${PROJECT_SOURCE_DIR}/include/podio/ROOTLegacyReader.h ${PROJECT_SOURCE_DIR}/include/podio/ROOTWriter.h + ${PROJECT_SOURCE_DIR}/include/podio/Reader.h + ${PROJECT_SOURCE_DIR}/include/podio/Writer.h ) if(ENABLE_RNTUPLE) list(APPEND root_headers diff --git a/src/Reader.cc b/src/Reader.cc new file mode 100644 index 000000000..f6c6024b0 --- /dev/null +++ b/src/Reader.cc @@ -0,0 +1,77 @@ +#include "podio/Reader.h" + +#include "podio/ROOTFrameReader.h" +#ifdef PODIO_ENABLE_RNTUPLE + #include "podio/ROOTRNTupleReader.h" +#endif +#ifdef PODIO_ENABLE_SIO + #include "podio/SIOFrameReader.h" +#endif + +#include "TFile.h" +#include "TKey.h" +#include + +namespace podio { + +template +Reader::Reader(std::unique_ptr reader) : m_self(std::make_unique>(reader.release())) { +} + +std::unique_ptr makeReader(const std::string& filename) { + return makeReader(std::vector{filename}); +} + +std::unique_ptr makeReader(const std::vector& filenames) { + + auto suffix = filenames[0].substr(filenames[0].find_last_of(".") + 1); + for (size_t i = 1; i < filenames.size(); ++i) { + if (filenames[i].substr(filenames[i].find_last_of(".") + 1) != suffix) { + std::cout << "ERROR: All files must have the same extension" << std::endl; + return nullptr; + } + } + + std::unique_ptr reader; + + if (suffix == "root") { + // Check only the first file for RNTuples + TFile* file = TFile::Open(filenames[0].c_str()); + bool hasRNTuple = false; + + for (auto key : *file->GetListOfKeys()) { + auto tkey = dynamic_cast(key); + + // if (tkey && tkey->GetClassName() == "ROOT::Experimental::RNTuple") { + if (tkey && std::string(tkey->GetClassName()) == "ROOT::Experimental::RNTuple") { + hasRNTuple = true; + break; + } + } + if (hasRNTuple) { +#ifdef PODIO_ENABLE_RNTUPLE + auto actualReader = std::make_unique(); + actualReader->openFiles(filenames); + reader = std::make_unique(std::move(actualReader)); +#else + throw std::runtime_error("ROOT RNTuple reader not available. Please recompile with ROOT RNTuple support."); +#endif + } else { + auto actualReader = std::make_unique(); + actualReader->openFiles(filenames); + reader = std::make_unique(std::move(actualReader)); + } + } else if (suffix == "sio") { +#ifdef PODIO_ENABLE_SIO + auto actualReader = std::make_unique(); + actualReader->openFiles(filenames); + reader = std::make_unique(std::move(actualReader)); +#else + throw std::runtime_error("SIO reader not available. Please recompile with SIO support."); +#endif + } + + return reader; +} + +} // namespace podio diff --git a/src/Writer.cc b/src/Writer.cc new file mode 100644 index 000000000..2874081d9 --- /dev/null +++ b/src/Writer.cc @@ -0,0 +1,52 @@ +#include "podio/Writer.h" + +#include "podio/ROOTFrameWriter.h" +#ifdef PODIO_ENABLE_RNTUPLE + #include "podio/ROOTRNTupleWriter.h" +#endif +#ifdef PODIO_ENABLE_SIO + #include "podio/SIOFrameWriter.h" +#endif + +#include + +namespace podio { + +template +Writer::Writer(std::unique_ptr reader) : m_self(std::make_unique>(reader.release())) { +} + +std::unique_ptr makeWriter(const std::string& filename, const std::string& type) { + + auto endsWith = [](const std::string& str, const std::string& suffix) { + return str.size() >= suffix.size() && 0 == str.compare(str.size() - suffix.size(), suffix.size(), suffix); + }; + + std::unique_ptr writer; + if ((type == "default" && endsWith(filename, ".root")) || type == "root") { + std::cout << "Calling makeWriter (root)" << std::endl; + auto actualWriter = std::make_unique(filename); + writer = std::make_unique(std::move(actualWriter)); + } else if (type == "rntuple") { +#ifdef PODIO_ENABLE_RNTUPLE + std::cout << "Calling makeWriter (rntuple)" << std::endl; + auto actualWriter = std::make_unique(filename); + writer = std::make_unique(std::move(actualWriter)); +#else + std::cout << "ERROR: RNTuple writer not enabled" << std::endl; +#endif + } else if ((type == "default" && endsWith(filename, ".sio")) || type == "sio") { +#ifdef PODIO_ENABLE_SIO + std::cout << "Calling makeWriter (sio)" << std::endl; + auto actualWriter = std::make_unique(filename); + writer = std::make_unique(std::move(actualWriter)); +#else + std::cout << "ERROR: SIO writer not enabled" << std::endl; +#endif + } else { + std::cout << "ERROR: Unknown writer type " << type << std::endl; + } + return writer; +} + +} // namespace podio diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index 1bc906755..20674dc5b 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -6,6 +6,8 @@ set(root_dependent_tests read_python_frame_root.cpp read_frame_root_multiple.cpp read_and_write_frame_root.cpp + write_interface.cpp + read_interface.cpp ) if(ENABLE_RNTUPLE) set(root_dependent_tests @@ -20,6 +22,8 @@ foreach( sourcefile ${root_dependent_tests} ) CREATE_PODIO_TEST(${sourcefile} "${root_libs}") endforeach() +set_property(TEST read_interface PROPERTY DEPENDS write_interface) + set_tests_properties( read_frame_root read_frame_root_multiple diff --git a/tests/root_io/read_interface.cpp b/tests/root_io/read_interface.cpp new file mode 100644 index 000000000..0886ac32a --- /dev/null +++ b/tests/root_io/read_interface.cpp @@ -0,0 +1,110 @@ +#include "read_frame.h" + +#include "podio/ROOTFrameReader.h" +#include "podio/Reader.h" +#ifdef PODIO_ENABLE_RNTUPLE + #include "podio/ROOTRNTupleReader.h" +#endif + +int read_frames(std::unique_ptr reader) { + + if (reader->getEntries(podio::Category::Event) != 10) { + std::cerr << "Could not read back the number of events correctly. " + << "(expected:" << 10 << ", actual: " << reader->getEntries(podio::Category::Event) << ")" << std::endl; + return 1; + } + + if (reader->getEntries(podio::Category::Event) != reader->getEntries("other_events")) { + std::cerr << "Could not read back the number of events correctly. " + << "(expected:" << 10 << ", actual: " << reader->getEntries("other_events") << ")" << std::endl; + return 1; + } + + // Read the frames in a different order than when writing them here to make + // sure that the writing/reading order does not impose any usage requirements + for (size_t i = 0; i < reader->getEntries(podio::Category::Event); ++i) { + auto frame = reader->readNextFrame(podio::Category::Event); + if (frame.get("emptySubsetColl") == nullptr) { + std::cerr << "Could not retrieve an empty subset collection" << std::endl; + return 1; + } + if (frame.get("emptyCollection") == nullptr) { + std::cerr << "Could not retrieve an empty collection" << std::endl; + return 1; + } + + processEvent(frame, i, reader->currentFileVersion()); + + auto otherFrame = reader->readNextFrame("other_events"); + processEvent(otherFrame, i + 100, reader->currentFileVersion()); + // The other_events category also holds external collections + processExtensions(otherFrame, i + 100, reader->currentFileVersion()); + // As well as a test for the vector members subset category + checkVecMemSubsetColl(otherFrame); + } + + // if (reader->readNextFrame(podio::Category::Event)) { + // std::cerr << "Trying to read more frame data than is present should return a nullptr" << std::endl; + // return 1; + // } + + std::cout << "========================================================\n" << std::endl; + // if (reader->readNextFrame("not_present")) { + // std::cerr << "Trying to read non-existant frame data should return a nullptr" << std::endl; + // return 1; + // } + + // Reading specific (jumping to) entry + { + auto frame = reader->readFrame(podio::Category::Event, 4); + processEvent(frame, 4, reader->currentFileVersion()); + // Reading the next entry after jump, continues from after the jump + auto nextFrame = reader->readNextFrame(podio::Category::Event); + processEvent(nextFrame, 5, reader->currentFileVersion()); + + auto otherFrame = reader->readFrame("other_events", 4); + processEvent(otherFrame, 4 + 100, reader->currentFileVersion()); + if (reader->currentFileVersion() > podio::version::Version{0, 16, 2}) { + processExtensions(otherFrame, 4 + 100, reader->currentFileVersion()); + } + + // Jumping back also works + auto previousFrame = reader->readFrame("other_events", 2); + processEvent(previousFrame, 2 + 100, reader->currentFileVersion()); + if (reader->currentFileVersion() > podio::version::Version{0, 16, 2}) { + processExtensions(previousFrame, 2 + 100, reader->currentFileVersion()); + } + + // Trying to read a Frame that is not present returns a nullptr + // if (reader->readFrame(podio::Category::Event, 10)) { + // std::cerr << "Trying to read a specific entry that does not exist should return a nullptr" << std::endl; + // return 1; + // } + } + + return 0; +} + +int main(int, char**) { + + auto reader = podio::makeReader("example_frame_interface.root"); + if (read_frames(std::move(reader))) { + return 1; + } + +#ifdef PODIO_ENABLE_RNTUPLE + auto readerRNTuple = podio::makeReader("example_frame_rntuple_interface.root"); + if (read_frames(std::move(readerRNTuple))) { + return 1; + } +#endif + +#ifdef PODIO_ENABLE_SIO + auto readerSIO = podio::makeReader("example_frame_sio_interface.sio"); + if (read_frames(std::move(readerSIO))) { + return 1; + } +#endif + + return 0; +} diff --git a/tests/root_io/write_interface.cpp b/tests/root_io/write_interface.cpp new file mode 100644 index 000000000..ec95110c7 --- /dev/null +++ b/tests/root_io/write_interface.cpp @@ -0,0 +1,63 @@ +#include "write_frame.h" + +#include "podio/Writer.h" + +// void write_frames(std::unique_ptr frameWriter) { +// for (int i = 0; i < 10; ++i) { +// auto frame = makeFrame(i); +// frameWriter->writeFrame(frame, podio::Category::Event, collsToWrite); +// } + +// for (int i = 100; i < 110; ++i) { +// auto frame = makeFrame(i); +// frameWriter->writeFrame(frame, "other_events"); +// } + +// frameWriter->finish(); +// } + +void write_frames(std::unique_ptr frameWriter) { + + for (int i = 0; i < 10; ++i) { + auto frame = makeFrame(i); + frameWriter->writeFrame(frame, podio::Category::Event, collsToWrite); + } + + for (int i = 100; i < 110; ++i) { + auto frame = makeFrame(i); + frameWriter->writeFrame(frame, "other_events"); + } +} + +int main(int, char**) { + + auto writer = podio::makeWriter("example_frame_interface.root"); + write_frames(std::move(writer)); + +#ifdef PODIO_ENABLE_RNTUPLE + auto writerRNTuple = podio::makeWriter("example_frame_rntuple_interface.root", "rntuple"); + write_frames(std::move(writerRNTuple)); +#endif + +#ifdef PODIO_ENABLE_SIO + auto writerSIO = podio::makeWriter("example_frame_sio_interface.sio", "sio"); + write_frames(std::move(writerSIO)); +#endif + + // std::unique_ptr frameWriter; + // frameWriter.reset(dynamic_cast(new + // podio::ROOTFrameWriter("example_frame_interface.root"))); + + // write_frames(std::move(frameWriter)); + + // #ifdef PODIO_ENABLE_RNTUPLE + // std::unique_ptr frameWriterRNTuple; + // frameWriterRNTuple.reset( + // dynamic_cast(new + // podio::ROOTRNTupleWriter("example_frame_rntuple_interface.root"))); + + // write_frames(std::move(frameWriterRNTuple)); + // #endif + + return 0; +} From 6588de8b60ef9747104517d9434c69d6ca5c4700 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Jan 2024 21:45:19 +0100 Subject: [PATCH 05/37] Change to RNTuple{Reader,Writer} instead ROOTRNTuple{Reader,Writer} --- src/RNTupleWriter.cc | 2 +- src/Reader.cc | 4 ++-- src/Writer.cc | 4 ++-- tests/root_io/read_interface.cpp | 2 +- tests/root_io/write_interface.cpp | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/RNTupleWriter.cc b/src/RNTupleWriter.cc index 38231cc18..0ced40fe4 100644 --- a/src/RNTupleWriter.cc +++ b/src/RNTupleWriter.cc @@ -69,7 +69,7 @@ void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& cat } void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, - const std::vector& collsToWrite) { + const std::vector& collsToWrite) { auto& catInfo = getCategoryInfo(category); // Use the writer as proxy to check whether this category has been initialized diff --git a/src/Reader.cc b/src/Reader.cc index f6c6024b0..24f2c3448 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -2,7 +2,7 @@ #include "podio/ROOTFrameReader.h" #ifdef PODIO_ENABLE_RNTUPLE - #include "podio/ROOTRNTupleReader.h" + #include "podio/RNTupleReader.h" #endif #ifdef PODIO_ENABLE_SIO #include "podio/SIOFrameReader.h" @@ -50,7 +50,7 @@ std::unique_ptr makeReader(const std::vector& filenames) { } if (hasRNTuple) { #ifdef PODIO_ENABLE_RNTUPLE - auto actualReader = std::make_unique(); + auto actualReader = std::make_unique(); actualReader->openFiles(filenames); reader = std::make_unique(std::move(actualReader)); #else diff --git a/src/Writer.cc b/src/Writer.cc index 2874081d9..e5692541f 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -2,7 +2,7 @@ #include "podio/ROOTFrameWriter.h" #ifdef PODIO_ENABLE_RNTUPLE - #include "podio/ROOTRNTupleWriter.h" + #include "podio/RNTupleWriter.h" #endif #ifdef PODIO_ENABLE_SIO #include "podio/SIOFrameWriter.h" @@ -30,7 +30,7 @@ std::unique_ptr makeWriter(const std::string& filename, const std::strin } else if (type == "rntuple") { #ifdef PODIO_ENABLE_RNTUPLE std::cout << "Calling makeWriter (rntuple)" << std::endl; - auto actualWriter = std::make_unique(filename); + auto actualWriter = std::make_unique(filename); writer = std::make_unique(std::move(actualWriter)); #else std::cout << "ERROR: RNTuple writer not enabled" << std::endl; diff --git a/tests/root_io/read_interface.cpp b/tests/root_io/read_interface.cpp index 0886ac32a..785417cbc 100644 --- a/tests/root_io/read_interface.cpp +++ b/tests/root_io/read_interface.cpp @@ -3,7 +3,7 @@ #include "podio/ROOTFrameReader.h" #include "podio/Reader.h" #ifdef PODIO_ENABLE_RNTUPLE - #include "podio/ROOTRNTupleReader.h" + #include "podio/RNTupleReader.h" #endif int read_frames(std::unique_ptr reader) { diff --git a/tests/root_io/write_interface.cpp b/tests/root_io/write_interface.cpp index ec95110c7..da0db3925 100644 --- a/tests/root_io/write_interface.cpp +++ b/tests/root_io/write_interface.cpp @@ -54,7 +54,7 @@ int main(int, char**) { // std::unique_ptr frameWriterRNTuple; // frameWriterRNTuple.reset( // dynamic_cast(new - // podio::ROOTRNTupleWriter("example_frame_rntuple_interface.root"))); + // podio::RNTupleWriter("example_frame_rntuple_interface.root"))); // write_frames(std::move(frameWriterRNTuple)); // #endif From 7417c483a51e58e34697c8d1919ad845caac2a86 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Jan 2024 22:01:13 +0100 Subject: [PATCH 06/37] Use std::unique_ptr --- include/podio/Reader.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 254a9f1ea..cb8cd0d1d 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -57,7 +57,7 @@ class Reader { podio::version::Version currentFileVersion() const override { return m_reader->currentFileVersion(); } - T* m_reader{nullptr}; + std::unique_ptr m_reader; }; std::unique_ptr m_self{nullptr}; From fe78005603526075e39501b6e21cb1a5e2f372a5 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Jan 2024 22:11:07 +0100 Subject: [PATCH 07/37] Push some changes needed for tests --- include/podio/Reader.h | 25 +++++++++++++++++++++++++ include/podio/Writer.h | 7 +++++++ src/root_selection.xml | 2 ++ tests/root_io/write_interface.cpp | 29 ----------------------------- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index cb8cd0d1d..17d50aa60 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -18,6 +18,9 @@ class Reader { virtual podio::Frame readEvent(size_t index) = 0; virtual size_t getEvents() = 0; virtual podio::version::Version currentFileVersion() const = 0; + virtual std::vector getAvailableCategories() const = 0; + virtual const std::string_view getDatamodelDefinition(const std::string& name) const = 0; + virtual std::vector getAvailableDatamodels() const = 0; }; template @@ -57,6 +60,19 @@ class Reader { podio::version::Version currentFileVersion() const override { return m_reader->currentFileVersion(); } + + std::vector getAvailableCategories() const override { + return m_reader->getAvailableCategories(); + } + + const std::string_view getDatamodelDefinition(const std::string& name) const override { + return m_reader->getDatamodelDefinition(name); + } + + std::vector getAvailableDatamodels() const override { + return m_reader->getAvailableDatamodels(); + } + std::unique_ptr m_reader; }; @@ -86,6 +102,15 @@ class Reader { podio::version::Version currentFileVersion() const { return m_self->currentFileVersion(); } + std::vector getAvailableCategories() const { + return m_self->getAvailableCategories(); + } + const std::string_view getDatamodelDefinition(const std::string& name) const { + return m_self->getDatamodelDefinition(name); + } + std::vector getAvailableDatamodels() const { + return m_self->getAvailableDatamodels(); + } }; std::unique_ptr makeReader(const std::string& filename); diff --git a/include/podio/Writer.h b/include/podio/Writer.h index 40a466dca..d0e33dfa5 100644 --- a/include/podio/Writer.h +++ b/include/podio/Writer.h @@ -16,6 +16,7 @@ class Writer { const std::vector& collections) = 0; virtual void writeEvent(const podio::Frame& frame) = 0; virtual void writeEvent(const podio::Frame& frame, const std::vector& collections) = 0; + virtual void finish() = 0; }; template @@ -38,6 +39,9 @@ class Writer { void writeEvent(const podio::Frame& frame, const std::vector& collections) override { return writeFrame(frame, podio::Category::Event, collections); } + void finish() override { + return m_writer->finish(); + } std::unique_ptr m_writer{nullptr}; }; @@ -58,6 +62,9 @@ class Writer { void writeEvent(const podio::Frame& frame, const std::vector& collections) { return writeFrame(frame, podio::Category::Event, collections); } + void finish() { + return m_self->finish(); + } }; std::unique_ptr makeWriter(const std::string& filename, const std::string& type = "default"); diff --git a/src/root_selection.xml b/src/root_selection.xml index 38db949c0..6753cedee 100644 --- a/src/root_selection.xml +++ b/src/root_selection.xml @@ -5,5 +5,7 @@ + + diff --git a/tests/root_io/write_interface.cpp b/tests/root_io/write_interface.cpp index da0db3925..5b434ff84 100644 --- a/tests/root_io/write_interface.cpp +++ b/tests/root_io/write_interface.cpp @@ -2,20 +2,6 @@ #include "podio/Writer.h" -// void write_frames(std::unique_ptr frameWriter) { -// for (int i = 0; i < 10; ++i) { -// auto frame = makeFrame(i); -// frameWriter->writeFrame(frame, podio::Category::Event, collsToWrite); -// } - -// for (int i = 100; i < 110; ++i) { -// auto frame = makeFrame(i); -// frameWriter->writeFrame(frame, "other_events"); -// } - -// frameWriter->finish(); -// } - void write_frames(std::unique_ptr frameWriter) { for (int i = 0; i < 10; ++i) { @@ -44,20 +30,5 @@ int main(int, char**) { write_frames(std::move(writerSIO)); #endif - // std::unique_ptr frameWriter; - // frameWriter.reset(dynamic_cast(new - // podio::ROOTFrameWriter("example_frame_interface.root"))); - - // write_frames(std::move(frameWriter)); - - // #ifdef PODIO_ENABLE_RNTUPLE - // std::unique_ptr frameWriterRNTuple; - // frameWriterRNTuple.reset( - // dynamic_cast(new - // podio::RNTupleWriter("example_frame_rntuple_interface.root"))); - - // write_frames(std::move(frameWriterRNTuple)); - // #endif - return 0; } From 1aee85ab687f2ac9667dc4398998c4d06d132221 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 23 Jan 2024 08:18:13 +0100 Subject: [PATCH 08/37] Return the reader and writer by value --- include/podio/Reader.h | 4 +-- include/podio/Writer.h | 2 +- src/Reader.cc | 20 +++++------ src/Writer.cc | 31 ++++++++++------- tests/root_io/read_interface.cpp | 58 +++++++++++++++---------------- tests/root_io/write_interface.cpp | 12 +++---- 6 files changed, 66 insertions(+), 61 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 17d50aa60..9f8a631ee 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -113,8 +113,8 @@ class Reader { } }; -std::unique_ptr makeReader(const std::string& filename); -std::unique_ptr makeReader(const std::vector& filename); +Reader makeReader(const std::string& filename); +Reader makeReader(const std::vector& filename); } // namespace podio diff --git a/include/podio/Writer.h b/include/podio/Writer.h index d0e33dfa5..823095a3a 100644 --- a/include/podio/Writer.h +++ b/include/podio/Writer.h @@ -67,7 +67,7 @@ class Writer { } }; -std::unique_ptr makeWriter(const std::string& filename, const std::string& type = "default"); +Writer makeWriter(const std::string& filename, const std::string& type = "default"); } // namespace podio diff --git a/src/Reader.cc b/src/Reader.cc index 24f2c3448..91834132d 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -18,22 +18,19 @@ template Reader::Reader(std::unique_ptr reader) : m_self(std::make_unique>(reader.release())) { } -std::unique_ptr makeReader(const std::string& filename) { +Reader makeReader(const std::string& filename) { return makeReader(std::vector{filename}); } -std::unique_ptr makeReader(const std::vector& filenames) { +Reader makeReader(const std::vector& filenames) { auto suffix = filenames[0].substr(filenames[0].find_last_of(".") + 1); for (size_t i = 1; i < filenames.size(); ++i) { if (filenames[i].substr(filenames[i].find_last_of(".") + 1) != suffix) { - std::cout << "ERROR: All files must have the same extension" << std::endl; - return nullptr; + throw std::runtime_error("All files must have the same extension"); } } - std::unique_ptr reader; - if (suffix == "root") { // Check only the first file for RNTuples TFile* file = TFile::Open(filenames[0].c_str()); @@ -52,26 +49,29 @@ std::unique_ptr makeReader(const std::vector& filenames) { #ifdef PODIO_ENABLE_RNTUPLE auto actualReader = std::make_unique(); actualReader->openFiles(filenames); - reader = std::make_unique(std::move(actualReader)); + Reader reader{std::move(actualReader)}; + return reader; #else throw std::runtime_error("ROOT RNTuple reader not available. Please recompile with ROOT RNTuple support."); #endif } else { auto actualReader = std::make_unique(); actualReader->openFiles(filenames); - reader = std::make_unique(std::move(actualReader)); + Reader reader{std::move(actualReader)}; + return reader; } } else if (suffix == "sio") { #ifdef PODIO_ENABLE_SIO auto actualReader = std::make_unique(); actualReader->openFiles(filenames); - reader = std::make_unique(std::move(actualReader)); + Reader reader{std::move(actualReader)}; + return reader; #else throw std::runtime_error("SIO reader not available. Please recompile with SIO support."); #endif } - return reader; + throw std::runtime_error("Unknown file extension: " + suffix); } } // namespace podio diff --git a/src/Writer.cc b/src/Writer.cc index e5692541f..aaee2569c 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -16,37 +16,42 @@ template Writer::Writer(std::unique_ptr reader) : m_self(std::make_unique>(reader.release())) { } -std::unique_ptr makeWriter(const std::string& filename, const std::string& type) { +Writer makeWriter(const std::string& filename, const std::string& type) { auto endsWith = [](const std::string& str, const std::string& suffix) { return str.size() >= suffix.size() && 0 == str.compare(str.size() - suffix.size(), suffix.size(), suffix); }; - std::unique_ptr writer; - if ((type == "default" && endsWith(filename, ".root")) || type == "root") { + auto lower = [](std::string str) { + std::transform(str.begin(), str.end(), str.begin(), [](unsigned char c) { return std::tolower(c); }); + return str; + }; + + if ((type == "default" && endsWith(filename, ".root")) || lower(type) == "root") { std::cout << "Calling makeWriter (root)" << std::endl; auto actualWriter = std::make_unique(filename); - writer = std::make_unique(std::move(actualWriter)); - } else if (type == "rntuple") { + Writer writer{std::move(actualWriter)}; + return writer; + } + if (lower(type) == "rntuple") { #ifdef PODIO_ENABLE_RNTUPLE std::cout << "Calling makeWriter (rntuple)" << std::endl; auto actualWriter = std::make_unique(filename); - writer = std::make_unique(std::move(actualWriter)); + Writer writer{std::move(actualWriter)}; #else - std::cout << "ERROR: RNTuple writer not enabled" << std::endl; + throw std::runtime_error("ROOT RNTuple writer not available. Please recompile with ROOT RNTuple support."); #endif - } else if ((type == "default" && endsWith(filename, ".sio")) || type == "sio") { + } + if ((type == "default" && endsWith(filename, ".sio")) || lower(type) == "sio") { #ifdef PODIO_ENABLE_SIO std::cout << "Calling makeWriter (sio)" << std::endl; auto actualWriter = std::make_unique(filename); - writer = std::make_unique(std::move(actualWriter)); + Writer writer{std::move(actualWriter)}; #else - std::cout << "ERROR: SIO writer not enabled" << std::endl; + throw std::runtime_error("SIO writer not available. Please recompile with SIO support."); #endif - } else { - std::cout << "ERROR: Unknown writer type " << type << std::endl; } - return writer; + throw std::runtime_error("Unknown file type"); } } // namespace podio diff --git a/tests/root_io/read_interface.cpp b/tests/root_io/read_interface.cpp index 785417cbc..b4a32778b 100644 --- a/tests/root_io/read_interface.cpp +++ b/tests/root_io/read_interface.cpp @@ -6,24 +6,24 @@ #include "podio/RNTupleReader.h" #endif -int read_frames(std::unique_ptr reader) { +int read_frames(podio::Reader& reader) { - if (reader->getEntries(podio::Category::Event) != 10) { + if (reader.getEntries(podio::Category::Event) != 10) { std::cerr << "Could not read back the number of events correctly. " - << "(expected:" << 10 << ", actual: " << reader->getEntries(podio::Category::Event) << ")" << std::endl; + << "(expected:" << 10 << ", actual: " << reader.getEntries(podio::Category::Event) << ")" << std::endl; return 1; } - if (reader->getEntries(podio::Category::Event) != reader->getEntries("other_events")) { + if (reader.getEntries(podio::Category::Event) != reader.getEntries("other_events")) { std::cerr << "Could not read back the number of events correctly. " - << "(expected:" << 10 << ", actual: " << reader->getEntries("other_events") << ")" << std::endl; + << "(expected:" << 10 << ", actual: " << reader.getEntries("other_events") << ")" << std::endl; return 1; } // Read the frames in a different order than when writing them here to make // sure that the writing/reading order does not impose any usage requirements - for (size_t i = 0; i < reader->getEntries(podio::Category::Event); ++i) { - auto frame = reader->readNextFrame(podio::Category::Event); + for (size_t i = 0; i < reader.getEntries(podio::Category::Event); ++i) { + auto frame = reader.readNextFrame(podio::Category::Event); if (frame.get("emptySubsetColl") == nullptr) { std::cerr << "Could not retrieve an empty subset collection" << std::endl; return 1; @@ -33,50 +33,50 @@ int read_frames(std::unique_ptr reader) { return 1; } - processEvent(frame, i, reader->currentFileVersion()); + processEvent(frame, i, reader.currentFileVersion()); - auto otherFrame = reader->readNextFrame("other_events"); - processEvent(otherFrame, i + 100, reader->currentFileVersion()); + auto otherFrame = reader.readNextFrame("other_events"); + processEvent(otherFrame, i + 100, reader.currentFileVersion()); // The other_events category also holds external collections - processExtensions(otherFrame, i + 100, reader->currentFileVersion()); + processExtensions(otherFrame, i + 100, reader.currentFileVersion()); // As well as a test for the vector members subset category checkVecMemSubsetColl(otherFrame); } - // if (reader->readNextFrame(podio::Category::Event)) { + // if (reader.readNextFrame(podio::Category::Event)) { // std::cerr << "Trying to read more frame data than is present should return a nullptr" << std::endl; // return 1; // } std::cout << "========================================================\n" << std::endl; - // if (reader->readNextFrame("not_present")) { + // if (reader.readNextFrame("not_present")) { // std::cerr << "Trying to read non-existant frame data should return a nullptr" << std::endl; // return 1; // } // Reading specific (jumping to) entry { - auto frame = reader->readFrame(podio::Category::Event, 4); - processEvent(frame, 4, reader->currentFileVersion()); + auto frame = reader.readFrame(podio::Category::Event, 4); + processEvent(frame, 4, reader.currentFileVersion()); // Reading the next entry after jump, continues from after the jump - auto nextFrame = reader->readNextFrame(podio::Category::Event); - processEvent(nextFrame, 5, reader->currentFileVersion()); + auto nextFrame = reader.readNextFrame(podio::Category::Event); + processEvent(nextFrame, 5, reader.currentFileVersion()); - auto otherFrame = reader->readFrame("other_events", 4); - processEvent(otherFrame, 4 + 100, reader->currentFileVersion()); - if (reader->currentFileVersion() > podio::version::Version{0, 16, 2}) { - processExtensions(otherFrame, 4 + 100, reader->currentFileVersion()); + auto otherFrame = reader.readFrame("other_events", 4); + processEvent(otherFrame, 4 + 100, reader.currentFileVersion()); + if (reader.currentFileVersion() > podio::version::Version{0, 16, 2}) { + processExtensions(otherFrame, 4 + 100, reader.currentFileVersion()); } // Jumping back also works - auto previousFrame = reader->readFrame("other_events", 2); - processEvent(previousFrame, 2 + 100, reader->currentFileVersion()); - if (reader->currentFileVersion() > podio::version::Version{0, 16, 2}) { - processExtensions(previousFrame, 2 + 100, reader->currentFileVersion()); + auto previousFrame = reader.readFrame("other_events", 2); + processEvent(previousFrame, 2 + 100, reader.currentFileVersion()); + if (reader.currentFileVersion() > podio::version::Version{0, 16, 2}) { + processExtensions(previousFrame, 2 + 100, reader.currentFileVersion()); } // Trying to read a Frame that is not present returns a nullptr - // if (reader->readFrame(podio::Category::Event, 10)) { + // if (reader.readFrame(podio::Category::Event, 10)) { // std::cerr << "Trying to read a specific entry that does not exist should return a nullptr" << std::endl; // return 1; // } @@ -88,20 +88,20 @@ int read_frames(std::unique_ptr reader) { int main(int, char**) { auto reader = podio::makeReader("example_frame_interface.root"); - if (read_frames(std::move(reader))) { + if (read_frames(reader)) { return 1; } #ifdef PODIO_ENABLE_RNTUPLE auto readerRNTuple = podio::makeReader("example_frame_rntuple_interface.root"); - if (read_frames(std::move(readerRNTuple))) { + if (read_frames(readerRNTuple)) { return 1; } #endif #ifdef PODIO_ENABLE_SIO auto readerSIO = podio::makeReader("example_frame_sio_interface.sio"); - if (read_frames(std::move(readerSIO))) { + if (read_frames(readerSIO)) { return 1; } #endif diff --git a/tests/root_io/write_interface.cpp b/tests/root_io/write_interface.cpp index 5b434ff84..6e55930bb 100644 --- a/tests/root_io/write_interface.cpp +++ b/tests/root_io/write_interface.cpp @@ -2,32 +2,32 @@ #include "podio/Writer.h" -void write_frames(std::unique_ptr frameWriter) { +void write_frames(podio::Writer& frameWriter) { for (int i = 0; i < 10; ++i) { auto frame = makeFrame(i); - frameWriter->writeFrame(frame, podio::Category::Event, collsToWrite); + frameWriter.writeFrame(frame, podio::Category::Event, collsToWrite); } for (int i = 100; i < 110; ++i) { auto frame = makeFrame(i); - frameWriter->writeFrame(frame, "other_events"); + frameWriter.writeFrame(frame, "other_events"); } } int main(int, char**) { auto writer = podio::makeWriter("example_frame_interface.root"); - write_frames(std::move(writer)); + write_frames(writer); #ifdef PODIO_ENABLE_RNTUPLE auto writerRNTuple = podio::makeWriter("example_frame_rntuple_interface.root", "rntuple"); - write_frames(std::move(writerRNTuple)); + write_frames(writerRNTuple); #endif #ifdef PODIO_ENABLE_SIO auto writerSIO = podio::makeWriter("example_frame_sio_interface.sio", "sio"); - write_frames(std::move(writerSIO)); + write_frames(writerSIO); #endif return 0; From fd70b54a2dfa5d3b492f89a9e3fd09cb45f8ee5d Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 23 Jan 2024 08:30:26 +0100 Subject: [PATCH 09/37] Add mising returns --- src/Writer.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Writer.cc b/src/Writer.cc index aaee2569c..2a2321a48 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -28,25 +28,24 @@ Writer makeWriter(const std::string& filename, const std::string& type) { }; if ((type == "default" && endsWith(filename, ".root")) || lower(type) == "root") { - std::cout << "Calling makeWriter (root)" << std::endl; auto actualWriter = std::make_unique(filename); Writer writer{std::move(actualWriter)}; return writer; } if (lower(type) == "rntuple") { #ifdef PODIO_ENABLE_RNTUPLE - std::cout << "Calling makeWriter (rntuple)" << std::endl; auto actualWriter = std::make_unique(filename); Writer writer{std::move(actualWriter)}; + return writer; #else throw std::runtime_error("ROOT RNTuple writer not available. Please recompile with ROOT RNTuple support."); #endif } if ((type == "default" && endsWith(filename, ".sio")) || lower(type) == "sio") { #ifdef PODIO_ENABLE_SIO - std::cout << "Calling makeWriter (sio)" << std::endl; auto actualWriter = std::make_unique(filename); Writer writer{std::move(actualWriter)}; + return writer; #else throw std::runtime_error("SIO writer not available. Please recompile with SIO support."); #endif From 7d84e151078b98fb56163c1c017d0b8074e52fdd Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 6 Feb 2024 18:04:38 +0100 Subject: [PATCH 10/37] Add python bindings for the reader and writer by reusing the current Reader and Writer --- python/podio/frame_iterator.py | 34 ++++++++++++++++++++++++++++------ python/podio/root_io.py | 15 ++++++++++----- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/python/podio/frame_iterator.py b/python/podio/frame_iterator.py index 1fa97d828..eb6bc04e8 100644 --- a/python/podio/frame_iterator.py +++ b/python/podio/frame_iterator.py @@ -4,6 +4,7 @@ # pylint: disable-next=import-error # gbl is a dynamic module from cppyy from cppyy.gbl import std from podio.frame import Frame +import cppyy class FrameCategoryIterator: @@ -26,12 +27,15 @@ def __iter__(self): return self def __next__(self): - """Get the next available Frame or stop.""" - frame_data = self._reader.readNextEntry(self._category) - if frame_data: - return Frame(std.move(frame_data)) - - raise StopIteration + """Get the next available Frame or stop.""" + try: + frame_data = self._reader.readNextFrame(self._category) + except AttributeError: + frame_data = self._reader.readNextEntry(self._category) + except std.runtime_error: + raise StopIteration + if frame_data: + return Frame(std.move(frame_data)) def __len__(self): """Get the number of available Frames for the passed category.""" @@ -46,6 +50,24 @@ def __getitem__(self, entry): # Handle python negative indexing to start from the end if entry < 0: entry = self._reader.getEntries(self._category) + entry + try: + frame_data = self._reader.readFrame(self._category, entry) + except AttributeError: + frame_data = self._reader.readEntry(self._category, entry) + except std.bad_function_call: + print('Error: Unable to read an entry of the input file. This can happen when the ' + 'ROOT model dictionaries are not in LD_LIBRARY_PATH. Make sure that LD_LIBRARY_PATH ' + 'points to the library folder of the installation of podio and also to the library ' + 'folder with your data model\n') + raise + except std.runtime_error: + raise IndexError('Unable to read frame, you might be trying to read beyond bounds or a ' + 'non-existent category') + + if frame_data: + if isinstance(frame_data, cppyy.gbl.podio.ROOTFrameData): + return Frame(std.move(frame_data)) + return Frame(frame_data) if entry < 0: # If we are below 0 now, we do not have enough entries to serve the request diff --git a/python/podio/root_io.py b/python/podio/root_io.py index 830eb62e7..df29b1e50 100644 --- a/python/podio/root_io.py +++ b/python/podio/root_io.py @@ -1,9 +1,9 @@ #!/usr/bin/env python3 """Python module for reading root files containing podio Frames""" -from ROOT import gSystem - -gSystem.Load("libpodioRootIO") # noqa: E402 +from ROOT import gSystem, gInterpreter +gSystem.Load('libpodioRootIO') # noqa: E402 +gInterpreter.LoadFile("podio/Reader.h") # noqa: E402 from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position # noqa: E402 @@ -22,12 +22,16 @@ def __init__(self, filenames): if isinstance(filenames, str): filenames = (filenames,) - self._reader = podio.ROOTReader() + self._reader = podio.makeReader(filenames) + # self._reader = podio.ROOTReader() self._reader.openFiles(filenames) super().__init__() + super().__init__() + + class RNTupleReader(BaseReaderMixin): """Reader class for reading podio RNTuple root files.""" @@ -78,10 +82,11 @@ def __init__(self, filename): Args: filename (str): The name of the output file """ - self._writer = podio.ROOTWriter(filename) + self._writer = podio.makeWriter(filename) super().__init__() + class RNTupleWriter(BaseWriterMixin): """Writer class for writing podio root files""" From c60a53c33597178c2a5ac653d276861dd5bf0f2e Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 23 Jan 2024 08:54:55 +0100 Subject: [PATCH 11/37] Remove std::move --- include/podio/Reader.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 9f8a631ee..72bd7c68d 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -33,7 +33,7 @@ class Reader { podio::Frame readNextFrame(const std::string& name) override { auto maybeFrame = m_reader->readNextEntry(name); if (maybeFrame) { - return std::move(maybeFrame); + return maybeFrame; } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } @@ -44,7 +44,7 @@ class Reader { podio::Frame readFrame(const std::string& name, size_t index) override { auto maybeFrame = m_reader->readEntry(name, index); if (maybeFrame) { - return std::move(maybeFrame); + return maybeFrame; } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } From da4b569e4557e30982a59d6be60a7f8e26e5e46d Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 23 Jan 2024 10:30:52 +0100 Subject: [PATCH 12/37] Fix #ifs --- src/Reader.cc | 8 ++++---- src/Writer.cc | 8 ++++---- tests/root_io/read_interface.cpp | 6 +++--- tests/root_io/write_interface.cpp | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Reader.cc b/src/Reader.cc index 91834132d..c7dee3cfa 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -1,10 +1,10 @@ #include "podio/Reader.h" #include "podio/ROOTFrameReader.h" -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE #include "podio/RNTupleReader.h" #endif -#ifdef PODIO_ENABLE_SIO +#if PODIO_ENABLE_SIO #include "podio/SIOFrameReader.h" #endif @@ -46,7 +46,7 @@ Reader makeReader(const std::vector& filenames) { } } if (hasRNTuple) { -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE auto actualReader = std::make_unique(); actualReader->openFiles(filenames); Reader reader{std::move(actualReader)}; @@ -61,7 +61,7 @@ Reader makeReader(const std::vector& filenames) { return reader; } } else if (suffix == "sio") { -#ifdef PODIO_ENABLE_SIO +#if PODIO_ENABLE_SIO auto actualReader = std::make_unique(); actualReader->openFiles(filenames); Reader reader{std::move(actualReader)}; diff --git a/src/Writer.cc b/src/Writer.cc index 2a2321a48..db84eded9 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -1,10 +1,10 @@ #include "podio/Writer.h" #include "podio/ROOTFrameWriter.h" -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE #include "podio/RNTupleWriter.h" #endif -#ifdef PODIO_ENABLE_SIO +#if PODIO_ENABLE_SIO #include "podio/SIOFrameWriter.h" #endif @@ -33,7 +33,7 @@ Writer makeWriter(const std::string& filename, const std::string& type) { return writer; } if (lower(type) == "rntuple") { -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE auto actualWriter = std::make_unique(filename); Writer writer{std::move(actualWriter)}; return writer; @@ -42,7 +42,7 @@ Writer makeWriter(const std::string& filename, const std::string& type) { #endif } if ((type == "default" && endsWith(filename, ".sio")) || lower(type) == "sio") { -#ifdef PODIO_ENABLE_SIO +#if PODIO_ENABLE_SIO auto actualWriter = std::make_unique(filename); Writer writer{std::move(actualWriter)}; return writer; diff --git a/tests/root_io/read_interface.cpp b/tests/root_io/read_interface.cpp index b4a32778b..00b5c0449 100644 --- a/tests/root_io/read_interface.cpp +++ b/tests/root_io/read_interface.cpp @@ -2,7 +2,7 @@ #include "podio/ROOTFrameReader.h" #include "podio/Reader.h" -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE #include "podio/RNTupleReader.h" #endif @@ -92,14 +92,14 @@ int main(int, char**) { return 1; } -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE auto readerRNTuple = podio::makeReader("example_frame_rntuple_interface.root"); if (read_frames(readerRNTuple)) { return 1; } #endif -#ifdef PODIO_ENABLE_SIO +#if PODIO_ENABLE_SIO auto readerSIO = podio::makeReader("example_frame_sio_interface.sio"); if (read_frames(readerSIO)) { return 1; diff --git a/tests/root_io/write_interface.cpp b/tests/root_io/write_interface.cpp index 6e55930bb..b886e5b95 100644 --- a/tests/root_io/write_interface.cpp +++ b/tests/root_io/write_interface.cpp @@ -20,12 +20,12 @@ int main(int, char**) { auto writer = podio::makeWriter("example_frame_interface.root"); write_frames(writer); -#ifdef PODIO_ENABLE_RNTUPLE +#if PODIO_ENABLE_RNTUPLE auto writerRNTuple = podio::makeWriter("example_frame_rntuple_interface.root", "rntuple"); write_frames(writerRNTuple); #endif -#ifdef PODIO_ENABLE_SIO +#if PODIO_ENABLE_SIO auto writerSIO = podio::makeWriter("example_frame_sio_interface.sio", "sio"); write_frames(writerSIO); #endif From d3262d49e6a607c80668bc214b1f999e727016d1 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 6 Feb 2024 18:11:34 +0100 Subject: [PATCH 13/37] Fix pre-commit --- python/podio/frame_iterator.py | 23 ++++++++++++++++------- python/podio/root_io.py | 5 +---- src/RNTupleWriter.cc | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/python/podio/frame_iterator.py b/python/podio/frame_iterator.py index eb6bc04e8..45fd2c02e 100644 --- a/python/podio/frame_iterator.py +++ b/python/podio/frame_iterator.py @@ -3,8 +3,8 @@ # pylint: disable-next=import-error # gbl is a dynamic module from cppyy from cppyy.gbl import std -from podio.frame import Frame import cppyy +from podio.frame import Frame class FrameCategoryIterator: @@ -32,8 +32,8 @@ def __next__(self): frame_data = self._reader.readNextFrame(self._category) except AttributeError: frame_data = self._reader.readNextEntry(self._category) - except std.runtime_error: - raise StopIteration + except std.runtime_error as error: + raise StopIteration from error if frame_data: return Frame(std.move(frame_data)) @@ -64,10 +64,19 @@ def __getitem__(self, entry): raise IndexError('Unable to read frame, you might be trying to read beyond bounds or a ' 'non-existent category') - if frame_data: - if isinstance(frame_data, cppyy.gbl.podio.ROOTFrameData): - return Frame(std.move(frame_data)) - return Frame(frame_data) + try: + frame_data = self._reader.readFrame(self._category, entry) + except AttributeError: + frame_data = self._reader.readEntry(self._category, entry) + except std.bad_function_call: + print('Error: Unable to read an entry of the input file. This can happen when the ' + 'ROOT model dictionaries are not in LD_LIBRARY_PATH. Make sure that LD_LIBRARY_PATH ' + 'points to the library folder of the installation of podio and also to the library ' + 'folder with your data model\n') + raise + except std.runtime_error as error: + raise IndexError('Unable to read frame, you might be trying to read beyond bounds or a ' + 'non-existent category') from error if entry < 0: # If we are below 0 now, we do not have enough entries to serve the request diff --git a/python/podio/root_io.py b/python/podio/root_io.py index df29b1e50..8228bf94f 100644 --- a/python/podio/root_io.py +++ b/python/podio/root_io.py @@ -3,7 +3,7 @@ from ROOT import gSystem, gInterpreter gSystem.Load('libpodioRootIO') # noqa: E402 -gInterpreter.LoadFile("podio/Reader.h") # noqa: E402 +gInterpreter.LoadFile("podio/Reader.h") # noqa: E402 from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position # noqa: E402 @@ -29,9 +29,6 @@ def __init__(self, filenames): super().__init__() - super().__init__() - - class RNTupleReader(BaseReaderMixin): """Reader class for reading podio RNTuple root files.""" diff --git a/src/RNTupleWriter.cc b/src/RNTupleWriter.cc index 0ced40fe4..38231cc18 100644 --- a/src/RNTupleWriter.cc +++ b/src/RNTupleWriter.cc @@ -69,7 +69,7 @@ void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& cat } void RNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, - const std::vector& collsToWrite) { + const std::vector& collsToWrite) { auto& catInfo = getCategoryInfo(category); // Use the writer as proxy to check whether this category has been initialized From 994c7f6af8c512d03cc1ac799c64eee4983f7631 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Thu, 25 Jan 2024 11:36:03 +0100 Subject: [PATCH 14/37] Exclude the new tests from running with sanitizers --- include/podio/Writer.h | 16 ++++++++++++++-- src/Writer.cc | 16 +++------------- tests/CTestCustom.cmake | 3 +++ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/include/podio/Writer.h b/include/podio/Writer.h index 823095a3a..f39f08d97 100644 --- a/include/podio/Writer.h +++ b/include/podio/Writer.h @@ -21,10 +21,14 @@ class Writer { template struct WriterModel : public WriterConcept { - WriterModel(T* writer) : m_writer(writer) { + WriterModel(std::unique_ptr writer) : m_writer(std::move(writer)) { } WriterModel(const WriterModel&) = delete; WriterModel& operator=(const WriterModel&) = delete; + WriterModel(WriterModel&&) = default; + WriterModel& operator=(WriterModel&&) = default; + + ~WriterModel() = default; void writeFrame(const podio::Frame& frame, const std::string& category) override { return m_writer->writeFrame(frame, category); @@ -48,7 +52,15 @@ class Writer { std::unique_ptr m_self{nullptr}; template - Writer(std::unique_ptr); + Writer(std::unique_ptr reader) : m_self(std::make_unique>(std::move(reader))) { + } + + Writer(const Writer&) = delete; + Writer& operator=(const Writer&) = delete; + Writer(Writer&&) = default; + Writer& operator=(Writer&&) = default; + + ~Writer() = default; void writeFrame(const podio::Frame& frame, const std::string& category) { return m_self->writeFrame(frame, category); diff --git a/src/Writer.cc b/src/Writer.cc index db84eded9..a579b275f 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -12,10 +12,6 @@ namespace podio { -template -Writer::Writer(std::unique_ptr reader) : m_self(std::make_unique>(reader.release())) { -} - Writer makeWriter(const std::string& filename, const std::string& type) { auto endsWith = [](const std::string& str, const std::string& suffix) { @@ -28,24 +24,18 @@ Writer makeWriter(const std::string& filename, const std::string& type) { }; if ((type == "default" && endsWith(filename, ".root")) || lower(type) == "root") { - auto actualWriter = std::make_unique(filename); - Writer writer{std::move(actualWriter)}; - return writer; + return Writer{std::make_unique(filename)}; } if (lower(type) == "rntuple") { #if PODIO_ENABLE_RNTUPLE - auto actualWriter = std::make_unique(filename); - Writer writer{std::move(actualWriter)}; - return writer; + return Writer{std::make_unique(filename)}; #else throw std::runtime_error("ROOT RNTuple writer not available. Please recompile with ROOT RNTuple support."); #endif } if ((type == "default" && endsWith(filename, ".sio")) || lower(type) == "sio") { #if PODIO_ENABLE_SIO - auto actualWriter = std::make_unique(filename); - Writer writer{std::move(actualWriter)}; - return writer; + return Writer{std::make_unique(filename)}; #else throw std::runtime_error("SIO writer not available. Please recompile with SIO support."); #endif diff --git a/tests/CTestCustom.cmake b/tests/CTestCustom.cmake index 396c32b78..911a3fe25 100644 --- a/tests/CTestCustom.cmake +++ b/tests/CTestCustom.cmake @@ -22,6 +22,9 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ write_frame_root read_frame_root + write_interface + read_interface + write_python_frame_sio read_python_frame_sio From bebfd07272d506fc6f4171cf55d2277cc5f0af5c Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Thu, 25 Jan 2024 13:19:20 +0100 Subject: [PATCH 15/37] Fix the RNTupleWriter header --- include/podio/RNTupleWriter.h | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/include/podio/RNTupleWriter.h b/include/podio/RNTupleWriter.h index 8820ba4d9..c504aa558 100644 --- a/include/podio/RNTupleWriter.h +++ b/include/podio/RNTupleWriter.h @@ -22,7 +22,6 @@ namespace podio { -<<<<<<<< HEAD:include/podio/RNTupleWriter.h class RNTupleWriter { public: RNTupleWriter(const std::string& filename); @@ -30,15 +29,6 @@ class RNTupleWriter { RNTupleWriter(const RNTupleWriter&) = delete; RNTupleWriter& operator=(const RNTupleWriter&) = delete; -======== -class ROOTRNTupleWriter { -public: - ROOTRNTupleWriter(const std::string& filename); - ~ROOTRNTupleWriter(); - - ROOTRNTupleWriter(const ROOTRNTupleWriter&) = delete; - ROOTRNTupleWriter& operator=(const ROOTRNTupleWriter&) = delete; ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleWriter.h template void fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry); @@ -101,8 +91,4 @@ class ROOTRNTupleWriter { } // namespace podio -<<<<<<<< HEAD:include/podio/RNTupleWriter.h #endif // PODIO_RNTUPLEWRITER_H -======== -#endif // PODIO_ROOTRNTUPLEWRITER_H ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleWriter.h From df2fe4546c7f49a0122684c291cfe2b66502432d Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Thu, 25 Jan 2024 13:19:37 +0100 Subject: [PATCH 16/37] Add another fix --- include/podio/RNTupleWriter.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/include/podio/RNTupleWriter.h b/include/podio/RNTupleWriter.h index c504aa558..2583e56db 100644 --- a/include/podio/RNTupleWriter.h +++ b/include/podio/RNTupleWriter.h @@ -1,10 +1,5 @@ -<<<<<<<< HEAD:include/podio/RNTupleWriter.h #ifndef PODIO_RNTUPLEWRITER_H #define PODIO_RNTUPLEWRITER_H -======== -#ifndef PODIO_ROOTRNTUPLEWRITER_H -#define PODIO_ROOTRNTUPLEWRITER_H ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleWriter.h #include "podio/CollectionBase.h" #include "podio/Frame.h" From 2b43a3daa3f7e8091b6062c8f5a466de1bb8800b Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Thu, 25 Jan 2024 13:21:08 +0100 Subject: [PATCH 17/37] Fix also the RNTupleReader --- include/podio/RNTupleReader.h | 16 -------------- src/RNTupleReader.cc | 40 ----------------------------------- 2 files changed, 56 deletions(-) diff --git a/include/podio/RNTupleReader.h b/include/podio/RNTupleReader.h index 05cc0b319..ef13aec27 100644 --- a/include/podio/RNTupleReader.h +++ b/include/podio/RNTupleReader.h @@ -1,10 +1,5 @@ -<<<<<<<< HEAD:include/podio/RNTupleReader.h #ifndef PODIO_RNTUPLEREADER_H #define PODIO_RNTUPLEREADER_H -======== -#ifndef PODIO_ROOTRNTUPLEREADER_H -#define PODIO_ROOTRNTUPLEREADER_H ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleReader.h #include "podio/CollectionBranches.h" #include "podio/ICollectionProvider.h" @@ -27,7 +22,6 @@ namespace podio { This class has the function to read available data from disk and to prepare collections and buffers. **/ -<<<<<<<< HEAD:include/podio/RNTupleReader.h class RNTupleReader { public: @@ -36,16 +30,6 @@ class RNTupleReader { RNTupleReader(const RNTupleReader&) = delete; RNTupleReader& operator=(const RNTupleReader&) = delete; -======== -class ROOTRNTupleReader { - -public: - ROOTRNTupleReader() = default; - ~ROOTRNTupleReader() = default; - - ROOTRNTupleReader(const ROOTRNTupleReader&) = delete; - ROOTRNTupleReader& operator=(const ROOTRNTupleReader&) = delete; ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):include/podio/ROOTRNTupleReader.h void openFile(const std::string& filename); void openFiles(const std::vector& filename); diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 706e3798d..a372f6c2f 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -1,8 +1,4 @@ -<<<<<<<< HEAD:src/RNTupleReader.cc #include "podio/RNTupleReader.h" -======== -#include "podio/ROOTRNTupleReader.h" ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc #include "podio/CollectionBase.h" #include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" @@ -18,11 +14,7 @@ namespace podio { template -<<<<<<<< HEAD:src/RNTupleReader.cc void RNTupleReader::readParams(const std::string& name, unsigned entNum, GenericParameters& params) { -======== -void ROOTRNTupleReader::readParams(const std::string& name, unsigned entNum, GenericParameters& params) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc auto keyView = m_readers[name][0]->GetView>(root_utils::getGPKeyName()); auto valueView = m_readers[name][0]->GetView>>(root_utils::getGPValueName()); @@ -31,11 +23,7 @@ void ROOTRNTupleReader::readParams(const std::string& name, unsigned entNum, Gen } } -<<<<<<<< HEAD:src/RNTupleReader.cc GenericParameters RNTupleReader::readEventMetaData(const std::string& name, unsigned entNum) { -======== -GenericParameters ROOTRNTupleReader::readEventMetaData(const std::string& name, unsigned entNum) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc GenericParameters params; readParams(name, entNum, params); @@ -46,11 +34,7 @@ GenericParameters ROOTRNTupleReader::readEventMetaData(const std::string& name, return params; } -<<<<<<<< HEAD:src/RNTupleReader.cc bool RNTupleReader::initCategory(const std::string& category) { -======== -bool ROOTRNTupleReader::initCategory(const std::string& category) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc if (std::find(m_availableCategories.begin(), m_availableCategories.end(), category) == m_availableCategories.end()) { return false; } @@ -81,19 +65,11 @@ bool ROOTRNTupleReader::initCategory(const std::string& category) { return true; } -<<<<<<<< HEAD:src/RNTupleReader.cc void RNTupleReader::openFile(const std::string& filename) { openFiles({filename}); } void RNTupleReader::openFiles(const std::vector& filenames) { -======== -void ROOTRNTupleReader::openFile(const std::string& filename) { - openFiles({filename}); -} - -void ROOTRNTupleReader::openFiles(const std::vector& filenames) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc m_filenames.insert(m_filenames.end(), filenames.begin(), filenames.end()); for (auto& filename : filenames) { @@ -117,11 +93,7 @@ void ROOTRNTupleReader::openFiles(const std::vector& filenames) { m_availableCategories = availableCategoriesField(0); } -<<<<<<<< HEAD:src/RNTupleReader.cc unsigned RNTupleReader::getEntries(const std::string& name) { -======== -unsigned ROOTRNTupleReader::getEntries(const std::string& name) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc if (m_readers.find(name) == m_readers.end()) { for (auto& filename : m_filenames) { try { @@ -136,11 +108,7 @@ unsigned ROOTRNTupleReader::getEntries(const std::string& name) { return m_totalEntries[name]; } -<<<<<<<< HEAD:src/RNTupleReader.cc std::vector RNTupleReader::getAvailableCategories() const { -======== -std::vector ROOTRNTupleReader::getAvailableCategories() const { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc std::vector cats; cats.reserve(m_availableCategories.size()); for (const auto& cat : m_availableCategories) { @@ -149,19 +117,11 @@ std::vector ROOTRNTupleReader::getAvailableCategories() const return cats; } -<<<<<<<< HEAD:src/RNTupleReader.cc std::unique_ptr RNTupleReader::readNextEntry(const std::string& name) { return readEntry(name, m_entries[name]); } std::unique_ptr RNTupleReader::readEntry(const std::string& category, const unsigned entNum) { -======== -std::unique_ptr ROOTRNTupleReader::readNextEntry(const std::string& name) { - return readEntry(name, m_entries[name]); -} - -std::unique_ptr ROOTRNTupleReader::readEntry(const std::string& category, const unsigned entNum) { ->>>>>>>> da92408 (Change ROOTNTuple{Reader,Writer} to ROOTRNTuple{Reader,Writer}):src/ROOTRNTupleReader.cc if (m_totalEntries.find(category) == m_totalEntries.end()) { getEntries(category); } From 2ea218bd62e71fda647f4dfde175b12e79046669 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Apr 2024 20:41:32 +0200 Subject: [PATCH 18/37] Update after a rebase --- include/podio/RNTupleReader.h | 80 ++++++++++++++++++++++++------- include/podio/RNTupleWriter.h | 88 ++++++++++++++++++++++++++++------- src/Reader.cc | 4 +- src/Writer.cc | 4 +- 4 files changed, 138 insertions(+), 38 deletions(-) diff --git a/include/podio/RNTupleReader.h b/include/podio/RNTupleReader.h index ef13aec27..1cffa149f 100644 --- a/include/podio/RNTupleReader.h +++ b/include/podio/RNTupleReader.h @@ -15,6 +15,10 @@ #include #include +#include +#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) + #include +#endif namespace podio { @@ -22,55 +26,95 @@ namespace podio { This class has the function to read available data from disk and to prepare collections and buffers. **/ +/// The RNTupleReader can be used to read files that have been written with the +/// RNTuple backend. +/// +/// The RNTupleReader provides the data as ROOTFrameData from which a podio::Frame +/// can be constructed. It can be used to read files written by the RNTupleWriter. class RNTupleReader { public: + /// Create a RNTupleReader RNTupleReader() = default; + /// Destructor ~RNTupleReader() = default; - + /// The RNTupleReader is not copy-able RNTupleReader(const RNTupleReader&) = delete; + /// The RNTupleReader is not copy-able RNTupleReader& operator=(const RNTupleReader&) = delete; + /// Open a single file for reading. + /// + /// @param filename The name of the input file void openFile(const std::string& filename); - void openFiles(const std::vector& filename); - /** - * Read the next data entry from which a Frame can be constructed for the - * given name. In case there are no more entries left for this name or in - * case there is no data for this name, this returns a nullptr. - */ + /// Open multiple files for reading and then treat them as if they are one file + /// + /// @note All of the files are assumed to have the same structure. Specifically + /// this means: + /// - The same categories are available from all files + /// - The collections that are contained in the individual categories are the + /// same across all files + /// - This usually boils down to "the files have been written with the same + /// "settings", e.g. they are outputs of a batched process. + /// + /// @param filenames The filenames of all input files that should be read + void openFiles(const std::vector& filenames); + + /// Read the next data entry for a given category. + /// + /// @param name The category name for which to read the next entry + /// + /// @returns FrameData from which a podio::Frame can be constructed if the + /// category exists and if there are still entries left to read. + /// Otherwise a nullptr std::unique_ptr readNextEntry(const std::string& name); - /** - * Read the specified data entry from which a Frame can be constructed for - * the given name. In case the entry does not exist for this name or in case - * there is no data for this name, this returns a nullptr. - */ + /// Read the desired data entry for a given category. + /// + /// @param name The category name for which to read the next entry + /// @param entry The entry number to read + /// + /// @returns FrameData from which a podio::Frame can be constructed if the + /// category and the desired entry exist. Otherwise a nullptr std::unique_ptr readEntry(const std::string& name, const unsigned entry); - /// Get the names of all the available Frame categories in the current file(s) + /// Get the names of all the available Frame categories in the current file(s). + /// + /// @returns The names of the available categores from the file std::vector getAvailableCategories() const; - /// Returns number of entries for the given name + /// Get the number of entries for the given name + /// + /// @param name The name of the category + /// + /// @returns The number of entries that are available for the category unsigned getEntries(const std::string& name); - /// Get the build version of podio that has been used to write the current file + /// Get the build version of podio that has been used to write the current + /// file + /// + /// @returns The podio build version podio::version::Version currentFileVersion() const { return m_fileVersion; } /// Get the datamodel definition for the given name + /// + /// @param name The name of the datamodel + /// + /// @returns The high level definition of the datamodel in JSON format const std::string_view getDatamodelDefinition(const std::string& name) const { return m_datamodelHolder.getDatamodelDefinition(name); } - /// Get all names of the datamodels that ara available from this reader + /// Get all names of the datamodels that are available from this reader + /// + /// @returns The names of the datamodels std::vector getAvailableDatamodels() const { return m_datamodelHolder.getAvailableDatamodels(); } - void closeFile(); - private: /** * Initialize the given category by filling the maps with metadata information diff --git a/include/podio/RNTupleWriter.h b/include/podio/RNTupleWriter.h index 2583e56db..ed0f3012c 100644 --- a/include/podio/RNTupleWriter.h +++ b/include/podio/RNTupleWriter.h @@ -10,6 +10,10 @@ #include "TFile.h" #include #include +#include +#if ROOT_VERSION_CODE >= ROOT_VERSION(6, 31, 0) + #include +#endif #include #include @@ -17,38 +21,90 @@ namespace podio { +/// The RNTupleWriter writes podio files into ROOT files using the new RNTuple +/// format. +/// +/// Each category gets its own RNTuple. Additionally, there is a podio_metadata +/// RNTuple that contains metadata that is necessary for interpreting the files +/// for reading. +/// +/// Files written with the RNTupleWriter can be read with the RNTupleReader. class RNTupleWriter { public: + /// Create a RNTupleWriter to write to a file. + /// + /// @note Existing files will be overwritten without warning. + /// + /// @param filename The path to the file that will be created. RNTupleWriter(const std::string& filename); + + /// RNTupleWriter destructor + /// + /// This also takes care of writing all the necessary metadata in order to be + /// able to read files back again. ~RNTupleWriter(); + /// The RNTupleWriter is not copy-able RNTupleWriter(const RNTupleWriter&) = delete; + /// The RNTupleWriter is not copy-able RNTupleWriter& operator=(const RNTupleWriter&) = delete; - template - void fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry); - + /// Store the given frame with the given category. + /// + /// This stores all available collections from the Frame. + /// + /// @note The contents of the first Frame that is written in this way + /// determines the contents that will be written for all subsequent Frames. + /// + /// @param frame The Frame to store + /// @param category The category name under which this Frame should be stored void writeFrame(const podio::Frame& frame, const std::string& category); + + /// Store the given Frame with the given category. + /// + /// This stores only the desired collections and not the complete frame. + /// + /// @note The contents of the first Frame that is written in this way + /// determines the contents that will be written for all subsequent Frames. + /// + /// @param frame The Frame to store + /// @param category The category name under which this Frame should be + /// stored + /// @param collsToWrite The collection names that should be written void writeFrame(const podio::Frame& frame, const std::string& category, const std::vector& collsToWrite); + + /// Write the current file, including all the necessary metadata to read it + /// again. + /// + /// @note The destructor will also call this, so letting a RNTupleWriter go out + /// of scope is also a viable way to write a readable file void finish(); - /** Check whether the collsToWrite are consistent with the state of the passed - * category. - * - * Return two vectors of collection names. The first one contains all the - * names that were missing from the collsToWrite but were present in the - * category. The second one contains the names that are present in the - * collsToWrite only. If both vectors are empty the category and the passed - * collsToWrite are consistent. - * - * NOTE: This will only be a meaningful check if the first Frame of the passed - * category has already been written. Also, this check is rather expensive as - * it has to effectively do two set differences. - */ + /// Check whether the collsToWrite are consistent with the state of the passed + /// category. + /// + /// @note This will only be a meaningful check if the first Frame of the passed + /// category has already been written. Also, this check is rather expensive as + /// it has to effectively do two set differences. + /// + /// + /// @param collsToWrite The collection names that should be checked for + /// consistency + /// @param category The category name for which consistency should be + /// checked + /// + /// @returns two vectors of collection names. The first one contains all the + /// names that were missing from the collsToWrite but were present in the + /// category. The second one contains the names that are present in the + /// collsToWrite only. If both vectors are empty the category and the passed + /// collsToWrite are consistent. std::tuple, std::vector> checkConsistency(const std::vector& collsToWrite, const std::string& category) const; private: + template + void fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry); + using StoreCollection = std::pair; std::unique_ptr createModels(const std::vector& collections); diff --git a/src/Reader.cc b/src/Reader.cc index c7dee3cfa..fde5c3054 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -1,6 +1,6 @@ #include "podio/Reader.h" -#include "podio/ROOTFrameReader.h" +#include "podio/ROOTReader.h" #if PODIO_ENABLE_RNTUPLE #include "podio/RNTupleReader.h" #endif @@ -55,7 +55,7 @@ Reader makeReader(const std::vector& filenames) { throw std::runtime_error("ROOT RNTuple reader not available. Please recompile with ROOT RNTuple support."); #endif } else { - auto actualReader = std::make_unique(); + auto actualReader = std::make_unique(); actualReader->openFiles(filenames); Reader reader{std::move(actualReader)}; return reader; diff --git a/src/Writer.cc b/src/Writer.cc index a579b275f..f9c7de6f0 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -1,6 +1,6 @@ #include "podio/Writer.h" -#include "podio/ROOTFrameWriter.h" +#include "podio/ROOTWriter.h" #if PODIO_ENABLE_RNTUPLE #include "podio/RNTupleWriter.h" #endif @@ -24,7 +24,7 @@ Writer makeWriter(const std::string& filename, const std::string& type) { }; if ((type == "default" && endsWith(filename, ".root")) || lower(type) == "root") { - return Writer{std::make_unique(filename)}; + return Writer{std::make_unique(filename)}; } if (lower(type) == "rntuple") { #if PODIO_ENABLE_RNTUPLE From 72e73379f6d9fb5adee7dc3cdf1e68cf3aafc2ca Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Apr 2024 20:44:37 +0200 Subject: [PATCH 19/37] Remove changes in python --- python/podio/frame_iterator.py | 43 +++++----------------------------- python/podio/root_io.py | 12 ++++------ 2 files changed, 11 insertions(+), 44 deletions(-) diff --git a/python/podio/frame_iterator.py b/python/podio/frame_iterator.py index 45fd2c02e..1fa97d828 100644 --- a/python/podio/frame_iterator.py +++ b/python/podio/frame_iterator.py @@ -3,7 +3,6 @@ # pylint: disable-next=import-error # gbl is a dynamic module from cppyy from cppyy.gbl import std -import cppyy from podio.frame import Frame @@ -27,15 +26,12 @@ def __iter__(self): return self def __next__(self): - """Get the next available Frame or stop.""" - try: - frame_data = self._reader.readNextFrame(self._category) - except AttributeError: - frame_data = self._reader.readNextEntry(self._category) - except std.runtime_error as error: - raise StopIteration from error - if frame_data: - return Frame(std.move(frame_data)) + """Get the next available Frame or stop.""" + frame_data = self._reader.readNextEntry(self._category) + if frame_data: + return Frame(std.move(frame_data)) + + raise StopIteration def __len__(self): """Get the number of available Frames for the passed category.""" @@ -50,33 +46,6 @@ def __getitem__(self, entry): # Handle python negative indexing to start from the end if entry < 0: entry = self._reader.getEntries(self._category) + entry - try: - frame_data = self._reader.readFrame(self._category, entry) - except AttributeError: - frame_data = self._reader.readEntry(self._category, entry) - except std.bad_function_call: - print('Error: Unable to read an entry of the input file. This can happen when the ' - 'ROOT model dictionaries are not in LD_LIBRARY_PATH. Make sure that LD_LIBRARY_PATH ' - 'points to the library folder of the installation of podio and also to the library ' - 'folder with your data model\n') - raise - except std.runtime_error: - raise IndexError('Unable to read frame, you might be trying to read beyond bounds or a ' - 'non-existent category') - - try: - frame_data = self._reader.readFrame(self._category, entry) - except AttributeError: - frame_data = self._reader.readEntry(self._category, entry) - except std.bad_function_call: - print('Error: Unable to read an entry of the input file. This can happen when the ' - 'ROOT model dictionaries are not in LD_LIBRARY_PATH. Make sure that LD_LIBRARY_PATH ' - 'points to the library folder of the installation of podio and also to the library ' - 'folder with your data model\n') - raise - except std.runtime_error as error: - raise IndexError('Unable to read frame, you might be trying to read beyond bounds or a ' - 'non-existent category') from error if entry < 0: # If we are below 0 now, we do not have enough entries to serve the request diff --git a/python/podio/root_io.py b/python/podio/root_io.py index 8228bf94f..830eb62e7 100644 --- a/python/podio/root_io.py +++ b/python/podio/root_io.py @@ -1,9 +1,9 @@ #!/usr/bin/env python3 """Python module for reading root files containing podio Frames""" -from ROOT import gSystem, gInterpreter -gSystem.Load('libpodioRootIO') # noqa: E402 -gInterpreter.LoadFile("podio/Reader.h") # noqa: E402 +from ROOT import gSystem + +gSystem.Load("libpodioRootIO") # noqa: E402 from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position # noqa: E402 @@ -22,8 +22,7 @@ def __init__(self, filenames): if isinstance(filenames, str): filenames = (filenames,) - self._reader = podio.makeReader(filenames) - # self._reader = podio.ROOTReader() + self._reader = podio.ROOTReader() self._reader.openFiles(filenames) super().__init__() @@ -79,11 +78,10 @@ def __init__(self, filename): Args: filename (str): The name of the output file """ - self._writer = podio.makeWriter(filename) + self._writer = podio.ROOTWriter(filename) super().__init__() - class RNTupleWriter(BaseWriterMixin): """Writer class for writing podio root files""" From 68e6a23a36fbdc1294bae4596784faa552c84f40 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Apr 2024 21:08:16 +0200 Subject: [PATCH 20/37] Remove a few methods from the concept and model --- include/podio/Reader.h | 12 ------------ src/Reader.cc | 2 -- 2 files changed, 14 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 72bd7c68d..68c8079b5 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -14,9 +14,6 @@ class Reader { virtual podio::Frame readNextFrame(const std::string& name) = 0; virtual podio::Frame readFrame(const std::string& name, size_t index) = 0; virtual size_t getEntries(const std::string& name) = 0; - virtual podio::Frame readNextEvent() = 0; - virtual podio::Frame readEvent(size_t index) = 0; - virtual size_t getEvents() = 0; virtual podio::version::Version currentFileVersion() const = 0; virtual std::vector getAvailableCategories() const = 0; virtual const std::string_view getDatamodelDefinition(const std::string& name) const = 0; @@ -37,9 +34,6 @@ class Reader { } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } - podio::Frame readNextEvent() override { - return readNextFrame(podio::Category::Event); - } podio::Frame readFrame(const std::string& name, size_t index) override { auto maybeFrame = m_reader->readEntry(name, index); @@ -48,15 +42,9 @@ class Reader { } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } - podio::Frame readEvent(size_t index) override { - return readFrame(podio::Category::Event, index); - } size_t getEntries(const std::string& name) override { return m_reader->getEntries(name); } - size_t getEvents() override { - return getEntries(podio::Category::Event); - } podio::version::Version currentFileVersion() const override { return m_reader->currentFileVersion(); } diff --git a/src/Reader.cc b/src/Reader.cc index fde5c3054..f4375c3c1 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -32,14 +32,12 @@ Reader makeReader(const std::vector& filenames) { } if (suffix == "root") { - // Check only the first file for RNTuples TFile* file = TFile::Open(filenames[0].c_str()); bool hasRNTuple = false; for (auto key : *file->GetListOfKeys()) { auto tkey = dynamic_cast(key); - // if (tkey && tkey->GetClassName() == "ROOT::Experimental::RNTuple") { if (tkey && std::string(tkey->GetClassName()) == "ROOT::Experimental::RNTuple") { hasRNTuple = true; break; From be7662c4e81b165b75c7720d1125fb9f3f4d8064 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Apr 2024 21:12:18 +0200 Subject: [PATCH 21/37] Fix clang 12 --- include/podio/Reader.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 68c8079b5..47141ce8c 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -30,7 +30,8 @@ class Reader { podio::Frame readNextFrame(const std::string& name) override { auto maybeFrame = m_reader->readNextEntry(name); if (maybeFrame) { - return maybeFrame; + // std::move seems to be needed by clang 12 + return std::move(maybeFrame); } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } From e30621e49855a8edc0701b29ac27e9769df5db53 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Apr 2024 21:15:36 +0200 Subject: [PATCH 22/37] Fix clang 12 --- include/podio/Reader.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 47141ce8c..29b7969a1 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -39,7 +39,7 @@ class Reader { podio::Frame readFrame(const std::string& name, size_t index) override { auto maybeFrame = m_reader->readEntry(name, index); if (maybeFrame) { - return maybeFrame; + return std::move(maybeFrame); } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } From 162a2932857711e21d63d67d5495fd86794a1df8 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 22 Apr 2024 21:25:57 +0200 Subject: [PATCH 23/37] Let clang 12 fail --- include/podio/Reader.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 29b7969a1..68c8079b5 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -30,8 +30,7 @@ class Reader { podio::Frame readNextFrame(const std::string& name) override { auto maybeFrame = m_reader->readNextEntry(name); if (maybeFrame) { - // std::move seems to be needed by clang 12 - return std::move(maybeFrame); + return maybeFrame; } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } @@ -39,7 +38,7 @@ class Reader { podio::Frame readFrame(const std::string& name, size_t index) override { auto maybeFrame = m_reader->readEntry(name, index); if (maybeFrame) { - return std::move(maybeFrame); + return maybeFrame; } throw std::runtime_error("Could not read frame (reading beyond bounds?)"); } From 98bee52a76c525783a3b5c82d3054967211e7144 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 07:59:37 +0200 Subject: [PATCH 24/37] Use final, make error messages a bit more verbose --- include/podio/Reader.h | 6 +++--- include/podio/Writer.h | 2 +- src/Reader.cc | 6 +++--- src/Writer.cc | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 68c8079b5..287205116 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -21,7 +21,7 @@ class Reader { }; template - struct ReaderModel : public ReaderConcept { + struct ReaderModel final : public ReaderConcept { ReaderModel(T* reader) : m_reader(reader) { } ReaderModel(const ReaderModel&) = delete; @@ -32,7 +32,7 @@ class Reader { if (maybeFrame) { return maybeFrame; } - throw std::runtime_error("Could not read frame (reading beyond bounds?)"); + throw std::runtime_error("Failed reading category " + name + " (reading beyond bounds?)"); } podio::Frame readFrame(const std::string& name, size_t index) override { @@ -40,7 +40,7 @@ class Reader { if (maybeFrame) { return maybeFrame; } - throw std::runtime_error("Could not read frame (reading beyond bounds?)"); + throw std::runtime_error("Failed reading category " + name + " at frame " + std::to_string(index) + " (reading beyond bounds?)"); } size_t getEntries(const std::string& name) override { return m_reader->getEntries(name); diff --git a/include/podio/Writer.h b/include/podio/Writer.h index f39f08d97..74c02025d 100644 --- a/include/podio/Writer.h +++ b/include/podio/Writer.h @@ -20,7 +20,7 @@ class Writer { }; template - struct WriterModel : public WriterConcept { + struct WriterModel final : public WriterConcept { WriterModel(std::unique_ptr writer) : m_writer(std::move(writer)) { } WriterModel(const WriterModel&) = delete; diff --git a/src/Reader.cc b/src/Reader.cc index f4375c3c1..3684634f9 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -5,7 +5,7 @@ #include "podio/RNTupleReader.h" #endif #if PODIO_ENABLE_SIO - #include "podio/SIOFrameReader.h" + #include "podio/SIOReader.h" #endif #include "TFile.h" @@ -60,8 +60,8 @@ Reader makeReader(const std::vector& filenames) { } } else if (suffix == "sio") { #if PODIO_ENABLE_SIO - auto actualReader = std::make_unique(); - actualReader->openFiles(filenames); + auto actualReader = std::make_unique(); + actualReader->openFile(filenames[0]); Reader reader{std::move(actualReader)}; return reader; #else diff --git a/src/Writer.cc b/src/Writer.cc index f9c7de6f0..a3eef5c93 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -5,7 +5,7 @@ #include "podio/RNTupleWriter.h" #endif #if PODIO_ENABLE_SIO - #include "podio/SIOFrameWriter.h" + #include "podio/SIOWriter.h" #endif #include @@ -35,12 +35,12 @@ Writer makeWriter(const std::string& filename, const std::string& type) { } if ((type == "default" && endsWith(filename, ".sio")) || lower(type) == "sio") { #if PODIO_ENABLE_SIO - return Writer{std::make_unique(filename)}; + return Writer{std::make_unique(filename)}; #else throw std::runtime_error("SIO writer not available. Please recompile with SIO support."); #endif } - throw std::runtime_error("Unknown file type"); + throw std::runtime_error("Unknown file type for file " + filename + " with type " + type); } } // namespace podio From 6168cb9d737916bfef4088c473d5978e0faca19d Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 08:13:19 +0200 Subject: [PATCH 25/37] Add a new podioIO library --- src/CMakeLists.txt | 19 +++++++++-- src/io_selection.xml | 4 +++ .../read_interface.cpp => read_interface.h} | 28 --------------- tests/root_io/CMakeLists.txt | 8 ++--- tests/root_io/read_interface_root.cpp | 18 ++++++++++ tests/root_io/write_interface.cpp | 34 ------------------- tests/root_io/write_interface_root.cpp | 14 ++++++++ tests/sio_io/CMakeLists.txt | 6 +++- tests/sio_io/read_interface_sio.cpp | 11 ++++++ tests/sio_io/write_interface_sio.cpp | 9 +++++ tests/write_interface.h | 16 +++++++++ 11 files changed, 98 insertions(+), 69 deletions(-) create mode 100644 src/io_selection.xml rename tests/{root_io/read_interface.cpp => read_interface.h} (85%) create mode 100644 tests/root_io/read_interface_root.cpp delete mode 100644 tests/root_io/write_interface.cpp create mode 100644 tests/root_io/write_interface_root.cpp create mode 100644 tests/sio_io/read_interface_sio.cpp create mode 100644 tests/sio_io/write_interface_sio.cpp create mode 100644 tests/write_interface.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a2dc67b10..a7fb53776 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -80,8 +80,6 @@ SET(root_sources ROOTWriter.cc ROOTReader.cc ROOTLegacyReader.cc - Reader.cc - Writer.cc ) if(ENABLE_RNTUPLE) list(APPEND root_sources @@ -139,6 +137,23 @@ if(ENABLE_SIO) LIST(APPEND INSTALL_LIBRARIES podioSioIO podioSioIODict) endif() +# --- IO +set(io_sources + Writer.cc + Reader.cc + ) + +PODIO_ADD_LIB_AND_DICT(podioIO "${core_headers}" "${io_sources}" io_selection.xml) +target_link_libraries(podioIO PUBLIC podio::podio ROOT::Core ROOT::RIO ROOT::Tree) +if(ENABLE_RNTUPLE) + target_link_libraries(podioIO PUBLIC ROOT::ROOTNTuple) + target_compile_definitions(podioIO PUBLIC PODIO_ENABLE_RNTUPLE=1) +endif() +if(ENABLE_SIO) + target_link_libraries(podioIO PUBLIC podio::podioSioIO) + target_compile_definitions(podioIO PUBLIC PODIO_ENABLE_SIO=1) +endif() + # --- Install everything install(TARGETS podio podioDict podioRootIO podioRootIODict ${INSTALL_LIBRARIES} EXPORT podioTargets diff --git a/src/io_selection.xml b/src/io_selection.xml new file mode 100644 index 000000000..cfd0d3af9 --- /dev/null +++ b/src/io_selection.xml @@ -0,0 +1,4 @@ + + + + diff --git a/tests/root_io/read_interface.cpp b/tests/read_interface.h similarity index 85% rename from tests/root_io/read_interface.cpp rename to tests/read_interface.h index 00b5c0449..2cd826ab0 100644 --- a/tests/root_io/read_interface.cpp +++ b/tests/read_interface.h @@ -1,10 +1,6 @@ #include "read_frame.h" -#include "podio/ROOTFrameReader.h" #include "podio/Reader.h" -#if PODIO_ENABLE_RNTUPLE - #include "podio/RNTupleReader.h" -#endif int read_frames(podio::Reader& reader) { @@ -84,27 +80,3 @@ int read_frames(podio::Reader& reader) { return 0; } - -int main(int, char**) { - - auto reader = podio::makeReader("example_frame_interface.root"); - if (read_frames(reader)) { - return 1; - } - -#if PODIO_ENABLE_RNTUPLE - auto readerRNTuple = podio::makeReader("example_frame_rntuple_interface.root"); - if (read_frames(readerRNTuple)) { - return 1; - } -#endif - -#if PODIO_ENABLE_SIO - auto readerSIO = podio::makeReader("example_frame_sio_interface.sio"); - if (read_frames(readerSIO)) { - return 1; - } -#endif - - return 0; -} diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index 20674dc5b..cb4d4636e 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -6,8 +6,8 @@ set(root_dependent_tests read_python_frame_root.cpp read_frame_root_multiple.cpp read_and_write_frame_root.cpp - write_interface.cpp - read_interface.cpp + write_interface_root.cpp + read_interface_root.cpp ) if(ENABLE_RNTUPLE) set(root_dependent_tests @@ -17,12 +17,12 @@ if(ENABLE_RNTUPLE) read_python_frame_rntuple.cpp ) endif() -set(root_libs TestDataModelDict ExtensionDataModelDict podio::podioRootIO) +set(root_libs TestDataModelDict ExtensionDataModelDict podio::podioRootIO podio::podioIO) foreach( sourcefile ${root_dependent_tests} ) CREATE_PODIO_TEST(${sourcefile} "${root_libs}") endforeach() -set_property(TEST read_interface PROPERTY DEPENDS write_interface) +set_property(TEST read_interface_root PROPERTY DEPENDS write_interface_root) set_tests_properties( read_frame_root diff --git a/tests/root_io/read_interface_root.cpp b/tests/root_io/read_interface_root.cpp new file mode 100644 index 000000000..397f4677b --- /dev/null +++ b/tests/root_io/read_interface_root.cpp @@ -0,0 +1,18 @@ +#include "read_interface.h" + +int main(int, char**) { + + auto reader = podio::makeReader("example_frame_interface.root"); + if (read_frames(reader)) { + return 1; + } + +#if PODIO_ENABLE_RNTUPLE + auto readerRNTuple = podio::makeReader("example_frame_rntuple_interface.root"); + if (read_frames(readerRNTuple)) { + return 1; + } +#endif + + return 0; +} diff --git a/tests/root_io/write_interface.cpp b/tests/root_io/write_interface.cpp deleted file mode 100644 index b886e5b95..000000000 --- a/tests/root_io/write_interface.cpp +++ /dev/null @@ -1,34 +0,0 @@ -#include "write_frame.h" - -#include "podio/Writer.h" - -void write_frames(podio::Writer& frameWriter) { - - for (int i = 0; i < 10; ++i) { - auto frame = makeFrame(i); - frameWriter.writeFrame(frame, podio::Category::Event, collsToWrite); - } - - for (int i = 100; i < 110; ++i) { - auto frame = makeFrame(i); - frameWriter.writeFrame(frame, "other_events"); - } -} - -int main(int, char**) { - - auto writer = podio::makeWriter("example_frame_interface.root"); - write_frames(writer); - -#if PODIO_ENABLE_RNTUPLE - auto writerRNTuple = podio::makeWriter("example_frame_rntuple_interface.root", "rntuple"); - write_frames(writerRNTuple); -#endif - -#if PODIO_ENABLE_SIO - auto writerSIO = podio::makeWriter("example_frame_sio_interface.sio", "sio"); - write_frames(writerSIO); -#endif - - return 0; -} diff --git a/tests/root_io/write_interface_root.cpp b/tests/root_io/write_interface_root.cpp new file mode 100644 index 000000000..9c7885014 --- /dev/null +++ b/tests/root_io/write_interface_root.cpp @@ -0,0 +1,14 @@ +#include "write_interface.h" + +int main(int, char**) { + + auto writer = podio::makeWriter("example_frame_interface.root"); + write_frames(writer); + +#if PODIO_ENABLE_RNTUPLE + auto writerRNTuple = podio::makeWriter("example_frame_rntuple_interface.root", "rntuple"); + write_frames(writerRNTuple); +#endif + + return 0; +} diff --git a/tests/sio_io/CMakeLists.txt b/tests/sio_io/CMakeLists.txt index b3987ba5a..7102c36c7 100644 --- a/tests/sio_io/CMakeLists.txt +++ b/tests/sio_io/CMakeLists.txt @@ -3,8 +3,10 @@ set(sio_dependent_tests write_frame_sio.cpp read_and_write_frame_sio.cpp read_python_frame_sio.cpp + write_interface_sio.cpp + read_interface_sio.cpp ) -set(sio_libs podio::podioSioIO) +set(sio_libs podio::podioSioIO podio::podioIO) foreach( sourcefile ${sio_dependent_tests} ) CREATE_PODIO_TEST(${sourcefile} "${sio_libs}") endforeach() @@ -18,6 +20,8 @@ set_tests_properties( write_frame_sio ) +set_property(TEST read_interface_sio PROPERTY DEPENDS write_interface_sio) + #--- Write via python and the SIO backend and see if we can read it back in in #--- c++ add_test(NAME write_python_frame_sio COMMAND python3 ${PROJECT_SOURCE_DIR}/tests/write_frame.py example_frame_with_py.sio sio_io.Writer) diff --git a/tests/sio_io/read_interface_sio.cpp b/tests/sio_io/read_interface_sio.cpp new file mode 100644 index 000000000..e411f2a96 --- /dev/null +++ b/tests/sio_io/read_interface_sio.cpp @@ -0,0 +1,11 @@ +#include "read_interface.h" + +int main(int, char**) { + + auto readerSIO = podio::makeReader("example_frame_sio_interface.sio"); + if (read_frames(readerSIO)) { + return 1; + } + + return 0; +} diff --git a/tests/sio_io/write_interface_sio.cpp b/tests/sio_io/write_interface_sio.cpp new file mode 100644 index 000000000..9def5db18 --- /dev/null +++ b/tests/sio_io/write_interface_sio.cpp @@ -0,0 +1,9 @@ +#include "write_interface.h" + +int main(int, char**) { + + auto writerSIO = podio::makeWriter("example_frame_sio_interface.sio", "sio"); + write_frames(writerSIO); + + return 0; +} diff --git a/tests/write_interface.h b/tests/write_interface.h new file mode 100644 index 000000000..b6baf21ea --- /dev/null +++ b/tests/write_interface.h @@ -0,0 +1,16 @@ +#include "write_frame.h" + +#include "podio/Writer.h" + +void write_frames(podio::Writer& frameWriter) { + + for (int i = 0; i < 10; ++i) { + auto frame = makeFrame(i); + frameWriter.writeFrame(frame, podio::Category::Event, collsToWrite); + } + + for (int i = 100; i < 110; ++i) { + auto frame = makeFrame(i); + frameWriter.writeFrame(frame, "other_events"); + } +} From 6655ff968b012b621ed7b31558a8aedaf135358d Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 08:18:08 +0200 Subject: [PATCH 26/37] Add sio files to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 286e8dda5..8da0b83df 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ tests/unittests/Manifest.toml # Data Files *.root *.dat +*.sio # Spack build folders spack* From 866731bace8f4e9c2f64b3e31642de101958a195 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 08:31:22 +0200 Subject: [PATCH 27/37] Link with podioRootIO --- src/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a7fb53776..130d71555 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -144,9 +144,8 @@ set(io_sources ) PODIO_ADD_LIB_AND_DICT(podioIO "${core_headers}" "${io_sources}" io_selection.xml) -target_link_libraries(podioIO PUBLIC podio::podio ROOT::Core ROOT::RIO ROOT::Tree) +target_link_libraries(podioIO PUBLIC podio::podio podio::podioRootIO) if(ENABLE_RNTUPLE) - target_link_libraries(podioIO PUBLIC ROOT::ROOTNTuple) target_compile_definitions(podioIO PUBLIC PODIO_ENABLE_RNTUPLE=1) endif() if(ENABLE_SIO) From fb9764b4808e3409c8e0d0c592f7ed7709f6411b Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 08:33:47 +0200 Subject: [PATCH 28/37] Remove public definitions --- src/CMakeLists.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 130d71555..27f558562 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -145,12 +145,8 @@ set(io_sources PODIO_ADD_LIB_AND_DICT(podioIO "${core_headers}" "${io_sources}" io_selection.xml) target_link_libraries(podioIO PUBLIC podio::podio podio::podioRootIO) -if(ENABLE_RNTUPLE) - target_compile_definitions(podioIO PUBLIC PODIO_ENABLE_RNTUPLE=1) -endif() if(ENABLE_SIO) target_link_libraries(podioIO PUBLIC podio::podioSioIO) - target_compile_definitions(podioIO PUBLIC PODIO_ENABLE_SIO=1) endif() # --- Install everything From 0ed53e9e52bd7a2b7e686ed5cbd11e573e09fa03 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 08:47:37 +0200 Subject: [PATCH 29/37] Fix style --- include/podio/Reader.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 287205116..8484fc1b2 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -40,7 +40,8 @@ class Reader { if (maybeFrame) { return maybeFrame; } - throw std::runtime_error("Failed reading category " + name + " at frame " + std::to_string(index) + " (reading beyond bounds?)"); + throw std::runtime_error("Failed reading category " + name + " at frame " + std::to_string(index) + + " (reading beyond bounds?)"); } size_t getEntries(const std::string& name) override { return m_reader->getEntries(name); From 648a90b59cdff4b6e9fc9dd9518316be9039d930 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 29 Apr 2024 09:09:45 +0200 Subject: [PATCH 30/37] Fix style --- tests/read_interface.h | 5 +++++ tests/write_interface.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/read_interface.h b/tests/read_interface.h index 2cd826ab0..278e03742 100644 --- a/tests/read_interface.h +++ b/tests/read_interface.h @@ -1,3 +1,6 @@ +#ifndef PODIO_TESTS_READ_INTERFACE_H // NOLINT(llvm-header-guard): folder structure not suitable +#define PODIO_TESTS_READ_INTERFACE_H // NOLINT(llvm-header-guard): folder structure not suitable + #include "read_frame.h" #include "podio/Reader.h" @@ -80,3 +83,5 @@ int read_frames(podio::Reader& reader) { return 0; } + +#endif // PODIO_TESTS_READ_INTERFACE_H diff --git a/tests/write_interface.h b/tests/write_interface.h index b6baf21ea..1acaa2383 100644 --- a/tests/write_interface.h +++ b/tests/write_interface.h @@ -1,3 +1,6 @@ +#ifndef PODIO_TESTS_WRITE_INTERFACE_H // NOLINT(llvm-header-guard): folder structure not suitable +#define PODIO_TESTS_WRITE_INTERFACE_H // NOLINT(llvm-header-guard): folder structure not suitable + #include "write_frame.h" #include "podio/Writer.h" @@ -14,3 +17,5 @@ void write_frames(podio::Writer& frameWriter) { frameWriter.writeFrame(frame, "other_events"); } } + +#endif // PODIO_TESTS_WRITE_INTERFACE_H From 249c59c2c71386bf9118a53af71c13cd07c77e2c Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 2 May 2024 16:17:18 +0200 Subject: [PATCH 31/37] Make sure test exclusion matches new test names --- tests/CTestCustom.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/CTestCustom.cmake b/tests/CTestCustom.cmake index 911a3fe25..18e8ddd7c 100644 --- a/tests/CTestCustom.cmake +++ b/tests/CTestCustom.cmake @@ -22,8 +22,8 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ write_frame_root read_frame_root - write_interface - read_interface + write_interface_root + read_interface_root write_python_frame_sio read_python_frame_sio From a74d7aadb5f05511e055e27be9c508ab9d0f8ec7 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 2 May 2024 16:17:38 +0200 Subject: [PATCH 32/37] Split RTNtuple based interface tests off to make sanitizers run on them --- tests/root_io/CMakeLists.txt | 3 +++ tests/root_io/read_interface_rntuple.cpp | 10 ++++++++++ tests/root_io/read_interface_root.cpp | 7 ------- tests/root_io/write_interface_rntuple.cpp | 8 ++++++++ tests/root_io/write_interface_root.cpp | 5 ----- 5 files changed, 21 insertions(+), 12 deletions(-) create mode 100644 tests/root_io/read_interface_rntuple.cpp create mode 100644 tests/root_io/write_interface_rntuple.cpp diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index cb4d4636e..6a4069a48 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -15,6 +15,8 @@ if(ENABLE_RNTUPLE) write_rntuple.cpp read_rntuple.cpp read_python_frame_rntuple.cpp + write_interface_rntuple.cpp + read_interface_rntuple.cpp ) endif() set(root_libs TestDataModelDict ExtensionDataModelDict podio::podioRootIO podio::podioIO) @@ -35,6 +37,7 @@ set_tests_properties( if(ENABLE_RNTUPLE) set_property(TEST read_rntuple PROPERTY DEPENDS write_rntuple) + set_property(TEST read_interface_rntuple PROPERTY DEPENDS write_interface_rntuple) endif() add_executable(read_frame_legacy_root read_frame_legacy_root.cpp) diff --git a/tests/root_io/read_interface_rntuple.cpp b/tests/root_io/read_interface_rntuple.cpp new file mode 100644 index 000000000..e2ee2a421 --- /dev/null +++ b/tests/root_io/read_interface_rntuple.cpp @@ -0,0 +1,10 @@ +#include "read_interface.h" + +int main(int, char**) { + auto reader = podio::makeReader("example_from_rntuple_interface.root"); + if (read_frames(reader)) { + return 1; + } + + return 0; +} diff --git a/tests/root_io/read_interface_root.cpp b/tests/root_io/read_interface_root.cpp index 397f4677b..8d33ec1c3 100644 --- a/tests/root_io/read_interface_root.cpp +++ b/tests/root_io/read_interface_root.cpp @@ -7,12 +7,5 @@ int main(int, char**) { return 1; } -#if PODIO_ENABLE_RNTUPLE - auto readerRNTuple = podio::makeReader("example_frame_rntuple_interface.root"); - if (read_frames(readerRNTuple)) { - return 1; - } -#endif - return 0; } diff --git a/tests/root_io/write_interface_rntuple.cpp b/tests/root_io/write_interface_rntuple.cpp new file mode 100644 index 000000000..c13658c61 --- /dev/null +++ b/tests/root_io/write_interface_rntuple.cpp @@ -0,0 +1,8 @@ +#include "write_interface.h" + +int main(int, char**) { + auto writer = podio::makeWriter("example_from_rntuple_interface.root", "rntuple"); + write_frames(writer); + + return 0; +} diff --git a/tests/root_io/write_interface_root.cpp b/tests/root_io/write_interface_root.cpp index 9c7885014..84775e6de 100644 --- a/tests/root_io/write_interface_root.cpp +++ b/tests/root_io/write_interface_root.cpp @@ -5,10 +5,5 @@ int main(int, char**) { auto writer = podio::makeWriter("example_frame_interface.root"); write_frames(writer); -#if PODIO_ENABLE_RNTUPLE - auto writerRNTuple = podio::makeWriter("example_frame_rntuple_interface.root", "rntuple"); - write_frames(writerRNTuple); -#endif - return 0; } From 70a529b29739ef4b0144e20a07ee7e0712773dbc Mon Sep 17 00:00:00 2001 From: tmadlener Date: Fri, 3 May 2024 09:15:30 +0200 Subject: [PATCH 33/37] Move unique_ptr instead of releasing and acquiring it --- include/podio/Reader.h | 2 +- src/Reader.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index 8484fc1b2..e644a8238 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -22,7 +22,7 @@ class Reader { template struct ReaderModel final : public ReaderConcept { - ReaderModel(T* reader) : m_reader(reader) { + ReaderModel(std::unique_ptr reader) : m_reader(std::move(reader)) { } ReaderModel(const ReaderModel&) = delete; ReaderModel& operator=(const ReaderModel&) = delete; diff --git a/src/Reader.cc b/src/Reader.cc index 3684634f9..2686419b8 100644 --- a/src/Reader.cc +++ b/src/Reader.cc @@ -15,7 +15,7 @@ namespace podio { template -Reader::Reader(std::unique_ptr reader) : m_self(std::make_unique>(reader.release())) { +Reader::Reader(std::unique_ptr reader) : m_self(std::make_unique>(std::move(reader))) { } Reader makeReader(const std::string& filename) { From df642d3829e606fa933f6e03984e50eabf3b6936 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 6 May 2024 08:09:16 +0200 Subject: [PATCH 34/37] Add read_interface_sio to be excluded when using the Address sanitizer --- tests/CTestCustom.cmake | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/CTestCustom.cmake b/tests/CTestCustom.cmake index 18e8ddd7c..3c660068a 100644 --- a/tests/CTestCustom.cmake +++ b/tests/CTestCustom.cmake @@ -74,13 +74,12 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ set(CTEST_CUSTOM_TESTS_IGNORE ${CTEST_CUSTOM_TESTS_IGNORE} - write_sio read_sio read_and_write_sio write_timed_sio read_timed_sio - write_frame_sio read_frame_sio + read_interface_sio read_frame_legacy_sio read_and_write_frame_sio ) From 94039e3d33544d2d7ba7f2af8b616c680295ce41 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Sun, 12 May 2024 18:24:06 +0200 Subject: [PATCH 35/37] Don't check type for SIO --- src/Writer.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Writer.cc b/src/Writer.cc index a3eef5c93..13f979ce8 100644 --- a/src/Writer.cc +++ b/src/Writer.cc @@ -25,15 +25,13 @@ Writer makeWriter(const std::string& filename, const std::string& type) { if ((type == "default" && endsWith(filename, ".root")) || lower(type) == "root") { return Writer{std::make_unique(filename)}; - } - if (lower(type) == "rntuple") { + } else if (lower(type) == "rntuple") { #if PODIO_ENABLE_RNTUPLE return Writer{std::make_unique(filename)}; #else throw std::runtime_error("ROOT RNTuple writer not available. Please recompile with ROOT RNTuple support."); #endif - } - if ((type == "default" && endsWith(filename, ".sio")) || lower(type) == "sio") { + } else if (endsWith(filename, ".sio")) { #if PODIO_ENABLE_SIO return Writer{std::make_unique(filename)}; #else From f7e67c5f135571a21647c58ad043fc56f7daba37 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 21 May 2024 11:46:25 +0200 Subject: [PATCH 36/37] Address comments --- src/CMakeLists.txt | 6 +++--- src/io_selection.xml | 2 ++ src/root_selection.xml | 2 -- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 27f558562..a1bc11dec 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -68,6 +68,8 @@ SET(core_headers ${PROJECT_SOURCE_DIR}/include/podio/DatamodelRegistry.h ${PROJECT_SOURCE_DIR}/include/podio/utilities/DatamodelRegistryIOHelpers.h ${PROJECT_SOURCE_DIR}/include/podio/GenericParameters.h + ${PROJECT_SOURCE_DIR}/include/podio/Reader.h + ${PROJECT_SOURCE_DIR}/include/podio/Writer.h ) PODIO_ADD_LIB_AND_DICT(podio "${core_headers}" "${core_sources}" selection.xml) @@ -92,8 +94,6 @@ SET(root_headers ${PROJECT_SOURCE_DIR}/include/podio/ROOTReader.h ${PROJECT_SOURCE_DIR}/include/podio/ROOTLegacyReader.h ${PROJECT_SOURCE_DIR}/include/podio/ROOTWriter.h - ${PROJECT_SOURCE_DIR}/include/podio/Reader.h - ${PROJECT_SOURCE_DIR}/include/podio/Writer.h ) if(ENABLE_RNTUPLE) list(APPEND root_headers @@ -150,7 +150,7 @@ if(ENABLE_SIO) endif() # --- Install everything -install(TARGETS podio podioDict podioRootIO podioRootIODict ${INSTALL_LIBRARIES} +install(TARGETS podio podioDict podioRootIO podioRootIODict podioIO ${INSTALL_LIBRARIES} EXPORT podioTargets DESTINATION "${CMAKE_INSTALL_LIBDIR}") diff --git a/src/io_selection.xml b/src/io_selection.xml index cfd0d3af9..ffeeb5eef 100644 --- a/src/io_selection.xml +++ b/src/io_selection.xml @@ -1,4 +1,6 @@ + + diff --git a/src/root_selection.xml b/src/root_selection.xml index 6753cedee..38db949c0 100644 --- a/src/root_selection.xml +++ b/src/root_selection.xml @@ -5,7 +5,5 @@ - - From 68053f57b7f3770733298009ca9f3b397d1aef11 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 21 May 2024 14:36:41 +0200 Subject: [PATCH 37/37] Fix a few more issues --- src/CMakeLists.txt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a1bc11dec..f129467d8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -68,8 +68,6 @@ SET(core_headers ${PROJECT_SOURCE_DIR}/include/podio/DatamodelRegistry.h ${PROJECT_SOURCE_DIR}/include/podio/utilities/DatamodelRegistryIOHelpers.h ${PROJECT_SOURCE_DIR}/include/podio/GenericParameters.h - ${PROJECT_SOURCE_DIR}/include/podio/Reader.h - ${PROJECT_SOURCE_DIR}/include/podio/Writer.h ) PODIO_ADD_LIB_AND_DICT(podio "${core_headers}" "${core_sources}" selection.xml) @@ -143,7 +141,12 @@ set(io_sources Reader.cc ) -PODIO_ADD_LIB_AND_DICT(podioIO "${core_headers}" "${io_sources}" io_selection.xml) +set(io_headers + ${PROJECT_SOURCE_DIR}/include/podio/Writer.h + ${PROJECT_SOURCE_DIR}/include/podio/Reader.h + ) + +PODIO_ADD_LIB_AND_DICT(podioIO "${io_headers}" "${io_sources}" io_selection.xml) target_link_libraries(podioIO PUBLIC podio::podio podio::podioRootIO) if(ENABLE_SIO) target_link_libraries(podioIO PUBLIC podio::podioSioIO) @@ -167,6 +170,8 @@ install(FILES ${CMAKE_CURRENT_BINARY_DIR}/libpodioDict_rdict.pcm ${CMAKE_CURRENT_BINARY_DIR}/podioRootIODictDict.rootmap ${CMAKE_CURRENT_BINARY_DIR}/libpodioRootIODict_rdict.pcm + ${CMAKE_CURRENT_BINARY_DIR}/podioIODictDict.rootmap + ${CMAKE_CURRENT_BINARY_DIR}/libpodioIODict_rdict.pcm DESTINATION "${CMAKE_INSTALL_LIBDIR}") if (ENABLE_SIO)