From 953ea9ac6f13a7587fef5317e51ec6f292dc2040 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 6 Feb 2023 17:53:40 +0100 Subject: [PATCH 1/9] Add RelationNames and code gen to populate it --- include/podio/CollectionBase.h | 3 +++ include/podio/CollectionBuffers.h | 15 +++++++++++++++ include/podio/UserDataCollection.h | 6 ++++++ python/templates/Collection.cc.jinja2 | 19 +++++++++++++++++++ python/templates/Collection.h.jinja2 | 7 +++++++ 5 files changed, 50 insertions(+) diff --git a/include/podio/CollectionBase.h b/include/podio/CollectionBase.h index 7e2a51b2c..8a4eb682e 100644 --- a/include/podio/CollectionBase.h +++ b/include/podio/CollectionBase.h @@ -53,6 +53,9 @@ class CollectionBase { /// number of elements in the collection virtual size_t size() const = 0; + /// Get the names of the relations and vector members + virtual RelationNames getRelationNames() const = 0; + /// fully qualified type name virtual std::string getTypeName() const = 0; /// fully qualified type name of elements - with namespace diff --git a/include/podio/CollectionBuffers.h b/include/podio/CollectionBuffers.h index 80b94c6dd..69eada62f 100644 --- a/include/podio/CollectionBuffers.h +++ b/include/podio/CollectionBuffers.h @@ -7,11 +7,26 @@ #include #include #include +#include #include #include namespace podio { +/** + * Information on the names of the OneTo[One|Many]Relations as well as the + * VectorMembers of a datatype + * + * The contents are populated by the code generation, where we simply generate + * static vectors that we make available as const& here. + */ +struct RelationNames { + /// The names of the relations (OneToMany before OneToOne) + const std::vector& relations; + /// The names of the vector members + const std::vector& vectorMembers; +}; + class CollectionBase; template diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index 4fe575996..94da2bbb8 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -132,6 +132,12 @@ class UserDataCollection : public CollectionBase { return _vec.size(); } + /// Get the names of the relations and vector members + podio::RelationNames getRelationNames() const override { + const static std::vector emptyVec{}; + return RelationNames{emptyVec, emptyVec}; + } + /// fully qualified type name std::string getTypeName() const override { return userDataCollTypeName(); diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index 1789a83f6..861106f56 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -50,6 +50,25 @@ std::size_t {{ collection_type }}::size() const { return m_storage.entries.size(); } +podio::RelationNames {{ collection_type }}::getRelationNames() const { +{% set comma = joiner(", ") %} + static const std::vector relationNames = { +{%- for relation in OneToManyRelations + OneToOneRelations -%} + {{ comma() }}"{{ relation.name }}" + {%- endfor -%} +}; + +{% set comma = joiner(", ") %} + static const std::vector vectorNames = { +{%- for member in VectorMembers -%} + {{ comma() }}"{{ member.name }}" +{%- endfor -%} +}; + + return podio::RelationNames{relationNames, vectorNames}; +} + + void {{ collection_type }}::setSubsetCollection(bool setSubset) { if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { throw std::logic_error("Cannot change the character of a collection that already contains elements"); diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index f91c02db5..5262a4bde 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -32,6 +32,10 @@ #include #include +namespace podio { + struct RelationNames; +} + {{ utils.namespace_open(class.namespace) }} @@ -77,6 +81,9 @@ public: /// number of elements in the collection std::size_t size() const final; + /// Get the names of the relations and vector members + podio::RelationNames getRelationNames() const final; + /// fully qualified type name std::string getTypeName() const final { return std::string("{{ (class | string ).strip(':')+"Collection" }}"); } /// fully qualified type name of elements - with namespace From 1d3c9fd6785f4079e726b09c52d7f9521245a26b Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 8 Feb 2023 14:35:14 +0100 Subject: [PATCH 2/9] Write branches with proper names in ROOTFrameWriter --- include/podio/CollectionBase.h | 5 +-- include/podio/CollectionBuffers.h | 14 ------- include/podio/DatamodelRegistry.h | 40 ++++++++++++++++++- include/podio/UserDataCollection.h | 6 --- python/podio_class_generator.py | 6 +++ python/templates/Collection.cc.jinja2 | 19 --------- python/templates/Collection.h.jinja2 | 3 -- python/templates/DatamodelDefinition.h.jinja2 | 17 +++++++- src/DatamodelRegistry.cc | 21 +++++++++- src/ROOTFrameWriter.cc | 40 ++++++++++--------- src/rootUtils.h | 13 ++++++ 11 files changed, 118 insertions(+), 66 deletions(-) diff --git a/include/podio/CollectionBase.h b/include/podio/CollectionBase.h index 8a4eb682e..45f179e96 100644 --- a/include/podio/CollectionBase.h +++ b/include/podio/CollectionBase.h @@ -14,6 +14,8 @@ namespace podio { // forward declarations class ICollectionProvider; +struct RelationNames; + class CollectionBase { protected: /// default constructor @@ -53,9 +55,6 @@ class CollectionBase { /// number of elements in the collection virtual size_t size() const = 0; - /// Get the names of the relations and vector members - virtual RelationNames getRelationNames() const = 0; - /// fully qualified type name virtual std::string getTypeName() const = 0; /// fully qualified type name of elements - with namespace diff --git a/include/podio/CollectionBuffers.h b/include/podio/CollectionBuffers.h index 69eada62f..8846af162 100644 --- a/include/podio/CollectionBuffers.h +++ b/include/podio/CollectionBuffers.h @@ -13,20 +13,6 @@ namespace podio { -/** - * Information on the names of the OneTo[One|Many]Relations as well as the - * VectorMembers of a datatype - * - * The contents are populated by the code generation, where we simply generate - * static vectors that we make available as const& here. - */ -struct RelationNames { - /// The names of the relations (OneToMany before OneToOne) - const std::vector& relations; - /// The names of the vector members - const std::vector& vectorMembers; -}; - class CollectionBase; template diff --git a/include/podio/DatamodelRegistry.h b/include/podio/DatamodelRegistry.h index a32aa8218..77f31192b 100644 --- a/include/podio/DatamodelRegistry.h +++ b/include/podio/DatamodelRegistry.h @@ -3,11 +3,39 @@ #include #include +#include +#include #include #include namespace podio { +/** + * Type alias for storing the names of all Relations and VectorMembers for all + * datatypes of an EDM. Populated for each EDM at code generation time. + * The structure is of each element in the outer vector is: + * - get<0>: The name of the datatype + * - get<1>: The names of all Relations, where OneToManyRelations comes before + * OneToOneRelations (in the order as they appear in the YAML file) + * - get<2>: The names of all VectorMembers (in the order of the file YAML) + */ +using RelationNameMapping = + std::vector, std::vector>>; + +/** + * Information on the names of the OneTo[One|Many]Relations as well as the + * VectorMembers of a datatype + * + * The contents are populated by the code generation, where we simply generate + * static vectors that we make available as const& here. + */ +struct RelationNames { + /// The names of the relations (OneToMany before OneToOne) + const std::vector& relations; + /// The names of the vector members + const std::vector& vectorMembers; +}; + /** * Global registry holding information about datamodels and datatypes defined * therein that are currently known by podio (i.e. which have been dynamically @@ -85,14 +113,24 @@ class DatamodelRegistry { * @param name The name of the EDM that should be registered * @param definition The datamodel definition from which this EDM has been * generated in JSON format + * @param relationNames the names of the relations and vector members for all + * datatypes that are defined for this EDM * */ - size_t registerDatamodel(std::string name, std::string_view definition); + size_t registerDatamodel(std::string name, std::string_view definition, + const podio::RelationNameMapping& relationNames); + + /** + * Get the names of the relations and vector members of a datatype + */ + RelationNames getRelationNames(std::string_view typeName) const; private: DatamodelRegistry() = default; /// The stored definitions std::vector> m_definitions{}; + + std::unordered_map m_relations{}; }; } // namespace podio diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index 94da2bbb8..4fe575996 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -132,12 +132,6 @@ class UserDataCollection : public CollectionBase { return _vec.size(); } - /// Get the names of the relations and vector members - podio::RelationNames getRelationNames() const override { - const static std::vector emptyVec{}; - return RelationNames{emptyVec, emptyVec}; - } - /// fully qualified type name std::string getTypeName() const override { return userDataCollTypeName(); diff --git a/python/podio_class_generator.py b/python/podio_class_generator.py index 0b37d706c..4f86f2a45 100755 --- a/python/podio_class_generator.py +++ b/python/podio_class_generator.py @@ -410,8 +410,14 @@ def _write_edm_def_file(self): 'edm_definition': model_encoder.encode(self.datamodel), 'incfolder': self.incfolder, 'schema_version': self.datamodel.schema_version, + 'datatypes': self.datamodel.datatypes, } + def quoted_sv(string): + return f"\"{string}\"sv" + + self.env.filters["quoted_sv"] = quoted_sv + self._write_file('DatamodelDefinition.h', self._eval_template('DatamodelDefinition.h.jinja2', data)) diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index 861106f56..1789a83f6 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -50,25 +50,6 @@ std::size_t {{ collection_type }}::size() const { return m_storage.entries.size(); } -podio::RelationNames {{ collection_type }}::getRelationNames() const { -{% set comma = joiner(", ") %} - static const std::vector relationNames = { -{%- for relation in OneToManyRelations + OneToOneRelations -%} - {{ comma() }}"{{ relation.name }}" - {%- endfor -%} -}; - -{% set comma = joiner(", ") %} - static const std::vector vectorNames = { -{%- for member in VectorMembers -%} - {{ comma() }}"{{ member.name }}" -{%- endfor -%} -}; - - return podio::RelationNames{relationNames, vectorNames}; -} - - void {{ collection_type }}::setSubsetCollection(bool setSubset) { if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { throw std::logic_error("Cannot change the character of a collection that already contains elements"); diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index 5262a4bde..b70e93b12 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -81,9 +81,6 @@ public: /// number of elements in the collection std::size_t size() const final; - /// Get the names of the relations and vector members - podio::RelationNames getRelationNames() const final; - /// fully qualified type name std::string getTypeName() const final { return std::string("{{ (class | string ).strip(':')+"Collection" }}"); } /// fully qualified type name of elements - with namespace diff --git a/python/templates/DatamodelDefinition.h.jinja2 b/python/templates/DatamodelDefinition.h.jinja2 index c424ded7f..74b92a50c 100644 --- a/python/templates/DatamodelDefinition.h.jinja2 +++ b/python/templates/DatamodelDefinition.h.jinja2 @@ -9,6 +9,21 @@ namespace {{ package_name }}::meta { */ static constexpr auto {{ package_name }}__JSONDefinition = R"DATAMODELDEF({{ edm_definition }})DATAMODELDEF"; +using namespace std::string_view_literals; + +/** + * The names of all relations and vector members for all datatypes + */ +const static podio::RelationNameMapping {{ package_name }}__relationNames = { +{% for typeName, type in datatypes.items() %} + {"{{ typeName }}", + { {{ (type.OneToManyRelations + type.OneToOneRelations) | map(attribute="name") | map("quoted_sv") | join(", ") }} }, + { {{ type.VectorMembers | map(attribute="name") | map("quoted_sv") | join(", ")}} }, + }, +{% endfor %} +}; + + /** * The helper class that takes care of registering the datamodel definition to * the DatamodelRegistry and to provide the index in that registry. @@ -19,7 +34,7 @@ static constexpr auto {{ package_name }}__JSONDefinition = R"DATAMODELDEF({{ edm class DatamodelRegistryIndex { public: static size_t value() { - static auto index = DatamodelRegistryIndex(podio::DatamodelRegistry::mutInstance().registerDatamodel("{{ package_name }}", {{ package_name }}__JSONDefinition)); + static auto index = DatamodelRegistryIndex(podio::DatamodelRegistry::mutInstance().registerDatamodel("{{ package_name }}", {{ package_name }}__JSONDefinition, {{ package_name }}__relationNames)); return index.m_value; } diff --git a/src/DatamodelRegistry.cc b/src/DatamodelRegistry.cc index d5a96e364..7c40b6b24 100644 --- a/src/DatamodelRegistry.cc +++ b/src/DatamodelRegistry.cc @@ -15,13 +15,19 @@ DatamodelRegistry& DatamodelRegistry::mutInstance() { return registryInstance; } -size_t DatamodelRegistry::registerDatamodel(std::string name, std::string_view definition) { +size_t DatamodelRegistry::registerDatamodel(std::string name, std::string_view definition, + const podio::RelationNameMapping& relationNames) { const auto it = std::find_if(m_definitions.cbegin(), m_definitions.cend(), [&name](const auto& kvPair) { return kvPair.first == name; }); if (it == m_definitions.cend()) { int index = m_definitions.size(); m_definitions.emplace_back(name, definition); + + for (const auto& [typeName, relations, vectorMembers] : relationNames) { + m_relations.emplace(typeName, RelationNames{relations, vectorMembers}); + } + return index; } @@ -60,4 +66,17 @@ const std::string& DatamodelRegistry::getDatamodelName(size_t index) const { return m_definitions[index].first; } +RelationNames DatamodelRegistry::getRelationNames(std::string_view typeName) const { + static std::vector emptyVec{}; + if (typeName.substr(0, 24) == "podio::UserDataCollection") { + return {emptyVec, emptyVec}; + } + + if (const auto it = m_relations.find(typeName); it != m_relations.end()) { + return it->second; + } + + return {emptyVec, emptyVec}; +} + } // namespace podio diff --git a/src/ROOTFrameWriter.cc b/src/ROOTFrameWriter.cc index e6fa85de6..ce9261b60 100644 --- a/src/ROOTFrameWriter.cc +++ b/src/ROOTFrameWriter.cc @@ -1,5 +1,6 @@ #include "podio/ROOTFrameWriter.h" #include "podio/CollectionBase.h" +#include "podio/DatamodelRegistry.h" #include "podio/Frame.h" #include "podio/GenericParameters.h" #include "podio/podioVersion.h" @@ -68,29 +69,32 @@ void ROOTFrameWriter::initBranches(CategoryInfo& catInfo, const std::vectorgetBuffers(); - - // data buffer branch, only for non-subset collections - if (buffers.data) { + // For subset collections we only fill one references branch + if (coll->isSubsetCollection()) { + auto& refColl = (*buffers.references)[0]; + const auto brName = root_utils::subsetBranch(name); + branches.refs.push_back(catInfo.tree->Branch(brName.c_str(), refColl.get())); + } else { + // For "proper" collections we populate all branches, starting with the data auto bufferDataType = "vector<" + coll->getDataTypeName() + ">"; branches.data = catInfo.tree->Branch(name.c_str(), bufferDataType.c_str(), buffers.data); - } - // reference collections - if (auto refColls = buffers.references) { - int i = 0; - for (auto& c : (*refColls)) { - const auto brName = root_utils::refBranch(name, i++); - branches.refs.push_back(catInfo.tree->Branch(brName.c_str(), c.get())); + const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(coll->getValueTypeName()); + if (auto refColls = buffers.references) { + int i = 0; + for (auto& c : (*refColls)) { + const auto brName = root_utils::refBranch(name, relVecNames.relations[i++]); + branches.refs.push_back(catInfo.tree->Branch(brName.c_str(), c.get())); + } } - } - // vector members - if (auto vmInfo = buffers.vectorMembers) { - int i = 0; - for (auto& [type, vec] : (*vmInfo)) { - const auto typeName = "vector<" + type + ">"; - const auto brName = root_utils::vecBranch(name, i++); - branches.vecs.push_back(catInfo.tree->Branch(brName.c_str(), typeName.c_str(), vec)); + if (auto vmInfo = buffers.vectorMembers) { + int i = 0; + for (auto& [type, vec] : (*vmInfo)) { + const auto typeName = "vector<" + type + ">"; + const auto brName = root_utils::vecBranch(name, relVecNames.vectorMembers[i++]); + branches.vecs.push_back(catInfo.tree->Branch(brName.c_str(), typeName.c_str(), vec)); + } } } diff --git a/src/rootUtils.h b/src/rootUtils.h index b4859bbf8..b65d36840 100644 --- a/src/rootUtils.h +++ b/src/rootUtils.h @@ -76,10 +76,23 @@ inline std::string refBranch(const std::string& name, size_t index) { return name + "#" + std::to_string(index); } +inline std::string refBranch(const std::string& name, std::string_view relName) { + return name + "_" + std::string(relName); +} + inline std::string vecBranch(const std::string& name, size_t index) { return name + "_" + std::to_string(index); } +inline std::string vecBranch(const std::string& name, std::string_view vecName) { + return name + "_" + std::string(vecName); +} + +/// The name for subset branches +inline std::string subsetBranch(const std::string& name) { + return name + "_objIdx"; +} + template inline void setCollectionAddresses(const BufferT& collBuffers, const CollectionBranches& branches) { From 9c5cb0fa628bcf73d26fbe5bf9c5198e17d33271 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 13 Apr 2023 11:05:01 +0200 Subject: [PATCH 3/9] Make the Frame reader use the registry to get branch names --- src/DatamodelRegistry.cc | 5 +++++ src/ROOTFrameReader.cc | 40 ++++++++++++++++++---------------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/DatamodelRegistry.cc b/src/DatamodelRegistry.cc index 7c40b6b24..16e387132 100644 --- a/src/DatamodelRegistry.cc +++ b/src/DatamodelRegistry.cc @@ -72,6 +72,11 @@ RelationNames DatamodelRegistry::getRelationNames(std::string_view typeName) con return {emptyVec, emptyVec}; } + // Strip Collection if necessary + if (typeName.size() > 10 && typeName.substr(typeName.size() - 10) == "Collection") { + typeName = typeName.substr(0, typeName.size() - 10); + } + if (const auto it = m_relations.find(typeName); it != m_relations.end()) { return it->second; } diff --git a/src/ROOTFrameReader.cc b/src/ROOTFrameReader.cc index c1a7461d8..fe3d74e9b 100644 --- a/src/ROOTFrameReader.cc +++ b/src/ROOTFrameReader.cc @@ -3,6 +3,7 @@ #include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" #include "podio/CollectionIDTable.h" +#include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" #include "rootUtils.h" @@ -100,7 +101,7 @@ podio::CollectionReadBuffers ROOTFrameReader::getCollectionBuffers(ROOTFrameRead if (auto* refCollections = collBuffers.references) { for (size_t i = 0; i < refCollections->size(); ++i) { const auto brName = root_utils::refBranch(name, i); - branches.refs[i] = root_utils::getBranch(catInfo.chain.get(), brName.c_str()); + branches.refs[i] = root_utils::getBranch(catInfo.chain.get(), branches.refs[i]->GetName()); } } @@ -108,7 +109,7 @@ podio::CollectionReadBuffers ROOTFrameReader::getCollectionBuffers(ROOTFrameRead if (auto* vecMembers = collBuffers.vectorMembers) { for (size_t i = 0; i < vecMembers->size(); ++i) { const auto brName = root_utils::vecBranch(name, i); - branches.vecs[i] = root_utils::getBranch(catInfo.chain.get(), brName.c_str()); + branches.vecs[i] = root_utils::getBranch(catInfo.chain.get(), branches.vecs[i]->GetName()); } } } @@ -270,33 +271,28 @@ createCollectionBranches(TChain* chain, const podio::CollectionIDTable& idTable, const auto name = idTable.name(collID); root_utils::CollectionBranches branches{}; - const auto collectionClass = TClass::GetClass(collType.c_str()); - - // Need the collection here to setup all the branches. Have to manage the - // temporary collection ourselves - auto collection = - std::unique_ptr(static_cast(collectionClass->New())); - collection->setSubsetCollection(isSubsetColl); - - if (!isSubsetColl) { + if (isSubsetColl) { + // Only one branch will exist and we can trivially get its name + const auto brName = root_utils::subsetBranch(name); + branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + } else { // This branch is guaranteed to exist since only collections that are // also written to file are in the info metadata that we work with here branches.data = root_utils::getBranch(chain, name.c_str()); - } - const auto buffers = collection->getBuffers(); - for (size_t i = 0; i < buffers.references->size(); ++i) { - const auto brName = root_utils::refBranch(name, i); - branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); - } - - for (size_t i = 0; i < buffers.vectorMembers->size(); ++i) { - const auto brName = root_utils::vecBranch(name, i); - branches.vecs.push_back(root_utils::getBranch(chain, brName.c_str())); + const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(collType); + for (const auto& relName : relVecNames.relations) { + const auto brName = root_utils::refBranch(name, relName); + branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + } + for (const auto& vecName : relVecNames.vectorMembers) { + const auto brName = root_utils::refBranch(name, vecName); + branches.vecs.push_back(root_utils::getBranch(chain, brName.c_str())); + } } storedClasses.emplace_back(name, std::make_tuple(collType, isSubsetColl, collSchemaVersion, collectionIndex++)); - collBranches.push_back(branches); + collBranches.emplace_back(std::move(branches)); } return {collBranches, storedClasses}; From 4c44520d335567b960473159d5d317c290bbaee2 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 13 Apr 2023 13:21:04 +0200 Subject: [PATCH 4/9] Make sure that the EDM always is in the registry --- python/templates/DatamodelDefinition.h.jinja2 | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/python/templates/DatamodelDefinition.h.jinja2 b/python/templates/DatamodelDefinition.h.jinja2 index 74b92a50c..8efe2fcb4 100644 --- a/python/templates/DatamodelDefinition.h.jinja2 +++ b/python/templates/DatamodelDefinition.h.jinja2 @@ -23,6 +23,10 @@ const static podio::RelationNameMapping {{ package_name }}__relationNames = { {% endfor %} }; +/** + * The schema version at generation time + */ +static constexpr podio::SchemaVersionT schemaVersion = {{ schema_version }}; /** * The helper class that takes care of registering the datamodel definition to @@ -43,9 +47,18 @@ private: size_t m_value{podio::DatamodelRegistry::NoDefinitionAvailable}; }; -/** - * The schema version at generation time - */ -static constexpr podio::SchemaVersionT schemaVersion = {{ schema_version }}; + +namespace static_registration { + // The usual trick via an IIFE and a const variable that we assign to, to + // ensure that we populate this before everything starts + inline bool ensureRegistration() { + const static auto reg = []() { + return {{ package_name }}::meta::DatamodelRegistryIndex::value() != podio::DatamodelRegistry::NoDefinitionAvailable; + }(); + return reg; + } + + const auto registrationEnsured = ensureRegistration(); +} } // namespace {{ package_name }}::meta From 4105ab30008e8504a1f36fb8873ba99f789ace12 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 13 Apr 2023 14:21:42 +0200 Subject: [PATCH 5/9] Make sure that the names are populated when they are used --- python/templates/DatamodelDefinition.h.jinja2 | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/python/templates/DatamodelDefinition.h.jinja2 b/python/templates/DatamodelDefinition.h.jinja2 index 8efe2fcb4..ce4cefb41 100644 --- a/python/templates/DatamodelDefinition.h.jinja2 +++ b/python/templates/DatamodelDefinition.h.jinja2 @@ -9,19 +9,21 @@ namespace {{ package_name }}::meta { */ static constexpr auto {{ package_name }}__JSONDefinition = R"DATAMODELDEF({{ edm_definition }})DATAMODELDEF"; -using namespace std::string_view_literals; /** * The names of all relations and vector members for all datatypes */ -const static podio::RelationNameMapping {{ package_name }}__relationNames = { +inline podio::RelationNameMapping {{ package_name }}__getRelationNames() { + using namespace std::string_view_literals; + return { {% for typeName, type in datatypes.items() %} - {"{{ typeName }}", - { {{ (type.OneToManyRelations + type.OneToOneRelations) | map(attribute="name") | map("quoted_sv") | join(", ") }} }, - { {{ type.VectorMembers | map(attribute="name") | map("quoted_sv") | join(", ")}} }, - }, + {"{{ typeName }}"sv, + { {{ (type.OneToManyRelations + type.OneToOneRelations) | map(attribute="name") | map("quoted_sv") | join(", ") }} }, + { {{ type.VectorMembers | map(attribute="name") | map("quoted_sv") | join(", ")}} }, + }, {% endfor %} -}; + }; +} /** * The schema version at generation time @@ -38,10 +40,10 @@ static constexpr podio::SchemaVersionT schemaVersion = {{ schema_version }}; class DatamodelRegistryIndex { public: static size_t value() { - static auto index = DatamodelRegistryIndex(podio::DatamodelRegistry::mutInstance().registerDatamodel("{{ package_name }}", {{ package_name }}__JSONDefinition, {{ package_name }}__relationNames)); + static const auto relationNames = {{ package_name }}__getRelationNames(); + static auto index = DatamodelRegistryIndex(podio::DatamodelRegistry::mutInstance().registerDatamodel("{{ package_name }}", {{ package_name }}__JSONDefinition, relationNames)); return index.m_value; } - private: DatamodelRegistryIndex(size_t v) : m_value(v) {} size_t m_value{podio::DatamodelRegistry::NoDefinitionAvailable}; From 9d24c5bcb34dcd1954d65f51e27d41d6b55150d0 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 23 May 2023 14:30:54 +0200 Subject: [PATCH 6/9] Make relation branch names start with an underscore Less prone to accidental collisions. Plus sort of conveying that they are "hidden" --- src/rootUtils.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rootUtils.h b/src/rootUtils.h index b65d36840..baf2531fc 100644 --- a/src/rootUtils.h +++ b/src/rootUtils.h @@ -77,7 +77,7 @@ inline std::string refBranch(const std::string& name, size_t index) { } inline std::string refBranch(const std::string& name, std::string_view relName) { - return name + "_" + std::string(relName); + return "_" + name + "_" + std::string(relName); } inline std::string vecBranch(const std::string& name, size_t index) { @@ -85,7 +85,7 @@ inline std::string vecBranch(const std::string& name, size_t index) { } inline std::string vecBranch(const std::string& name, std::string_view vecName) { - return name + "_" + std::string(vecName); + return "_" + name + "_" + std::string(vecName); } /// The name for subset branches From 5e012d1d509de3b6bd966d941488db0d7813b21d Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 25 Apr 2023 11:13:29 +0200 Subject: [PATCH 7/9] Make it possible to read index based files as well - Keep branch names around for resetting them (since invalidated branches can no longer provide that information) --- include/podio/CollectionBranches.h | 3 + src/ROOTFrameReader.cc | 89 ++++++++++++++++++++++-------- src/rootUtils.h | 17 ++++++ 3 files changed, 87 insertions(+), 22 deletions(-) diff --git a/include/podio/CollectionBranches.h b/include/podio/CollectionBranches.h index ddbce7ec2..9b20016aa 100644 --- a/include/podio/CollectionBranches.h +++ b/include/podio/CollectionBranches.h @@ -3,6 +3,7 @@ #include "TBranch.h" +#include #include namespace podio::root_utils { @@ -15,6 +16,8 @@ struct CollectionBranches { TBranch* data{nullptr}; std::vector refs{}; std::vector vecs{}; + std::vector refNames{}; ///< The names of the relation branches + std::vector vecNames{}; ///< The names of the vector member branches }; } // namespace podio::root_utils diff --git a/src/ROOTFrameReader.cc b/src/ROOTFrameReader.cc index fe3d74e9b..df038e023 100644 --- a/src/ROOTFrameReader.cc +++ b/src/ROOTFrameReader.cc @@ -23,6 +23,10 @@ std::tuple, std::vector& collInfo); +std::tuple, std::vector>> +createCollectionBranchesIndexBased(TChain* chain, const podio::CollectionIDTable& idTable, + const std::vector& collInfo); + GenericParameters ROOTFrameReader::readEntryParameters(ROOTFrameReader::CategoryInfo& catInfo, bool reloadBranches, unsigned int localEntry) { // Parameter branch is always the last one @@ -95,23 +99,7 @@ podio::CollectionReadBuffers ROOTFrameReader::getCollectionBuffers(ROOTFrameRead auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{}); if (reloadBranches) { - branches.data = root_utils::getBranch(catInfo.chain.get(), name.c_str()); - - // reference collections - if (auto* refCollections = collBuffers.references) { - for (size_t i = 0; i < refCollections->size(); ++i) { - const auto brName = root_utils::refBranch(name, i); - branches.refs[i] = root_utils::getBranch(catInfo.chain.get(), branches.refs[i]->GetName()); - } - } - - // vector members - if (auto* vecMembers = collBuffers.vectorMembers) { - for (size_t i = 0; i < vecMembers->size(); ++i) { - const auto brName = root_utils::vecBranch(name, i); - branches.vecs[i] = root_utils::getBranch(catInfo.chain.get(), branches.vecs[i]->GetName()); - } - } + root_utils::resetBranches(catInfo.chain.get(), branches, name); } // set the addresses and read the data @@ -165,8 +153,15 @@ void ROOTFrameReader::initCategory(CategoryInfo& catInfo, const std::string& cat collInfoBranch->GetEntry(0); } - std::tie(catInfo.branches, catInfo.storedClasses) = - createCollectionBranches(catInfo.chain.get(), *catInfo.table, *collInfo); + // For backwards compatibility make it possible to read the index based files + // from older versions + if (m_fileVersion <= podio::version::Version{0, 16, 4}) { + std::tie(catInfo.branches, catInfo.storedClasses) = + createCollectionBranchesIndexBased(catInfo.chain.get(), *catInfo.table, *collInfo); + } else { + std::tie(catInfo.branches, catInfo.storedClasses) = + createCollectionBranches(catInfo.chain.get(), *catInfo.table, *collInfo); + } delete collInfo; @@ -255,6 +250,53 @@ std::vector ROOTFrameReader::getAvailableCategories() const { return cats; } +std::tuple, std::vector>> +createCollectionBranchesIndexBased(TChain* chain, const podio::CollectionIDTable& idTable, + const std::vector& collInfo) { + + size_t collectionIndex{0}; + std::vector collBranches; + collBranches.reserve(collInfo.size() + 1); + std::vector> storedClasses; + storedClasses.reserve(collInfo.size()); + + for (const auto& [collID, collType, isSubsetColl, collSchemaVersion] : collInfo) { + // We only write collections that are in the collectionIDTable, so no need + // to check here + const auto name = idTable.name(collID); + + const auto collectionClass = TClass::GetClass(collType.c_str()); + // Need the collection here to setup all the branches. Have to manage the + // temporary collection ourselves + auto collection = + std::unique_ptr(static_cast(collectionClass->New())); + root_utils::CollectionBranches branches{}; + if (!isSubsetColl) { + // This branch is guaranteed to exist since only collections that are + // also written to file are in the info metadata that we work with here + branches.data = root_utils::getBranch(chain, name.c_str()); + } + + const auto buffers = collection->getBuffers(); + for (size_t i = 0; i < buffers.references->size(); ++i) { + auto brName = root_utils::refBranch(name, i); + branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.refNames.emplace_back(std::move(brName)); + } + + for (size_t i = 0; i < buffers.vectorMembers->size(); ++i) { + auto brName = root_utils::vecBranch(name, i); + branches.vecs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.vecNames.emplace_back(std::move(brName)); + } + + storedClasses.emplace_back(name, std::make_tuple(collType, isSubsetColl, collSchemaVersion, collectionIndex++)); + collBranches.emplace_back(std::move(branches)); + } + + return {collBranches, storedClasses}; +} + std::tuple, std::vector>> createCollectionBranches(TChain* chain, const podio::CollectionIDTable& idTable, const std::vector& collInfo) { @@ -273,8 +315,9 @@ createCollectionBranches(TChain* chain, const podio::CollectionIDTable& idTable, root_utils::CollectionBranches branches{}; if (isSubsetColl) { // Only one branch will exist and we can trivially get its name - const auto brName = root_utils::subsetBranch(name); + auto brName = root_utils::subsetBranch(name); branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.refNames.emplace_back(std::move(brName)); } else { // This branch is guaranteed to exist since only collections that are // also written to file are in the info metadata that we work with here @@ -282,12 +325,14 @@ createCollectionBranches(TChain* chain, const podio::CollectionIDTable& idTable, const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(collType); for (const auto& relName : relVecNames.relations) { - const auto brName = root_utils::refBranch(name, relName); + auto brName = root_utils::refBranch(name, relName); branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.refNames.emplace_back(std::move(brName)); } for (const auto& vecName : relVecNames.vectorMembers) { - const auto brName = root_utils::refBranch(name, vecName); + auto brName = root_utils::refBranch(name, vecName); branches.vecs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.vecNames.emplace_back(std::move(brName)); } } diff --git a/src/rootUtils.h b/src/rootUtils.h index baf2531fc..d6072fa09 100644 --- a/src/rootUtils.h +++ b/src/rootUtils.h @@ -93,6 +93,23 @@ inline std::string subsetBranch(const std::string& name) { return name + "_objIdx"; } +/** + * Reset all the branches that by getting them from the TTree again + */ +inline void resetBranches(TTree* chain, CollectionBranches& branches, const std::string& name) { + if (branches.data) { + branches.data = getBranch(chain, name); + } + + for (size_t i = 0; i < branches.refs.size(); ++i) { + branches.refs[i] = getBranch(chain, branches.refNames[i]); + } + + for (size_t i = 0; i < branches.vecs.size(); ++i) { + branches.vecs[i] = getBranch(chain, branches.vecNames[i]); + } +} + template inline void setCollectionAddresses(const BufferT& collBuffers, const CollectionBranches& branches) { From 4b4fd5c6c1c900307a20ae569eef03f396ffce06 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 30 May 2023 19:51:37 +0200 Subject: [PATCH 8/9] Fix legacy reading --- src/ROOTFrameReader.cc | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/ROOTFrameReader.cc b/src/ROOTFrameReader.cc index df038e023..d7da9dfa9 100644 --- a/src/ROOTFrameReader.cc +++ b/src/ROOTFrameReader.cc @@ -155,7 +155,7 @@ void ROOTFrameReader::initCategory(CategoryInfo& catInfo, const std::string& cat // For backwards compatibility make it possible to read the index based files // from older versions - if (m_fileVersion <= podio::version::Version{0, 16, 4}) { + if (m_fileVersion <= podio::version::Version{0, 16, 5}) { std::tie(catInfo.branches, catInfo.storedClasses) = createCollectionBranchesIndexBased(catInfo.chain.get(), *catInfo.table, *collInfo); } else { @@ -271,23 +271,28 @@ createCollectionBranchesIndexBased(TChain* chain, const podio::CollectionIDTable auto collection = std::unique_ptr(static_cast(collectionClass->New())); root_utils::CollectionBranches branches{}; - if (!isSubsetColl) { + if (isSubsetColl) { + // Only one branch will exist and we can trivially get its name + auto brName = root_utils::refBranch(name, 0); + branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.refNames.emplace_back(std::move(brName)); + } else { // This branch is guaranteed to exist since only collections that are // also written to file are in the info metadata that we work with here branches.data = root_utils::getBranch(chain, name.c_str()); - } - const auto buffers = collection->getBuffers(); - for (size_t i = 0; i < buffers.references->size(); ++i) { - auto brName = root_utils::refBranch(name, i); - branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); - branches.refNames.emplace_back(std::move(brName)); - } + const auto buffers = collection->getBuffers(); + for (size_t i = 0; i < buffers.references->size(); ++i) { + auto brName = root_utils::refBranch(name, i); + branches.refs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.refNames.emplace_back(std::move(brName)); + } - for (size_t i = 0; i < buffers.vectorMembers->size(); ++i) { - auto brName = root_utils::vecBranch(name, i); - branches.vecs.push_back(root_utils::getBranch(chain, brName.c_str())); - branches.vecNames.emplace_back(std::move(brName)); + for (size_t i = 0; i < buffers.vectorMembers->size(); ++i) { + auto brName = root_utils::vecBranch(name, i); + branches.vecs.push_back(root_utils::getBranch(chain, brName.c_str())); + branches.vecNames.emplace_back(std::move(brName)); + } } storedClasses.emplace_back(name, std::make_tuple(collType, isSubsetColl, collSchemaVersion, collectionIndex++)); From feabec413aff1ea67b025ea8f94b62b037cbde39 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 30 May 2023 19:51:46 +0200 Subject: [PATCH 9/9] Manually increase patch to make if clause work properly --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 700536436..577fac281 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ project(podio) #--- Version ------------------------------------------------------------------- SET( ${PROJECT_NAME}_VERSION_MAJOR 0 ) SET( ${PROJECT_NAME}_VERSION_MINOR 16 ) -SET( ${PROJECT_NAME}_VERSION_PATCH 5 ) +SET( ${PROJECT_NAME}_VERSION_PATCH 6 ) SET( ${PROJECT_NAME}_VERSION "${${PROJECT_NAME}_VERSION_MAJOR}.${${PROJECT_NAME}_VERSION_MINOR}.${${PROJECT_NAME}_VERSION_PATCH}" )