From 23af0ab68e0c295585ce880f469694ca4e7834bd Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Sun, 19 Oct 2025 14:08:55 +0200 Subject: [PATCH 01/17] Make test read all collections Remove the need to specifically state which collections to read --- .../code_gen/datatypes_remove_type/check.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/schema_evolution/code_gen/datatypes_remove_type/check.cpp b/tests/schema_evolution/code_gen/datatypes_remove_type/check.cpp index 2516e4049..599cdf1a1 100644 --- a/tests/schema_evolution/code_gen/datatypes_remove_type/check.cpp +++ b/tests/schema_evolution/code_gen/datatypes_remove_type/check.cpp @@ -19,15 +19,6 @@ int main() { elem2.t(99); event.put(std::move(coll2), "evolution_collection"); }); -#ifdef PODIO_SCHEMA_EVOLUTION_TEST_READ - auto reader = ReaderT(); - std::cout << "Reading file " << TEST_CASE FILE_SUFFIX << std::endl; - reader.openFile((TEST_CASE FILE_SUFFIX)); - // TODO: Make this work when reading all collections - auto event = podio::Frame(reader.readNextEntry(podio::Category::Event, {"evolution_collection"})); - const auto& coll = event.get("evolution_collection"); - const auto elem = coll[0]; - ASSERT_EQUAL(elem.t(), 99, "Member variables unrelated to schema evolution have changed"); -#endif - return 0; + READ_AS(ExampleHitStaysCollection, + { ASSERT_EQUAL(elem.t(), 99, "Member variables unrelated to schema evolution have changed"); }); } From 637d4c34f1b1a5aa5aedcbc03395b8082a27b586 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Sun, 19 Oct 2025 13:50:21 +0200 Subject: [PATCH 02/17] Make the ROOTReader skip unreadable collections --- include/podio/ROOTReader.h | 4 ++-- src/ROOTReader.cc | 20 +++++++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/podio/ROOTReader.h b/include/podio/ROOTReader.h index b58c2f52b..ab945bd22 100644 --- a/include/podio/ROOTReader.h +++ b/include/podio/ROOTReader.h @@ -195,8 +195,8 @@ class ROOTReader { const std::vector& collsToRead); /// Get / read the buffers at index iColl in the passed category information - podio::CollectionReadBuffers getCollectionBuffers(CategoryInfo& catInfo, size_t iColl, bool reloadBranches, - unsigned int localEntry); + std::optional getCollectionBuffers(CategoryInfo& catInfo, size_t iColl, + bool reloadBranches, unsigned int localEntry); std::unique_ptr m_metaChain{nullptr}; ///< The metadata tree std::unordered_map m_categories{}; ///< All categories diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index 3ec3418cb..c8c29ea7c 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -131,7 +131,13 @@ std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& c if (!collsToRead.empty() && std::ranges::find(collsToRead, catInfo.storedClasses[i].name) == collsToRead.end()) { continue; } - buffers.emplace(catInfo.storedClasses[i].name, getCollectionBuffers(catInfo, i, reloadBranches, localEntry)); + auto collBuffers = getCollectionBuffers(catInfo, i, reloadBranches, localEntry); + if (!collBuffers) { + std::cerr << "WARNING: Could not get buffers for collection " << catInfo.storedClasses[i].name + << "(type=" << std::get(catInfo.storedClasses[i].info) << ") from file" << std::endl; + continue; + } + buffers.emplace(catInfo.storedClasses[i].name, std::move(collBuffers.value())); } auto parameters = readEntryParameters(catInfo, reloadBranches, localEntry); @@ -140,8 +146,9 @@ std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& c return std::make_unique(std::move(buffers), catInfo.table, std::move(parameters)); } -podio::CollectionReadBuffers ROOTReader::getCollectionBuffers(ROOTReader::CategoryInfo& catInfo, size_t iColl, - bool reloadBranches, unsigned int localEntry) { +std::optional ROOTReader::getCollectionBuffers(ROOTReader::CategoryInfo& catInfo, + size_t iColl, bool reloadBranches, + unsigned int localEntry) { const auto& name = catInfo.storedClasses[iColl].name; const auto& [collType, isSubsetColl, schemaVersion, index] = catInfo.storedClasses[iColl].info; auto& branches = catInfo.branches[index]; @@ -149,8 +156,11 @@ podio::CollectionReadBuffers ROOTReader::getCollectionBuffers(ROOTReader::Catego const auto& bufferFactory = podio::CollectionBufferFactory::instance(); auto maybeBuffers = bufferFactory.createBuffers(collType, schemaVersion, isSubsetColl); - // TODO: Error handling of empty optional - auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{}); + if (!maybeBuffers) { + return std::nullopt; + } + + auto collBuffers = maybeBuffers.value(); if (reloadBranches) { root_utils::resetBranches(catInfo.chain.get(), branches, name); From 52aa71ca6d4ca008b1aeefa8a153054b18d049b2 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 12:18:51 +0200 Subject: [PATCH 03/17] Make the RNTupleReader skip unreadable collections --- src/RNTupleReader.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 5aa7ece53..28bed41ee 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -189,13 +189,13 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ const auto& collType = coll.dataType; const auto& bufferFactory = podio::CollectionBufferFactory::instance(); const auto maybeBuffers = bufferFactory.createBuffers(collType, coll.schemaVersion, coll.isSubset); - const auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{}); if (!maybeBuffers) { std::cout << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType << " and schema version " << coll.schemaVersion << std::endl; - return nullptr; + continue; } + const auto collBuffers = maybeBuffers.value(); if (coll.isSubset) { const auto brName = root_utils::subsetBranch(coll.name); From a765aec0ba54ffe21cabfefcecd9e25232d0b1be Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 12:41:34 +0200 Subject: [PATCH 04/17] Add ReadOptions and incorporate collsToRead Makes it possible to configure at runtime what should happen with collections that can't be read, rather than making it a compile time decision. Keeping old overloads undeprecated for the moment. --- include/podio/RNTupleReader.h | 10 ++++++++-- include/podio/ROOTReader.h | 13 ++++++++++--- include/podio/ReadOptions.h | 14 ++++++++++++++ src/RNTupleReader.cc | 31 +++++++++++++++++++++++++------ src/ROOTReader.cc | 30 +++++++++++++++++++++++------- 5 files changed, 80 insertions(+), 18 deletions(-) create mode 100644 include/podio/ReadOptions.h diff --git a/include/podio/RNTupleReader.h b/include/podio/RNTupleReader.h index ea908f5da..dd73fd700 100644 --- a/include/podio/RNTupleReader.h +++ b/include/podio/RNTupleReader.h @@ -2,6 +2,7 @@ #define PODIO_RNTUPLEREADER_H #include "podio/ROOTFrameData.h" +#include "podio/ReadOptions.h" #include "podio/podioVersion.h" #include "podio/utilities/DatamodelRegistryIOHelpers.h" #include "podio/utilities/RootHelpers.h" @@ -75,8 +76,10 @@ class RNTupleReader { /// @throws std::invalid_argument in case collsToRead contains collection /// names that are not available std::unique_ptr readNextEntry(const std::string& name, - const std::vector& collsToRead = {}); + const std::vector& collsToRead); + std::unique_ptr readNextEntry(const std::string& name, + const podio::ReadOptions& readOptions = {}); /// Read the desired data entry for a given category. /// /// @param name The category name for which to read the next entry @@ -90,7 +93,10 @@ class RNTupleReader { /// @throws std::invalid_argument in case collsToRead contains collection /// names that are not available std::unique_ptr readEntry(const std::string& name, const unsigned entry, - const std::vector& collsToRead = {}); + const std::vector& collsToRead); + + std::unique_ptr readEntry(const std::string& name, const unsigned entry, + const podio::ReadOptions& readOptions = {}); /// Get the names of all the available Frame categories in the current file(s). /// diff --git a/include/podio/ROOTReader.h b/include/podio/ROOTReader.h index ab945bd22..f5d9a7702 100644 --- a/include/podio/ROOTReader.h +++ b/include/podio/ROOTReader.h @@ -2,6 +2,7 @@ #define PODIO_ROOTREADER_H #include "podio/ROOTFrameData.h" +#include "podio/ReadOptions.h" #include "podio/podioVersion.h" #include "podio/utilities/DatamodelRegistryIOHelpers.h" #include "podio/utilities/RootHelpers.h" @@ -89,7 +90,10 @@ class ROOTReader { /// @throws std::invalid_argument in case collsToRead contains collection /// names that are not available std::unique_ptr readNextEntry(const std::string& name, - const std::vector& collsToRead = {}); + const std::vector& collsToRead); + + std::unique_ptr readNextEntry(const std::string& name, + const podio::ReadOptions& readOptions = {}); /// Read the desired data entry for a given category. /// @@ -104,7 +108,10 @@ class ROOTReader { /// @throws std::invalid_argument in case collsToRead contains collection /// names that are not available std::unique_ptr readEntry(const std::string& name, const unsigned entry, - const std::vector& collsToRead = {}); + const std::vector& collsToRead); + + std::unique_ptr readEntry(const std::string& name, const unsigned entry, + const podio::ReadOptions& collsToRead = {}); /// Get the number of entries for the given name /// @@ -192,7 +199,7 @@ class ROOTReader { /// counter afterwards. In case the requested entry is larger than the /// available number of entries, return a nullptr. std::unique_ptr readEntry(ROOTReader::CategoryInfo& catInfo, - const std::vector& collsToRead); + const podio::ReadOptions& readOptions); /// Get / read the buffers at index iColl in the passed category information std::optional getCollectionBuffers(CategoryInfo& catInfo, size_t iColl, diff --git a/include/podio/ReadOptions.h b/include/podio/ReadOptions.h new file mode 100644 index 000000000..86583d4b9 --- /dev/null +++ b/include/podio/ReadOptions.h @@ -0,0 +1,14 @@ +#ifndef PODIO_READOPTIONS_H +#define PODIO_READOPTIONS_H + +#include +#include + +namespace podio { +struct ReadOptions { + std::vector collsToRead{}; + bool skipUnredable{false}; +}; +} // namespace podio + +#endif diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 28bed41ee..6a959cbef 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -3,6 +3,7 @@ #include "podio/CollectionBuffers.h" #include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" +#include "podio/ReadOptions.h" #include "podio/utilities/RootHelpers.h" #include "rootUtils.h" @@ -141,11 +142,24 @@ std::vector RNTupleReader::getAvailableCategories() const { std::unique_ptr RNTupleReader::readNextEntry(const std::string& name, const std::vector& collsToRead) { - return readEntry(name, m_entries[name], collsToRead); + return readEntry(name, m_entries[name], {collsToRead, false}); +} + +std::unique_ptr RNTupleReader::readNextEntry(const std::string& name, + const podio::ReadOptions& readOptions) { + return readEntry(name, m_entries[name], readOptions); } std::unique_ptr RNTupleReader::readEntry(const std::string& category, const unsigned entNum, const std::vector& collsToRead) { + return readEntry(category, entNum, {collsToRead, false}); +} + +std::unique_ptr RNTupleReader::readEntry(const std::string& category, const unsigned entNum, + const podio::ReadOptions& readOptions) { + if (m_totalEntries.find(category) == m_totalEntries.end()) { + getEntries(category); + } if (entNum >= m_totalEntries[category]) { return nullptr; } @@ -158,8 +172,8 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ const auto& collInfo = m_collectionInfo[category]; // Make sure to not silently ignore non-existant but requested collections - if (!collsToRead.empty()) { - for (const auto& name : collsToRead) { + if (!readOptions.collsToRead.empty()) { + for (const auto& name : readOptions.collsToRead) { if (std::ranges::find(collInfo, name, &root_utils::CollectionWriteInfo::name) == collInfo.end()) { throw std::invalid_argument(name + " is not available from Frame"); } @@ -183,7 +197,8 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ const auto dentry = m_readers[category][readerIndex]->GetModel().CreateEntry(); for (const auto& coll : collInfo) { - if (!collsToRead.empty() && std::ranges::find(collsToRead, coll.name) == collsToRead.end()) { + if (!readOptions.collsToRead.empty() && + std::ranges::find(readOptions.collsToRead, coll.name) == readOptions.collsToRead.end()) { continue; } const auto& collType = coll.dataType; @@ -193,9 +208,13 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ if (!maybeBuffers) { std::cout << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType << " and schema version " << coll.schemaVersion << std::endl; - continue; + if (readOptions.skipUnredable) { + continue; + } else { + return nullptr; + } } - const auto collBuffers = maybeBuffers.value(); + const auto& collBuffers = maybeBuffers.value(); if (coll.isSubset) { const auto brName = root_utils::subsetBranch(coll.name); diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index c8c29ea7c..ddd16ec54 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -5,6 +5,7 @@ #include "podio/CollectionIDTable.h" #include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" +#include "podio/ReadOptions.h" #include "podio/podioVersion.h" #include "podio/utilities/RootHelpers.h" #include "rootUtils.h" @@ -87,19 +88,29 @@ GenericParameters ROOTReader::readEntryParameters(ROOTReader::CategoryInfo& catI std::unique_ptr ROOTReader::readNextEntry(const std::string& name, const std::vector& collsToRead) { + return readNextEntry(name, {collsToRead, false}); +} + +std::unique_ptr ROOTReader::readNextEntry(const std::string& name, + const podio::ReadOptions& readOptions) { auto& catInfo = getCategoryInfo(name); - return readEntry(catInfo, collsToRead); + return readEntry(catInfo, readOptions); } std::unique_ptr ROOTReader::readEntry(const std::string& name, const unsigned entNum, const std::vector& collsToRead) { + return readEntry(name, entNum, {collsToRead, false}); +} + +std::unique_ptr ROOTReader::readEntry(const std::string& name, const unsigned entNum, + const podio::ReadOptions& readOptions) { auto& catInfo = getCategoryInfo(name); catInfo.entry = entNum; - return readEntry(catInfo, collsToRead); + return readEntry(catInfo, readOptions); } std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& catInfo, - const std::vector& collsToRead) { + const podio::ReadOptions& readOptions) { if (!catInfo.chain) { return nullptr; } @@ -108,8 +119,8 @@ std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& c } // Make sure to not silently ignore non-existant but requested collections - if (!collsToRead.empty()) { - for (const auto& name : collsToRead) { + if (!readOptions.collsToRead.empty()) { + for (const auto& name : readOptions.collsToRead) { if (std::ranges::find(catInfo.storedClasses, name, &detail::NamedCollInfo::name) == catInfo.storedClasses.end()) { throw std::invalid_argument(name + " is not available from Frame"); } @@ -128,14 +139,19 @@ std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& c ROOTFrameData::BufferMap buffers; for (size_t i = 0; i < catInfo.storedClasses.size(); ++i) { - if (!collsToRead.empty() && std::ranges::find(collsToRead, catInfo.storedClasses[i].name) == collsToRead.end()) { + if (!readOptions.collsToRead.empty() && + std::ranges::find(readOptions.collsToRead, catInfo.storedClasses[i].name) == readOptions.collsToRead.end()) { continue; } auto collBuffers = getCollectionBuffers(catInfo, i, reloadBranches, localEntry); if (!collBuffers) { std::cerr << "WARNING: Could not get buffers for collection " << catInfo.storedClasses[i].name << "(type=" << std::get(catInfo.storedClasses[i].info) << ") from file" << std::endl; - continue; + if (readOptions.skipUnredable) { + continue; + } else { + return nullptr; + } } buffers.emplace(catInfo.storedClasses[i].name, std::move(collBuffers.value())); } From 9da1e88de9be4259b5d25432fccca2a31d4c4b83 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 12:50:07 +0200 Subject: [PATCH 05/17] Add some useful static constructors for good defaults --- include/podio/ReadOptions.h | 8 ++++++++ tests/schema_evolution/code_gen/check_base.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/podio/ReadOptions.h b/include/podio/ReadOptions.h index 86583d4b9..9513ab155 100644 --- a/include/podio/ReadOptions.h +++ b/include/podio/ReadOptions.h @@ -8,6 +8,14 @@ namespace podio { struct ReadOptions { std::vector collsToRead{}; bool skipUnredable{false}; + + inline static ReadOptions SkipUnreadable() { + return {{}, true}; + } + + inline static ReadOptions Only(std::vector collections) { + return {std::move(collections), false}; + } }; } // namespace podio diff --git a/tests/schema_evolution/code_gen/check_base.h b/tests/schema_evolution/code_gen/check_base.h index 0f417b278..c4ce085d5 100644 --- a/tests/schema_evolution/code_gen/check_base.h +++ b/tests/schema_evolution/code_gen/check_base.h @@ -79,7 +79,7 @@ using ReaderT = podio::ROOTReader; {{auto reader = READER_TYPE(); \ std::cout << "Reading file " << FILENAME << std::endl; \ reader.openFile(FILENAME); \ - auto event = podio::Frame(reader.readEntry(podio::Category::Event, 0)); \ + auto event = podio::Frame(reader.readEntry(podio::Category::Event, 0, podio::ReadOptions::SkipUnreadable())); \ {__VA_ARGS__}; \ return 0; \ } \ From 29eeaf6d64995bd2a21e311e03a5b34232c2ad3d Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 13:46:47 +0200 Subject: [PATCH 06/17] Add ReadOptions to SIOReader --- include/podio/SIOReader.h | 11 +++++++++-- src/SIOReader.cc | 20 +++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/include/podio/SIOReader.h b/include/podio/SIOReader.h index a081dbb82..b23c65250 100644 --- a/include/podio/SIOReader.h +++ b/include/podio/SIOReader.h @@ -1,6 +1,7 @@ #ifndef PODIO_SIOREADER_H #define PODIO_SIOREADER_H +#include "podio/ReadOptions.h" #include "podio/SIOBlock.h" #include "podio/SIOFrameData.h" #include "podio/podioVersion.h" @@ -51,7 +52,10 @@ class SIOReader { /// category exists and if there are still entries left to read. /// Otherwise a nullptr std::unique_ptr readNextEntry(const std::string& name, - const std::vector& collsToRead = {}); + const std::vector& collsToRead); + + std::unique_ptr readNextEntry(const std::string& name, + const podio::ReadOptions& readOptions = {}); /// Read the desired data entry for a given category. /// @@ -69,7 +73,10 @@ class SIOReader { /// @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, - const std::vector& collsToRead = {}); + const std::vector& collsToRead); + + std::unique_ptr readEntry(const std::string& name, const unsigned entry, + const podio::ReadOptions& = {}); /// Get the number of entries for the given name /// diff --git a/src/SIOReader.cc b/src/SIOReader.cc index b7c4a420c..96dd0c068 100644 --- a/src/SIOReader.cc +++ b/src/SIOReader.cc @@ -1,5 +1,6 @@ #include "podio/SIOReader.h" +#include "podio/ReadOptions.h" #include "sioUtils.h" #include @@ -26,8 +27,7 @@ void SIOReader::openFile(const std::string& filename) { readEDMDefinitions(); // Potentially could do this lazily } -std::unique_ptr SIOReader::readNextEntry(const std::string& name, - const std::vector& collsToRead) { +std::unique_ptr SIOReader::readNextEntry(const std::string& name, const podio::ReadOptions& readOptions) { // Skip to where the next record of this name starts in the file, based on // how many times we have already read this name // @@ -45,15 +45,25 @@ std::unique_ptr SIOReader::readNextEntry(const std::string& name, m_nameCtr[name]++; return std::make_unique(std::move(dataBuffer), dataInfo._uncompressed_length, std::move(tableBuffer), - tableInfo._uncompressed_length, collsToRead); + tableInfo._uncompressed_length, readOptions.collsToRead); +} + +std::unique_ptr SIOReader::readNextEntry(const std::string& name, + const std::vector& collsToRead) { + return readNextEntry(name, podio::ReadOptions::Only(collsToRead)); } std::unique_ptr SIOReader::readEntry(const std::string& name, const unsigned entry, - const std::vector& collsToRead) { + const podio::ReadOptions& readOptions) { // NOTE: Will create or overwrite the entry counter // All checks are done in the following function m_nameCtr[name] = entry; - return readNextEntry(name, collsToRead); + return readNextEntry(name, readOptions); +} + +std::unique_ptr SIOReader::readEntry(const std::string& name, const unsigned entry, + const std::vector& collsToRead) { + return readEntry(name, entry, podio::ReadOptions::Only(collsToRead)); } std::vector SIOReader::getAvailableCategories() const { From 4606eecedd0469661e16f5a8c20c914e5207cd03 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 13:46:55 +0200 Subject: [PATCH 07/17] Make Reader interface understand ReadOptions --- include/podio/Reader.h | 46 ++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/include/podio/Reader.h b/include/podio/Reader.h index b7aa3235a..a8fbbd6b0 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -2,6 +2,7 @@ #define PODIO_READER_H #include "podio/Frame.h" +#include "podio/ReadOptions.h" #include "podio/podioVersion.h" namespace podio { @@ -22,8 +23,8 @@ class Reader { struct ReaderConcept { virtual ~ReaderConcept() = default; - virtual podio::Frame readNextFrame(const std::string& name, const std::vector&) = 0; - virtual podio::Frame readFrame(const std::string& name, size_t index, const std::vector&) = 0; + virtual podio::Frame readNextFrame(const std::string& name, const podio::ReadOptions& readOptions) = 0; + virtual podio::Frame readFrame(const std::string& name, size_t index, const podio::ReadOptions& readOptions) = 0; virtual size_t getEntries(const std::string& name) const = 0; virtual podio::version::Version currentFileVersion() const = 0; virtual std::optional currentFileVersion(const std::string& name) const = 0; @@ -44,17 +45,16 @@ class Reader { ~ReaderModel() override = default; - podio::Frame readNextFrame(const std::string& name, const std::vector& collsToRead) override { - auto maybeFrame = m_reader->readNextEntry(name, collsToRead); + podio::Frame readNextFrame(const std::string& name, const podio::ReadOptions& readOptions) override { + auto maybeFrame = m_reader->readNextEntry(name, readOptions); if (maybeFrame) { return maybeFrame; } throw std::runtime_error("Failed reading category " + name + " (reading beyond bounds?)"); } - podio::Frame readFrame(const std::string& name, size_t index, - const std::vector& collsToRead) override { - auto maybeFrame = m_reader->readEntry(name, index, collsToRead); + podio::Frame readFrame(const std::string& name, size_t index, const podio::ReadOptions& readOptions) override { + auto maybeFrame = m_reader->readEntry(name, index, readOptions); if (maybeFrame) { return maybeFrame; } @@ -113,8 +113,12 @@ class Reader { /// /// @throws std::invalid_argument in case the category is not available or in /// case no more entries are available - podio::Frame readNextFrame(const std::string& name, const std::vector& collsToRead = {}) { - return m_self->readNextFrame(name, collsToRead); + podio::Frame readNextFrame(const std::string& name, const std::vector& collsToRead) { + return m_self->readNextFrame(name, podio::ReadOptions::Only(collsToRead)); + } + + podio::Frame readNextFrame(const std::string& name, const podio::ReadOptions& readOptions = {}) { + return m_self->readNextFrame(name, readOptions); } /// Read the next frame of the "events" category @@ -125,8 +129,12 @@ class Reader { /// @returns A fully constructed Frame with the contents read from file /// /// @throws std::invalid_argument in case no (more) events are available - podio::Frame readNextEvent(const std::vector& collsToRead = {}) { - return readNextFrame(podio::Category::Event, collsToRead); + podio::Frame readNextEvent(const std::vector& collsToRead) { + return readNextFrame(podio::Category::Event, podio::ReadOptions::Only(collsToRead)); + } + + podio::Frame readNextEvent(const podio::ReadOptions& readOptions = {}) { + return readNextFrame(podio::Category::Event, readOptions); } /// Read a specific frame for a given category @@ -140,8 +148,12 @@ class Reader { /// /// @throws std::invalid_argument in case the category is not available or in /// case the specified entry is not available - podio::Frame readFrame(const std::string& name, size_t index, const std::vector& collsToRead = {}) { - return m_self->readFrame(name, index, collsToRead); + podio::Frame readFrame(const std::string& name, size_t index, const std::vector& collsToRead) { + return m_self->readFrame(name, index, podio::ReadOptions::Only(collsToRead)); + } + + podio::Frame readFrame(const std::string& name, size_t index, const podio::ReadOptions& readOptions = {}) { + return m_self->readFrame(name, index, readOptions); } /// Read a specific frame of the "events" category @@ -153,8 +165,12 @@ class Reader { /// @returns A fully constructed Frame with the contents read from file /// /// @throws std::invalid_argument in case the desired event is not available - podio::Frame readEvent(size_t index, const std::vector& collsToRead = {}) { - return readFrame(podio::Category::Event, index, collsToRead); + podio::Frame readEvent(size_t index, const std::vector& collsToRead) { + return readFrame(podio::Category::Event, index, podio::ReadOptions::Only(collsToRead)); + } + + podio::Frame readEvent(size_t index, const podio::ReadOptions& readOptions = {}) { + return readFrame(podio::Category::Event, index, readOptions); } /// Get the number of entries for the given name From b0fb666cbbccbe5864ff89543f6eb052cba3accb Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 13:47:13 +0200 Subject: [PATCH 08/17] Make podio-dump be permissive by default --- tools/src/podio-dump-tool.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/src/podio-dump-tool.cpp b/tools/src/podio-dump-tool.cpp index 09d26cb67..ba8fc4b5e 100644 --- a/tools/src/podio-dump-tool.cpp +++ b/tools/src/podio-dump-tool.cpp @@ -1,4 +1,5 @@ #include "argparseUtils.h" +#include "podio/ReadOptions.h" #include "tabulate.h" #include "podio/Frame.h" @@ -258,7 +259,7 @@ int main(int argc, char* argv[]) { for (const auto event : args.events) { try { - const auto& frame = reader.readFrame(args.category, event); + const auto& frame = reader.readFrame(args.category, event, podio::ReadOptions::SkipUnreadable()); printFrame(frame, args.category, event, args.detailed); } catch (std::runtime_error& err) { fmt::println(stderr, "{}", err.what()); From 67637092a318999f5907a665304dabb6e0b3605c Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 14:30:04 +0200 Subject: [PATCH 09/17] Fix typo in member name --- include/podio/ReadOptions.h | 2 +- src/RNTupleReader.cc | 2 +- src/ROOTReader.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/podio/ReadOptions.h b/include/podio/ReadOptions.h index 9513ab155..e04b84526 100644 --- a/include/podio/ReadOptions.h +++ b/include/podio/ReadOptions.h @@ -7,7 +7,7 @@ namespace podio { struct ReadOptions { std::vector collsToRead{}; - bool skipUnredable{false}; + bool skipUnreadable{false}; inline static ReadOptions SkipUnreadable() { return {{}, true}; diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 6a959cbef..7bbf2d8fc 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -208,7 +208,7 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ if (!maybeBuffers) { std::cout << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType << " and schema version " << coll.schemaVersion << std::endl; - if (readOptions.skipUnredable) { + if (readOptions.skipUnreadable) { continue; } else { return nullptr; diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index ddd16ec54..deb9b664c 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -147,7 +147,7 @@ std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& c if (!collBuffers) { std::cerr << "WARNING: Could not get buffers for collection " << catInfo.storedClasses[i].name << "(type=" << std::get(catInfo.storedClasses[i].info) << ") from file" << std::endl; - if (readOptions.skipUnredable) { + if (readOptions.skipUnreadable) { continue; } else { return nullptr; From dfc80eaa210e2c1b61365af8b320795ea8be44c9 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 14:30:31 +0200 Subject: [PATCH 10/17] Ensure header is installed --- src/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 638dfcb59..0de5caf9c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -75,6 +75,7 @@ SET(core_headers ${PROJECT_SOURCE_DIR}/include/podio/GenericParameters.h ${PROJECT_SOURCE_DIR}/include/podio/LinkCollection.h ${PROJECT_SOURCE_DIR}/include/podio/utilities/Glob.h + ${PROJECT_SOURCE_DIR}/include/podio/ReadOptions.h ) PODIO_ADD_LIB_AND_DICT(podio "${core_headers}" "${core_sources}" selection.xml) From 93b1b91319d5052e607b332e411e16eebd7a5c38 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 14:30:40 +0200 Subject: [PATCH 11/17] Expose ReadOptions to python bindings --- python/podio/__init__.py | 13 ++++++++++++- src/selection.xml | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/python/podio/__init__.py b/python/podio/__init__.py index 147dc96c0..3bd353bd0 100644 --- a/python/podio/__init__.py +++ b/python/podio/__init__.py @@ -27,4 +27,15 @@ except ImportError: pass -__all__ = ["__version__", "Frame", "root_io", "sio_io", "reading", "data_source", "version"] +ReadOptions = podio.ReadOptions + +__all__ = [ + "__version__", + "Frame", + "root_io", + "sio_io", + "reading", + "data_source", + "version", + "ReadOptions", +] diff --git a/src/selection.xml b/src/selection.xml index c3541060f..a19b7db4c 100644 --- a/src/selection.xml +++ b/src/selection.xml @@ -49,5 +49,7 @@ + + From 88762b8a48c0a30c31cc70d4930074b0cf728491 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 14:51:11 +0200 Subject: [PATCH 12/17] Skip types that have changed enough to become unreadable --- src/RNTupleReader.cc | 44 ++++++++++++++++++++++++++------------------ src/ROOTReader.cc | 4 +++- src/rootUtils.h | 10 +++++++++- 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 7bbf2d8fc..4868085c3 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -216,27 +216,35 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ } const auto& collBuffers = maybeBuffers.value(); - if (coll.isSubset) { - const auto brName = root_utils::subsetBranch(coll.name); - const auto vec = new std::vector; - dentry->BindRawPtr(brName, vec); - collBuffers.references->at(0) = std::unique_ptr>(vec); - } else { - dentry->BindRawPtr(coll.name, collBuffers.data); - - const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(collType); - for (size_t j = 0; j < relVecNames.relations.size(); ++j) { - const auto relName = relVecNames.relations[j]; + try { + if (coll.isSubset) { + const auto brName = root_utils::subsetBranch(coll.name); const auto vec = new std::vector; - const auto brName = root_utils::refBranch(coll.name, relName); dentry->BindRawPtr(brName, vec); - collBuffers.references->at(j) = std::unique_ptr>(vec); + collBuffers.references->at(0) = std::unique_ptr>(vec); + } else { + dentry->BindRawPtr(coll.name, collBuffers.data); + + const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(collType); + for (size_t j = 0; j < relVecNames.relations.size(); ++j) { + const auto relName = relVecNames.relations[j]; + const auto vec = new std::vector; + const auto brName = root_utils::refBranch(coll.name, relName); + dentry->BindRawPtr(brName, vec); + collBuffers.references->at(j) = std::unique_ptr>(vec); + } + + for (size_t j = 0; j < relVecNames.vectorMembers.size(); ++j) { + const auto vecName = relVecNames.vectorMembers[j]; + const auto brName = root_utils::vecBranch(coll.name, vecName); + dentry->BindRawPtr(brName, collBuffers.vectorMembers->at(j).second); + } } - - for (size_t j = 0; j < relVecNames.vectorMembers.size(); ++j) { - const auto vecName = relVecNames.vectorMembers[j]; - const auto brName = root_utils::vecBranch(coll.name, vecName); - dentry->BindRawPtr(brName, collBuffers.vectorMembers->at(j).second); + } catch (const RException&) { + if (readOptions.skipUnreadable) { + continue; + } else { + return nullptr; } } diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index deb9b664c..d2ba88570 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -183,7 +183,9 @@ std::optional ROOTReader::getCollectionBuffers(ROO } // set the addresses and read the data - root_utils::setCollectionAddresses(collBuffers, branches); + if (!root_utils::setCollectionAddresses(collBuffers, branches)) { + return std::nullopt; + } root_utils::readBranchesData(branches, localEntry); collBuffers.recast(collBuffers); diff --git a/src/rootUtils.h b/src/rootUtils.h index d427c4d33..638e4fc11 100644 --- a/src/rootUtils.h +++ b/src/rootUtils.h @@ -207,7 +207,7 @@ inline void resetBranches(TTree* chain, CollectionBranches& branches, const std: } template -inline void setCollectionAddresses(const BufferT& collBuffers, const CollectionBranches& branches) { +inline bool setCollectionAddresses(const BufferT& collBuffers, const CollectionBranches& branches) { if (const auto buffer = collBuffers.data) { branches.data->SetAddress(buffer); @@ -215,15 +215,23 @@ inline void setCollectionAddresses(const BufferT& collBuffers, const CollectionB if (const auto refCollections = collBuffers.references) { for (size_t i = 0; i < refCollections->size(); ++i) { + if (!branches.refs[i]) { + return false; + } branches.refs[i]->SetAddress(&(*refCollections)[i]); } } if (const auto vecMembers = collBuffers.vectorMembers) { for (size_t i = 0; i < vecMembers->size(); ++i) { + if (!branches.vecs[i]) { + return false; + } branches.vecs[i]->SetAddress((*vecMembers)[i].second); } } + + return true; } inline void readBranchesData(const CollectionBranches& branches, Long64_t entry) { From 87ce6329b78e9903e71d64ad1538f2894c22016d Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 15:41:03 +0200 Subject: [PATCH 13/17] Add docstrings for the new overloads and the options --- include/podio/RNTupleReader.h | 24 ++++++++++++++++++++++ include/podio/ROOTReader.h | 26 +++++++++++++++++++++++- include/podio/ReadOptions.h | 7 +++++++ include/podio/Reader.h | 38 +++++++++++++++++++++++++++++++++++ include/podio/SIOReader.h | 32 ++++++++++++++++++++++++++++- 5 files changed, 125 insertions(+), 2 deletions(-) diff --git a/include/podio/RNTupleReader.h b/include/podio/RNTupleReader.h index dd73fd700..4b6e66b6f 100644 --- a/include/podio/RNTupleReader.h +++ b/include/podio/RNTupleReader.h @@ -78,6 +78,18 @@ class RNTupleReader { std::unique_ptr readNextEntry(const std::string& name, const std::vector& collsToRead); + /// Read the next data entry for a given category. + /// + /// @param name The category name for which to read the next entry + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @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 + /// + /// @throws std::invalid_argument in case readOptions.collsToRead contains collection + /// names that are not available std::unique_ptr readNextEntry(const std::string& name, const podio::ReadOptions& readOptions = {}); /// Read the desired data entry for a given category. @@ -95,6 +107,18 @@ class RNTupleReader { std::unique_ptr readEntry(const std::string& name, const unsigned entry, const std::vector& collsToRead); + /// 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 + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @returns FrameData from which a podio::Frame can be constructed if the + /// category and the desired entry exist. Otherwise a nullptr + /// + /// @throws std::invalid_argument in case readOptions.collsToRead contains collection + /// names that are not available std::unique_ptr readEntry(const std::string& name, const unsigned entry, const podio::ReadOptions& readOptions = {}); diff --git a/include/podio/ROOTReader.h b/include/podio/ROOTReader.h index f5d9a7702..a1465b778 100644 --- a/include/podio/ROOTReader.h +++ b/include/podio/ROOTReader.h @@ -92,6 +92,18 @@ class ROOTReader { std::unique_ptr readNextEntry(const std::string& name, const std::vector& collsToRead); + /// Read the next data entry for a given category. + /// + /// @param name The category name for which to read the next entry + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @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 + /// + /// @throws std::invalid_argument in case readOptions.collsToRead contains collection + /// names that are not available std::unique_ptr readNextEntry(const std::string& name, const podio::ReadOptions& readOptions = {}); @@ -110,8 +122,20 @@ class ROOTReader { std::unique_ptr readEntry(const std::string& name, const unsigned entry, const std::vector& collsToRead); + /// 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 + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @returns FrameData from which a podio::Frame can be constructed if the + /// category and the desired entry exist. Otherwise a nullptr + /// + /// @throws std::invalid_argument in case readOptions.collsToRead contains collection + /// names that are not available std::unique_ptr readEntry(const std::string& name, const unsigned entry, - const podio::ReadOptions& collsToRead = {}); + const podio::ReadOptions& readOptions = {}); /// Get the number of entries for the given name /// diff --git a/include/podio/ReadOptions.h b/include/podio/ReadOptions.h index e04b84526..a004e84ca 100644 --- a/include/podio/ReadOptions.h +++ b/include/podio/ReadOptions.h @@ -5,14 +5,21 @@ #include namespace podio { + +/// Options for configuring read operations struct ReadOptions { + /// Collections to read. If empty, all collections will be read std::vector collsToRead{}; + /// Whether to skip collections that cannot be read bool skipUnreadable{false}; + /// Create ReadOptions that skip unreadable collections inline static ReadOptions SkipUnreadable() { return {{}, true}; } + /// Create ReadOptions that only read specific collections + /// @param collections list of collection names to read inline static ReadOptions Only(std::vector collections) { return {std::move(collections), false}; } diff --git a/include/podio/Reader.h b/include/podio/Reader.h index a8fbbd6b0..c787df83e 100644 --- a/include/podio/Reader.h +++ b/include/podio/Reader.h @@ -117,6 +117,16 @@ class Reader { return m_self->readNextFrame(name, podio::ReadOptions::Only(collsToRead)); } + /// Read the next frame of a given category + /// + /// @param name The category name for which to read the next frame + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @returns A fully constructed Frame with the contents read from file + /// + /// @throws std::invalid_argument in case the category is not available or in + /// case no more entries are available podio::Frame readNextFrame(const std::string& name, const podio::ReadOptions& readOptions = {}) { return m_self->readNextFrame(name, readOptions); } @@ -133,6 +143,14 @@ class Reader { return readNextFrame(podio::Category::Event, podio::ReadOptions::Only(collsToRead)); } + /// Read the next frame of the "events" category + /// + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @returns A fully constructed Frame with the contents read from file + /// + /// @throws std::invalid_argument in case no (more) events are available podio::Frame readNextEvent(const podio::ReadOptions& readOptions = {}) { return readNextFrame(podio::Category::Event, readOptions); } @@ -152,6 +170,17 @@ class Reader { return m_self->readFrame(name, index, podio::ReadOptions::Only(collsToRead)); } + /// Read a specific frame for a given category + /// + /// @param name The category name for which to read the next entry + /// @param index The entry number to read + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @returns A fully constructed Frame with the contents read from file + /// + /// @throws std::invalid_argument in case the category is not available or in + /// case the specified entry is not available podio::Frame readFrame(const std::string& name, size_t index, const podio::ReadOptions& readOptions = {}) { return m_self->readFrame(name, index, readOptions); } @@ -169,6 +198,15 @@ class Reader { return readFrame(podio::Category::Event, index, podio::ReadOptions::Only(collsToRead)); } + /// Read a specific frame of the "events" category + /// + /// @param index The event number to read + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @returns A fully constructed Frame with the contents read from file + /// + /// @throws std::invalid_argument in case the desired event is not available podio::Frame readEvent(size_t index, const podio::ReadOptions& readOptions = {}) { return readFrame(podio::Category::Event, index, readOptions); } diff --git a/include/podio/SIOReader.h b/include/podio/SIOReader.h index b23c65250..33e79779c 100644 --- a/include/podio/SIOReader.h +++ b/include/podio/SIOReader.h @@ -54,6 +54,21 @@ class SIOReader { std::unique_ptr readNextEntry(const std::string& name, const std::vector& collsToRead); + /// Read the next data entry for a given category. + /// + /// @note Given how the SIO files are currently laid out it is in fact not + /// possible to only read a subset of a Frame. Rather the subset of + /// collections to read will be an artificial limit on the returned + /// SIOFrameData. Limiting the collections to read will not improve I/O + /// performance. + /// + /// @param name The category name for which to read the next entry + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @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, const podio::ReadOptions& readOptions = {}); @@ -75,8 +90,23 @@ class SIOReader { std::unique_ptr readEntry(const std::string& name, const unsigned entry, const std::vector& collsToRead); + /// Read the desired data entry for a given category. + /// + /// @note Given how the SIO files are currently laid out it is in fact not + /// possible to only read a subset of a Frame. Rather the subset of + /// collections to read will be an artificial limit on the returned + /// SIOFrameData. Limiting the collections to read will not improve I/O + /// performance. + /// + /// @param name The category name for which to read the next entry + /// @param entry The entry number to read + /// @param readOptions Options for configuring the read operation, including + /// which collections to read and whether to skip unreadable ones + /// + /// @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, - const podio::ReadOptions& = {}); + const podio::ReadOptions& readOptions = {}); /// Get the number of entries for the given name /// From 8fa0bca78b06f8b582865a1ac15bc363bddb81d5 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 17:19:09 +0200 Subject: [PATCH 14/17] Skip one python unittest for old ROOT versions --- python/podio/test_Reader.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/python/podio/test_Reader.py b/python/podio/test_Reader.py index 24fb39a06..ed2f989ed 100644 --- a/python/podio/test_Reader.py +++ b/python/podio/test_Reader.py @@ -1,7 +1,15 @@ #!/usr/bin/env python3 """Unit tests for podio readers""" -from podio.version import build_version +import unittest + +from ROOT import __version__ as root_version + +from podio.version import build_version, parse + +# We are only interested in the major and minor version and splitting on '/' +# allows us to make this works over the switch in versioning format in ROOT +ROOT_VERSION = parse(root_version.split("/")[0]) class ReaderTestCaseMixin: @@ -96,6 +104,10 @@ def test_limited_collections(self): event = self.reader.get("events", ["hits", "info", "links"])[0] self.assertEqual(set(event.getAvailableCollections()), {"hits", "info", "links"}) + @unittest.skipIf( + ROOT_VERSION <= parse("6.30"), + "cppyy version shipped with ROOT fails on overload resolution", + ) def test_invalid_limited_collections(self): """Ensure that requesting non existant collections raises a value error""" with self.assertRaises(ValueError): From a5c39e8dec457f1a456036b92a9e21216bfb1e60 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 17:53:43 +0200 Subject: [PATCH 15/17] Harmonize warnings between TTree and RNTuple reading --- src/RNTupleReader.cc | 2 +- src/ROOTReader.cc | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 4868085c3..faf4d6a65 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -206,7 +206,7 @@ std::unique_ptr RNTupleReader::readEntry(const std::string& categ const auto maybeBuffers = bufferFactory.createBuffers(collType, coll.schemaVersion, coll.isSubset); if (!maybeBuffers) { - std::cout << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType + std::cerr << "WARNING: Buffers couldn't be created for collection " << coll.name << " of type " << coll.dataType << " and schema version " << coll.schemaVersion << std::endl; if (readOptions.skipUnreadable) { continue; diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index d2ba88570..db2c48ae8 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -145,8 +145,9 @@ std::unique_ptr ROOTReader::readEntry(ROOTReader::CategoryInfo& c } auto collBuffers = getCollectionBuffers(catInfo, i, reloadBranches, localEntry); if (!collBuffers) { - std::cerr << "WARNING: Could not get buffers for collection " << catInfo.storedClasses[i].name - << "(type=" << std::get(catInfo.storedClasses[i].info) << ") from file" << std::endl; + std::cerr << "WARNING: Buffers couldn't be created for collection " << catInfo.storedClasses[i].name + << " of type " << std::get(catInfo.storedClasses[i].info) << " and schema version " + << std::get<2>(catInfo.storedClasses[i].info) << std::endl; if (readOptions.skipUnreadable) { continue; } else { From 531a6bd11097cade6c4850deb4f428c9d229e9cc Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 20 Oct 2025 19:23:25 +0200 Subject: [PATCH 16/17] Make it possible to check non supported evolution behavior --- .../schema_evolution/code_gen/CMakeLists.txt | 4 ++ tests/schema_evolution/code_gen/README.md | 6 +++ .../datatypes_rename_relation/check.cpp | 34 +++++++++++++ .../datatypes_rename_relation/new.yaml | 22 +++++++++ .../datatypes_rename_relation/old.yaml | 22 +++++++++ .../code_gen/test_utilities.cmake | 48 +++++++++++++------ 6 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 tests/schema_evolution/code_gen/datatypes_rename_relation/check.cpp create mode 100644 tests/schema_evolution/code_gen/datatypes_rename_relation/new.yaml create mode 100644 tests/schema_evolution/code_gen/datatypes_rename_relation/old.yaml diff --git a/tests/schema_evolution/code_gen/CMakeLists.txt b/tests/schema_evolution/code_gen/CMakeLists.txt index 8061785c0..85a127d9c 100644 --- a/tests/schema_evolution/code_gen/CMakeLists.txt +++ b/tests/schema_evolution/code_gen/CMakeLists.txt @@ -25,6 +25,8 @@ ADD_SCHEMA_EVOLUTION_TEST(datatypes_rename_member WITH_EVOLUTION) ADD_SCHEMA_EVOLUTION_TEST(array_component_new_member) ADD_SCHEMA_EVOLUTION_TEST(datatypes_remove_type) +ADD_SCHEMA_EVOLUTION_TEST(datatypes_rename_relation NO_EVOLUTION_CHECKS) + if(ENABLE_RNTUPLE) ADD_SCHEMA_EVOLUTION_TEST(no_change RNTUPLE NO_GENERATE_MODELS) endif() @@ -39,6 +41,8 @@ if(ENABLE_RNTUPLE AND ROOT_VERSION VERSION_GREATER_EQUAL 6.36) ADD_SCHEMA_EVOLUTION_TEST(components_new_member RNTUPLE NO_GENERATE_MODELS) ADD_SCHEMA_EVOLUTION_TEST(array_component_new_member RNTUPLE NO_GENERATE_MODELS) ADD_SCHEMA_EVOLUTION_TEST(datatypes_remove_type RNTUPLE NO_GENERATE_MODELS) + + ADD_SCHEMA_EVOLUTION_TEST(datatypes_rename_relation RNTUPLE NO_GENERATE_MODELS) endif() # These do not yet work. diff --git a/tests/schema_evolution/code_gen/README.md b/tests/schema_evolution/code_gen/README.md index 43cf76ce8..3aea99e32 100644 --- a/tests/schema_evolution/code_gen/README.md +++ b/tests/schema_evolution/code_gen/README.md @@ -27,6 +27,12 @@ If the `RNTUPLE` argument / flag is passed to `ADD_SCHEMA_EVOLUTION_TEST` the test will use the `RNTuple{Writer,Reader}` instead of the default `ROOT{Writer,Reader}`. +If the `NO_EVOLUTION_CHECKS` argument / flag is passed the code generation for +the new model will not take into account the old model yaml defintion. Hence, +there will be no checks on whether the evolution is actually supported at the +moment. This makes it possible to write checks that ensure certain behavior even +if the evolution is not supported. + To avoid re-generating the datamodel again for the same check with different backends, the `NO_GENERATE_MODELS` flag can be passed to `ADD_SCHEMA_EVOLUTION_TEST`. diff --git a/tests/schema_evolution/code_gen/datatypes_rename_relation/check.cpp b/tests/schema_evolution/code_gen/datatypes_rename_relation/check.cpp new file mode 100644 index 000000000..89944a37c --- /dev/null +++ b/tests/schema_evolution/code_gen/datatypes_rename_relation/check.cpp @@ -0,0 +1,34 @@ +#include "datamodel/ExampleClusterCollection.h" +#include "datamodel/ExampleHitCollection.h" + +#include "check_base.h" + +int main() { + WRITE_WITH(WriterT, (TEST_CASE FILE_SUFFIX), { + auto hits = ExampleHitCollection(); + auto hit = hits.create(); + hit.x(1.23); + hit.y(2.34); + hit.z(3.45); + hit.energy(5.67); + + auto clusters = ExampleClusterCollection(); + auto cluster = clusters.create(); + cluster.addoldHits(hit); + + event.put(std::move(hits), "related_collection"); + event.put(std::move(clusters), "evolution_collection"); + }) + + READ_WITH(ReaderT, (TEST_CASE FILE_SUFFIX), { + const auto& hitColl = event.get("related_collection"); + ASSERT_EQUAL(hitColl.hasID(), true, "Readable collection should be available"); + ASSERT_EQUAL(hitColl[0].x(), 1.23, "Readable elements should not change"); + ASSERT_EQUAL(hitColl[0].y(), 2.34, "Readable elements should not change"); + ASSERT_EQUAL(hitColl[0].z(), 3.45, "Readable elements should not change"); + ASSERT_EQUAL(hitColl[0].energy(), 5.67, "Readable elements should not change"); + + const auto& clusterColl = event.get("evolution_collection"); + ASSERT_EQUAL(clusterColl.hasID(), false, "Unreadable collection should be skipped"); + }) +} diff --git a/tests/schema_evolution/code_gen/datatypes_rename_relation/new.yaml b/tests/schema_evolution/code_gen/datatypes_rename_relation/new.yaml new file mode 100644 index 000000000..b6eee0aa4 --- /dev/null +++ b/tests/schema_evolution/code_gen/datatypes_rename_relation/new.yaml @@ -0,0 +1,22 @@ +options: + includeSubfolder: true + +schema_version: 2 + +datatypes: + ExampleHit: + Description: "Example hit" + Author: "B. Hegner" + Members: + - double x // x coordinate + - double y // y coordinate + - double z // z coordinate + - double energy // measured energy deposit + + ExampleCluster: + Description: "Cluster" + Author: "B. Hegner" + Members: + - double energy // cluster energy + OneToManyRelations: + - ExampleHit newHits // hits in this cluster (renamed from oldHits) diff --git a/tests/schema_evolution/code_gen/datatypes_rename_relation/old.yaml b/tests/schema_evolution/code_gen/datatypes_rename_relation/old.yaml new file mode 100644 index 000000000..ae4045dbc --- /dev/null +++ b/tests/schema_evolution/code_gen/datatypes_rename_relation/old.yaml @@ -0,0 +1,22 @@ +options: + includeSubfolder: true + +schema_version: 1 + +datatypes: + ExampleHit: + Description: "Example hit" + Author: "B. Hegner" + Members: + - double x // x coordinate + - double y // y coordinate + - double z // z coordinate + - double energy // measured energy deposit + + ExampleCluster: + Description: "Cluster" + Author: "B. Hegner" + Members: + - double energy // cluster energy + OneToManyRelations: + - ExampleHit oldHits // hits in this cluster diff --git a/tests/schema_evolution/code_gen/test_utilities.cmake b/tests/schema_evolution/code_gen/test_utilities.cmake index 0f8324db5..b7224023c 100644 --- a/tests/schema_evolution/code_gen/test_utilities.cmake +++ b/tests/schema_evolution/code_gen/test_utilities.cmake @@ -1,16 +1,17 @@ -#--- GENERATE_DATAMODEL(test_case model_version [WITH_EVOLUTION]) +#--- GENERATE_DATAMODEL(test_case model_version [WITH_EVOLUTION] [NO_EVOLUTION_CHECKS]) # # Arguments: -# test_case The name of the test case -# model_version which version of the model to generate (old or new) -# WITH_EVOLUTION (Optional) pass an evolution.yaml file to the generation of the model +# test_case The name of the test case +# model_version which version of the model to generate (old or new) +# WITH_EVOLUTION (Optional) pass an evolution.yaml file to the generation of the model +# NO_EVOLUTION_CHECKS (Optional) skip passing OLD_DESCRIPTION to PODIO_GENERATE_DATAMODEL # # Generate the necessary code and build all required libraries for the specified # datamodel and version. Make sure to put all generated and compiled binary # outputs into a distinct subfolder such that at (test) runtime the models can # be individually "toggled" function(GENERATE_DATAMODEL test_case model_version) - cmake_parse_arguments(PARSED_ARGS "WITH_EVOLUTION" "" "" ${ARGN}) + cmake_parse_arguments(PARSED_ARGS "WITH_EVOLUTION;NO_EVOLUTION_CHECKS" "" "" ${ARGN}) set(model_base ${test_case}_${model_version}Model) set(output_base ${CMAKE_CURRENT_BINARY_DIR}/${test_case}/${model_version}_model) @@ -25,11 +26,18 @@ function(GENERATE_DATAMODEL test_case model_version) SCHEMA_EVOLUTION ${test_case}/evolution.yaml ) else() - PODIO_GENERATE_DATAMODEL(datamodel ${test_case}/${model_version}.yaml headers sources - IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} - OUTPUT_FOLDER ${output_base} - OLD_DESCRIPTION ${test_case}/old.yaml - ) + if(PARSED_ARGS_NO_EVOLUTION_CHECKS) + PODIO_GENERATE_DATAMODEL(datamodel ${test_case}/${model_version}.yaml headers sources + IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} + OUTPUT_FOLDER ${output_base} + ) + else() + PODIO_GENERATE_DATAMODEL(datamodel ${test_case}/${model_version}.yaml headers sources + IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} + OUTPUT_FOLDER ${output_base} + OLD_DESCRIPTION ${test_case}/old.yaml + ) + endif() endif() else() PODIO_GENERATE_DATAMODEL(datamodel ${test_case}/${model_version}.yaml headers sources @@ -92,15 +100,25 @@ endfunction() # See the README for more details on which parts need to be implemented for # adding a new test case. function(ADD_SCHEMA_EVOLUTION_TEST test_case) - cmake_parse_arguments(PARSED_ARGS "RNTUPLE;NO_GENERATE_MODELS;WITH_EVOLUTION" "" "" ${ARGN}) + cmake_parse_arguments(PARSED_ARGS "RNTUPLE;NO_GENERATE_MODELS;WITH_EVOLUTION;NO_EVOLUTION_CHECKS" "" "" ${ARGN}) # Generate datamodels if(NOT PARSED_ARGS_NO_GENERATE_MODELS) if(PARSED_ARGS_WITH_EVOLUTION) - GENERATE_DATAMODEL(${test_case} old) - GENERATE_DATAMODEL(${test_case} new WITH_EVOLUTION) + if(PARSED_ARGS_NO_EVOLUTION_CHECKS) + GENERATE_DATAMODEL(${test_case} old) + GENERATE_DATAMODEL(${test_case} new WITH_EVOLUTION NO_EVOLUTION_CHECKS) + else() + GENERATE_DATAMODEL(${test_case} old) + GENERATE_DATAMODEL(${test_case} new WITH_EVOLUTION) + endif() else() - GENERATE_DATAMODEL(${test_case} old) - GENERATE_DATAMODEL(${test_case} new) + if(PARSED_ARGS_NO_EVOLUTION_CHECKS) + GENERATE_DATAMODEL(${test_case} old) + GENERATE_DATAMODEL(${test_case} new NO_EVOLUTION_CHECKS) + else() + GENERATE_DATAMODEL(${test_case} old) + GENERATE_DATAMODEL(${test_case} new) + endif() endif() endif() From 21f403f5c5e36b1b3f5c7d752f310afe925c9c93 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Tue, 21 Oct 2025 15:51:11 +0200 Subject: [PATCH 17/17] Add tests for files only containing subset collections --- tests/root_io/CMakeLists.txt | 15 +++++++++++++++ tests/root_io/write_subset_only.cpp | 9 +++++++++ tests/root_io/write_subset_only_rntuple.cpp | 9 +++++++++ tests/sio_io/CMakeLists.txt | 7 +++++++ tests/sio_io/write_subset_only_sio.cpp | 9 +++++++++ tests/write_frame.h | 17 +++++++++++++++++ tools/CMakeLists.txt | 5 +++++ 7 files changed, 71 insertions(+) create mode 100644 tests/root_io/write_subset_only.cpp create mode 100644 tests/root_io/write_subset_only_rntuple.cpp create mode 100644 tests/sio_io/write_subset_only_sio.cpp diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index 132e01606..18d7ccd26 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -10,6 +10,7 @@ set(root_dependent_tests read_interface_root.cpp read_glob.cpp selected_colls_roundtrip_root.cpp + write_subset_only.cpp ) if(ENABLE_RNTUPLE) set(root_dependent_tests @@ -20,6 +21,7 @@ if(ENABLE_RNTUPLE) write_interface_rntuple.cpp read_interface_rntuple.cpp selected_colls_roundtrip_rntuple.cpp + write_subset_only_rntuple.cpp ) endif() if(ENABLE_DATASOURCE) @@ -40,6 +42,19 @@ set_tests_properties(write_frame_root PROPERTIES FIXTURES_SETUP podio_write_root set_tests_properties(write_interface_root PROPERTIES FIXTURES_SETUP podio_write_interface_root_fixture) set_tests_properties(read_interface_root PROPERTIES FIXTURES_REQUIRED podio_write_interface_root_fixture) +set_tests_properties( + write_subset_only + PROPERTIES + FIXTURES_SETUP podio_write_subset_only +) + +set_tests_properties( + write_subset_only_rntuple + PROPERTIES + FIXTURES_SETUP podio_write_subset_only_rntuple +) + + if(ENABLE_RNTUPLE) set_tests_properties(write_rntuple PROPERTIES FIXTURES_SETUP podio_write_rntuple_fixture) diff --git a/tests/root_io/write_subset_only.cpp b/tests/root_io/write_subset_only.cpp new file mode 100644 index 000000000..ea605e698 --- /dev/null +++ b/tests/root_io/write_subset_only.cpp @@ -0,0 +1,9 @@ +#include "write_frame.h" + +#include "podio/ROOTWriter.h" + +int main() { + write_subsets_only("example_subsets_only.root"); + + return 0; +} diff --git a/tests/root_io/write_subset_only_rntuple.cpp b/tests/root_io/write_subset_only_rntuple.cpp new file mode 100644 index 000000000..c2bde4239 --- /dev/null +++ b/tests/root_io/write_subset_only_rntuple.cpp @@ -0,0 +1,9 @@ +#include "write_frame.h" + +#include "podio/RNTupleWriter.h" + +int main() { + write_subsets_only("example_subsets_only_rntuple.root"); + + return 0; +} diff --git a/tests/sio_io/CMakeLists.txt b/tests/sio_io/CMakeLists.txt index cdd492215..61ec4eec8 100644 --- a/tests/sio_io/CMakeLists.txt +++ b/tests/sio_io/CMakeLists.txt @@ -6,6 +6,7 @@ set(sio_dependent_tests write_interface_sio.cpp read_interface_sio.cpp selected_colls_roundtrip_sio.cpp + write_subset_only_sio.cpp ) set(sio_libs podio::podioSioIO podio::podioIO) foreach( sourcefile ${sio_dependent_tests} ) @@ -24,6 +25,12 @@ set_tests_properties( FIXTURES_REQUIRED podio_write_sio_fixture ) +set_tests_properties( + write_subset_only_sio + PROPERTIES + FIXTURES_SETUP podio_write_subset_only_sio +) + set_tests_properties(read_interface_sio PROPERTIES FIXTURES_REQUIRED podio_write_interface_sio_fixture) #--- Write via python and the SIO backend and see if we can read it back in in diff --git a/tests/sio_io/write_subset_only_sio.cpp b/tests/sio_io/write_subset_only_sio.cpp new file mode 100644 index 000000000..911fea540 --- /dev/null +++ b/tests/sio_io/write_subset_only_sio.cpp @@ -0,0 +1,9 @@ +#include "write_frame.h" + +#include "podio/SIOWriter.h" + +int main() { + write_subsets_only("example_subsets_only.sio"); + + return 0; +} diff --git a/tests/write_frame.h b/tests/write_frame.h index 3f19fb60b..7466fd76e 100644 --- a/tests/write_frame.h +++ b/tests/write_frame.h @@ -557,4 +557,21 @@ void write_frames(const std::string& filename) { writer.finish(); } +template +void write_subsets_only(const std::string& filename) { + WriterT writer(filename); + + auto mcps = createMCCollection(); + auto subsetMCPs = ExampleMCCollection(); + subsetMCPs.setSubsetCollection(); + for (const auto mc : mcps) { + subsetMCPs.push_back(mc); + } + + auto frame = podio::Frame(); + frame.put(std::move(subsetMCPs), "subsetMCParticles"); + + writer.writeFrame(frame, "events"); +} + #endif // PODIO_TESTS_WRITE_FRAME_H diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index ebab88f2a..b01111ca1 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -87,9 +87,12 @@ if(BUILD_TESTING) CREATE_LEGACY_DUMP_TEST("root" v00-16-06 v00-16-06-example.root) CREATE_LEGACY_DUMP_TEST("root-detailed" v00-16-06 v00-16-06-example.root --detailed --entries 2:3) + CREATE_DUMP_TEST(podio-dump-subset-only "podio_write_subset_only" ${PROJECT_BINARY_DIR}/tests/root_io/example_subsets_only.root) + if (ENABLE_SIO) CREATE_DUMP_TEST(podio-dump-sio "podio_write_sio_fixture" --entries 4:7 ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio) CREATE_DUMP_TEST(podio-dump-detailed-sio "podio_write_sio_fixture" --detailed --entries 9 ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio) + CREATE_DUMP_TEST(podio-dump-subset-only-sio "podio_write_subset_only_sio" ${PROJECT_BINARY_DIR}/tests/sio_io/example_subsets_only.sio) CREATE_LEGACY_DUMP_TEST("sio" v00-16-06 v00-16-06-example.sio) CREATE_LEGACY_DUMP_TEST("sio-detailed" v00-16-06 v00-16-06-example.sio --detailed --entries 2:3) @@ -98,6 +101,8 @@ if(BUILD_TESTING) if (ENABLE_RNTUPLE) CREATE_DUMP_TEST(podio-dump-rntuple "podio_write_rntuple_fixture" ${PROJECT_BINARY_DIR}/tests/root_io/example_rntuple.root) CREATE_DUMP_TEST(podio-dump-detailed-rntuple "podio_write_rntuple_fixture" --detailed --category events --entries 1:3 ${PROJECT_BINARY_DIR}/tests/root_io/example_rntuple.root) + + CREATE_DUMP_TEST(podio-dump-subset-only-rntuple "podio_write_subset_only_rntuple" ${PROJECT_BINARY_DIR}/tests/root_io/example_subsets_only_rntuple.root) endif() endif()