From c0d22c92798dc97c8b33d1c01e463a40b7abbeca Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 11 Sep 2024 17:17:56 +0200 Subject: [PATCH 01/79] Use more suitable name for header --- include/podio/detail/RelationIOHelpers.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/podio/detail/RelationIOHelpers.h b/include/podio/detail/RelationIOHelpers.h index 36dd4de34..a52c36bdb 100644 --- a/include/podio/detail/RelationIOHelpers.h +++ b/include/podio/detail/RelationIOHelpers.h @@ -29,7 +29,7 @@ namespace podio::detail { /// @param id The ObjectID of the element that we are currently looking for template void tryAddTo(T, std::vector& relElements, const podio::CollectionBase* coll, const podio::ObjectID id) { - if (auto typedColl = dynamic_cast(coll)) { + if (auto typedColl = dynamic_cast(coll)) { const T tmp = (*typedColl)[id.index]; relElements.emplace_back(tmp); } @@ -87,7 +87,7 @@ void addMultiRelation(std::vector& relElements, const podio::Collection if constexpr (podio::detail::isInterfaceType) { addInterfaceToMultiRelation(relElements, coll, id); } else { - const auto* typeColl = static_cast(coll); + const auto* typeColl = static_cast(coll); relElements.emplace_back((*typeColl)[id.index]); } } From 9cbd24019b67a6b8cf7165ac118763fea40492d5 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 11 Sep 2024 17:35:45 +0200 Subject: [PATCH 02/79] Make things work again (again) with c++17 --- include/podio/detail/RelationIOHelpers.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/podio/detail/RelationIOHelpers.h b/include/podio/detail/RelationIOHelpers.h index a52c36bdb..36dd4de34 100644 --- a/include/podio/detail/RelationIOHelpers.h +++ b/include/podio/detail/RelationIOHelpers.h @@ -29,7 +29,7 @@ namespace podio::detail { /// @param id The ObjectID of the element that we are currently looking for template void tryAddTo(T, std::vector& relElements, const podio::CollectionBase* coll, const podio::ObjectID id) { - if (auto typedColl = dynamic_cast(coll)) { + if (auto typedColl = dynamic_cast(coll)) { const T tmp = (*typedColl)[id.index]; relElements.emplace_back(tmp); } @@ -87,7 +87,7 @@ void addMultiRelation(std::vector& relElements, const podio::Collection if constexpr (podio::detail::isInterfaceType) { addInterfaceToMultiRelation(relElements, coll, id); } else { - const auto* typeColl = static_cast(coll); + const auto* typeColl = static_cast(coll); relElements.emplace_back((*typeColl)[id.index]); } } From aaf4b519939d4ded590630feaf2751d23b168eb4 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 26 Jan 2022 20:23:16 +0100 Subject: [PATCH 03/79] [wip] Start working on templated associations --- include/podio/Association.h | 132 +++++++++++++++++++++++++++++++ include/podio/AssociationObj.h | 55 +++++++++++++ tests/unittests/CMakeLists.txt | 2 +- tests/unittests/associations.cpp | 16 ++++ 4 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 include/podio/Association.h create mode 100644 include/podio/AssociationObj.h create mode 100644 tests/unittests/associations.cpp diff --git a/include/podio/Association.h b/include/podio/Association.h new file mode 100644 index 000000000..003d06a29 --- /dev/null +++ b/include/podio/Association.h @@ -0,0 +1,132 @@ +#ifndef PODIO_ASSOCIATION_H +#define PODIO_ASSOCIATION_H + +#include "podio/AssociationObj.h" + +#include +#include // std::swap + +namespace podio { + +/** + * Generalized Association type for both Mutable and immutable (default) + * versions. User facing clases with the expected naming scheme are defined via + * template aliases are defined just below + */ +template +class AssociationT { +public: + /// Constructor + AssociationT() : m_obj(new AssociationObj()) { + m_obj->acquire(); + } + + /// Constructor with weight + AssociationT(float weight) : m_obj(new AssociationObj()) { + m_obj->acquire(); + m_obj->weight = weight; + } + + /// Constructor from existing AssociationObj + AssociationT(AssociationObj* obj) : m_obj(obj) { + if (m_obj) { + m_obj->acquire(); + } + } + + /// Copy constructor + AssociationT(const AssociationT& other) : m_obj(other.m_obj) { + if (m_obj) { + m_obj->acquire(); + } + } + + /// Assignment operator + AssociationT& operator=(AssociationT other) { + swap(*this, other); + return *this; + } + + /// Destructor + ~AssociationT() { + if (m_obj) { + m_obj->release(); + } + } + + float getWeight() const { + return m_obj->weight; + } + + /// Access the related-from object + const FromT getFrom() const { + if (!m_obj->m_from) { + return FromT(nullptr); + } + return FromT(m_obj->m_from); + } + + /// Access the related-to object + const ToT getTo() const { + if (!m_obj->m_to) { + return FromT(nullptr); + } + return FromT(m_obj->m_to); + } + + /// check whether the object is actually available + bool isAvailable() const { + return m_obj; + } + + /// disconnect from Association instance + void unlink() { + m_obj = nullptr; + } + + /// Get the ObjectID + podio::ObjectID getObjectID() const { + if (m_obj) { + return m_obj->id; + } + return podio::ObjectID{podio::ObjectID::invalid, podio::ObjectID::invalid}; + } + + unsigned int id() const { + return getObjectID().collectionID * 10000000 + getObjectID().index; + } + + bool operator==(const AssociationT& other) const { + return m_obj == other.m_obj; + } + + bool operator<(const AssociationT& other) const { + return m_obj < other.m_obj; + } + + friend void swap(AssociationT& a, AssociationT& b) { + using std::swap; + swap(a.m_obj, b.m_obj); // swap out the internal pointers + } + +private: + AssociationObj* m_obj{nullptr}; +}; // namespace podio + +template +using Association = AssociationT; + +template +using MutableAssociation = AssociationT; + +template +std::ostream& operator<<(std::ostream& os, const Association& assoc) { + return os << " id: " << assoc.id() << '\n' + << " weight: " << assoc.getWeight() << '\n' + << " from: " << assoc.getFrom().id() << '\n' + << " to: " << assoc.getTo().id() << '\n'; +} + +} // namespace podio + +#endif // PODIO_ASSOCIATION_H diff --git a/include/podio/AssociationObj.h b/include/podio/AssociationObj.h new file mode 100644 index 000000000..1cec8e376 --- /dev/null +++ b/include/podio/AssociationObj.h @@ -0,0 +1,55 @@ +#ifndef PODIO_ASSOCIATIONOBJ_H +#define PODIO_ASSOCIATIONOBJ_H + +#include "podio/ObjBase.h" +#include "podio/ObjectID.h" + +namespace podio { + +template +class AssociationObj : public podio::ObjBase { +public: + /// Constructor + AssociationObj() : + ObjBase{{podio::ObjectID::untracked, podio::ObjectID::untracked}, 0}, + weight(0.0f), + m_from(nullptr), + m_to(nullptr) { + } + + /// Constructor from ObjectID and weight (does not initialize relations yet!) + AssociationObj(const podio::ObjectID id_, float weight_) : ObjBase{id_, 0}, weight(weight_) { + } + + /// Copy constructor (deep-copy of relations) + AssociationObj(const AssociationObj& other) : + ObjBase{{podio::ObjectID::untracked, podio::ObjectID::untracked}, 0}, + weight(other.weight), + m_from(nullptr), + m_to(nullptr) { + if (other.m_from) { + m_from = new FromT(*other.m_from); + } + if (other.m_to) { + m_to = new ToT(*other.m_to); + } + } + + /// No assignment operator + AssociationObj& operator=(const AssociationObj&) = delete; + + /// Destructor + ~AssociationObj() { + delete m_from; + delete m_to; + } + +private: + float weight{}; + FromT* m_from{nullptr}; + ToT* m_to{nullptr}; +}; + +} // namespace podio + +#endif // PODIO_ASSOCIATIONOBJ_H diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index 0cb3a7214..144b97115 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -40,7 +40,7 @@ if(NOT Catch2_FOUND) endif() find_package(Threads REQUIRED) -add_executable(unittest_podio unittest.cpp frame.cpp buffer_factory.cpp interface_types.cpp std_interoperability.cpp) +add_executable(unittest_podio unittest.cpp frame.cpp buffer_factory.cpp interface_types.cpp std_interoperability.cpp associations.cpp) target_link_libraries(unittest_podio PUBLIC TestDataModel PRIVATE Catch2::Catch2WithMain Threads::Threads podio::podioRootIO) if (ENABLE_SIO) target_link_libraries(unittest_podio PRIVATE podio::podioSioIO) diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp new file mode 100644 index 000000000..d83ade5a9 --- /dev/null +++ b/tests/unittests/associations.cpp @@ -0,0 +1,16 @@ +#include "catch2/catch_test_macros.hpp" + +#include "podio/Association.h" + +#include "datamodel/ExampleCluster.h" +#include "datamodel/ExampleHit.h" + +// Test datatypes + +TEST_CASE("Association basics") { + using TestA = podio::Association; + + auto assoc = TestA(); + + REQUIRE(true); +} From 9b9456010abb4e9be2f2c61ceb9eef8f81d319fc Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 27 Jan 2022 16:25:39 +0100 Subject: [PATCH 04/79] [wip] Almost complete implementation of Association --- include/podio/Association.h | 70 +++++++++++--- include/podio/AssociationFwd.h | 35 +++++++ include/podio/AssociationObj.h | 5 + python/templates/MutableObject.h.jinja2 | 3 + python/templates/Object.h.jinja2 | 3 + tests/unittests/associations.cpp | 121 +++++++++++++++++++++++- 6 files changed, 219 insertions(+), 18 deletions(-) create mode 100644 include/podio/AssociationFwd.h diff --git a/include/podio/Association.h b/include/podio/Association.h index 003d06a29..b30595cd9 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -1,6 +1,7 @@ #ifndef PODIO_ASSOCIATION_H #define PODIO_ASSOCIATION_H +#include "podio/AssociationFwd.h" #include "podio/AssociationObj.h" #include @@ -15,20 +16,30 @@ namespace podio { */ template class AssociationT { + // The typedefs in AssociationFwd.h should make sure that at this point + // Mutable classes are stripped, i.e. the user should never be able to trigger + // these! + static_assert(std::is_same_v, FromT>, + "Associations need to be instantiated with the default types!"); + static_assert(std::is_same_v, ToT>, + "Associations need to be instantiated with the default types!"); + + using AssociationObjT = AssociationObj; + public: /// Constructor - AssociationT() : m_obj(new AssociationObj()) { + AssociationT() : m_obj(new AssociationObjT()) { m_obj->acquire(); } /// Constructor with weight - AssociationT(float weight) : m_obj(new AssociationObj()) { + AssociationT(float weight) : m_obj(new AssociationObjT()) { m_obj->acquire(); m_obj->weight = weight; } /// Constructor from existing AssociationObj - AssociationT(AssociationObj* obj) : m_obj(obj) { + AssociationT(AssociationObjT* obj) : m_obj(obj) { if (m_obj) { m_obj->acquire(); } @@ -47,6 +58,20 @@ class AssociationT { return *this; } + /// Implicit conversion of mutable to immutable associations + template && std::is_same_v>> + operator AssociationT() const { + return AssociationT(m_obj); + } + + /// Create a mutable deep-copy with identical relations + template && std::is_same_v>> + MutableAssociation clone() const { + return {new AssociationObjT(*m_obj)}; + } + /// Destructor ~AssociationT() { if (m_obj) { @@ -54,24 +79,47 @@ class AssociationT { } } + /// Get the weight of the association float getWeight() const { return m_obj->weight; } + /// Set the weight of the association + template > + void setWeight(float value) { + m_obj->weight = value; + } + /// Access the related-from object - const FromT getFrom() const { + FromT getFrom() const { if (!m_obj->m_from) { return FromT(nullptr); } - return FromT(m_obj->m_from); + return FromT(*(m_obj->m_from)); + } + + /// Set the related-from object + template , FromT>>> + void setFrom(FromU value) { + delete m_obj->m_from; + m_obj->m_from = new detail::GetDefT(value); } /// Access the related-to object - const ToT getTo() const { + ToT getTo() const { if (!m_obj->m_to) { - return FromT(nullptr); + return ToT(nullptr); } - return FromT(m_obj->m_to); + return ToT(*(m_obj->m_to)); + } + + /// Set the related-to object + template , ToT>>> + void setTo(ToU value) { + delete m_obj->m_to; + m_obj->m_to = new detail::GetDefT(value); } /// check whether the object is actually available @@ -113,12 +161,6 @@ class AssociationT { AssociationObj* m_obj{nullptr}; }; // namespace podio -template -using Association = AssociationT; - -template -using MutableAssociation = AssociationT; - template std::ostream& operator<<(std::ostream& os, const Association& assoc) { return os << " id: " << assoc.id() << '\n' diff --git a/include/podio/AssociationFwd.h b/include/podio/AssociationFwd.h new file mode 100644 index 000000000..1017ac68f --- /dev/null +++ b/include/podio/AssociationFwd.h @@ -0,0 +1,35 @@ +#ifndef PODIO_ASSOCIATIONFWD_H +#define PODIO_ASSOCIATIONFWD_H + +namespace podio { +namespace detail { + template + struct GetMutType { + using type = typename T::MutT; + }; + + template + using GetMutT = typename GetMutType::type; + + template + struct GetDefType { + using type = typename T::DefT; + }; + + template + using GetDefT = typename GetDefType::type; + +} // namespace detail + +// Forward declarations for a bit less typing below +template +class AssociationT; + +template +using Association = AssociationT, detail::GetDefT, false>; + +template +using MutableAssociation = AssociationT, detail::GetDefT, true>; +} // namespace podio + +#endif // PODIO_ASSOCIATIONFWD_H diff --git a/include/podio/AssociationObj.h b/include/podio/AssociationObj.h index 1cec8e376..491987878 100644 --- a/include/podio/AssociationObj.h +++ b/include/podio/AssociationObj.h @@ -1,6 +1,7 @@ #ifndef PODIO_ASSOCIATIONOBJ_H #define PODIO_ASSOCIATIONOBJ_H +#include "podio/AssociationFwd.h" #include "podio/ObjBase.h" #include "podio/ObjectID.h" @@ -8,6 +9,10 @@ namespace podio { template class AssociationObj : public podio::ObjBase { + + friend Association; + friend MutableAssociation; + public: /// Constructor AssociationObj() : diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2 index be0cd8805..2a7d22345 100644 --- a/python/templates/MutableObject.h.jinja2 +++ b/python/templates/MutableObject.h.jinja2 @@ -39,6 +39,9 @@ public: {{ macros.constructors_destructors(class.bare_type, Members, prefix='Mutable') }} public: + // Some typedefs for the Association handling + using MutT = Mutable{{ class.bare_type }}; + using DefT = {{ class.bare_type }}; {{ macros.member_getters(Members, use_get_syntax) }} {{ macros.single_relation_getters(OneToOneRelations, use_get_syntax) }} diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2 index 077fe8763..b36454d0a 100644 --- a/python/templates/Object.h.jinja2 +++ b/python/templates/Object.h.jinja2 @@ -49,6 +49,9 @@ public: static {{ class.bare_type }} makeEmpty(); public: + // Some typedefs for the Association handling + using MutT = Mutable{{ class.bare_type }}; + using DefT = {{ class.bare_type }}; {{ macros.member_getters(Members, use_get_syntax) }} {{ macros.single_relation_getters(OneToOneRelations, use_get_syntax) }} diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index d83ade5a9..aaa0a2a92 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -4,13 +4,126 @@ #include "datamodel/ExampleCluster.h" #include "datamodel/ExampleHit.h" +#include "datamodel/MutableExampleCluster.h" +#include "datamodel/MutableExampleHit.h" + +#include // Test datatypes +using TestA = podio::Association; +using TestMA = podio::MutableAssociation; + +TEST_CASE("Association constness", "[associations][static-checks]") { + STATIC_REQUIRE(std::is_same_v().getFrom()), ExampleHit>); + STATIC_REQUIRE(std::is_same_v().getTo()), ExampleCluster>); + + STATIC_REQUIRE(std::is_same_v().getFrom()), ExampleHit>); + STATIC_REQUIRE(std::is_same_v().getTo()), ExampleCluster>); +} + +TEST_CASE("Association basics", "[associations]") { + auto cluster = MutableExampleCluster(); + auto hit = MutableExampleHit(); + + auto mutAssoc = TestMA(); + mutAssoc.setWeight(3.14f); + mutAssoc.setFrom(hit); + mutAssoc.setTo(cluster); + + REQUIRE(mutAssoc.getWeight() == 3.14f); + REQUIRE(mutAssoc.getFrom() == hit); + REQUIRE(mutAssoc.getTo() == cluster); + + SECTION("Copying") { + auto otherAssoc = mutAssoc; + REQUIRE(otherAssoc.getWeight() == 3.14f); + REQUIRE(otherAssoc.getFrom() == hit); + REQUIRE(otherAssoc.getTo() == cluster); + + auto otherCluster = ExampleCluster(); + auto otherHit = ExampleHit(); + otherAssoc.setFrom(otherHit); + otherAssoc.setTo(otherCluster); + otherAssoc.setWeight(42.0f); + REQUIRE(otherAssoc.getWeight() == 42.0f); + REQUIRE(otherAssoc.getFrom() == otherHit); + REQUIRE(otherAssoc.getTo() == otherCluster); + + // Make sure original association changes as well + REQUIRE(mutAssoc.getWeight() == 42.0f); + REQUIRE(mutAssoc.getFrom() == otherHit); + REQUIRE(mutAssoc.getTo() == otherCluster); + } + + SECTION("Assignment") { + auto otherAssoc = TestMA(); + otherAssoc = mutAssoc; + REQUIRE(otherAssoc.getWeight() == 3.14f); + REQUIRE(otherAssoc.getFrom() == hit); + REQUIRE(otherAssoc.getTo() == cluster); + + auto otherCluster = ExampleCluster(); + auto otherHit = ExampleHit(); + otherAssoc.setFrom(otherHit); + otherAssoc.setTo(otherCluster); + otherAssoc.setWeight(42.0f); + REQUIRE(otherAssoc.getWeight() == 42.0f); + REQUIRE(otherAssoc.getFrom() == otherHit); + REQUIRE(otherAssoc.getTo() == otherCluster); + + // Make sure original association changes as well + REQUIRE(mutAssoc.getWeight() == 42.0f); + REQUIRE(mutAssoc.getFrom() == otherHit); + REQUIRE(mutAssoc.getTo() == otherCluster); + } + + SECTION("Implicit conversion") { + // Use an immediately invoked lambda to check that the implicit conversion + // is working as desired + [hit, cluster](TestA assoc) { + REQUIRE(assoc.getWeight() == 3.14f); + REQUIRE(assoc.getFrom() == hit); + REQUIRE(assoc.getTo() == cluster); + }(mutAssoc); + } + + SECTION("Cloning") { + auto otherAssoc = mutAssoc.clone(); + REQUIRE(otherAssoc.getWeight() == 3.14f); + REQUIRE(otherAssoc.getFrom() == hit); + REQUIRE(otherAssoc.getTo() == cluster); + + auto otherCluster = ExampleCluster(); + auto otherHit = ExampleHit(); + otherAssoc.setFrom(otherHit); + otherAssoc.setTo(otherCluster); + otherAssoc.setWeight(42.0f); + REQUIRE(otherAssoc.getWeight() == 42.0f); + REQUIRE(otherAssoc.getFrom() == otherHit); + REQUIRE(otherAssoc.getTo() == otherCluster); + + // Make sure original association is unchanged + REQUIRE(mutAssoc.getWeight() == 3.14f); + REQUIRE(mutAssoc.getFrom() == hit); + REQUIRE(mutAssoc.getTo() == cluster); -TEST_CASE("Association basics") { - using TestA = podio::Association; + // Check cloning from an immutable one + TestA assoc = mutAssoc; + auto anotherAssoc = assoc.clone(); + anotherAssoc.setFrom(otherHit); + anotherAssoc.setTo(otherCluster); + anotherAssoc.setWeight(42.0f); + REQUIRE(anotherAssoc.getWeight() == 42.0f); + REQUIRE(anotherAssoc.getFrom() == otherHit); + REQUIRE(anotherAssoc.getTo() == otherCluster); + } - auto assoc = TestA(); + SECTION("Equality operator") { + auto otherAssoc = mutAssoc; + REQUIRE(otherAssoc == mutAssoc); - REQUIRE(true); + // Mutable and immutable associations should be comparable + TestA assoc = mutAssoc; + REQUIRE(assoc == mutAssoc); + } } From 309d777069acb38fa64c63aa29f5ed26dc576b0a Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 27 Jan 2022 16:53:54 +0100 Subject: [PATCH 05/79] [wip] Scaffolding of AssociationCollection --- include/podio/AssociationCollection.h | 83 +++++++++++++++++++++++++ python/templates/MutableObject.h.jinja2 | 1 + python/templates/Object.h.jinja2 | 2 + tests/unittests/associations.cpp | 11 +++- 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 include/podio/AssociationCollection.h diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h new file mode 100644 index 000000000..48bcd9f73 --- /dev/null +++ b/include/podio/AssociationCollection.h @@ -0,0 +1,83 @@ +#ifndef PODIO_ASSOCIATIONCOLLECTION_H +#define PODIO_ASSOCIATIONCOLLECTION_H + +#include "podio/Association.h" +#include "podio/AssociationFwd.h" +#include "podio/AssociationObj.h" +#include "podio/CollectionBase.h" +#include "podio/CollectionBuffers.h" +#include "podio/ICollectionProvider.h" + +namespace podio { + +template +class AssociationCollection : public podio::CollectionBase { + static_assert(std::is_same_v>, + "Associations need to be instantiated with the default types!"); + static_assert(std::is_same_v>, + "Associations need to be instantiated with the default types!"); + +public: + size_t size() const override { + // TODO + } + + void clear() override { + // TODO + } + + bool isValid() const override { + // TODO + } + + podio::CollectionBuffers getBuffers() override { + // TOOD + } + + std::string getTypeName() const override { + return std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; + } + + std::string getValueTypeName() const override { + return std::string("podio::Association<") + FromT::TypeName + "," + ToT::TypeName + ">"; + } + + std::string getDataTypeName() const override { + return "float"; + } + + bool isSubsetCollection() const override { + // TODO + } + + void setSubsetCollection(bool) override { + // TODO + } + + void setID(unsigned id) override { + m_collectionID = id; + } + + unsigned getID() const override { + return m_collectionID; + } + + void prepareForWrite() override { + // TODO + } + + void prepareAfterRead() override { + // TODO + } + + bool setReferences(const ICollectionProvider*) override { + // TODO + } + +private: + unsigned m_collectionID{}; +}; + +} // namespace podio + +#endif // PODIO_ASSOCIATIONCOLLECTION_H diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2 index 2a7d22345..8062e2e1b 100644 --- a/python/templates/MutableObject.h.jinja2 +++ b/python/templates/MutableObject.h.jinja2 @@ -42,6 +42,7 @@ public: // Some typedefs for the Association handling using MutT = Mutable{{ class.bare_type }}; using DefT = {{ class.bare_type }}; + static constexpr auto TypeName = "{{ class.namespace }}::Mutable{{ class.bare_type }}"; {{ macros.member_getters(Members, use_get_syntax) }} {{ macros.single_relation_getters(OneToOneRelations, use_get_syntax) }} diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2 index b36454d0a..4df1f3fad 100644 --- a/python/templates/Object.h.jinja2 +++ b/python/templates/Object.h.jinja2 @@ -53,6 +53,8 @@ public: using MutT = Mutable{{ class.bare_type }}; using DefT = {{ class.bare_type }}; + static constexpr auto TypeName = "{{ class.full_type }}"; + {{ macros.member_getters(Members, use_get_syntax) }} {{ macros.single_relation_getters(OneToOneRelations, use_get_syntax) }} {{ macros.multi_relation_handling(OneToManyRelations + VectorMembers, use_get_syntax) }} diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index aaa0a2a92..f8739ebfa 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -1,6 +1,6 @@ #include "catch2/catch_test_macros.hpp" -#include "podio/Association.h" +#include "podio/AssociationCollection.h" #include "datamodel/ExampleCluster.h" #include "datamodel/ExampleHit.h" @@ -12,6 +12,7 @@ // Test datatypes using TestA = podio::Association; using TestMA = podio::MutableAssociation; +using TestAColl = podio::AssociationCollection; TEST_CASE("Association constness", "[associations][static-checks]") { STATIC_REQUIRE(std::is_same_v().getFrom()), ExampleHit>); @@ -127,3 +128,11 @@ TEST_CASE("Association basics", "[associations]") { REQUIRE(assoc == mutAssoc); } } + +TEST_CASE("AssociationCollection", "[associations]") { + auto coll = TestAColl(); + + REQUIRE(coll.getValueTypeName() == "podio::Association"); + + REQUIRE(true); +} From 56df83b287e7c0008c31f7994201b299332702d4 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 28 Jan 2022 14:26:40 +0100 Subject: [PATCH 06/79] [wip] AssociationColllection implementation --- include/podio/Association.h | 1 + include/podio/AssociationCollection.h | 137 +++++++++++-- include/podio/AssociationCollectionData.h | 189 ++++++++++++++++++ include/podio/AssociationCollectionIterator.h | 46 +++++ include/podio/AssociationFwd.h | 35 +++- include/podio/AssociationObj.h | 2 +- python/podio_gen/cpp_generator.py | 3 + python/templates/MutableObject.h.jinja2 | 6 +- python/templates/Object.h.jinja2 | 5 +- tests/unittests/associations.cpp | 101 +++++++++- 10 files changed, 496 insertions(+), 29 deletions(-) create mode 100644 include/podio/AssociationCollectionData.h create mode 100644 include/podio/AssociationCollectionIterator.h diff --git a/include/podio/Association.h b/include/podio/Association.h index b30595cd9..cdc5548ec 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -25,6 +25,7 @@ class AssociationT { "Associations need to be instantiated with the default types!"); using AssociationObjT = AssociationObj; + friend AssociationCollectionIteratorT; public: /// Constructor diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index 48bcd9f73..d7288fd92 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -2,12 +2,16 @@ #define PODIO_ASSOCIATIONCOLLECTION_H #include "podio/Association.h" +#include "podio/AssociationCollectionData.h" +#include "podio/AssociationCollectionIterator.h" #include "podio/AssociationFwd.h" #include "podio/AssociationObj.h" #include "podio/CollectionBase.h" #include "podio/CollectionBuffers.h" #include "podio/ICollectionProvider.h" +#include + namespace podio { template @@ -17,21 +21,98 @@ class AssociationCollection : public podio::CollectionBase { static_assert(std::is_same_v>, "Associations need to be instantiated with the default types!"); + // convenience typedefs + using AssocT = Association; + using MutableAssocT = MutableAssociation; + public: + using const_iterator = AssociationCollectionIterator; + using iterator = AssociationMutableCollectionIterator; + + /// Append a new association to the collection and return this object + MutableAssocT create() { + if (m_isSubsetColl) { + throw std::logic_error("Cannot create new elements on a subset collection"); + } + + auto obj = m_storage.entries.emplace_back(new AssociationObj()); + obj->id = {int(m_storage.entries.size() - 1), m_collectionID}; + return MutableAssocT(obj); + } + + /// Returns the immutable object of given index + AssocT operator[](unsigned int index) const { + return AssocT(m_storage.entries[index]); + } + /// Returns the mutable object of given index + MutableAssocT operator[](unsigned int index) { + return MutableAssocT(m_storage.entries[index]); + } + /// Returns the immutable object of given index + AssocT at(unsigned int index) const { + return AssocT(m_storage.entries.at(index)); + } + /// Returns the mutable object of given index + MutableAssocT at(unsigned int index) { + return MutableAssocT(m_storage.entries.at(index)); + } + + void push_back(AssocT object) { + // We have to do different things here depending on whether this is a + // subset collection or not. A normal collection cannot collect objects + // that are already part of another collection, while a subset collection + // can only collect such objects + if (!m_isSubsetColl) { + auto obj = object.m_obj; + if (obj->id.index == podio::ObjectID::untracked) { + const auto size = m_storage.entries.size(); + obj->id = {(int)size, m_collectionID}; + m_storage.entries.push_back(obj); + } else { + throw std::invalid_argument("Object already in a collection. Cannot add it to a second collection"); + } + } else { + const auto obj = object.m_obj; + if (obj->id.index < 0) { + throw std::invalid_argument( + "Objects can only be stored in a subset collection if they are already elements of a collection"); + } + m_storage.entries.push_back(obj); + // No need to handle any relations here, since this is already done by the + // "owning" collection + } + } + + /// Number of elements in the collection size_t size() const override { - // TODO + return m_storage.entries.size(); } void clear() override { - // TODO + m_storage.clear(m_isSubsetColl); + m_isPrepared = false; + } + + // support for the iterator protocol + const_iterator begin() const { + return const_iterator(0, &m_storage.entries); + } + const_iterator end() const { + return const_iterator(m_storage.entries.size(), &m_storage.entries); + } + iterator begin() { + return iterator(0, &m_storage.entries); + } + iterator end() { + return iterator(m_storage.entries.size(), &m_storage.entries); } bool isValid() const override { - // TODO + return m_isValid; } podio::CollectionBuffers getBuffers() override { - // TOOD + return m_storage.getCollectionBuffers(m_isSubsetColl); } std::string getTypeName() const override { @@ -47,11 +128,18 @@ class AssociationCollection : public podio::CollectionBase { } bool isSubsetCollection() const override { - // TODO + return m_isSubsetColl; } - void setSubsetCollection(bool) override { - // TODO + void setSubsetCollection(bool setSubset) override { + if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { + throw std::logic_error("Cannot change the character of a collection that already contains elements"); + } + + if (setSubset) { + m_storage.makeSubsetCollection(); + } + m_isSubsetColl = setSubset; } void setID(unsigned id) override { @@ -63,19 +151,44 @@ class AssociationCollection : public podio::CollectionBase { } void prepareForWrite() override { - // TODO + // If the collection has been prepared already there is nothing to do + if (m_isPrepared) { + return; + } + m_storage.prepareForWrite(m_isSubsetColl); + m_isPrepared = true; } void prepareAfterRead() override { - // TODO + // No need to go through this again if we have already done it + if (m_isPrepared) { + return; + } + + if (!m_isSubsetColl) { + // Subset collections do not store any data that would require post-processing + m_storage.prepareAfterRead(m_collectionID); + } + // Preparing a collection doesn't affect the underlying I/O buffers, so this + // collection is still prepared + m_isPrepared = true; } - bool setReferences(const ICollectionProvider*) override { - // TODO + bool setReferences(const ICollectionProvider* collectionProvider) override { + return m_storage.setReferences(collectionProvider, m_isSubsetColl); } private: - unsigned m_collectionID{}; + // For setReferences, we need to give our own CollectionData access to our + // private entries. Otherwise we would need to expose a public member function + // that gives access to the Obj* which is definitely not what we want + friend class AssociationCollectionData; + + bool m_isValid{false}; + bool m_isPrepared{false}; + bool m_isSubsetColl{false}; + unsigned m_collectionID{0}; + AssociationCollectionData m_storage; }; } // namespace podio diff --git a/include/podio/AssociationCollectionData.h b/include/podio/AssociationCollectionData.h new file mode 100644 index 000000000..25b533917 --- /dev/null +++ b/include/podio/AssociationCollectionData.h @@ -0,0 +1,189 @@ +#ifndef PODIO_ASSOCIATIONCOLLECTIONDATA_H +#define PODIO_ASSOCIATIONCOLLECTIONDATA_H + +#include "podio/AssociationFwd.h" +#include "podio/AssociationObj.h" +#include "podio/CollectionBase.h" +#include "podio/CollectionBuffers.h" +#include "podio/ICollectionProvider.h" + +#include +#include +#include + +namespace podio { + +template +class AssociationCollectionData { +public: + AssociationObjPointerContainer entries{}; + + AssociationCollectionData() : + m_rel_from(new std::vector()), m_rel_to(new std::vector()), m_data(new AssociationDataContainer()) { + m_refCollections.emplace_back(std::make_unique>()); + m_refCollections.emplace_back(std::make_unique>()); + } + AssociationCollectionData(const AssociationCollectionData&) = delete; + AssociationCollectionData& operator=(const AssociationCollectionData&) = delete; + AssociationCollectionData(AssociationCollectionData&&) = default; + AssociationCollectionData& operator=(AssociationCollectionData&&) = default; + ~AssociationCollectionData() = default; + + podio::CollectionBuffers getCollectionBuffers(bool isSubsetColl) { + return {isSubsetColl ? nullptr : (void*)&m_data, &m_refCollections, nullptr}; + } + + void clear(bool isSubsetColl) { + if (isSubsetColl) { + // We don't own the objects so no cleanup to do here + entries.clear(); + // Clear the ObjectID I/O buffer + for (auto& pointer : m_refCollections) { + pointer->clear(); + } + return; + } + + // Normal collections manage a bit more and have to clean up a bit more + if (m_data) { + m_data->clear(); + } + for (auto& pointer : m_refCollections) { + pointer->clear(); + } + if (m_rel_from) { + for (auto& item : (*m_rel_from)) { + item.unlink(); + } + m_rel_from->clear(); + } + + if (m_rel_to) { + for (auto& item : (*m_rel_to)) { + item.unlink(); + } + m_rel_to->clear(); + } + + for (auto& obj : entries) { + delete obj; + } + entries.clear(); + } + + void prepareForWrite(bool isSubsetColl) { + for (auto& pointer : m_refCollections) { + pointer->clear(); + } + + // If this is a subset collection use the relation storing mechanism to + // store the ObjectIDs of all reference objects and nothing else + if (isSubsetColl) { + for (const auto* obj : entries) { + m_refCollections[0]->emplace_back(obj->id); + } + return; + } + + m_data->reserve(entries.size()); + for (const auto obj : entries) { + m_data->push_back(obj->weight); + + if (obj->m_from) { + m_refCollections[0]->emplace_back(obj->m_from->getObjectID()); + } else { + m_refCollections[0]->push_back({podio::ObjectID::invalid, podio::ObjectID::invalid}); + } + + if (obj->m_to) { + m_refCollections[1]->emplace_back(obj->m_to->getObjectID()); + } else { + m_refCollections[1]->push_back({podio::ObjectID::invalid, podio::ObjectID::invalid}); + } + } + } + + void prepareAfterRead(int collectionID) { + int index = 0; + for (const auto data : *m_data) { + auto obj = new AssociationObj({index++, collectionID}, data); + entries.emplace_back(obj); + } + + // Keep the I/O data buffer to keep the preparedForWrite state intact + } + + bool setReferences(const podio::ICollectionProvider* collectionProvider, bool isSubsetColl) { + if (isSubsetColl) { + for (const auto& id : *m_refCollections[0]) { + podio::CollectionBase* coll{nullptr}; + AssociationObj* obj{nullptr}; + if (collectionProvider->get(id.collectionID, coll)) { + auto* tmp_coll = static_cast*>(coll); + obj = tmp_coll->m_storage.entries[id.index]; + } + entries.push_back(obj); + } + return true; // TODO: check success, how? + } + + // Normal collections have to resolve all relations + for (size_t i = 0; i < entries.size(); ++i) { + const auto id = (*m_refCollections[0])[i]; + if (id.index != podio::ObjectID::invalid) { + podio::CollectionBase* coll = nullptr; + if (!collectionProvider->get(id.collectionID, coll)) { + entries[i]->m_from = nullptr; + continue; + } + auto* tmp_coll = static_cast*>(coll); + entries[i]->m_from = new FromT((*tmp_coll)[id.index]); + } else { + entries[i]->m_from = nullptr; + } + } + + for (size_t i = 0; i < entries.size(); ++i) { + const auto id = (*m_refCollections[1])[i]; + if (id.index != podio::ObjectID::invalid) { + podio::CollectionBase* coll = nullptr; + if (!collectionProvider->get(id.collectionID, coll)) { + entries[i]->m_to = nullptr; + continue; + } + auto* tmp_coll = static_cast*>(coll); + entries[i]->m_to = new ToT((*tmp_coll)[id.index]); + } else { + entries[i]->m_to = nullptr; + } + } + + return true; // TODO: check success, how? + } + + void makeSubsetCollection() { + // Subset collections do not need all the data buffers that normal + // collections need, so we can free them here + m_data.reset(nullptr); + + m_rel_from.reset(nullptr); + m_rel_to.reset(nullptr); + + // Subset collections need one vector of ObjectIDs for I/O purposes. + m_refCollections.resize(1); + m_refCollections[0] = std::make_unique>(); + } + +private: + // members to handle relations + podio::UVecPtr m_rel_from{}; + podio::UVecPtr m_rel_to{}; + + // I/O related buffers (as far as necessary) + podio::CollRefCollection m_refCollections{}; + std::unique_ptr m_data{nullptr}; +}; + +} // namespace podio + +#endif // PODIO_ASSOCIATIONCOLLECTIONDATA_H diff --git a/include/podio/AssociationCollectionIterator.h b/include/podio/AssociationCollectionIterator.h new file mode 100644 index 000000000..0d3946140 --- /dev/null +++ b/include/podio/AssociationCollectionIterator.h @@ -0,0 +1,46 @@ +#ifndef PODIO_ASSOCIATIONCOLLECTIONITERATOR_H +#define PODIO_ASSOCIATIONCOLLECTIONITERATOR_H + +#include "podio/AssociationCollectionData.h" +#include "podio/AssociationFwd.h" + +namespace podio { +template +class AssociationCollectionIteratorT { + using AssocT = AssociationT; + +public: + AssociationCollectionIteratorT(size_t index, const AssociationObjPointerContainer* coll) : + m_index(index), m_object(nullptr), m_collection(coll) { + } + + AssociationCollectionIteratorT(const AssociationCollectionIteratorT&) = delete; + AssociationCollectionIteratorT& operator=(const AssociationCollectionIteratorT&) = delete; + + bool operator!=(const AssociationCollectionIteratorT& other) const { + return m_index != other.m_index; // TODO: may not be complete + } + + AssocT operator*() { + m_object.m_obj = (*m_collection)[m_index]; + return m_object; + } + + AssocT* operator->() { + m_object.m_obj = (*m_collection)[m_index]; + return &m_object; + } + + AssociationCollectionIteratorT& operator++() { + ++m_index; + return *this; + } + +private: + size_t m_index; + AssocT m_object; + const AssociationObjPointerContainer* m_collection; +}; +} // namespace podio + +#endif // PODIO_ASSOCIATIONCOLLECTIONITERATOR_H diff --git a/include/podio/AssociationFwd.h b/include/podio/AssociationFwd.h index 1017ac68f..3e36dfc90 100644 --- a/include/podio/AssociationFwd.h +++ b/include/podio/AssociationFwd.h @@ -1,6 +1,9 @@ #ifndef PODIO_ASSOCIATIONFWD_H #define PODIO_ASSOCIATIONFWD_H +#include +#include + namespace podio { namespace detail { template @@ -19,9 +22,26 @@ namespace detail { template using GetDefT = typename GetDefType::type; + template + struct GetCollType { + using type = typename T::CollT; + }; + + template + using GetCollT = typename GetCollType::type; + } // namespace detail -// Forward declarations for a bit less typing below +// Forward declarations and typedefs used throughout the whole Association +// business +template +class AssociationObj; + +template +using AssociationObjPointerContainer = std::deque*>; + +using AssociationDataContainer = std::vector; + template class AssociationT; @@ -30,6 +50,19 @@ using Association = AssociationT, detail::GetDefT, f template using MutableAssociation = AssociationT, detail::GetDefT, true>; + +template +class AssociationCollection; + +template +class AssociationCollectionIteratorT; + +template +using AssociationCollectionIterator = AssociationCollectionIteratorT; + +template +using AssociationMutableCollectionIterator = AssociationCollectionIteratorT; + } // namespace podio #endif // PODIO_ASSOCIATIONFWD_H diff --git a/include/podio/AssociationObj.h b/include/podio/AssociationObj.h index 491987878..d5beffee4 100644 --- a/include/podio/AssociationObj.h +++ b/include/podio/AssociationObj.h @@ -49,7 +49,7 @@ class AssociationObj : public podio::ObjBase { delete m_to; } -private: +public: float weight{}; FromT* m_from{nullptr}; ToT* m_to{nullptr}; diff --git a/python/podio_gen/cpp_generator.py b/python/podio_gen/cpp_generator.py index 61fdea6dd..09e17141c 100644 --- a/python/podio_gen/cpp_generator.py +++ b/python/podio_gen/cpp_generator.py @@ -217,6 +217,9 @@ def _preprocess_for_class(self, datatype): """Do the preprocessing that is necessary for the classes and Mutable classes""" includes = set(datatype["includes_data"]) fwd_declarations = defaultdict(list) + fwd_declarations[datatype["class"].namespace] = [ + f"{datatype['class'].bare_type}Collection" + ] includes_cc = set() for member in datatype["Members"]: diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2 index 8062e2e1b..d10db4733 100644 --- a/python/templates/MutableObject.h.jinja2 +++ b/python/templates/MutableObject.h.jinja2 @@ -40,8 +40,10 @@ public: public: // Some typedefs for the Association handling - using MutT = Mutable{{ class.bare_type }}; - using DefT = {{ class.bare_type }}; + using MutT = {{ class.namespace }}::Mutable{{ class.bare_type }}; + using DefT = {{ class.namespace }}::{{ class.bare_type }}; + using CollT = {{ class.namespace }}::{{ class.bare_type }}Collection; + static constexpr auto TypeName = "{{ class.namespace }}::Mutable{{ class.bare_type }}"; {{ macros.member_getters(Members, use_get_syntax) }} diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2 index 4df1f3fad..e9ba70305 100644 --- a/python/templates/Object.h.jinja2 +++ b/python/templates/Object.h.jinja2 @@ -50,8 +50,9 @@ public: public: // Some typedefs for the Association handling - using MutT = Mutable{{ class.bare_type }}; - using DefT = {{ class.bare_type }}; + using MutT = {{ class.namespace }}::Mutable{{ class.bare_type }}; + using DefT = {{ class.namespace }}::{{ class.bare_type }}; + using CollT = {{ class.namespace }}::{{ class.bare_type }}Collection; static constexpr auto TypeName = "{{ class.full_type }}"; diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index f8739ebfa..e80382fe8 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -2,21 +2,22 @@ #include "podio/AssociationCollection.h" -#include "datamodel/ExampleCluster.h" -#include "datamodel/ExampleHit.h" -#include "datamodel/MutableExampleCluster.h" -#include "datamodel/MutableExampleHit.h" +#include "datamodel/ExampleClusterCollection.h" +#include "datamodel/ExampleHitCollection.h" #include -// Test datatypes +// Test datatypes (spelling them out here explicitly to make sure that +// assumptions about typedefs actually hold) using TestA = podio::Association; -using TestMA = podio::MutableAssociation; +using TestMutA = podio::MutableAssociation; using TestAColl = podio::AssociationCollection; +using TestAIter = podio::AssociationCollectionIterator; +using TestAMutIter = podio::AssociationMutableCollectionIterator; TEST_CASE("Association constness", "[associations][static-checks]") { - STATIC_REQUIRE(std::is_same_v().getFrom()), ExampleHit>); - STATIC_REQUIRE(std::is_same_v().getTo()), ExampleCluster>); + STATIC_REQUIRE(std::is_same_v().getFrom()), ExampleHit>); + STATIC_REQUIRE(std::is_same_v().getTo()), ExampleCluster>); STATIC_REQUIRE(std::is_same_v().getFrom()), ExampleHit>); STATIC_REQUIRE(std::is_same_v().getTo()), ExampleCluster>); @@ -26,7 +27,7 @@ TEST_CASE("Association basics", "[associations]") { auto cluster = MutableExampleCluster(); auto hit = MutableExampleHit(); - auto mutAssoc = TestMA(); + auto mutAssoc = TestMutA(); mutAssoc.setWeight(3.14f); mutAssoc.setFrom(hit); mutAssoc.setTo(cluster); @@ -57,7 +58,7 @@ TEST_CASE("Association basics", "[associations]") { } SECTION("Assignment") { - auto otherAssoc = TestMA(); + auto otherAssoc = TestMutA(); otherAssoc = mutAssoc; REQUIRE(otherAssoc.getWeight() == 3.14f); REQUIRE(otherAssoc.getFrom() == hit); @@ -129,10 +130,88 @@ TEST_CASE("Association basics", "[associations]") { } } -TEST_CASE("AssociationCollection", "[associations]") { +TEST_CASE("AssociationCollection basics", "[associations]") { auto coll = TestAColl(); REQUIRE(coll.getValueTypeName() == "podio::Association"); REQUIRE(true); } + +TEST_CASE("AssociationCollection constness", "[associations][static-checks][const-correctness]") { + // Test type-aliases in AssociationCollection + STATIC_REQUIRE(std::is_same_v); + STATIC_REQUIRE(std::is_same_v); + + SECTION("const collections with const iterators") { + const auto coll = TestAColl(); + // this essentially checks the whole "chain" from begin() / end() through + // iterator operators + for (auto assoc : coll) { + STATIC_REQUIRE(std::is_same_v); // const collection iterators should only return + // immutable objects + } + + // check the individual steps again from above, to see where things fail if they fail + STATIC_REQUIRE(std::is_same_v().begin()), + TestAColl::const_iterator>); // const collectionb begin() should return a + // AssociationCollectionIterator + + STATIC_REQUIRE(std::is_same_v().end()), + TestAColl::const_iterator>); // const collectionb end() should return a + // AssociationCollectionIterator + + STATIC_REQUIRE(std::is_same_v().begin()), + TestA>); // AssociationCollectionIterator should only give access to immutable + // objects + + STATIC_REQUIRE(std::is_same_v().operator->()), + TestA*>); // AssociationCollectionIterator should only give access to immutable + // objects + } + + SECTION("non-const collections with mutable iterators") { + auto coll = TestAColl(); + // this essentially checks the whole "chain" from begin() / end() through + // iterator operators + for (auto assoc : coll) { + STATIC_REQUIRE(std::is_same_v); // collection iterators should return return + // mutable objects + } + + // check the individual steps again from above, to see where things fail if they fail + STATIC_REQUIRE(std::is_same_v().begin()), + TestAColl::iterator>); // collection begin() should return a + // MutableCollectionIterator + + STATIC_REQUIRE(std::is_same_v().end()), + TestAColl::iterator>); // collectionb end() should return a + // MutableCollectionIterator + + STATIC_REQUIRE(std::is_same_v().begin()), + TestMutA>); // MutableCollectionIterator should give access to immutable + // mutable objects + + STATIC_REQUIRE(std::is_same_v().operator->()), + TestMutA*>); // MutableCollectionIterator should give access to immutable + // mutable objects + } + + SECTION("const correct indexed access to const collections") { + STATIC_REQUIRE(std::is_same_v()[0]), + TestA>); // const collections should only have indexed indexed access to immutable + // objects + + STATIC_REQUIRE(std::is_same_v().at(0)), + TestA>); // const collections should only have indexed indexed access to immutable + // objects + } + + SECTION("const correct indexed access to collections") { + STATIC_REQUIRE(std::is_same_v()[0]), + TestMutA>); // collections should have indexed indexed access to mutable objects + + STATIC_REQUIRE(std::is_same_v().at(0)), + TestMutA>); // collections should have indexed indexed access to mutable objects + } +} From f9cdaed61f0563bce24398530619408e4989502b Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 28 Jan 2022 14:44:10 +0100 Subject: [PATCH 07/79] [wip] unit tests for subset collection functionality --- include/podio/Association.h | 1 + include/podio/AssociationCollection.h | 4 +-- tests/unittests/associations.cpp | 42 +++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/include/podio/Association.h b/include/podio/Association.h index cdc5548ec..dd147b0e2 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -25,6 +25,7 @@ class AssociationT { "Associations need to be instantiated with the default types!"); using AssociationObjT = AssociationObj; + friend AssociationCollection; friend AssociationCollectionIteratorT; public: diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index d7288fd92..5de46bf72 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -131,7 +131,7 @@ class AssociationCollection : public podio::CollectionBase { return m_isSubsetColl; } - void setSubsetCollection(bool setSubset) override { + void setSubsetCollection(bool setSubset = true) override { if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { throw std::logic_error("Cannot change the character of a collection that already contains elements"); } @@ -187,7 +187,7 @@ class AssociationCollection : public podio::CollectionBase { bool m_isValid{false}; bool m_isPrepared{false}; bool m_isSubsetColl{false}; - unsigned m_collectionID{0}; + int m_collectionID{0}; AssociationCollectionData m_storage; }; diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index e80382fe8..023d9aff2 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -215,3 +215,45 @@ TEST_CASE("AssociationCollection constness", "[associations][static-checks][cons TestMutA>); // collections should have indexed indexed access to mutable objects } } + +TEST_CASE("AssociationCollection subset collection", "[associations]") { + auto assocs = TestAColl(); + auto assoc1 = assocs.create(); + assoc1.setWeight(1.0f); + auto assoc2 = assocs.create(); + assoc2.setWeight(2.0f); + + auto assocRefs = TestAColl(); + assocRefs.setSubsetCollection(); + for (const auto a : assocs) { + assocRefs.push_back(a); + } + + SECTION("Collection iterators work with subset collections") { + + // index-based looping / access + for (size_t i = 0; i < assocRefs.size(); ++i) { + REQUIRE(assocRefs[i].getWeight() == i + 1); + } + + // range-based for loop + int index = 1; + for (const auto a : assocRefs) { + REQUIRE(a.getWeight() == index++); + } + } + + SECTION("Conversion failures") { + // Cannot convert into a subset collection if elements already present + REQUIRE_THROWS_AS(assocs.setSubsetCollection(), std::logic_error); + + // Connot convert a subset collection into a normal collection + REQUIRE_THROWS_AS(assocRefs.setSubsetCollection(false), std::logic_error); + } + + SECTION("Subset collection only handles tracked objects") { + auto assoc = TestA(); + REQUIRE_THROWS_AS(assocRefs.push_back(assoc), std::invalid_argument); + REQUIRE_THROWS_AS(assocRefs.create(), std::logic_error); + } +} From aa77526fb0fe92766f6abd2d7624dc7912f55e38 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 28 Jan 2022 17:20:36 +0100 Subject: [PATCH 08/79] [wip] Proper move semantics --- include/podio/AssociationCollection.h | 15 ++++- tests/unittests/associations.cpp | 91 ++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index 5de46bf72..e7d577c5d 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -29,6 +29,19 @@ class AssociationCollection : public podio::CollectionBase { using const_iterator = AssociationCollectionIterator; using iterator = AssociationMutableCollectionIterator; + AssociationCollection() = default; + + // Move-only type + AssociationCollection(const AssociationCollection&) = delete; + AssociationCollection& operator=(const AssociationCollection&) = delete; + AssociationCollection(AssociationCollection&&) = default; + AssociationCollection& operator=(AssociationCollection&&) = default; + + ~AssociationCollection() { + // Need the storage how to clean up + m_storage.clear(m_isSubsetColl); + } + /// Append a new association to the collection and return this object MutableAssocT create() { if (m_isSubsetColl) { @@ -188,7 +201,7 @@ class AssociationCollection : public podio::CollectionBase { bool m_isPrepared{false}; bool m_isSubsetColl{false}; int m_collectionID{0}; - AssociationCollectionData m_storage; + AssociationCollectionData m_storage{}; }; } // namespace podio diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index 023d9aff2..635173e37 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -82,7 +82,7 @@ TEST_CASE("Association basics", "[associations]") { SECTION("Implicit conversion") { // Use an immediately invoked lambda to check that the implicit conversion // is working as desired - [hit, cluster](TestA assoc) { + [hit, cluster](TestA assoc) { // NOLINT(performance-unnecessary-value-param) REQUIRE(assoc.getWeight() == 3.14f); REQUIRE(assoc.getFrom() == hit); REQUIRE(assoc.getTo() == cluster); @@ -216,7 +216,7 @@ TEST_CASE("AssociationCollection constness", "[associations][static-checks][cons } } -TEST_CASE("AssociationCollection subset collection", "[associations]") { +TEST_CASE("AssociationCollection subset collection", "[associations][subset-colls]") { auto assocs = TestAColl(); auto assoc1 = assocs.create(); assoc1.setWeight(1.0f); @@ -257,3 +257,90 @@ TEST_CASE("AssociationCollection subset collection", "[associations]") { REQUIRE_THROWS_AS(assocRefs.create(), std::logic_error); } } + +auto createAssocCollections(const size_t nElements = 3u) { + auto colls = std::make_tuple(TestAColl(), ExampleHitCollection(), ExampleClusterCollection()); + + auto& [assocColl, hitColl, clusterColl] = colls; + for (auto i = 0u; i < nElements; ++i) { + auto hit = hitColl.create(); + auto cluster = clusterColl.create(); + } + + for (auto i = 0u; i < nElements; ++i) { + auto assoc = assocColl.create(); + assoc.setWeight(i); + // Fill the relations in opposite orders to at least uncover issues that + // could be hidden by running the indices in parallel + assoc.setFrom(hitColl[i]); + assoc.setTo(clusterColl[nElements - i - 1]); + } + + return colls; +} + +void checkCollections(const TestAColl& assocs, const ExampleHitCollection& hits, + const ExampleClusterCollection& clusters, const size_t nElements = 3u) { + REQUIRE(assocs.size() == 3); + REQUIRE(hits.size() == 3); + REQUIRE(clusters.size() == 3); + + size_t index = 0; + for (auto assoc : assocs) { + REQUIRE(assoc.getWeight() == index); + REQUIRE(assoc.getFrom() == hits[index]); + REQUIRE(assoc.getTo() == clusters[nElements - index - 1]); + + index++; + } +} + +TEST_CASE("AssociationCollection movability", "[associations][move-semantics][collections]") { + // Setup a few collections for testing + auto [assocColl, hitColl, clusterColl] = createAssocCollections(); + + // Check that after the setup everything is as expected + checkCollections(assocColl, hitColl, clusterColl); + + SECTION("Move constructor and assignment") { + // NOTE: moving datatype collections is already covered by respective tests + auto newAssocs = std::move(assocColl); + checkCollections(newAssocs, hitColl, clusterColl); + + auto newerAssocs = TestAColl(); + newerAssocs = std::move(newAssocs); + checkCollections(newerAssocs, hitColl, clusterColl); + } + + SECTION("Prepared collections can be move assigned/constructed") { + assocColl.prepareForWrite(); + auto newAssocs = std::move(assocColl); + // checkCollections(newAssocs, hitColl, clusterColl); + + newAssocs.prepareForWrite(); + auto newerAssocs = TestAColl(); + newerAssocs = std::move(newAssocs); + // checkCollections(newAssocs, hitColl, clusterColl); + } + + SECTION("Subset collections can be moved") { + // Create a subset collection to move from + auto subsetAssocs = TestAColl(); + subsetAssocs.setSubsetCollection(); + for (auto a : assocColl) { + subsetAssocs.push_back(a); + } + checkCollections(subsetAssocs, hitColl, clusterColl); + + // Move constructor + auto newSubsetAssocs = std::move(subsetAssocs); + checkCollections(newSubsetAssocs, hitColl, clusterColl); + REQUIRE(newSubsetAssocs.isSubsetCollection()); + + // Move assignment + auto evenNewerAssocs = TestAColl(); + evenNewerAssocs = std::move(newSubsetAssocs); + checkCollections(evenNewerAssocs, hitColl, clusterColl); + REQUIRE(evenNewerAssocs.isSubsetCollection()); + } +} From 8b59fe99df1c9920e826aee537096576a69927c1 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 28 Jan 2022 20:30:25 +0100 Subject: [PATCH 09/79] [wip] Add necessary functionality for SIO backend --- include/podio/AssociationCollection.h | 2 +- include/podio/AssociationFwd.h | 12 +++++ include/podio/AssociationSIOBlock.h | 78 +++++++++++++++++++++++++++ tests/unittests/associations.cpp | 4 +- 4 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 include/podio/AssociationSIOBlock.h diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index e7d577c5d..ba4dfb276 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -133,7 +133,7 @@ class AssociationCollection : public podio::CollectionBase { } std::string getValueTypeName() const override { - return std::string("podio::Association<") + FromT::TypeName + "," + ToT::TypeName + ">"; + return podio::detail::associationSIOName(); } std::string getDataTypeName() const override { diff --git a/include/podio/AssociationFwd.h b/include/podio/AssociationFwd.h index 3e36dfc90..ed2524a35 100644 --- a/include/podio/AssociationFwd.h +++ b/include/podio/AssociationFwd.h @@ -1,7 +1,9 @@ #ifndef PODIO_ASSOCIATIONFWD_H #define PODIO_ASSOCIATIONFWD_H +#include #include +#include #include namespace podio { @@ -22,6 +24,10 @@ namespace detail { template using GetDefT = typename GetDefType::type; + /** + * Helper template struct and type-alias to retrieve the collection type from + * a given data type + */ template struct GetCollType { using type = typename T::CollT; @@ -30,6 +36,12 @@ namespace detail { template using GetCollT = typename GetCollType::type; + template + inline std::string associationSIOName() { + auto n = std::string("Association_FROM_") + FromT::TypeName + "_TO_" + FromT::TypeName; + std::replace(n.begin(), n.end(), ':', '_'); + return n; + } } // namespace detail // Forward declarations and typedefs used throughout the whole Association diff --git a/include/podio/AssociationSIOBlock.h b/include/podio/AssociationSIOBlock.h new file mode 100644 index 000000000..00683b803 --- /dev/null +++ b/include/podio/AssociationSIOBlock.h @@ -0,0 +1,78 @@ +#ifndef PODIO_ASSOCIATIONSIOBLOCK_H +#define PODIO_ASSOCIATIONSIOBLOCK_H + +#include "podio/AssociationFwd.h" +#include "podio/SIOBlock.h" + +#include +#include +#include + +#include +#include + +namespace podio { +template +class AssociationSIOBlock : public podio::SIOBlock { +public: + AssociationSIOBlock() : + SIOBlock(podio::detail::associationSIOName(), sio::version::encode_version(0, 1)) { + podio::SIOBlockFactory::instance().registerBlockForCollection(podio::detail::associationSIOName(), + this); + } + + AssociationSIOBlock(const std::string& name) : SIOBlock(name, sio::version::encode_version(0, 1)) { + } + + void read(sio::read_device& device, sio::version_type) override { + auto buffers = _col->getBuffers(); + if (!_col->isSubsetCollection()) { + auto* dataVec = buffers.dataAsVector(); + unsigned size{0}; + device.data(size); + dataVec->resize(size); + podio::handlePODDataSIO(device, dataVec->data(), size); + } + + // ---- references + auto* refColls = buffers.references; + for (auto& refC : *refColls) { + unsigned size{0}; + device.data(size); + refC->resize(size); + podio::handlePODDataSIO(device, refC->data(), size); + } + } + + void write(sio::write_device& device) override { + _col->prepareForWrite(); + auto buffers = _col->getBuffers(); + if (!_col->isSubsetCollection()) { + auto* dataVec = buffers.dataAsVector(); + unsigned size = dataVec->size(); + device.data(size); + podio::handlePODDataSIO(device, dataVec->data(), size); + } + + // ---- references + auto* refColls = buffers.references; + for (auto& refC : *refColls) { + unsigned size = refC->size(); + device.data(size); + podio::handlePODDataSIO(device, refC->data(), size); + } + } + + void createCollection(const bool subsetCollection = false) override { + setCollection(new AssociationCollection()); + _col->setSubsetCollection(subsetCollection); + } + + SIOBlock* create(const std::string& name) const override { + return new AssociationSIOBlock(name); + } +}; + +} // namespace podio + +#endif // PODIO_ASSOCIATIONBLOCK_H diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index 635173e37..07fee8459 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -133,9 +133,7 @@ TEST_CASE("Association basics", "[associations]") { TEST_CASE("AssociationCollection basics", "[associations]") { auto coll = TestAColl(); - REQUIRE(coll.getValueTypeName() == "podio::Association"); - - REQUIRE(true); + // TODO: basics without I/O } TEST_CASE("AssociationCollection constness", "[associations][static-checks][const-correctness]") { From 7bb8c21ef4f0257205a9c69c0827ab0448418d18 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Sat, 29 Jan 2022 13:15:16 +0100 Subject: [PATCH 10/79] [wip] clang-tidy fixes --- include/podio/Association.h | 2 +- include/podio/AssociationSIOBlock.h | 2 +- tests/unittests/associations.cpp | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/podio/Association.h b/include/podio/Association.h index dd147b0e2..385d743b2 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -77,7 +77,7 @@ class AssociationT { /// Destructor ~AssociationT() { if (m_obj) { - m_obj->release(); + m_obj->release(); // NOLINT(clang-analyzer-cplusplus.NewDelete) issue #174 } } diff --git a/include/podio/AssociationSIOBlock.h b/include/podio/AssociationSIOBlock.h index 00683b803..80a0c17be 100644 --- a/include/podio/AssociationSIOBlock.h +++ b/include/podio/AssociationSIOBlock.h @@ -75,4 +75,4 @@ class AssociationSIOBlock : public podio::SIOBlock { } // namespace podio -#endif // PODIO_ASSOCIATIONBLOCK_H +#endif // PODIO_ASSOCIATIONSIOBLOCK_H diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index 07fee8459..033754f73 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -27,7 +27,8 @@ TEST_CASE("Association basics", "[associations]") { auto cluster = MutableExampleCluster(); auto hit = MutableExampleHit(); - auto mutAssoc = TestMutA(); + auto mutAssoc = TestMutA(); // NOLINT(clang-analyzer-cplusplus.NewDelete) necessary due to clang-tidy and Catch2 + // SECTIONs interaction (common setup seems to confuse clang-tidy) mutAssoc.setWeight(3.14f); mutAssoc.setFrom(hit); mutAssoc.setTo(cluster); @@ -121,7 +122,7 @@ TEST_CASE("Association basics", "[associations]") { } SECTION("Equality operator") { - auto otherAssoc = mutAssoc; + auto otherAssoc = mutAssoc; // NOLINT(performance-unnecessary-copy-initialization) REQUIRE(otherAssoc == mutAssoc); // Mutable and immutable associations should be comparable From e0a337379ec92af98caf8192503d12a09a8408cf Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 2 Dec 2022 19:07:12 +0100 Subject: [PATCH 11/79] Move non public headers to detail folder --- include/podio/Association.h | 4 ++-- include/podio/AssociationCollection.h | 7 ++++--- include/podio/AssociationCollectionIterator.h | 4 ++-- include/podio/{ => detail}/AssociationCollectionData.h | 5 +++-- include/podio/{ => detail}/AssociationFwd.h | 0 include/podio/{ => detail}/AssociationObj.h | 3 ++- include/podio/{ => detail}/AssociationSIOBlock.h | 3 ++- include/podio/utilities/DeprecationMacros.h | 7 +++++++ 8 files changed, 22 insertions(+), 11 deletions(-) rename include/podio/{ => detail}/AssociationCollectionData.h (98%) rename include/podio/{ => detail}/AssociationFwd.h (100%) rename include/podio/{ => detail}/AssociationObj.h (97%) rename include/podio/{ => detail}/AssociationSIOBlock.h (98%) create mode 100644 include/podio/utilities/DeprecationMacros.h diff --git a/include/podio/Association.h b/include/podio/Association.h index 385d743b2..d7c855fbf 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -1,8 +1,8 @@ #ifndef PODIO_ASSOCIATION_H #define PODIO_ASSOCIATION_H -#include "podio/AssociationFwd.h" -#include "podio/AssociationObj.h" +#include "podio/detail/AssociationFwd.h" +#include "podio/detail/AssociationObj.h" #include #include // std::swap diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index ba4dfb276..bb11ceb88 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -1,11 +1,12 @@ #ifndef PODIO_ASSOCIATIONCOLLECTION_H #define PODIO_ASSOCIATIONCOLLECTION_H +#include "podio/detail/AssociationCollectionData.h" +#include "podio/detail/AssociationFwd.h" +#include "podio/detail/AssociationObj.h" + #include "podio/Association.h" -#include "podio/AssociationCollectionData.h" #include "podio/AssociationCollectionIterator.h" -#include "podio/AssociationFwd.h" -#include "podio/AssociationObj.h" #include "podio/CollectionBase.h" #include "podio/CollectionBuffers.h" #include "podio/ICollectionProvider.h" diff --git a/include/podio/AssociationCollectionIterator.h b/include/podio/AssociationCollectionIterator.h index 0d3946140..8608f94cc 100644 --- a/include/podio/AssociationCollectionIterator.h +++ b/include/podio/AssociationCollectionIterator.h @@ -1,8 +1,8 @@ #ifndef PODIO_ASSOCIATIONCOLLECTIONITERATOR_H #define PODIO_ASSOCIATIONCOLLECTIONITERATOR_H -#include "podio/AssociationCollectionData.h" -#include "podio/AssociationFwd.h" +#include "podio/detail/AssociationCollectionData.h" +#include "podio/detail/AssociationFwd.h" namespace podio { template diff --git a/include/podio/AssociationCollectionData.h b/include/podio/detail/AssociationCollectionData.h similarity index 98% rename from include/podio/AssociationCollectionData.h rename to include/podio/detail/AssociationCollectionData.h index 25b533917..cc792d51a 100644 --- a/include/podio/AssociationCollectionData.h +++ b/include/podio/detail/AssociationCollectionData.h @@ -1,8 +1,9 @@ #ifndef PODIO_ASSOCIATIONCOLLECTIONDATA_H #define PODIO_ASSOCIATIONCOLLECTIONDATA_H -#include "podio/AssociationFwd.h" -#include "podio/AssociationObj.h" +#include "podio/detail/AssociationFwd.h" +#include "podio/detail/AssociationObj.h" + #include "podio/CollectionBase.h" #include "podio/CollectionBuffers.h" #include "podio/ICollectionProvider.h" diff --git a/include/podio/AssociationFwd.h b/include/podio/detail/AssociationFwd.h similarity index 100% rename from include/podio/AssociationFwd.h rename to include/podio/detail/AssociationFwd.h diff --git a/include/podio/AssociationObj.h b/include/podio/detail/AssociationObj.h similarity index 97% rename from include/podio/AssociationObj.h rename to include/podio/detail/AssociationObj.h index d5beffee4..45d6267e2 100644 --- a/include/podio/AssociationObj.h +++ b/include/podio/detail/AssociationObj.h @@ -1,7 +1,8 @@ #ifndef PODIO_ASSOCIATIONOBJ_H #define PODIO_ASSOCIATIONOBJ_H -#include "podio/AssociationFwd.h" +#include "podio/detail/AssociationFwd.h" + #include "podio/ObjBase.h" #include "podio/ObjectID.h" diff --git a/include/podio/AssociationSIOBlock.h b/include/podio/detail/AssociationSIOBlock.h similarity index 98% rename from include/podio/AssociationSIOBlock.h rename to include/podio/detail/AssociationSIOBlock.h index 80a0c17be..5b5a820a3 100644 --- a/include/podio/AssociationSIOBlock.h +++ b/include/podio/detail/AssociationSIOBlock.h @@ -1,7 +1,8 @@ #ifndef PODIO_ASSOCIATIONSIOBLOCK_H #define PODIO_ASSOCIATIONSIOBLOCK_H -#include "podio/AssociationFwd.h" +#include "podio/detail/AssociationFwd.h" + #include "podio/SIOBlock.h" #include diff --git a/include/podio/utilities/DeprecationMacros.h b/include/podio/utilities/DeprecationMacros.h new file mode 100644 index 000000000..077967ae4 --- /dev/null +++ b/include/podio/utilities/DeprecationMacros.h @@ -0,0 +1,7 @@ +#ifndef PODIO_UTILITIES_DEPRECATIONMACROS_H +#define PODIO_UTILITIES_DEPRECATIONMACROS_H + +#define DEPRECATED_USE_FRAME \ + [[deprecated("Legacy I/O functionality will be removed. Switch to Frame based I/O instead.")]] + +#endif From de7ce7a37537c8587ce24c5d535e58e6350ec7b7 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 2 Dec 2022 20:07:58 +0100 Subject: [PATCH 12/79] Fix tests after Frame --- tests/read_test.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/read_test.h b/tests/read_test.h index f7439e17c..6b5f37ef1 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -410,6 +410,21 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi } } } + + // ======================= Associations ========================== + auto& associations = store.template get("associations"); + if (associations.size() != nmspaces.size()) { + throw std::runtime_error("AssociationsCollection does not have the expected size"); + } + const auto nNameSpc = nmspaces.size(); + int assocIndex = 0; + for (auto assoc : associations) { + if (!((assoc.getWeight() == 0.5 * assocIndex) && (assoc.getFrom() == mcps[assocIndex]) && + (assoc.getTo() == nmspaces[nNameSpc - 1 - assocIndex]))) { + throw std::runtime_error("Association does not have expected content"); + } + assocIndex++; + } } #endif // PODIO_TESTS_READ_TEST_H From 510561d1a364e863f544a2ef4139d9635cbeb4f7 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 2 Dec 2022 20:08:23 +0100 Subject: [PATCH 13/79] [wip] Make everything compile again --- include/podio/AssociationCollection.h | 82 +++++++++++++++++-- .../podio/detail/AssociationCollectionData.h | 20 ++++- include/podio/detail/AssociationFwd.h | 3 + include/podio/detail/AssociationSIOBlock.h | 44 ++++++---- 4 files changed, 124 insertions(+), 25 deletions(-) diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index bb11ceb88..4f1d21823 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -11,7 +11,12 @@ #include "podio/CollectionBuffers.h" #include "podio/ICollectionProvider.h" +#include +#include +#include +#include #include +#include namespace podio { @@ -26,12 +31,19 @@ class AssociationCollection : public podio::CollectionBase { using AssocT = Association; using MutableAssocT = MutableAssociation; + using CollectionT = podio::AssociationCollection; + using CollectionDataT = podio::AssociationCollectionData; + public: using const_iterator = AssociationCollectionIterator; using iterator = AssociationMutableCollectionIterator; AssociationCollection() = default; + AssociationCollection(CollectionDataT&& data, bool isSubsetColl) : + m_isSubsetColl(isSubsetColl), m_collectionID(0), m_storage(std::move(data)) { + } + // Move-only type AssociationCollection(const AssociationCollection&) = delete; AssociationCollection& operator=(const AssociationCollection&) = delete; @@ -107,6 +119,13 @@ class AssociationCollection : public podio::CollectionBase { m_isPrepared = false; } + void print(std::ostream& os = std::cout, bool flush = true) const override { + os << *this; + if (flush) { + os.flush(); + } + } + // support for the iterator protocol const_iterator begin() const { return const_iterator(0, &m_storage.entries); @@ -125,10 +144,30 @@ class AssociationCollection : public podio::CollectionBase { return m_isValid; } - podio::CollectionBuffers getBuffers() override { + podio::CollectionWriteBuffers getBuffers() override { return m_storage.getCollectionBuffers(m_isSubsetColl); } + podio::CollectionReadBuffers createBuffers() override /*const*/ { + // Very cumbersome way at the moment. We get the actual buffers to have the + // references and vector members sized appropriately (we will use this + // information to create new buffers outside) + auto collBuffers = m_storage.getCollectionBuffers(m_isSubsetColl); + auto readBuffers = podio::CollectionReadBuffers{}; + readBuffers.references = collBuffers.references; + readBuffers.vectorMembers = collBuffers.vectorMembers; + readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + CollectionDataT data(buffers, isSubsetColl); + return std::make_unique(std::move(data), isSubsetColl); + }; + readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { + if (buffers.data) { + buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); + } + }; + return readBuffers; + } + std::string getTypeName() const override { return std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; } @@ -164,8 +203,20 @@ class AssociationCollection : public podio::CollectionBase { return m_collectionID; } - void prepareForWrite() override { - // If the collection has been prepared already there is nothing to do + void prepareForWrite() const override { + // TODO: Replace this double locking pattern with an atomic and only one + // lock. Problem: std::atomic is not default movable + { + std::lock_guard lock{*m_storageMtx}; + // If the collection has been prepared already there is nothing to do + if (m_isPrepared) { + return; + } + } + + std::lock_guard lock{*m_storageMtx}; + // by the time we acquire the lock another thread might have already + // succeeded, so we need to check again now if (m_isPrepared) { return; } @@ -196,15 +247,34 @@ class AssociationCollection : public podio::CollectionBase { // For setReferences, we need to give our own CollectionData access to our // private entries. Otherwise we would need to expose a public member function // that gives access to the Obj* which is definitely not what we want - friend class AssociationCollectionData; + friend CollectionDataT; bool m_isValid{false}; - bool m_isPrepared{false}; + mutable bool m_isPrepared{false}; bool m_isSubsetColl{false}; int m_collectionID{0}; - AssociationCollectionData m_storage{}; + mutable std::unique_ptr m_storageMtx{std::make_unique()}; + mutable CollectionDataT m_storage{}; }; +template +std::ostream& operator<<(std::ostream& o, const AssociationCollection& v) { + const auto old_flags = o.flags(); + o << " id: weight:" << '\n'; + for (const auto&& el : v) { + o << std::scientific << std::showpos << std::setw(12) << el.id() << " " << std::setw(12) << " " << el.getWeight() + << '\n'; + + o << " from : "; + o << el.getFrom().id() << std::endl; + o << " to : "; + o << el.getTo().id() << std::endl; + } + + o.flags(old_flags); + return o; +} + } // namespace podio #endif // PODIO_ASSOCIATIONCOLLECTION_H diff --git a/include/podio/detail/AssociationCollectionData.h b/include/podio/detail/AssociationCollectionData.h index cc792d51a..90fa035c3 100644 --- a/include/podio/detail/AssociationCollectionData.h +++ b/include/podio/detail/AssociationCollectionData.h @@ -21,17 +21,28 @@ class AssociationCollectionData { AssociationCollectionData() : m_rel_from(new std::vector()), m_rel_to(new std::vector()), m_data(new AssociationDataContainer()) { + m_refCollections.reserve(2); m_refCollections.emplace_back(std::make_unique>()); m_refCollections.emplace_back(std::make_unique>()); } + + AssociationCollectionData(podio::CollectionReadBuffers buffers, bool isSubsetColl) : + m_rel_from(new std::vector()), + m_rel_to(new std::vector()), + m_refCollections(std::move(*buffers.references)) { + if (!isSubsetColl) { + m_data.reset(buffers.dataAsVector()); + } + } + AssociationCollectionData(const AssociationCollectionData&) = delete; AssociationCollectionData& operator=(const AssociationCollectionData&) = delete; AssociationCollectionData(AssociationCollectionData&&) = default; AssociationCollectionData& operator=(AssociationCollectionData&&) = default; ~AssociationCollectionData() = default; - podio::CollectionBuffers getCollectionBuffers(bool isSubsetColl) { - return {isSubsetColl ? nullptr : (void*)&m_data, &m_refCollections, nullptr}; + podio::CollectionWriteBuffers getCollectionBuffers(bool isSubsetColl) { + return {isSubsetColl ? nullptr : (void*)&m_data, &m_refCollections, &m_vecInfo}; } void clear(bool isSubsetColl) { @@ -177,11 +188,12 @@ class AssociationCollectionData { private: // members to handle relations - podio::UVecPtr m_rel_from{}; - podio::UVecPtr m_rel_to{}; + podio::UVecPtr m_rel_from{nullptr}; + podio::UVecPtr m_rel_to{nullptr}; // I/O related buffers (as far as necessary) podio::CollRefCollection m_refCollections{}; + podio::VectorMembersInfo m_vecInfo{}; std::unique_ptr m_data{nullptr}; }; diff --git a/include/podio/detail/AssociationFwd.h b/include/podio/detail/AssociationFwd.h index ed2524a35..2d69a4b17 100644 --- a/include/podio/detail/AssociationFwd.h +++ b/include/podio/detail/AssociationFwd.h @@ -66,6 +66,9 @@ using MutableAssociation = AssociationT, detail::GetDefT< template class AssociationCollection; +template +class AssociationCollectionData; + template class AssociationCollectionIteratorT; diff --git a/include/podio/detail/AssociationSIOBlock.h b/include/podio/detail/AssociationSIOBlock.h index 5b5a820a3..9ddb586c1 100644 --- a/include/podio/detail/AssociationSIOBlock.h +++ b/include/podio/detail/AssociationSIOBlock.h @@ -3,6 +3,8 @@ #include "podio/detail/AssociationFwd.h" +#include "podio/CollectionBuffers.h" + #include "podio/SIOBlock.h" #include @@ -26,17 +28,21 @@ class AssociationSIOBlock : public podio::SIOBlock { } void read(sio::read_device& device, sio::version_type) override { - auto buffers = _col->getBuffers(); - if (!_col->isSubsetCollection()) { - auto* dataVec = buffers.dataAsVector(); + m_buffers.references->emplace_back(std::make_unique>()); + if (!m_subsetColl) { + m_buffers.references->emplace_back(std::make_unique>()); + } + + if (!m_subsetColl) { unsigned size{0}; device.data(size); - dataVec->resize(size); + m_buffers.data = new std::vector(size); + auto* dataVec = m_buffers.dataAsVector(); podio::handlePODDataSIO(device, dataVec->data(), size); } - // ---- references - auto* refColls = buffers.references; + // ---- read ref collections + auto* refColls = m_buffers.references; for (auto& refC : *refColls) { unsigned size{0}; device.data(size); @@ -46,17 +52,15 @@ class AssociationSIOBlock : public podio::SIOBlock { } void write(sio::write_device& device) override { - _col->prepareForWrite(); - auto buffers = _col->getBuffers(); - if (!_col->isSubsetCollection()) { - auto* dataVec = buffers.dataAsVector(); + if (!m_subsetColl) { + auto* dataVec = podio::CollectionWriteBuffers::asVector(m_buffers.data); unsigned size = dataVec->size(); device.data(size); podio::handlePODDataSIO(device, dataVec->data(), size); } - // ---- references - auto* refColls = buffers.references; + // ---- wirte ref collections ------ + auto* refColls = m_buffers.references; for (auto& refC : *refColls) { unsigned size = refC->size(); device.data(size); @@ -64,9 +68,19 @@ class AssociationSIOBlock : public podio::SIOBlock { } } - void createCollection(const bool subsetCollection = false) override { - setCollection(new AssociationCollection()); - _col->setSubsetCollection(subsetCollection); + void createBuffers(const bool subsetCollection = false) override { + m_subsetColl = subsetCollection; + + m_buffers.references = new podio::CollRefCollection(); + m_buffers.vectorMembers = new podio::VectorMembersInfo(); + + m_buffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + AssociationCollectionData data(buffers, isSubsetColl); + return std::make_unique>(std::move(data), isSubsetColl); + }; + + // setCollection(new AssociationCollection()); + // _col->setSubsetCollection(subsetCollection); } SIOBlock* create(const std::string& name) const override { From 2919668c38549fe4c2203a0840198d443c43e113 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 5 Dec 2022 17:39:46 +0100 Subject: [PATCH 14/79] Add json output support for associations --- include/podio/Association.h | 17 +++++++++++++++++ include/podio/AssociationCollection.h | 14 ++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/include/podio/Association.h b/include/podio/Association.h index d7c855fbf..52ccfa8f4 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -4,6 +4,10 @@ #include "podio/detail/AssociationFwd.h" #include "podio/detail/AssociationObj.h" +#ifdef PODIO_JSON_OUTPUT + #include "nlohmann/json.hpp" +#endif + #include #include // std::swap @@ -171,6 +175,19 @@ std::ostream& operator<<(std::ostream& os, const Association& assoc) << " to: " << assoc.getTo().id() << '\n'; } +#ifdef PODIO_JSON_OUTPUT +template +void to_json(nlohmann::json& j, const Association& assoc) { + j = nlohmann::json{{"weight", assoc.getWeight()}}; + + j["from"] = nlohmann::json{{"collectionID", assoc.getFrom().getObjectID().collectionID}, + {"index", assoc.getFrom().getObjectID().index}}; + + j["to"] = nlohmann::json{{"collectionID", assoc.getTo().getObjectID().collectionID}, + {"index", assoc.getTo().getObjectID().index}}; +} +#endif + } // namespace podio #endif // PODIO_ASSOCIATION_H diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index 4f1d21823..d5b704771 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -11,6 +11,10 @@ #include "podio/CollectionBuffers.h" #include "podio/ICollectionProvider.h" +#ifdef PODIO_JSON_OUTPUT + #include "nlohmann/json.hpp" +#endif + #include #include #include @@ -275,6 +279,16 @@ std::ostream& operator<<(std::ostream& o, const AssociationCollection +void to_json(nlohmann::json& j, const AssociationCollection& collection) { + j = nlohmann::json::array(); + for (auto&& elem : collection) { + j.emplace_back(elem); + } +} +#endif + } // namespace podio #endif // PODIO_ASSOCIATIONCOLLECTION_H From 79e8127fd7b67cd44f3d461f143bb0dbbd7eb2ee Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 5 Dec 2022 17:46:50 +0100 Subject: [PATCH 15/79] Generate all possible combinations for root dicts This makes it easy for the users but compile times explode for the dictionaries as we are instantiating N**2 templates in static variables in order for dictionary generation to pick them up properly. --- python/podio_gen/cpp_generator.py | 49 ++++++++++++++++++++++++-- python/templates/Associations.h.jinja2 | 11 ++++++ python/templates/CMakeLists.txt | 1 + python/templates/selection.xml.jinja2 | 9 +++++ src/CMakeLists.txt | 1 + 5 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 python/templates/Associations.h.jinja2 diff --git a/python/podio_gen/cpp_generator.py b/python/podio_gen/cpp_generator.py index 09e17141c..b4e4400c4 100644 --- a/python/podio_gen/cpp_generator.py +++ b/python/podio_gen/cpp_generator.py @@ -7,6 +7,7 @@ from enum import IntEnum from collections import defaultdict from collections.abc import Mapping +from itertools import combinations_with_replacement, product from podio_schema_evolution import DataModelComparator from podio_schema_evolution import RenamedMember, root_filter, RootIoRule @@ -94,9 +95,14 @@ def pre_process(self): def post_process(self, _): """Do the cpp specific post processing""" self._write_edm_def_file() + + # all possible associations + assocs = self._instantiate_associations() + if "ROOT" in self.io_handlers: self._prepare_iorules() - self._create_selection_xml() + self._create_selection_xml(assocs) + self._write_all_collections_header() self._write_cmake_lists_file() @@ -535,7 +541,7 @@ def quoted_sv(string): self._eval_template("DatamodelDefinition.h.jinja2", data), ) - def _create_selection_xml(self): + def _create_selection_xml(self, assoc_combinations): """Create the selection xml that is necessary for ROOT I/O""" data = { "version": self.datamodel.schema_version, @@ -546,6 +552,7 @@ def _create_selection_xml(self): for d in self.root_schema_datatype_names | self.root_schema_component_names ], # noqa "iorules": self.root_schema_iorules, + "associations": assoc_combinations, } self._write_file("selection.xml", self._eval_template("selection.xml.jinja2", data)) @@ -565,6 +572,44 @@ def _get_member_includes(self, members): return self._sort_includes(includes) + def _instantiate_associations(self): + """Instantiate all the associations in a dedicated .cc file and return + the combination of all instantiated things + """ + datatypes = [DataType(d) for d in self.datamodel.datatypes] + includes = [ + self._build_include_for_class(f"{d.bare_type}Collection", IncludeFrom.INTERNAL) + for d in datatypes + ] + + # We want all combinations of our datamodel + combinations = tuple(combinations_with_replacement(datatypes, 2)) + + if self.upstream_edm: + ext_datatypes = [DataType(d) for d in self.upstream_edm.datatypes] + includes.extend( + [ + self._build_include_for_class(f"{d.bare_type}Collection", IncludeFrom.EXTERNAL) + for d in ext_datatypes + ] + ) + + combinations += tuple(product(ext_datatypes, datatypes)) + + self._write_file( + "Associations.h", + self._eval_template( + "Associations.h.jinja2", + { + "package_name": self.package_name, + "includes": includes, + "combinations": combinations, + }, + ), + ) + + return combinations + def _needs_include(self, classname) -> IncludeFrom: """Check whether the member needs an include from within the datamodel""" if ( diff --git a/python/templates/Associations.h.jinja2 b/python/templates/Associations.h.jinja2 new file mode 100644 index 000000000..8a590de7f --- /dev/null +++ b/python/templates/Associations.h.jinja2 @@ -0,0 +1,11 @@ +// AUTOMATICALLY GENERATED FILE - DO NOT EDIT +#include "podio/AssociationCollection.h" + +// Datamodel includes +{% for include in includes %} +{{ include }} +{% endfor %} + +{% for (from_t, to_t) in combinations %} +static podio::AssociationCollection<{{from_t.full_type}}, {{to_t.full_type}}> podio__association_dummy__{{ package_name }}__{{ loop.index0 }}{}; +{% endfor %} diff --git a/python/templates/CMakeLists.txt b/python/templates/CMakeLists.txt index 014b4e0bd..fcceb4d35 100644 --- a/python/templates/CMakeLists.txt +++ b/python/templates/CMakeLists.txt @@ -18,6 +18,7 @@ set(PODIO_TEMPLATES ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.cc.jinja2 ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/DatamodelDefinition.h.jinja2 + ${CMAKE_CURRENT_LIST_DIR}/Associations.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/collections.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/declarations.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/implementations.jinja2 diff --git a/python/templates/selection.xml.jinja2 b/python/templates/selection.xml.jinja2 index e651a61b2..7ff109c41 100644 --- a/python/templates/selection.xml.jinja2 +++ b/python/templates/selection.xml.jinja2 @@ -13,6 +13,10 @@ {{ iorule.code }} ]]> + +{% macro assoc_selection(from_t, to_t) %} + + {% endmacro %} @@ -38,6 +42,11 @@ {% for class in old_schema_components %} {{ class_selection(class) }} +{% endfor %} + + +{% for (from_t, to_t) in associations %} +{{ assoc_selection(from_t, to_t) }} {% endfor %} diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cb3818c3f..75ad37247 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -68,6 +68,7 @@ SET(core_headers ${PROJECT_SOURCE_DIR}/include/podio/DatamodelRegistry.h ${PROJECT_SOURCE_DIR}/include/podio/utilities/DatamodelRegistryIOHelpers.h ${PROJECT_SOURCE_DIR}/include/podio/GenericParameters.h + ${PROJECT_SOURCE_DIR}/include/podio/AssociationCollection.h ) PODIO_ADD_LIB_AND_DICT(podio "${core_headers}" "${core_sources}" selection.xml) From 52a9cb1beca97a23cbebe5349ec4d332a6701283 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 5 Dec 2022 18:31:18 +0100 Subject: [PATCH 16/79] Add writing of Association collection to Frame I/O tests --- python/podio/test_Frame.py | 2 ++ tests/frame_test_common.h | 3 ++- tests/write_frame.h | 24 +++++++++++++++++++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/python/podio/test_Frame.py b/python/podio/test_Frame.py index 1ad16bd8f..451a79342 100644 --- a/python/podio/test_Frame.py +++ b/python/podio/test_Frame.py @@ -34,7 +34,9 @@ "WithNamespaceRelationCopy", "emptyCollection", "emptySubsetColl", + "associations", } + # The expected collections from the extension (only present in the other_events category) EXPECTED_EXTENSION_COLL_NAMES = { "extension_Contained", diff --git a/tests/frame_test_common.h b/tests/frame_test_common.h index 78862b40d..f4df71257 100644 --- a/tests/frame_test_common.h +++ b/tests/frame_test_common.h @@ -23,6 +23,7 @@ static const std::vector collsToWrite = {"mcparticles", "WithNamespaceRelation", "WithNamespaceRelationCopy", "emptyCollection", - "emptySubsetColl"}; + "emptySubsetColl", + "associations"}; #endif // PODIO_TESTS_FRAME_TEST_COMMON_H diff --git a/tests/write_frame.h b/tests/write_frame.h index 1e1c9f966..b6f3136f5 100644 --- a/tests/write_frame.h +++ b/tests/write_frame.h @@ -20,12 +20,16 @@ #include "extension_model/ExternalComponentTypeCollection.h" #include "extension_model/ExternalRelationTypeCollection.h" +#include "podio/AssociationCollection.h" #include "podio/Frame.h" #include "podio/UserDataCollection.h" #include #include +// Define an association that is used for the I/O tests +using TestAssocCollection = podio::AssociationCollection; + auto createMCCollection() { auto mcps = ExampleMCCollection(); @@ -380,6 +384,23 @@ auto createExampleWithInterfaceCollection(const ExampleHitCollection& hits, cons return coll; } +auto createAssociationCollection(const ExampleMCCollection& mcps, + const ex42::ExampleWithARelationCollection& namesprels) { + TestAssocCollection associations; + const auto nNameSpc = namesprels.size(); + for (size_t iA = 0; iA < nNameSpc; ++iA) { + auto assoc = associations.create(); + assoc.setWeight(0.5 * iA); + + // Fill in opposite "order" to at least make sure that we uncover issues + // that would otherwise be masked by parallel running of indices + assoc.setFrom(mcps[iA]); + assoc.setTo(namesprels[nNameSpc - 1 - iA]); + } + + return associations; +} + podio::Frame makeFrame(int iFrame) { podio::Frame frame{}; @@ -415,7 +436,7 @@ podio::Frame makeFrame(int iFrame) { auto [namesps, namespsrels, cpytest] = createNamespaceRelationCollection(iFrame); frame.put(std::move(namesps), "WithNamespaceMember"); - frame.put(std::move(namespsrels), "WithNamespaceRelation"); + const auto& namespaceRels = frame.put(std::move(namespsrels), "WithNamespaceRelation"); frame.put(std::move(cpytest), "WithNamespaceRelationCopy"); // Parameters @@ -440,6 +461,7 @@ podio::Frame makeFrame(int iFrame) { frame.put(createExtensionExternalRelationCollection(iFrame, hits, clusters), "extension_ExternalRelation"); frame.put(createExampleWithInterfaceCollection(hits, clusters, mcps), "interface_examples"); + frame.put(createAssociationCollection(mcps, namespaceRels), "associations"); return frame; } From 0c344f6a400143515dab7f4ff7117e7e76a5104b Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 5 Dec 2022 18:48:37 +0100 Subject: [PATCH 17/79] Generate SIOBlocks instantiations for associations Similar to root dictionary generation to make them available to the SIO backend. Same caveats and concerns as with ROOT in this case. --- python/podio_gen/cpp_generator.py | 23 +++++++++++-------- ...h.jinja2 => AssociationsRootDict.h.jinja2} | 0 .../templates/AssociationsSIOBlock.cc.jinja2 | 11 +++++++++ python/templates/CMakeLists.txt | 2 ++ 4 files changed, 27 insertions(+), 9 deletions(-) rename python/templates/{Associations.h.jinja2 => AssociationsRootDict.h.jinja2} (100%) create mode 100644 python/templates/AssociationsSIOBlock.cc.jinja2 diff --git a/python/podio_gen/cpp_generator.py b/python/podio_gen/cpp_generator.py index b4e4400c4..cd9b71ac8 100644 --- a/python/podio_gen/cpp_generator.py +++ b/python/podio_gen/cpp_generator.py @@ -596,18 +596,23 @@ def _instantiate_associations(self): combinations += tuple(product(ext_datatypes, datatypes)) + assoc_data = { + "package_name": self.package_name, + "includes": includes, + "combinations": combinations, + } + self._write_file( - "Associations.h", - self._eval_template( - "Associations.h.jinja2", - { - "package_name": self.package_name, - "includes": includes, - "combinations": combinations, - }, - ), + "AssociationsRootDict.h", + self._eval_template("AssociationsRootDict.h.jinja2", assoc_data), ) + if "SIO" in self.io_handlers: + self._write_file( + "AssociationsSIOBlock.cc", + self._eval_template("AssociationsSIOBlock.cc.jinja2", assoc_data), + ) + return combinations def _needs_include(self, classname) -> IncludeFrom: diff --git a/python/templates/Associations.h.jinja2 b/python/templates/AssociationsRootDict.h.jinja2 similarity index 100% rename from python/templates/Associations.h.jinja2 rename to python/templates/AssociationsRootDict.h.jinja2 diff --git a/python/templates/AssociationsSIOBlock.cc.jinja2 b/python/templates/AssociationsSIOBlock.cc.jinja2 new file mode 100644 index 000000000..8a590de7f --- /dev/null +++ b/python/templates/AssociationsSIOBlock.cc.jinja2 @@ -0,0 +1,11 @@ +// AUTOMATICALLY GENERATED FILE - DO NOT EDIT +#include "podio/AssociationCollection.h" + +// Datamodel includes +{% for include in includes %} +{{ include }} +{% endfor %} + +{% for (from_t, to_t) in combinations %} +static podio::AssociationCollection<{{from_t.full_type}}, {{to_t.full_type}}> podio__association_dummy__{{ package_name }}__{{ loop.index0 }}{}; +{% endfor %} diff --git a/python/templates/CMakeLists.txt b/python/templates/CMakeLists.txt index fcceb4d35..a7e0f1768 100644 --- a/python/templates/CMakeLists.txt +++ b/python/templates/CMakeLists.txt @@ -19,6 +19,8 @@ set(PODIO_TEMPLATES ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/DatamodelDefinition.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/Associations.h.jinja2 + ${CMAKE_CURRENT_LIST_DIR}/AssociationsRootDict.h.jinja2 + ${CMAKE_CURRENT_LIST_DIR}/AssociationsSIOBlock.cc.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/collections.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/declarations.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/implementations.jinja2 From cc9786a994015234c16f8bc9bbc8250d034ff436 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 5 Dec 2022 18:51:39 +0100 Subject: [PATCH 18/79] Make association read tests version dependent --- .../templates/AssociationsSIOBlock.cc.jinja2 | 3 ++- tests/read_test.h | 26 ++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/python/templates/AssociationsSIOBlock.cc.jinja2 b/python/templates/AssociationsSIOBlock.cc.jinja2 index 8a590de7f..024befdfd 100644 --- a/python/templates/AssociationsSIOBlock.cc.jinja2 +++ b/python/templates/AssociationsSIOBlock.cc.jinja2 @@ -1,5 +1,6 @@ // AUTOMATICALLY GENERATED FILE - DO NOT EDIT #include "podio/AssociationCollection.h" +#include "podio/detail/AssociationSIOBlock.h" // Datamodel includes {% for include in includes %} @@ -7,5 +8,5 @@ {% endfor %} {% for (from_t, to_t) in combinations %} -static podio::AssociationCollection<{{from_t.full_type}}, {{to_t.full_type}}> podio__association_dummy__{{ package_name }}__{{ loop.index0 }}{}; +static podio::AssociationSIOBlock<{{from_t.full_type}}, {{to_t.full_type}}> podio__association_dummy__{{ package_name }}__sio_block__{{ loop.index0 }}{}; {% endfor %} diff --git a/tests/read_test.h b/tests/read_test.h index 6b5f37ef1..d112d463c 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -412,18 +412,20 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi } // ======================= Associations ========================== - auto& associations = store.template get("associations"); - if (associations.size() != nmspaces.size()) { - throw std::runtime_error("AssociationsCollection does not have the expected size"); - } - const auto nNameSpc = nmspaces.size(); - int assocIndex = 0; - for (auto assoc : associations) { - if (!((assoc.getWeight() == 0.5 * assocIndex) && (assoc.getFrom() == mcps[assocIndex]) && - (assoc.getTo() == nmspaces[nNameSpc - 1 - assocIndex]))) { - throw std::runtime_error("Association does not have expected content"); - } - assocIndex++; + if (fileVersion >= podio::version::Version{0, 16, 1}) { + auto& associations = store.template get("associations"); + if (associations.size() != nmspaces.size()) { + throw std::runtime_error("AssociationsCollection does not have the expected size"); + } + const auto nNameSpc = nmspaces.size(); + int assocIndex = 0; + for (auto assoc : associations) { + if (!((assoc.getWeight() == 0.5 * assocIndex) && (assoc.getFrom() == mcps[assocIndex]) && + (assoc.getTo() == nmspaces[nNameSpc - 1 - assocIndex]))) { + throw std::runtime_error("Association does not have expected content"); + } + assocIndex++; + } } } From 034fc72186cdbc5550333247a78652beaf8e79b8 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 5 Dec 2022 19:43:23 +0100 Subject: [PATCH 19/79] Fix include guards to comply with clang-tidy --- include/podio/detail/AssociationCollectionData.h | 6 +++--- include/podio/detail/AssociationFwd.h | 6 +++--- include/podio/detail/AssociationObj.h | 6 +++--- include/podio/detail/AssociationSIOBlock.h | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/podio/detail/AssociationCollectionData.h b/include/podio/detail/AssociationCollectionData.h index 90fa035c3..ba520d5c3 100644 --- a/include/podio/detail/AssociationCollectionData.h +++ b/include/podio/detail/AssociationCollectionData.h @@ -1,5 +1,5 @@ -#ifndef PODIO_ASSOCIATIONCOLLECTIONDATA_H -#define PODIO_ASSOCIATIONCOLLECTIONDATA_H +#ifndef PODIO_DETAIL_ASSOCIATIONCOLLECTIONDATA_H +#define PODIO_DETAIL_ASSOCIATIONCOLLECTIONDATA_H #include "podio/detail/AssociationFwd.h" #include "podio/detail/AssociationObj.h" @@ -199,4 +199,4 @@ class AssociationCollectionData { } // namespace podio -#endif // PODIO_ASSOCIATIONCOLLECTIONDATA_H +#endif // PODIO_DETAIL_ASSOCIATIONCOLLECTIONDATA_H diff --git a/include/podio/detail/AssociationFwd.h b/include/podio/detail/AssociationFwd.h index 2d69a4b17..20d5c9d30 100644 --- a/include/podio/detail/AssociationFwd.h +++ b/include/podio/detail/AssociationFwd.h @@ -1,5 +1,5 @@ -#ifndef PODIO_ASSOCIATIONFWD_H -#define PODIO_ASSOCIATIONFWD_H +#ifndef PODIO_DETAIL_ASSOCIATIONFWD_H +#define PODIO_DETAIL_ASSOCIATIONFWD_H #include #include @@ -80,4 +80,4 @@ using AssociationMutableCollectionIterator = AssociationCollectionIteratorT Date: Wed, 22 Mar 2023 10:25:36 +0100 Subject: [PATCH 20/79] [wip] Make things compile again after rebase --- include/podio/AssociationCollection.h | 50 ++++++++++++++-------- include/podio/detail/AssociationSIOBlock.h | 29 +++++++------ python/templates/CMakeLists.txt | 1 - 3 files changed, 46 insertions(+), 34 deletions(-) diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index d5b704771..13f77f943 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -9,7 +9,9 @@ #include "podio/AssociationCollectionIterator.h" #include "podio/CollectionBase.h" #include "podio/CollectionBuffers.h" +#include "podio/DatamodelRegistry.h" #include "podio/ICollectionProvider.h" +#include "podio/SchemaEvolution.h" #ifdef PODIO_JSON_OUTPUT #include "nlohmann/json.hpp" @@ -152,25 +154,25 @@ class AssociationCollection : public podio::CollectionBase { return m_storage.getCollectionBuffers(m_isSubsetColl); } - podio::CollectionReadBuffers createBuffers() override /*const*/ { - // Very cumbersome way at the moment. We get the actual buffers to have the - // references and vector members sized appropriately (we will use this - // information to create new buffers outside) - auto collBuffers = m_storage.getCollectionBuffers(m_isSubsetColl); - auto readBuffers = podio::CollectionReadBuffers{}; - readBuffers.references = collBuffers.references; - readBuffers.vectorMembers = collBuffers.vectorMembers; - readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { - CollectionDataT data(buffers, isSubsetColl); - return std::make_unique(std::move(data), isSubsetColl); - }; - readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { - if (buffers.data) { - buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); - } - }; - return readBuffers; - } + // podio::CollectionReadBuffers createBuffers() override /*const*/ { + // // Very cumbersome way at the moment. We get the actual buffers to have the + // // references and vector members sized appropriately (we will use this + // // information to create new buffers outside) + // auto collBuffers = m_storage.getCollectionBuffers(m_isSubsetColl); + // auto readBuffers = podio::CollectionReadBuffers{}; + // readBuffers.references = collBuffers.references; + // readBuffers.vectorMembers = collBuffers.vectorMembers; + // readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + // CollectionDataT data(buffers, isSubsetColl); + // return std::make_unique(std::move(data), isSubsetColl); + // }; + // readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { + // if (buffers.data) { + // buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); + // } + // }; + // return readBuffers; + // } std::string getTypeName() const override { return std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; @@ -247,6 +249,16 @@ class AssociationCollection : public podio::CollectionBase { return m_storage.setReferences(collectionProvider, m_isSubsetColl); } + static constexpr SchemaVersionT schemaVersion = 1; + + SchemaVersionT getSchemaVersion() const override { + return schemaVersion; + } + + size_t getDatamodelRegistryIndex() const override { + return podio::DatamodelRegistry::NoDefinitionNecessary; + } + private: // For setReferences, we need to give our own CollectionData access to our // private entries. Otherwise we would need to expose a public member function diff --git a/include/podio/detail/AssociationSIOBlock.h b/include/podio/detail/AssociationSIOBlock.h index 587bb965c..4c9a2767d 100644 --- a/include/podio/detail/AssociationSIOBlock.h +++ b/include/podio/detail/AssociationSIOBlock.h @@ -4,7 +4,6 @@ #include "podio/detail/AssociationFwd.h" #include "podio/CollectionBuffers.h" - #include "podio/SIOBlock.h" #include @@ -19,12 +18,14 @@ template class AssociationSIOBlock : public podio::SIOBlock { public: AssociationSIOBlock() : - SIOBlock(podio::detail::associationSIOName(), sio::version::encode_version(0, 1)) { + SIOBlock(podio::detail::associationSIOName(), + sio::version::encode_version(AssociationCollection::schemaVersion, 0)) { podio::SIOBlockFactory::instance().registerBlockForCollection(podio::detail::associationSIOName(), this); } - AssociationSIOBlock(const std::string& name) : SIOBlock(name, sio::version::encode_version(0, 1)) { + AssociationSIOBlock(const std::string& name) : + SIOBlock(name, sio::version::encode_version(AssociationCollection::schemaVersion, 0)) { } void read(sio::read_device& device, sio::version_type) override { @@ -68,20 +69,20 @@ class AssociationSIOBlock : public podio::SIOBlock { } } - void createBuffers(const bool subsetCollection = false) override { - m_subsetColl = subsetCollection; + // void createBuffers(const bool subsetCollection = false) override { + // m_subsetColl = subsetCollection; - m_buffers.references = new podio::CollRefCollection(); - m_buffers.vectorMembers = new podio::VectorMembersInfo(); + // m_buffers.references = new podio::CollRefCollection(); + // m_buffers.vectorMembers = new podio::VectorMembersInfo(); - m_buffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { - AssociationCollectionData data(buffers, isSubsetColl); - return std::make_unique>(std::move(data), isSubsetColl); - }; + // m_buffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + // AssociationCollectionData data(buffers, isSubsetColl); + // return std::make_unique>(std::move(data), isSubsetColl); + // }; - // setCollection(new AssociationCollection()); - // _col->setSubsetCollection(subsetCollection); - } + // // setCollection(new AssociationCollection()); + // // _col->setSubsetCollection(subsetCollection); + // } SIOBlock* create(const std::string& name) const override { return new AssociationSIOBlock(name); diff --git a/python/templates/CMakeLists.txt b/python/templates/CMakeLists.txt index a7e0f1768..76dc27359 100644 --- a/python/templates/CMakeLists.txt +++ b/python/templates/CMakeLists.txt @@ -18,7 +18,6 @@ set(PODIO_TEMPLATES ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.cc.jinja2 ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/DatamodelDefinition.h.jinja2 - ${CMAKE_CURRENT_LIST_DIR}/Associations.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/AssociationsRootDict.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/AssociationsSIOBlock.cc.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/collections.jinja2 From 9dbecea206aacd6f6b66d565e8f79d6ef92b10f5 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 22 Mar 2023 16:08:21 +0100 Subject: [PATCH 21/79] [wip] Make associations work with Root I/O --- include/podio/AssociationCollection.h | 52 +++++++++++++++++++++++++++ tests/read_test.h | 3 ++ tests/write_frame.h | 4 ++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index 13f77f943..9fe6b7f99 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -1,6 +1,7 @@ #ifndef PODIO_ASSOCIATIONCOLLECTION_H #define PODIO_ASSOCIATIONCOLLECTION_H +#include "podio/CollectionBufferFactory.h" #include "podio/detail/AssociationCollectionData.h" #include "podio/detail/AssociationFwd.h" #include "podio/detail/AssociationObj.h" @@ -8,10 +9,12 @@ #include "podio/Association.h" #include "podio/AssociationCollectionIterator.h" #include "podio/CollectionBase.h" +#include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" #include "podio/DatamodelRegistry.h" #include "podio/ICollectionProvider.h" #include "podio/SchemaEvolution.h" +#include "podio/utilities/TypeHelpers.h" #ifdef PODIO_JSON_OUTPUT #include "nlohmann/json.hpp" @@ -26,6 +29,13 @@ namespace podio { +template +std::string associationCollTypeName() { + const static std::string typeName = + std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; + return typeName; +} + template class AssociationCollection : public podio::CollectionBase { static_assert(std::is_same_v>, @@ -301,6 +311,48 @@ void to_json(nlohmann::json& j, const AssociationCollection& collect } #endif +template +bool registerAssociationCollection(const std::string& assocTypeName) { + const static auto reg = [&assocTypeName]() { + auto& factory = CollectionBufferFactory::mutInstance(); + factory.registerCreationFunc(assocTypeName, AssociationCollection::schemaVersion, [](bool subsetColl) { + auto readBuffers = podio::CollectionReadBuffers{}; + readBuffers.data = subsetColl ? nullptr : new AssociationDataContainer(); + + // Either it is a subset collection or we have two relations + const auto nRefs = subsetColl ? 1 : 2; + readBuffers.references = new podio::CollRefCollection(nRefs); + for (auto& ref : *readBuffers.references) { + // Make sure to place usable buffer pointers here + ref = std::make_unique>(); + } + + readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + AssociationCollectionData data(buffers, isSubsetColl); + return std::make_unique>(std::move(data), isSubsetColl); + }; + + readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { + if (buffers.data) { + buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); + } + }; + + return readBuffers; + }); + + return true; + }(); + return reg; +} + +#define PODIO_DECLARE_ASSOCIATION(TypeName, FromT, ToT) \ + namespace { \ + using TypeName = podio::AssociationCollection; \ + const auto registerAssociation = \ + podio::registerAssociationCollection(podio::associationCollTypeName()); \ + } // namespace podio + } // namespace podio #endif // PODIO_ASSOCIATIONCOLLECTION_H diff --git a/tests/read_test.h b/tests/read_test.h index d112d463c..87694678e 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -26,6 +26,9 @@ #include #include +// using TestAssocCollection = podio::AssociationCollection; +PODIO_DECLARE_ASSOCIATION(TestAssocCollection, ExampleMC, ex42::ExampleWithARelation) + template bool check_fixed_width_value(FixedWidthT actual, FixedWidthT expected, const std::string& type) { if (actual != expected) { diff --git a/tests/write_frame.h b/tests/write_frame.h index b6f3136f5..47c30cd13 100644 --- a/tests/write_frame.h +++ b/tests/write_frame.h @@ -28,7 +28,9 @@ #include // Define an association that is used for the I/O tests -using TestAssocCollection = podio::AssociationCollection; +// using TestAssocCollection = podio::AssociationCollection; + +PODIO_DECLARE_ASSOCIATION(TestAssocCollection, ExampleMC, ex42::ExampleWithARelation) auto createMCCollection() { auto mcps = ExampleMCCollection(); From 942d01a255a0032974971900a8848c54b60342d4 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 22 Mar 2023 18:52:04 +0100 Subject: [PATCH 22/79] [wip] Make SIO Frame reading (almost) work again --- include/podio/AssociationCollection.h | 123 ++++++++---------- include/podio/detail/AssociationFwd.h | 9 +- include/podio/detail/AssociationSIOBlock.h | 34 ++--- python/podio_gen/cpp_generator.py | 52 +------- .../templates/AssociationsRootDict.h.jinja2 | 11 -- .../templates/AssociationsSIOBlock.cc.jinja2 | 12 -- python/templates/CMakeLists.txt | 2 - python/templates/selection.xml.jinja2 | 9 -- tests/read_test.h | 8 ++ tests/sio_io/write_frame_sio.cpp | 5 + 10 files changed, 89 insertions(+), 176 deletions(-) delete mode 100644 python/templates/AssociationsRootDict.h.jinja2 delete mode 100644 python/templates/AssociationsSIOBlock.cc.jinja2 diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index 9fe6b7f99..972e166bd 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -29,13 +29,6 @@ namespace podio { -template -std::string associationCollTypeName() { - const static std::string typeName = - std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; - return typeName; -} - template class AssociationCollection : public podio::CollectionBase { static_assert(std::is_same_v>, @@ -164,28 +157,8 @@ class AssociationCollection : public podio::CollectionBase { return m_storage.getCollectionBuffers(m_isSubsetColl); } - // podio::CollectionReadBuffers createBuffers() override /*const*/ { - // // Very cumbersome way at the moment. We get the actual buffers to have the - // // references and vector members sized appropriately (we will use this - // // information to create new buffers outside) - // auto collBuffers = m_storage.getCollectionBuffers(m_isSubsetColl); - // auto readBuffers = podio::CollectionReadBuffers{}; - // readBuffers.references = collBuffers.references; - // readBuffers.vectorMembers = collBuffers.vectorMembers; - // readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { - // CollectionDataT data(buffers, isSubsetColl); - // return std::make_unique(std::move(data), isSubsetColl); - // }; - // readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { - // if (buffers.data) { - // buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); - // } - // }; - // return readBuffers; - // } - std::string getTypeName() const override { - return std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; + return podio::detail::associationCollTypeName(); } std::string getValueTypeName() const override { @@ -311,48 +284,62 @@ void to_json(nlohmann::json& j, const AssociationCollection& collect } #endif -template -bool registerAssociationCollection(const std::string& assocTypeName) { - const static auto reg = [&assocTypeName]() { - auto& factory = CollectionBufferFactory::mutInstance(); - factory.registerCreationFunc(assocTypeName, AssociationCollection::schemaVersion, [](bool subsetColl) { - auto readBuffers = podio::CollectionReadBuffers{}; - readBuffers.data = subsetColl ? nullptr : new AssociationDataContainer(); - - // Either it is a subset collection or we have two relations - const auto nRefs = subsetColl ? 1 : 2; - readBuffers.references = new podio::CollRefCollection(nRefs); - for (auto& ref : *readBuffers.references) { - // Make sure to place usable buffer pointers here - ref = std::make_unique>(); - } - - readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { - AssociationCollectionData data(buffers, isSubsetColl); - return std::make_unique>(std::move(data), isSubsetColl); - }; - - readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { - if (buffers.data) { - buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); - } - }; - - return readBuffers; - }); +namespace detail { + template + bool registerAssociationCollection(const std::string& assocTypeName) { + const static auto reg = [&assocTypeName]() { + auto& factory = CollectionBufferFactory::mutInstance(); + factory.registerCreationFunc( + assocTypeName, AssociationCollection::schemaVersion, [](bool subsetColl) { + auto readBuffers = podio::CollectionReadBuffers{}; + readBuffers.data = subsetColl ? nullptr : new AssociationDataContainer(); + + // Either it is a subset collection or we have two relations + const auto nRefs = subsetColl ? 1 : 2; + readBuffers.references = new podio::CollRefCollection(nRefs); + for (auto& ref : *readBuffers.references) { + // Make sure to place usable buffer pointers here + ref = std::make_unique>(); + } + + readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + AssociationCollectionData data(buffers, isSubsetColl); + return std::make_unique>(std::move(data), isSubsetColl); + }; + + readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { + if (buffers.data) { + buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); + } + }; + + return readBuffers; + }); + + return true; + }(); + return reg; + } +} // namespace detail - return true; - }(); - return reg; -} +} // namespace podio +/** + * Main macro for declaring associations. Takes care of the following things: - + * - A type alias with the name TypeName: using TypeAlias = + * AssociationCollection + * - Registering the necessary buffer creation functionality with the + * CollectionBufferFactory. + * + * NOTE: The passed TypeName cannot have a namespace qualifier. If you want the + * type alias to appear in a namespace place the macro call into that namespace. + * + * TODO: Split off the SIOBlock dependency cleanly (i.e. not needing a dedicated + * include, and only present when building with SIO) + */ #define PODIO_DECLARE_ASSOCIATION(TypeName, FromT, ToT) \ - namespace { \ - using TypeName = podio::AssociationCollection; \ - const auto registerAssociation = \ - podio::registerAssociationCollection(podio::associationCollTypeName()); \ - } // namespace podio - -} // namespace podio + using TypeName = podio::AssociationCollection; \ + const static auto REGISTERED_ASSOCIATION_##TypeName = \ + podio::detail::registerAssociationCollection(podio::detail::associationCollTypeName()); #endif // PODIO_ASSOCIATIONCOLLECTION_H diff --git a/include/podio/detail/AssociationFwd.h b/include/podio/detail/AssociationFwd.h index 20d5c9d30..56733cec1 100644 --- a/include/podio/detail/AssociationFwd.h +++ b/include/podio/detail/AssociationFwd.h @@ -36,9 +36,16 @@ namespace detail { template using GetCollT = typename GetCollType::type; + template + std::string associationCollTypeName() { + const static std::string typeName = + std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; + return typeName; + } + template inline std::string associationSIOName() { - auto n = std::string("Association_FROM_") + FromT::TypeName + "_TO_" + FromT::TypeName; + auto n = std::string("ASSOCIATION_FROM_") + FromT::TypeName + "_TO_" + ToT::TypeName; std::replace(n.begin(), n.end(), ':', '_'); return n; } diff --git a/include/podio/detail/AssociationSIOBlock.h b/include/podio/detail/AssociationSIOBlock.h index 4c9a2767d..cc8a2f3ec 100644 --- a/include/podio/detail/AssociationSIOBlock.h +++ b/include/podio/detail/AssociationSIOBlock.h @@ -1,7 +1,8 @@ #ifndef PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H #define PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H -#include "podio/detail/AssociationFwd.h" +#include "podio/AssociationCollection.h" +#include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" #include "podio/SIOBlock.h" @@ -28,17 +29,19 @@ class AssociationSIOBlock : public podio::SIOBlock { SIOBlock(name, sio::version::encode_version(AssociationCollection::schemaVersion, 0)) { } - void read(sio::read_device& device, sio::version_type) override { - m_buffers.references->emplace_back(std::make_unique>()); - if (!m_subsetColl) { - m_buffers.references->emplace_back(std::make_unique>()); - } + void read(sio::read_device& device, sio::version_type version) override { + auto& bufferFactory = podio::CollectionBufferFactory::instance(); + // TODO: + // - Error handling of empty optional + auto maybeBuffers = bufferFactory.createBuffers(podio::detail::associationCollTypeName(), + sio::version::major_version(version), m_subsetColl); + m_buffers = maybeBuffers.value(); if (!m_subsetColl) { unsigned size{0}; device.data(size); - m_buffers.data = new std::vector(size); auto* dataVec = m_buffers.dataAsVector(); + dataVec->resize(size); podio::handlePODDataSIO(device, dataVec->data(), size); } @@ -60,7 +63,7 @@ class AssociationSIOBlock : public podio::SIOBlock { podio::handlePODDataSIO(device, dataVec->data(), size); } - // ---- wirte ref collections ------ + // ---- write ref collections ------ auto* refColls = m_buffers.references; for (auto& refC : *refColls) { unsigned size = refC->size(); @@ -69,21 +72,6 @@ class AssociationSIOBlock : public podio::SIOBlock { } } - // void createBuffers(const bool subsetCollection = false) override { - // m_subsetColl = subsetCollection; - - // m_buffers.references = new podio::CollRefCollection(); - // m_buffers.vectorMembers = new podio::VectorMembersInfo(); - - // m_buffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { - // AssociationCollectionData data(buffers, isSubsetColl); - // return std::make_unique>(std::move(data), isSubsetColl); - // }; - - // // setCollection(new AssociationCollection()); - // // _col->setSubsetCollection(subsetCollection); - // } - SIOBlock* create(const std::string& name) const override { return new AssociationSIOBlock(name); } diff --git a/python/podio_gen/cpp_generator.py b/python/podio_gen/cpp_generator.py index cd9b71ac8..0607a5301 100644 --- a/python/podio_gen/cpp_generator.py +++ b/python/podio_gen/cpp_generator.py @@ -7,7 +7,6 @@ from enum import IntEnum from collections import defaultdict from collections.abc import Mapping -from itertools import combinations_with_replacement, product from podio_schema_evolution import DataModelComparator from podio_schema_evolution import RenamedMember, root_filter, RootIoRule @@ -96,12 +95,9 @@ def post_process(self, _): """Do the cpp specific post processing""" self._write_edm_def_file() - # all possible associations - assocs = self._instantiate_associations() - if "ROOT" in self.io_handlers: self._prepare_iorules() - self._create_selection_xml(assocs) + self._create_selection_xml() self._write_all_collections_header() self._write_cmake_lists_file() @@ -541,7 +537,7 @@ def quoted_sv(string): self._eval_template("DatamodelDefinition.h.jinja2", data), ) - def _create_selection_xml(self, assoc_combinations): + def _create_selection_xml(self): """Create the selection xml that is necessary for ROOT I/O""" data = { "version": self.datamodel.schema_version, @@ -552,7 +548,6 @@ def _create_selection_xml(self, assoc_combinations): for d in self.root_schema_datatype_names | self.root_schema_component_names ], # noqa "iorules": self.root_schema_iorules, - "associations": assoc_combinations, } self._write_file("selection.xml", self._eval_template("selection.xml.jinja2", data)) @@ -572,49 +567,6 @@ def _get_member_includes(self, members): return self._sort_includes(includes) - def _instantiate_associations(self): - """Instantiate all the associations in a dedicated .cc file and return - the combination of all instantiated things - """ - datatypes = [DataType(d) for d in self.datamodel.datatypes] - includes = [ - self._build_include_for_class(f"{d.bare_type}Collection", IncludeFrom.INTERNAL) - for d in datatypes - ] - - # We want all combinations of our datamodel - combinations = tuple(combinations_with_replacement(datatypes, 2)) - - if self.upstream_edm: - ext_datatypes = [DataType(d) for d in self.upstream_edm.datatypes] - includes.extend( - [ - self._build_include_for_class(f"{d.bare_type}Collection", IncludeFrom.EXTERNAL) - for d in ext_datatypes - ] - ) - - combinations += tuple(product(ext_datatypes, datatypes)) - - assoc_data = { - "package_name": self.package_name, - "includes": includes, - "combinations": combinations, - } - - self._write_file( - "AssociationsRootDict.h", - self._eval_template("AssociationsRootDict.h.jinja2", assoc_data), - ) - - if "SIO" in self.io_handlers: - self._write_file( - "AssociationsSIOBlock.cc", - self._eval_template("AssociationsSIOBlock.cc.jinja2", assoc_data), - ) - - return combinations - def _needs_include(self, classname) -> IncludeFrom: """Check whether the member needs an include from within the datamodel""" if ( diff --git a/python/templates/AssociationsRootDict.h.jinja2 b/python/templates/AssociationsRootDict.h.jinja2 deleted file mode 100644 index 8a590de7f..000000000 --- a/python/templates/AssociationsRootDict.h.jinja2 +++ /dev/null @@ -1,11 +0,0 @@ -// AUTOMATICALLY GENERATED FILE - DO NOT EDIT -#include "podio/AssociationCollection.h" - -// Datamodel includes -{% for include in includes %} -{{ include }} -{% endfor %} - -{% for (from_t, to_t) in combinations %} -static podio::AssociationCollection<{{from_t.full_type}}, {{to_t.full_type}}> podio__association_dummy__{{ package_name }}__{{ loop.index0 }}{}; -{% endfor %} diff --git a/python/templates/AssociationsSIOBlock.cc.jinja2 b/python/templates/AssociationsSIOBlock.cc.jinja2 deleted file mode 100644 index 024befdfd..000000000 --- a/python/templates/AssociationsSIOBlock.cc.jinja2 +++ /dev/null @@ -1,12 +0,0 @@ -// AUTOMATICALLY GENERATED FILE - DO NOT EDIT -#include "podio/AssociationCollection.h" -#include "podio/detail/AssociationSIOBlock.h" - -// Datamodel includes -{% for include in includes %} -{{ include }} -{% endfor %} - -{% for (from_t, to_t) in combinations %} -static podio::AssociationSIOBlock<{{from_t.full_type}}, {{to_t.full_type}}> podio__association_dummy__{{ package_name }}__sio_block__{{ loop.index0 }}{}; -{% endfor %} diff --git a/python/templates/CMakeLists.txt b/python/templates/CMakeLists.txt index 76dc27359..014b4e0bd 100644 --- a/python/templates/CMakeLists.txt +++ b/python/templates/CMakeLists.txt @@ -18,8 +18,6 @@ set(PODIO_TEMPLATES ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.cc.jinja2 ${CMAKE_CURRENT_LIST_DIR}/SIOBlock.h.jinja2 ${CMAKE_CURRENT_LIST_DIR}/DatamodelDefinition.h.jinja2 - ${CMAKE_CURRENT_LIST_DIR}/AssociationsRootDict.h.jinja2 - ${CMAKE_CURRENT_LIST_DIR}/AssociationsSIOBlock.cc.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/collections.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/declarations.jinja2 ${CMAKE_CURRENT_LIST_DIR}/macros/implementations.jinja2 diff --git a/python/templates/selection.xml.jinja2 b/python/templates/selection.xml.jinja2 index 7ff109c41..e651a61b2 100644 --- a/python/templates/selection.xml.jinja2 +++ b/python/templates/selection.xml.jinja2 @@ -13,10 +13,6 @@ {{ iorule.code }} ]]> - -{% macro assoc_selection(from_t, to_t) %} - - {% endmacro %} @@ -42,11 +38,6 @@ {% for class in old_schema_components %} {{ class_selection(class) }} -{% endfor %} - - -{% for (from_t, to_t) in associations %} -{{ assoc_selection(from_t, to_t) }} {% endfor %} diff --git a/tests/read_test.h b/tests/read_test.h index 87694678e..0486e2e90 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -16,6 +16,7 @@ // podio specific includes #include "podio/Frame.h" #include "podio/UserDataCollection.h" +#include "podio/detail/AssociationSIOBlock.h" #include "podio/podioVersion.h" // STL @@ -28,6 +29,13 @@ // using TestAssocCollection = podio::AssociationCollection; PODIO_DECLARE_ASSOCIATION(TestAssocCollection, ExampleMC, ex42::ExampleWithARelation) +PODIO_DECLARE_ASSOCIATION(TestAssocCollection2, ExampleMC, ExampleHit) + +namespace ex42 { +PODIO_DECLARE_ASSOCIATION(AssocInNamespace, ExampleWithARelation, ExampleMC) +} + +using A = ex42::AssocInNamespace; template bool check_fixed_width_value(FixedWidthT actual, FixedWidthT expected, const std::string& type) { diff --git a/tests/sio_io/write_frame_sio.cpp b/tests/sio_io/write_frame_sio.cpp index 21cd740ca..9496f6445 100644 --- a/tests/sio_io/write_frame_sio.cpp +++ b/tests/sio_io/write_frame_sio.cpp @@ -1,7 +1,12 @@ +#include "datamodel/ExampleWithARelation.h" #include "write_frame.h" #include "podio/SIOWriter.h" +#include "podio/detail/AssociationSIOBlock.h" + +const static auto foo = podio::AssociationSIOBlock{}; + int main(int, char**) { write_frames("example_frame.sio"); return 0; From 4d7627190bcda4fcaacbeebb849d52794fde0291 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 22 Mar 2023 19:37:33 +0100 Subject: [PATCH 23/79] "fix" legacy sio frame test --- tests/sio_io/read_frame_legacy_sio.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/sio_io/read_frame_legacy_sio.cpp b/tests/sio_io/read_frame_legacy_sio.cpp index 5dbc123d3..6d77c49e9 100644 --- a/tests/sio_io/read_frame_legacy_sio.cpp +++ b/tests/sio_io/read_frame_legacy_sio.cpp @@ -2,9 +2,12 @@ #include "podio/Frame.h" #include "podio/SIOLegacyReader.h" +#include "podio/detail/AssociationSIOBlock.h" #include +const static auto foo = podio::AssociationSIOBlock{}; + int main(int argc, char* argv[]) { if (argc != 2) { std::cerr << "usage: read_frame_legacy_sio inputfile" << std::endl; From a1b0f3d87511b9c9a43356453b4eaaf02062662d Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 23 Mar 2023 13:28:13 +0100 Subject: [PATCH 24/79] Make the AssociationSIOBlock public --- include/podio/{detail => }/AssociationSIOBlock.h | 6 +++--- tests/read_test.h | 1 - tests/sio_io/read_frame_legacy_sio.cpp | 2 +- tests/sio_io/write_frame_sio.cpp | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) rename include/podio/{detail => }/AssociationSIOBlock.h (95%) diff --git a/include/podio/detail/AssociationSIOBlock.h b/include/podio/AssociationSIOBlock.h similarity index 95% rename from include/podio/detail/AssociationSIOBlock.h rename to include/podio/AssociationSIOBlock.h index cc8a2f3ec..a59d3e130 100644 --- a/include/podio/detail/AssociationSIOBlock.h +++ b/include/podio/AssociationSIOBlock.h @@ -1,5 +1,5 @@ -#ifndef PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H -#define PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H +#ifndef PODIO_ASSOCIATIONSIOBLOCK_H +#define PODIO_ASSOCIATIONSIOBLOCK_H #include "podio/AssociationCollection.h" #include "podio/CollectionBufferFactory.h" @@ -79,4 +79,4 @@ class AssociationSIOBlock : public podio::SIOBlock { } // namespace podio -#endif // PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H +#endif // PODIO_ASSOCIATIONSIOBLOCK_H diff --git a/tests/read_test.h b/tests/read_test.h index 0486e2e90..05727400b 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -16,7 +16,6 @@ // podio specific includes #include "podio/Frame.h" #include "podio/UserDataCollection.h" -#include "podio/detail/AssociationSIOBlock.h" #include "podio/podioVersion.h" // STL diff --git a/tests/sio_io/read_frame_legacy_sio.cpp b/tests/sio_io/read_frame_legacy_sio.cpp index 6d77c49e9..0bf0c98bc 100644 --- a/tests/sio_io/read_frame_legacy_sio.cpp +++ b/tests/sio_io/read_frame_legacy_sio.cpp @@ -2,7 +2,7 @@ #include "podio/Frame.h" #include "podio/SIOLegacyReader.h" -#include "podio/detail/AssociationSIOBlock.h" +#include "podio/AssociationSIOBlock.h" #include diff --git a/tests/sio_io/write_frame_sio.cpp b/tests/sio_io/write_frame_sio.cpp index 9496f6445..6674f7073 100644 --- a/tests/sio_io/write_frame_sio.cpp +++ b/tests/sio_io/write_frame_sio.cpp @@ -3,7 +3,7 @@ #include "podio/SIOWriter.h" -#include "podio/detail/AssociationSIOBlock.h" +#include "podio/AssociationSIOBlock.h" const static auto foo = podio::AssociationSIOBlock{}; From 6cc3c1eb9cbbb1be09eaec0f87ff49320e51488b Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 12:41:48 +0100 Subject: [PATCH 25/79] [wip] Introduce macros for registering associations --- include/podio/AssociationCollection.h | 367 ++-------------------- include/podio/AssociationCollectionImpl.h | 329 +++++++++++++++++++ include/podio/AssociationSIOBlock.h | 1 - tests/read_test.h | 13 +- tests/sio_io/read_frame_legacy_sio.cpp | 3 - tests/sio_io/write_frame_sio.cpp | 5 - tests/write_frame.h | 6 +- 7 files changed, 367 insertions(+), 357 deletions(-) create mode 100644 include/podio/AssociationCollectionImpl.h diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index 972e166bd..c2af89d22 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -1,345 +1,40 @@ #ifndef PODIO_ASSOCIATIONCOLLECTION_H #define PODIO_ASSOCIATIONCOLLECTION_H +#include "podio/AssociationCollectionImpl.h" -#include "podio/CollectionBufferFactory.h" -#include "podio/detail/AssociationCollectionData.h" -#include "podio/detail/AssociationFwd.h" -#include "podio/detail/AssociationObj.h" +// Preprocessor helper macros for concatenating tokens at preprocessing times +// Necessary because we use __COUNTER__ below for unique names of static +// variables for values returned by registration function calls +#define PP_CONCAT_IMPL(x, y) x##y +#define PP_CONCAT(x, y) PP_CONCAT_IMPL(x, y) -#include "podio/Association.h" -#include "podio/AssociationCollectionIterator.h" -#include "podio/CollectionBase.h" -#include "podio/CollectionBufferFactory.h" -#include "podio/CollectionBuffers.h" -#include "podio/DatamodelRegistry.h" -#include "podio/ICollectionProvider.h" -#include "podio/SchemaEvolution.h" -#include "podio/utilities/TypeHelpers.h" - -#ifdef PODIO_JSON_OUTPUT - #include "nlohmann/json.hpp" +#ifndef PODIO_ENABLE_SIO + #define PODIO_ENABLE_SIO 0 #endif -#include -#include -#include -#include -#include -#include - -namespace podio { - -template -class AssociationCollection : public podio::CollectionBase { - static_assert(std::is_same_v>, - "Associations need to be instantiated with the default types!"); - static_assert(std::is_same_v>, - "Associations need to be instantiated with the default types!"); - - // convenience typedefs - using AssocT = Association; - using MutableAssocT = MutableAssociation; - - using CollectionT = podio::AssociationCollection; - using CollectionDataT = podio::AssociationCollectionData; - -public: - using const_iterator = AssociationCollectionIterator; - using iterator = AssociationMutableCollectionIterator; - - AssociationCollection() = default; - - AssociationCollection(CollectionDataT&& data, bool isSubsetColl) : - m_isSubsetColl(isSubsetColl), m_collectionID(0), m_storage(std::move(data)) { - } - - // Move-only type - AssociationCollection(const AssociationCollection&) = delete; - AssociationCollection& operator=(const AssociationCollection&) = delete; - AssociationCollection(AssociationCollection&&) = default; - AssociationCollection& operator=(AssociationCollection&&) = default; - - ~AssociationCollection() { - // Need the storage how to clean up - m_storage.clear(m_isSubsetColl); - } - - /// Append a new association to the collection and return this object - MutableAssocT create() { - if (m_isSubsetColl) { - throw std::logic_error("Cannot create new elements on a subset collection"); - } - - auto obj = m_storage.entries.emplace_back(new AssociationObj()); - obj->id = {int(m_storage.entries.size() - 1), m_collectionID}; - return MutableAssocT(obj); - } - - /// Returns the immutable object of given index - AssocT operator[](unsigned int index) const { - return AssocT(m_storage.entries[index]); - } - /// Returns the mutable object of given index - MutableAssocT operator[](unsigned int index) { - return MutableAssocT(m_storage.entries[index]); - } - /// Returns the immutable object of given index - AssocT at(unsigned int index) const { - return AssocT(m_storage.entries.at(index)); - } - /// Returns the mutable object of given index - MutableAssocT at(unsigned int index) { - return MutableAssocT(m_storage.entries.at(index)); - } - - void push_back(AssocT object) { - // We have to do different things here depending on whether this is a - // subset collection or not. A normal collection cannot collect objects - // that are already part of another collection, while a subset collection - // can only collect such objects - if (!m_isSubsetColl) { - auto obj = object.m_obj; - if (obj->id.index == podio::ObjectID::untracked) { - const auto size = m_storage.entries.size(); - obj->id = {(int)size, m_collectionID}; - m_storage.entries.push_back(obj); - } else { - throw std::invalid_argument("Object already in a collection. Cannot add it to a second collection"); - } - } else { - const auto obj = object.m_obj; - if (obj->id.index < 0) { - throw std::invalid_argument( - "Objects can only be stored in a subset collection if they are already elements of a collection"); - } - m_storage.entries.push_back(obj); - // No need to handle any relations here, since this is already done by the - // "owning" collection - } - } - - /// Number of elements in the collection - size_t size() const override { - return m_storage.entries.size(); - } - - void clear() override { - m_storage.clear(m_isSubsetColl); - m_isPrepared = false; - } - - void print(std::ostream& os = std::cout, bool flush = true) const override { - os << *this; - if (flush) { - os.flush(); - } - } - - // support for the iterator protocol - const_iterator begin() const { - return const_iterator(0, &m_storage.entries); - } - const_iterator end() const { - return const_iterator(m_storage.entries.size(), &m_storage.entries); - } - iterator begin() { - return iterator(0, &m_storage.entries); - } - iterator end() { - return iterator(m_storage.entries.size(), &m_storage.entries); - } - - bool isValid() const override { - return m_isValid; - } - - podio::CollectionWriteBuffers getBuffers() override { - return m_storage.getCollectionBuffers(m_isSubsetColl); - } - - std::string getTypeName() const override { - return podio::detail::associationCollTypeName(); - } - - std::string getValueTypeName() const override { - return podio::detail::associationSIOName(); - } - - std::string getDataTypeName() const override { - return "float"; - } - - bool isSubsetCollection() const override { - return m_isSubsetColl; - } - - void setSubsetCollection(bool setSubset = true) override { - if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { - throw std::logic_error("Cannot change the character of a collection that already contains elements"); - } - - if (setSubset) { - m_storage.makeSubsetCollection(); - } - m_isSubsetColl = setSubset; - } - - void setID(unsigned id) override { - m_collectionID = id; - } - - unsigned getID() const override { - return m_collectionID; - } - - void prepareForWrite() const override { - // TODO: Replace this double locking pattern with an atomic and only one - // lock. Problem: std::atomic is not default movable - { - std::lock_guard lock{*m_storageMtx}; - // If the collection has been prepared already there is nothing to do - if (m_isPrepared) { - return; - } - } - - std::lock_guard lock{*m_storageMtx}; - // by the time we acquire the lock another thread might have already - // succeeded, so we need to check again now - if (m_isPrepared) { - return; - } - m_storage.prepareForWrite(m_isSubsetColl); - m_isPrepared = true; - } - - void prepareAfterRead() override { - // No need to go through this again if we have already done it - if (m_isPrepared) { - return; - } - - if (!m_isSubsetColl) { - // Subset collections do not store any data that would require post-processing - m_storage.prepareAfterRead(m_collectionID); - } - // Preparing a collection doesn't affect the underlying I/O buffers, so this - // collection is still prepared - m_isPrepared = true; - } - - bool setReferences(const ICollectionProvider* collectionProvider) override { - return m_storage.setReferences(collectionProvider, m_isSubsetColl); - } - - static constexpr SchemaVersionT schemaVersion = 1; - - SchemaVersionT getSchemaVersion() const override { - return schemaVersion; - } - - size_t getDatamodelRegistryIndex() const override { - return podio::DatamodelRegistry::NoDefinitionNecessary; - } - -private: - // For setReferences, we need to give our own CollectionData access to our - // private entries. Otherwise we would need to expose a public member function - // that gives access to the Obj* which is definitely not what we want - friend CollectionDataT; - - bool m_isValid{false}; - mutable bool m_isPrepared{false}; - bool m_isSubsetColl{false}; - int m_collectionID{0}; - mutable std::unique_ptr m_storageMtx{std::make_unique()}; - mutable CollectionDataT m_storage{}; -}; - -template -std::ostream& operator<<(std::ostream& o, const AssociationCollection& v) { - const auto old_flags = o.flags(); - o << " id: weight:" << '\n'; - for (const auto&& el : v) { - o << std::scientific << std::showpos << std::setw(12) << el.id() << " " << std::setw(12) << " " << el.getWeight() - << '\n'; - - o << " from : "; - o << el.getFrom().id() << std::endl; - o << " to : "; - o << el.getTo().id() << std::endl; - } - - o.flags(old_flags); - return o; -} - -#ifdef PODIO_JSON_OUTPUT -template -void to_json(nlohmann::json& j, const AssociationCollection& collection) { - j = nlohmann::json::array(); - for (auto&& elem : collection) { - j.emplace_back(elem); - } -} +#if PODIO_ENABLE_SIO && __has_include("podio/AssociationSIOBlock.h") + #include "podio/AssociationSIOBlock.h" + /** + * Main macro for declaring associations. Takes care of the following things: + * - Registering the necessary buffer creation functionality with the + * CollectionBufferFactory. + * - Registering the necessary SIOBlock with the SIOBlock factory + */ + #define PODIO_DECLARE_ASSOCIATION(FromT, ToT) \ + const static auto PP_CONCAT(REGISTERED_ASSOCIATION_, __COUNTER__) = \ + podio::detail::registerAssociationCollection( \ + podio::detail::associationCollTypeName()); \ + const static auto PP_CONCAT(ASSOCIATION_SIO_BLOCK_, __COUNTER__) = podio::AssociationSIOBlock{}; +#else + /** + * Main macro for declaring associations. Takes care of the following things: + * - Registering the necessary buffer creation functionality with the + * CollectionBufferFactory. + */ + #define PODIO_DECLARE_ASSOCIATION(FromT, ToT) \ + const static auto PP_CONCAT(REGISTERED_ASSOCIATION_, __COUNTER__) = \ + podio::detail::registerAssociationCollection( \ + podio::detail::associationCollTypeName()); #endif -namespace detail { - template - bool registerAssociationCollection(const std::string& assocTypeName) { - const static auto reg = [&assocTypeName]() { - auto& factory = CollectionBufferFactory::mutInstance(); - factory.registerCreationFunc( - assocTypeName, AssociationCollection::schemaVersion, [](bool subsetColl) { - auto readBuffers = podio::CollectionReadBuffers{}; - readBuffers.data = subsetColl ? nullptr : new AssociationDataContainer(); - - // Either it is a subset collection or we have two relations - const auto nRefs = subsetColl ? 1 : 2; - readBuffers.references = new podio::CollRefCollection(nRefs); - for (auto& ref : *readBuffers.references) { - // Make sure to place usable buffer pointers here - ref = std::make_unique>(); - } - - readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { - AssociationCollectionData data(buffers, isSubsetColl); - return std::make_unique>(std::move(data), isSubsetColl); - }; - - readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { - if (buffers.data) { - buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); - } - }; - - return readBuffers; - }); - - return true; - }(); - return reg; - } -} // namespace detail - -} // namespace podio - -/** - * Main macro for declaring associations. Takes care of the following things: - - * - A type alias with the name TypeName: using TypeAlias = - * AssociationCollection - * - Registering the necessary buffer creation functionality with the - * CollectionBufferFactory. - * - * NOTE: The passed TypeName cannot have a namespace qualifier. If you want the - * type alias to appear in a namespace place the macro call into that namespace. - * - * TODO: Split off the SIOBlock dependency cleanly (i.e. not needing a dedicated - * include, and only present when building with SIO) - */ -#define PODIO_DECLARE_ASSOCIATION(TypeName, FromT, ToT) \ - using TypeName = podio::AssociationCollection; \ - const static auto REGISTERED_ASSOCIATION_##TypeName = \ - podio::detail::registerAssociationCollection(podio::detail::associationCollTypeName()); - #endif // PODIO_ASSOCIATIONCOLLECTION_H diff --git a/include/podio/AssociationCollectionImpl.h b/include/podio/AssociationCollectionImpl.h new file mode 100644 index 000000000..37d74df62 --- /dev/null +++ b/include/podio/AssociationCollectionImpl.h @@ -0,0 +1,329 @@ +#ifndef PODIO_ASSOCIATIONCOLLECTIONIMPL_H +#define PODIO_ASSOCIATIONCOLLECTIONIMPL_H + +#include "podio/CollectionBufferFactory.h" +#include "podio/detail/AssociationCollectionData.h" +#include "podio/detail/AssociationFwd.h" +#include "podio/detail/AssociationObj.h" + +#include "podio/Association.h" +#include "podio/AssociationCollectionIterator.h" +#include "podio/CollectionBase.h" +#include "podio/CollectionBufferFactory.h" +#include "podio/CollectionBuffers.h" +#include "podio/DatamodelRegistry.h" +#include "podio/ICollectionProvider.h" +#include "podio/SchemaEvolution.h" +#include "podio/utilities/TypeHelpers.h" + +#ifdef PODIO_JSON_OUTPUT + #include "nlohmann/json.hpp" +#endif + +#include +#include +#include +#include +#include +#include + +namespace podio { + +template +class AssociationCollection : public podio::CollectionBase { + static_assert(std::is_same_v>, + "Associations need to be instantiated with the default types!"); + static_assert(std::is_same_v>, + "Associations need to be instantiated with the default types!"); + + // convenience typedefs + using AssocT = Association; + using MutableAssocT = MutableAssociation; + + using CollectionT = podio::AssociationCollection; + using CollectionDataT = podio::AssociationCollectionData; + +public: + using const_iterator = AssociationCollectionIterator; + using iterator = AssociationMutableCollectionIterator; + + AssociationCollection() = default; + + AssociationCollection(CollectionDataT&& data, bool isSubsetColl) : + m_isSubsetColl(isSubsetColl), m_collectionID(0), m_storage(std::move(data)) { + } + + // Move-only type + AssociationCollection(const AssociationCollection&) = delete; + AssociationCollection& operator=(const AssociationCollection&) = delete; + AssociationCollection(AssociationCollection&&) = default; + AssociationCollection& operator=(AssociationCollection&&) = default; + + ~AssociationCollection() { + // Need the storage how to clean up + m_storage.clear(m_isSubsetColl); + } + + /// Append a new association to the collection and return this object + MutableAssocT create() { + if (m_isSubsetColl) { + throw std::logic_error("Cannot create new elements on a subset collection"); + } + + auto obj = m_storage.entries.emplace_back(new AssociationObj()); + obj->id = {int(m_storage.entries.size() - 1), m_collectionID}; + return MutableAssocT(obj); + } + + /// Returns the immutable object of given index + AssocT operator[](unsigned int index) const { + return AssocT(m_storage.entries[index]); + } + /// Returns the mutable object of given index + MutableAssocT operator[](unsigned int index) { + return MutableAssocT(m_storage.entries[index]); + } + /// Returns the immutable object of given index + AssocT at(unsigned int index) const { + return AssocT(m_storage.entries.at(index)); + } + /// Returns the mutable object of given index + MutableAssocT at(unsigned int index) { + return MutableAssocT(m_storage.entries.at(index)); + } + + void push_back(AssocT object) { + // We have to do different things here depending on whether this is a + // subset collection or not. A normal collection cannot collect objects + // that are already part of another collection, while a subset collection + // can only collect such objects + if (!m_isSubsetColl) { + auto obj = object.m_obj; + if (obj->id.index == podio::ObjectID::untracked) { + const auto size = m_storage.entries.size(); + obj->id = {(int)size, m_collectionID}; + m_storage.entries.push_back(obj); + } else { + throw std::invalid_argument("Object already in a collection. Cannot add it to a second collection"); + } + } else { + const auto obj = object.m_obj; + if (obj->id.index < 0) { + throw std::invalid_argument( + "Objects can only be stored in a subset collection if they are already elements of a collection"); + } + m_storage.entries.push_back(obj); + // No need to handle any relations here, since this is already done by the + // "owning" collection + } + } + + /// Number of elements in the collection + size_t size() const override { + return m_storage.entries.size(); + } + + void clear() override { + m_storage.clear(m_isSubsetColl); + m_isPrepared = false; + } + + void print(std::ostream& os = std::cout, bool flush = true) const override { + os << *this; + if (flush) { + os.flush(); + } + } + + // support for the iterator protocol + const_iterator begin() const { + return const_iterator(0, &m_storage.entries); + } + const_iterator end() const { + return const_iterator(m_storage.entries.size(), &m_storage.entries); + } + iterator begin() { + return iterator(0, &m_storage.entries); + } + iterator end() { + return iterator(m_storage.entries.size(), &m_storage.entries); + } + + bool isValid() const override { + return m_isValid; + } + + podio::CollectionWriteBuffers getBuffers() override { + return m_storage.getCollectionBuffers(m_isSubsetColl); + } + + std::string getTypeName() const override { + return podio::detail::associationCollTypeName(); + } + + std::string getValueTypeName() const override { + return podio::detail::associationSIOName(); + } + + std::string getDataTypeName() const override { + return "float"; + } + + bool isSubsetCollection() const override { + return m_isSubsetColl; + } + + void setSubsetCollection(bool setSubset = true) override { + if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { + throw std::logic_error("Cannot change the character of a collection that already contains elements"); + } + + if (setSubset) { + m_storage.makeSubsetCollection(); + } + m_isSubsetColl = setSubset; + } + + void setID(unsigned id) override { + m_collectionID = id; + } + + unsigned getID() const override { + return m_collectionID; + } + + void prepareForWrite() const override { + // TODO: Replace this double locking pattern with an atomic and only one + // lock. Problem: std::atomic is not default movable + { + std::lock_guard lock{*m_storageMtx}; + // If the collection has been prepared already there is nothing to do + if (m_isPrepared) { + return; + } + } + + std::lock_guard lock{*m_storageMtx}; + // by the time we acquire the lock another thread might have already + // succeeded, so we need to check again now + if (m_isPrepared) { + return; + } + m_storage.prepareForWrite(m_isSubsetColl); + m_isPrepared = true; + } + + void prepareAfterRead() override { + // No need to go through this again if we have already done it + if (m_isPrepared) { + return; + } + + if (!m_isSubsetColl) { + // Subset collections do not store any data that would require post-processing + m_storage.prepareAfterRead(m_collectionID); + } + // Preparing a collection doesn't affect the underlying I/O buffers, so this + // collection is still prepared + m_isPrepared = true; + } + + bool setReferences(const ICollectionProvider* collectionProvider) override { + return m_storage.setReferences(collectionProvider, m_isSubsetColl); + } + + static constexpr SchemaVersionT schemaVersion = 1; + + SchemaVersionT getSchemaVersion() const override { + return schemaVersion; + } + + size_t getDatamodelRegistryIndex() const override { + return podio::DatamodelRegistry::NoDefinitionNecessary; + } + +private: + // For setReferences, we need to give our own CollectionData access to our + // private entries. Otherwise we would need to expose a public member function + // that gives access to the Obj* which is definitely not what we want + friend CollectionDataT; + + bool m_isValid{false}; + mutable bool m_isPrepared{false}; + bool m_isSubsetColl{false}; + int m_collectionID{0}; + mutable std::unique_ptr m_storageMtx{std::make_unique()}; + mutable CollectionDataT m_storage{}; +}; + +template +std::ostream& operator<<(std::ostream& o, const AssociationCollection& v) { + const auto old_flags = o.flags(); + o << " id: weight:" << '\n'; + for (const auto&& el : v) { + o << std::scientific << std::showpos << std::setw(12) << el.id() << " " << std::setw(12) << " " << el.getWeight() + << '\n'; + + o << " from : "; + o << el.getFrom().id() << std::endl; + o << " to : "; + o << el.getTo().id() << std::endl; + } + + o.flags(old_flags); + return o; +} + +#ifdef PODIO_JSON_OUTPUT +template +void to_json(nlohmann::json& j, const AssociationCollection& collection) { + j = nlohmann::json::array(); + for (auto&& elem : collection) { + j.emplace_back(elem); + } +} +#endif + +namespace detail { + template + podio::CollectionReadBuffers createAssociationBuffers(bool subsetColl) { + auto readBuffers = podio::CollectionReadBuffers{}; + readBuffers.data = subsetColl ? nullptr : new AssociationDataContainer(); + + // Either it is a subset collection or we have two relations + const auto nRefs = subsetColl ? 1 : 2; + readBuffers.references = new podio::CollRefCollection(nRefs); + for (auto& ref : *readBuffers.references) { + // Make sure to place usable buffer pointers here + ref = std::make_unique>(); + } + + readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) { + AssociationCollectionData data(buffers, isSubsetColl); + return std::make_unique>(std::move(data), isSubsetColl); + }; + + readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { + if (buffers.data) { + buffers.data = podio::CollectionWriteBuffers::asVector(buffers.data); + } + }; + + return readBuffers; + } + + template + bool registerAssociationCollection(const std::string& assocTypeName) { + const static auto reg = [&assocTypeName]() { + auto& factory = CollectionBufferFactory::mutInstance(); + factory.registerCreationFunc(assocTypeName, AssociationCollection::schemaVersion, + createAssociationBuffers); + return true; + }(); + return reg; + } +} // namespace detail + +} // namespace podio + +#endif // PODIO_ASSOCIATIONCOLLECTIONIMPL_H diff --git a/include/podio/AssociationSIOBlock.h b/include/podio/AssociationSIOBlock.h index a59d3e130..6aa0687aa 100644 --- a/include/podio/AssociationSIOBlock.h +++ b/include/podio/AssociationSIOBlock.h @@ -11,7 +11,6 @@ #include #include -#include #include namespace podio { diff --git a/tests/read_test.h b/tests/read_test.h index 05727400b..19d0077ef 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -26,15 +26,10 @@ #include #include -// using TestAssocCollection = podio::AssociationCollection; -PODIO_DECLARE_ASSOCIATION(TestAssocCollection, ExampleMC, ex42::ExampleWithARelation) -PODIO_DECLARE_ASSOCIATION(TestAssocCollection2, ExampleMC, ExampleHit) - -namespace ex42 { -PODIO_DECLARE_ASSOCIATION(AssocInNamespace, ExampleWithARelation, ExampleMC) -} - -using A = ex42::AssocInNamespace; +// Define an association that is used for the I/O tests +using TestAssocCollection = podio::AssociationCollection; +// Make sure the I/O parts are dynamically registered +PODIO_DECLARE_ASSOCIATION(ExampleMC, ex42::ExampleWithARelation) template bool check_fixed_width_value(FixedWidthT actual, FixedWidthT expected, const std::string& type) { diff --git a/tests/sio_io/read_frame_legacy_sio.cpp b/tests/sio_io/read_frame_legacy_sio.cpp index 0bf0c98bc..5dbc123d3 100644 --- a/tests/sio_io/read_frame_legacy_sio.cpp +++ b/tests/sio_io/read_frame_legacy_sio.cpp @@ -2,12 +2,9 @@ #include "podio/Frame.h" #include "podio/SIOLegacyReader.h" -#include "podio/AssociationSIOBlock.h" #include -const static auto foo = podio::AssociationSIOBlock{}; - int main(int argc, char* argv[]) { if (argc != 2) { std::cerr << "usage: read_frame_legacy_sio inputfile" << std::endl; diff --git a/tests/sio_io/write_frame_sio.cpp b/tests/sio_io/write_frame_sio.cpp index 6674f7073..21cd740ca 100644 --- a/tests/sio_io/write_frame_sio.cpp +++ b/tests/sio_io/write_frame_sio.cpp @@ -1,12 +1,7 @@ -#include "datamodel/ExampleWithARelation.h" #include "write_frame.h" #include "podio/SIOWriter.h" -#include "podio/AssociationSIOBlock.h" - -const static auto foo = podio::AssociationSIOBlock{}; - int main(int, char**) { write_frames("example_frame.sio"); return 0; diff --git a/tests/write_frame.h b/tests/write_frame.h index 47c30cd13..1b09daf54 100644 --- a/tests/write_frame.h +++ b/tests/write_frame.h @@ -28,9 +28,9 @@ #include // Define an association that is used for the I/O tests -// using TestAssocCollection = podio::AssociationCollection; - -PODIO_DECLARE_ASSOCIATION(TestAssocCollection, ExampleMC, ex42::ExampleWithARelation) +using TestAssocCollection = podio::AssociationCollection; +// Make sure the I/O parts are dynamically registered +PODIO_DECLARE_ASSOCIATION(ExampleMC, ex42::ExampleWithARelation) auto createMCCollection() { auto mcps = ExampleMCCollection(); From 2d3277e0e93552a2ecdccd170515fc07731fa154 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 15:51:55 +0100 Subject: [PATCH 26/79] Make Association I/O work with python --- tests/CMakeLists.txt | 2 ++ tests/DatamodelAssociations.cc | 6 ++++++ 2 files changed, 8 insertions(+) create mode 100644 tests/DatamodelAssociations.cc diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1c0345447..6142b337c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -19,6 +19,7 @@ PODIO_GENERATE_DATAMODEL(datamodel datalayout.yaml headers sources # Use the cmake building blocks to add the different parts (conditionally) PODIO_ADD_DATAMODEL_CORE_LIB(TestDataModel "${headers}" "${sources}") +target_sources(TestDataModel PRIVATE DatamodelAssociations.cc) find_package(nlohmann_json 3.10) if (nlohmann_json_FOUND) message(STATUS "Found compatible version of JSON library, will add JSON support to test datamodel") @@ -28,6 +29,7 @@ endif() PODIO_ADD_ROOT_IO_DICT(TestDataModelDict TestDataModel "${headers}" src/selection.xml) PODIO_ADD_SIO_IO_BLOCKS(TestDataModel "${headers}" "${sources}") +target_sources(TestDataModelSioBlocks PRIVATE DatamodelAssociations.cc) # Build the extension data model and link it against the upstream model PODIO_GENERATE_DATAMODEL(extension_model datalayout_extension.yaml ext_headers ext_sources diff --git a/tests/DatamodelAssociations.cc b/tests/DatamodelAssociations.cc new file mode 100644 index 000000000..6a1c025e9 --- /dev/null +++ b/tests/DatamodelAssociations.cc @@ -0,0 +1,6 @@ +#include "podio/AssociationCollection.h" + +#include "datamodel/ExampleMCCollection.h" +#include "datamodel/ExampleWithARelationCollection.h" + +PODIO_DECLARE_ASSOCIATION(ExampleMC, ex42::ExampleWithARelation) From c6d90efb7a4ba01c6c549f7ea06a02bc7294c9ca Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 16:02:06 +0100 Subject: [PATCH 27/79] Avoid unnecessary string copies, add documentation --- include/podio/detail/AssociationFwd.h | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/include/podio/detail/AssociationFwd.h b/include/podio/detail/AssociationFwd.h index 56733cec1..a1291dc07 100644 --- a/include/podio/detail/AssociationFwd.h +++ b/include/podio/detail/AssociationFwd.h @@ -36,16 +36,33 @@ namespace detail { template using GetCollT = typename GetCollType::type; + /** + * Get the collection type name for an AssociationCollection + * + * @tparam FromT the From type of the association + * @tparam ToT the To type of the association + * @returns a type string that is a valid c++ template instantiation + */ template - std::string associationCollTypeName() { + inline const std::string& associationCollTypeName() { const static std::string typeName = std::string("podio::AssociationCollection<") + FromT::TypeName + "," + ToT::TypeName + ">"; return typeName; } + /** + * Get an SIO friendly type name for an AssociationCollection (necessary for + * registration in the SIOBlockFactory) + * + * @tparam FromT the From type of the association + * @tparam ToT the To type of + * the association + * @returns a string that uniquely identifies this combination + * of From and To types + */ template - inline std::string associationSIOName() { - auto n = std::string("ASSOCIATION_FROM_") + FromT::TypeName + "_TO_" + ToT::TypeName; + inline const std::string& associationSIOName() { + static auto n = std::string("ASSOCIATION_FROM_") + FromT::TypeName + "_TO_" + ToT::TypeName; std::replace(n.begin(), n.end(), ':', '_'); return n; } From 1b7ab3ed1e95f4647df20f2ec5165008fe209fc7 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 17:38:34 +0100 Subject: [PATCH 28/79] Add templated get/set and structured bindings to Associations --- include/podio/Association.h | 61 +++++++++++++++++++++++++++ include/podio/detail/AssociationFwd.h | 26 ++++++++++++ tests/unittests/associations.cpp | 32 ++++++++++++++ 3 files changed, 119 insertions(+) diff --git a/include/podio/Association.h b/include/podio/Association.h index 52ccfa8f4..dc4595373 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -128,6 +128,67 @@ class AssociationT { m_obj->m_to = new detail::GetDefT(value); } + /** + * Templated version for getting an element of the association by type. Only + * available for Associations where FromT and ToT are **not the same type**, + * and if the requested type is actually part of the Association. It is only + * possible to get the immutable types from this. Will result in a compilation + * error if any of these conditions is not met. + * + * @tparam T the desired type + * @returns T the element of the Association + * */ + template && detail::isFromOrToT>> + T get() const { + if constexpr (std::is_same_v) { + return getFrom(); + } else { + return getTo(); + } + } + + /** + * Tuple like index based access to the elements of the Association. Returns + * only immutable types of the associations. This method enables structured + * bindings for Associations. + * + * @tparam Index an index (smaller than 3) to access an element of the Association + * @returns Depending on the value of Index: + * - 0: The From element of the Association + * - 1: The To element of the Association + * - 2: The weight of the Association + */ + template > + auto get() const { + if constexpr (Index == 0) { + return getFrom(); + } else if constexpr (Index == 1) { + return getTo(); + } else { + return getWeight(); + } + } + + /** + * Templated version for setting an element of the association by type. Only + * available for Associations where FromT and ToT are **not the same type**, + * and if the requested type is actually part of the Association. Will result + * in a compilation error if any of these conditions is not met. + * + * @tparam T type of value (**infered!**) + * @param value the element to set for this association. + */ + template < + typename T, bool Mut = Mutable, + typename = std::enable_if_t && detail::isMutableFromOrToT>> + void set(T value) { + if constexpr (std::is_same_v) { + setFrom(std::move(value)); + } else { + setTo(std::move(value)); + } + } + /// check whether the object is actually available bool isAvailable() const { return m_obj; diff --git a/include/podio/detail/AssociationFwd.h b/include/podio/detail/AssociationFwd.h index a1291dc07..f52ae6bda 100644 --- a/include/podio/detail/AssociationFwd.h +++ b/include/podio/detail/AssociationFwd.h @@ -1,6 +1,8 @@ #ifndef PODIO_DETAIL_ASSOCIATIONFWD_H #define PODIO_DETAIL_ASSOCIATIONFWD_H +#include "podio/utilities/TypeHelpers.h" + #include #include #include @@ -36,6 +38,20 @@ namespace detail { template using GetCollT = typename GetCollType::type; + /** + * Variable template to for determining whether T is either FromT or ToT. + * Mainly defined for convenience + */ + template + static constexpr bool isFromOrToT = detail::isInTuple>; + + /** + * Variable template to for determining whether T is either FromT or ToT or + * any of their mutable versions. + */ + template + static constexpr bool isMutableFromOrToT = detail::isInTuple, GetMutT>>; + /** * Get the collection type name for an AssociationCollection * @@ -104,4 +120,14 @@ using AssociationMutableCollectionIterator = AssociationCollectionIteratorT +struct tuple_size> : std::integral_constant {}; + +/// Specialization for enabling structure bindings for Associations +template +struct tuple_element> : tuple_element> {}; +} // namespace std + #endif // PODIO_DETAIL_ASSOCIATIONFWD_H diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index 033754f73..cf8c6a256 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -131,6 +131,38 @@ TEST_CASE("Association basics", "[associations]") { } } +TEST_CASE("Associations templated accessors", "[associations]") { + ExampleHit hit; + ExampleCluster cluster; + + TestMutA assoc; + assoc.set(hit); + assoc.set(cluster); + assoc.setWeight(1.0); + + SECTION("Mutable Association") { + REQUIRE(hit == assoc.get()); + REQUIRE(cluster == assoc.get()); + + const auto [h, c, w] = assoc; + REQUIRE(h == hit); + REQUIRE(c == cluster); + REQUIRE(w == 1.0); + } + + SECTION("Immutable association") { + TestA a{assoc}; + + REQUIRE(hit == a.get()); + REQUIRE(cluster == a.get()); + + const auto [h, c, w] = a; + REQUIRE(h == hit); + REQUIRE(c == cluster); + REQUIRE(w == 1.0); + } +} + TEST_CASE("AssociationCollection basics", "[associations]") { auto coll = TestAColl(); From ad77dcd7366c31aa399e92391f4f4b92c1b1a09f Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 18:13:33 +0100 Subject: [PATCH 29/79] Introduce typedefs for consistency with other types --- include/podio/Association.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/podio/Association.h b/include/podio/Association.h index dc4595373..ca7f84602 100644 --- a/include/podio/Association.h +++ b/include/podio/Association.h @@ -33,6 +33,10 @@ class AssociationT { friend AssociationCollectionIteratorT; public: + using MutT = podio::MutableAssociation; + using DefT = podio::Association; + using CollT = podio::AssociationCollection; + /// Constructor AssociationT() : m_obj(new AssociationObjT()) { m_obj->acquire(); From dd29b5613008970cf6fbc4083638e2ecba507e03 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 18:21:28 +0100 Subject: [PATCH 30/79] Move all but the public one into detail directory --- include/podio/AssociationCollection.h | 6 +++--- include/podio/{ => detail}/Association.h | 6 +++--- .../podio/{ => detail}/AssociationCollectionImpl.h | 11 +++++------ .../{ => detail}/AssociationCollectionIterator.h | 6 +++--- include/podio/{ => detail}/AssociationSIOBlock.h | 10 +++++----- 5 files changed, 19 insertions(+), 20 deletions(-) rename include/podio/{ => detail}/Association.h (98%) rename include/podio/{ => detail}/AssociationCollectionImpl.h (97%) rename include/podio/{ => detail}/AssociationCollectionIterator.h (87%) rename include/podio/{ => detail}/AssociationSIOBlock.h (93%) diff --git a/include/podio/AssociationCollection.h b/include/podio/AssociationCollection.h index c2af89d22..b78618829 100644 --- a/include/podio/AssociationCollection.h +++ b/include/podio/AssociationCollection.h @@ -1,6 +1,6 @@ #ifndef PODIO_ASSOCIATIONCOLLECTION_H #define PODIO_ASSOCIATIONCOLLECTION_H -#include "podio/AssociationCollectionImpl.h" +#include "podio/detail/AssociationCollectionImpl.h" // Preprocessor helper macros for concatenating tokens at preprocessing times // Necessary because we use __COUNTER__ below for unique names of static @@ -12,8 +12,8 @@ #define PODIO_ENABLE_SIO 0 #endif -#if PODIO_ENABLE_SIO && __has_include("podio/AssociationSIOBlock.h") - #include "podio/AssociationSIOBlock.h" +#if PODIO_ENABLE_SIO && __has_include("podio/detail/AssociationSIOBlock.h") + #include "podio/detail/AssociationSIOBlock.h" /** * Main macro for declaring associations. Takes care of the following things: * - Registering the necessary buffer creation functionality with the diff --git a/include/podio/Association.h b/include/podio/detail/Association.h similarity index 98% rename from include/podio/Association.h rename to include/podio/detail/Association.h index ca7f84602..654e2a0ae 100644 --- a/include/podio/Association.h +++ b/include/podio/detail/Association.h @@ -1,5 +1,5 @@ -#ifndef PODIO_ASSOCIATION_H -#define PODIO_ASSOCIATION_H +#ifndef PODIO_DETAIL_ASSOCIATION_H +#define PODIO_DETAIL_ASSOCIATION_H #include "podio/detail/AssociationFwd.h" #include "podio/detail/AssociationObj.h" @@ -255,4 +255,4 @@ void to_json(nlohmann::json& j, const Association& assoc) { } // namespace podio -#endif // PODIO_ASSOCIATION_H +#endif // PODIO_DETAIL_ASSOCIATION_H diff --git a/include/podio/AssociationCollectionImpl.h b/include/podio/detail/AssociationCollectionImpl.h similarity index 97% rename from include/podio/AssociationCollectionImpl.h rename to include/podio/detail/AssociationCollectionImpl.h index 37d74df62..e547a3685 100644 --- a/include/podio/AssociationCollectionImpl.h +++ b/include/podio/detail/AssociationCollectionImpl.h @@ -1,13 +1,12 @@ -#ifndef PODIO_ASSOCIATIONCOLLECTIONIMPL_H -#define PODIO_ASSOCIATIONCOLLECTIONIMPL_H +#ifndef PODIO_DETAIL_ASSOCIATIONCOLLECTIONIMPL_H +#define PODIO_DETAIL_ASSOCIATIONCOLLECTIONIMPL_H -#include "podio/CollectionBufferFactory.h" +#include "podio/detail/Association.h" #include "podio/detail/AssociationCollectionData.h" +#include "podio/detail/AssociationCollectionIterator.h" #include "podio/detail/AssociationFwd.h" #include "podio/detail/AssociationObj.h" -#include "podio/Association.h" -#include "podio/AssociationCollectionIterator.h" #include "podio/CollectionBase.h" #include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" @@ -326,4 +325,4 @@ namespace detail { } // namespace podio -#endif // PODIO_ASSOCIATIONCOLLECTIONIMPL_H +#endif // PODIO_DETAIL_ASSOCIATIONCOLLECTIONIMPL_H diff --git a/include/podio/AssociationCollectionIterator.h b/include/podio/detail/AssociationCollectionIterator.h similarity index 87% rename from include/podio/AssociationCollectionIterator.h rename to include/podio/detail/AssociationCollectionIterator.h index 8608f94cc..3ea23a5ce 100644 --- a/include/podio/AssociationCollectionIterator.h +++ b/include/podio/detail/AssociationCollectionIterator.h @@ -1,5 +1,5 @@ -#ifndef PODIO_ASSOCIATIONCOLLECTIONITERATOR_H -#define PODIO_ASSOCIATIONCOLLECTIONITERATOR_H +#ifndef PODIO_DETAIL_ASSOCIATIONCOLLECTIONITERATOR_H +#define PODIO_DETAIL_ASSOCIATIONCOLLECTIONITERATOR_H #include "podio/detail/AssociationCollectionData.h" #include "podio/detail/AssociationFwd.h" @@ -43,4 +43,4 @@ class AssociationCollectionIteratorT { }; } // namespace podio -#endif // PODIO_ASSOCIATIONCOLLECTIONITERATOR_H +#endif // PODIO_DETAIL_ASSOCIATIONCOLLECTIONITERATOR_H diff --git a/include/podio/AssociationSIOBlock.h b/include/podio/detail/AssociationSIOBlock.h similarity index 93% rename from include/podio/AssociationSIOBlock.h rename to include/podio/detail/AssociationSIOBlock.h index 6aa0687aa..cccf95b17 100644 --- a/include/podio/AssociationSIOBlock.h +++ b/include/podio/detail/AssociationSIOBlock.h @@ -1,9 +1,9 @@ -#ifndef PODIO_ASSOCIATIONSIOBLOCK_H -#define PODIO_ASSOCIATIONSIOBLOCK_H +#ifndef PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H +#define PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H -#include "podio/AssociationCollection.h" -#include "podio/CollectionBufferFactory.h" +#include "podio/detail/AssociationCollectionImpl.h" +#include "podio/CollectionBufferFactory.h" #include "podio/CollectionBuffers.h" #include "podio/SIOBlock.h" @@ -78,4 +78,4 @@ class AssociationSIOBlock : public podio::SIOBlock { } // namespace podio -#endif // PODIO_ASSOCIATIONSIOBLOCK_H +#endif // PODIO_DETAIL_ASSOCIATIONSIOBLOCK_H From 80ef0e8c99c44dd3aca2fb5dcfa4dff1f8d30b44 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 19:15:14 +0100 Subject: [PATCH 31/79] Fix clang-tidy warning --- tests/unittests/associations.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unittests/associations.cpp b/tests/unittests/associations.cpp index cf8c6a256..b751a5a79 100644 --- a/tests/unittests/associations.cpp +++ b/tests/unittests/associations.cpp @@ -144,7 +144,7 @@ TEST_CASE("Associations templated accessors", "[associations]") { REQUIRE(hit == assoc.get()); REQUIRE(cluster == assoc.get()); - const auto [h, c, w] = assoc; + const auto& [h, c, w] = assoc; REQUIRE(h == hit); REQUIRE(c == cluster); REQUIRE(w == 1.0); @@ -156,7 +156,7 @@ TEST_CASE("Associations templated accessors", "[associations]") { REQUIRE(hit == a.get()); REQUIRE(cluster == a.get()); - const auto [h, c, w] = a; + const auto& [h, c, w] = a; REQUIRE(h == hit); REQUIRE(c == cluster); REQUIRE(w == 1.0); From cd4de43f170ff2c546b3bcfbe35b7757a91fe6e0 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 19:30:25 +0100 Subject: [PATCH 32/79] Default initialize weight to 1 --- include/podio/detail/AssociationObj.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/podio/detail/AssociationObj.h b/include/podio/detail/AssociationObj.h index 000d2aa35..6f7dadcbd 100644 --- a/include/podio/detail/AssociationObj.h +++ b/include/podio/detail/AssociationObj.h @@ -51,7 +51,7 @@ class AssociationObj : public podio::ObjBase { } public: - float weight{}; + float weight{1.0f}; FromT* m_from{nullptr}; ToT* m_to{nullptr}; }; From 2d6b499c2d34020b34dacf99bb31cc04bc208232 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 21:11:26 +0100 Subject: [PATCH 33/79] Simplify and fortify SFINAE logic for mutable associations --- include/podio/detail/Association.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/include/podio/detail/Association.h b/include/podio/detail/Association.h index 654e2a0ae..bf77944b9 100644 --- a/include/podio/detail/Association.h +++ b/include/podio/detail/Association.h @@ -69,8 +69,8 @@ class AssociationT { } /// Implicit conversion of mutable to immutable associations - template && std::is_same_v>> + template && std::is_same_v>> operator AssociationT() const { return AssociationT(m_obj); } @@ -95,7 +95,7 @@ class AssociationT { } /// Set the weight of the association - template > + template > void setWeight(float value) { m_obj->weight = value; } @@ -109,8 +109,7 @@ class AssociationT { } /// Set the related-from object - template , FromT>>> + template , FromT>>> void setFrom(FromU value) { delete m_obj->m_from; m_obj->m_from = new detail::GetDefT(value); @@ -125,8 +124,7 @@ class AssociationT { } /// Set the related-to object - template , ToT>>> + template , ToT>>> void setTo(ToU value) { delete m_obj->m_to; m_obj->m_to = new detail::GetDefT(value); @@ -183,8 +181,8 @@ class AssociationT { * @param value the element to set for this association. */ template < - typename T, bool Mut = Mutable, - typename = std::enable_if_t && detail::isMutableFromOrToT>> + typename T, + typename = std::enable_if_t && detail::isMutableFromOrToT>> void set(T value) { if constexpr (std::is_same_v) { setFrom(std::move(value)); @@ -229,7 +227,7 @@ class AssociationT { } private: - AssociationObj* m_obj{nullptr}; + AssociationObjT* m_obj{nullptr}; }; // namespace podio template From 9b62ef64213682e2e0332fa0f7391891ef17f508 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 24 Mar 2023 21:20:01 +0100 Subject: [PATCH 34/79] Add markdown documentation describing usage and some implemenation details --- doc/associations.md | 233 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 doc/associations.md diff --git a/doc/associations.md b/doc/associations.md new file mode 100644 index 000000000..68717a912 --- /dev/null +++ b/doc/associations.md @@ -0,0 +1,233 @@ +# Associationg unrelated objects with each other +Sometimes it is necessary to build relations between objects whose datatypes are +not related via a `OneToOneRelation` or a `OneToManyRelation`. These *external +relations* are called *Associations* in podio, and they are implemented as a +templated version of the code that would be generated by the following yaml +snippet (in this case between generic `FromT` and `ToT` datatypes): + +```yaml +Association: + Description: "A weighted association between a FromT and a ToT" + Author: "P. O. Dio" + Members: + - float weight // the weight of the association + OneToOneRelations: + - FromT from // reference to the FromT + - ToT to // reference to the ToT +``` + +## `Association` basics +`Association`s are implemented as templated classes forming a similar structure +as other podio generated classes, with several layers of which users only ever +interact with the *User layer*. This layer has the following basic classes +```cpp +/// The collection class that forms the basis of I/O and also is the main entry point +template +class AssociationCollection; + +/// The default (immutable) class that one gets after reading a collection +template +class Association; + +/// The mutable class for creating associations before writing them +template +class MutableAssociation; +``` + +Although the names of the template parameters, `FromT` and `ToT` imply a +direction of the association, from a technical point of view nothing actually +enforces this direction, unless `FromT` and `ToT` are both of the same type. +Hence, associations can effectively be treated as bi-directional, and one +combination of `FromT` and `ToT` should be enough for all use cases (see also +the [usage section](#how-to-use-associations)). + +For a more detailed explanation of the internals and the actual implementation +see [the implementation details](#implementation-details). + +## How to use `Association`s +Using `Association`s is quite simple. In line with other datatypes that are +generated by podio all the functionality can be gained by including the +corresponding `Collection` header. After that it is generally recommended to +introduce a type alias for easier usage. **As a general rule `Associations` need +to be declared with the default (immutable) types.** Trying to instantiate them +with `Mutable` types will result in a compilation error. + +```cpp +#include "podio/AssociationCollection.h" + +#include "edm4hep/MCParticleCollection.h" +#include "edm4hep/ReconstructedParticleCollection.h" + +// declare a new assocation type +using MCRecoParticleAssociationCollection = podio::AssociationCollection; +``` + +This can now be used exactly as any other podio generated collection, i.e. +```cpp +edm4hep::MCParticle mcParticle{}; +edm4hep::ReconstructedParticle recoParticle{}; + +auto mcRecoAssocs = MCRecoParticleAssociationCollection{}; +auto assoc = mcRecoAssocs.create(); // create an association; +assoc.setFrom(mcParticle); +assoc.setTo(recoParticle); +assoc.setWeight(1.0); // This is also the default value! +``` + +and similar for getting the associated objects +```cpp +auto mcP = assoc.getFrom(); +auto recoP = assoc.getTo(); +auto weight = assoc.getWeight(); +``` + +In the above examples the `From` and `To` in the method names imply a direction, +but it is also possible to use a templated `get` and `set` method to retrieve +the associated objects via their type: + +```cpp +assoc.set(mcParticle); +assoc.set(recoParticle); + +auto mcP = assoc.get(); +auto recoP = assoc.get(); +auto weight = assoc.getWeight(); +``` + +It is also possible to access the elments of an association via an index based +`get` (similar to `std::tuple`). In this case `0` corresponds to `getFrom`, `1` +corresponds to `getTo` and `2` corresponds to the weight. The main purpose of +this feature is to enable structured bindings: + +```cpp +const auto& [mcP, recoP, weight] = assoc; +``` + +The above three examples are three equivalent ways of retrieving the same things +from an `Association`. **The templated `get` and `set` methods are only availble +if `FromT` and `ToT` are not the same type** and will lead to a compilation +error otherwise. + +### Enabling I/O capabilities for `Association`s + +`Association`s do not have I/O support out of the box. This has to be enabled via +the `PODIO_DECLARE_ASSOCIATION` macro (defined in the `AssociationCollection.h` +header). If you simply want to be able to read / write `Association`s in a +standalone executable, it is enough to use this macro somewhere in the +executable, e.g. to enable I/O capabilities for the `MCRecoParticleAssociation`s +used above this would look like: + +```cpp +PODIO_DECLARE_ASSOCIATION(edm4hep::MCParticle, edm4hep::ReconstructedParticle) +``` + +The macro will also enable SIO support if the `PODIO_ENABLE_SIO=1` is passed to +the compiler. This is done by default when linking against the +`podio::podioSioIO` library in CMake. + +For enabling I/O support for shared datamodel libraries, it is necessary to have +all the necessary combinations of types declared via `PODIO_DECLARE_ASSOCIATION` +and have that compiled into the library. This is necessary if you want to use +the python bindings, since they rely on dynamically loading the datamodel +libraries. + +## Implementation details + +In order to give a slightly easier entry to the details of the implementation +and also to make it easier to find where things in the generated documentation, +we give a brief description of the main ideas and design choices here. With +those it should be possible to dive deeper if necessary or to understand the +template structure that is visible in the documentation, but should be fairly +invisible in usage. We will focus mainly on the user facing classes, as those +deal with the most complexity, the underlying layers are more or less what could +be obtained by generating them via the yaml snippet above and sprinkling some +`` templates where necessary. + +### File structure + +The user facing `"podio/AssociationCollection.h"` header essentially just +defines the `PODIO_DECLARE_ASSOCIATION` macro (depending on whether SIO support +is desired and possible or not). All the actual implementation is done in the +following files: + +- [`"podio/detail/AssociationCollectionImpl.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/AssociationCollectionImpl.h): + for the collection functionality +- [`"podio/detail/Association.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/Association.h): + for the functionality of single association +- [`"podio/detail/AssociationCollectionIterator.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/AssociationCollectionIterator.h): + for the collection iterator functionality +- [`"podio/detail/AssociationObj.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/AssociationObj.h): + for the object layer functionality + - [`"podio/detail/AssociationCollectionData.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/AssociationCollectionData.h): + for the collection data functionality +- [`"podio/detail/AssociationFwd.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/AssociationFwd.h): + for some type helper functionality and some forward declarations that are used + throughout the othe headers +- [`"podio/detail/AssociationSIOBlock.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/AssociationSIOBlock.h): + for defining the SIOBlocks that are necessary to use SIO + +As is visible from this structure, we did not introduce an `AssociationData` +class, since that would effectively just be a `float` wrapped inside a `struct`. + +### Default and `Mutable` `Association` classes + +A quick look into the `AssociationFwd.h` header will reveal that the default and +`Mutable` `Association` classes are in fact just partial specialization of the +`AssociationT` class that takes a `bool Mutable` as third template argument. The +same approach is also followed by the `AssocationCollectionIterator`s: + +```cpp +template +class AssociationT; + +template +using Association = AssociationT; + +template +using MutableAssociation = AssociationT; +``` + +All podio generated datatypes have the following three public typedefs +- `DefT` yields the default type +- `MutT` yields the mutable type +- `CollT` yields the collection type + +There are corresponding template helpers to retrieve these types in +`AssocationFwd.h`. Note that these are not +[*SFINAE*](https://en.cppreference.com/w/cpp/language/sfinae) friendly. However, +since they are only used in contexts internally in non-SFINAE contexts, this +doesn't really matter. + +Throught the implementation it is assumed that `FromT` and `ToT` always are the +default types. This is ensured through `static_assert`s, resp. through usage of +the aforementioned template helpers at the possible entry points. With this in +mind, effectively all mutating operations on `Association`s are defined using +SFINAE using the following template structure (taking here `setFrom` as an +example) + +```cpp +template , FromT>>> +void setFrom(FromU value); +``` + +This is a SFINAE friendly way to ensure that this definition is only viable if +the following conditions are met +- The object this method is called on has to be `Mutable`. (first part inside the `std::enable_if`) +- The passed in `value` is either a `Mutable` or default class of type `FromT`. (second part inside the `std::enable_if`) + +In some cases the template signature looks like this + +```cpp +template> +void setWeight(float weight); +``` + +The reason to have a defaulted `bool` template parameter here is the same as the +one for having a `typename FromU` template parameter above: SFINAE only works +with deduced types. Using `Mut && Mutable` in the `std::enable_if` makes sure +that users cannot bypass the immutability by specifying a template parameter +themselves. From 3f6ccc95579d4973e5a9232c17cb36606db2c8fb Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 30 Jan 2024 14:51:13 +0100 Subject: [PATCH 35/79] Make things compile again after PR revival --- include/podio/detail/Association.h | 72 +++++++++---------- .../podio/detail/AssociationCollectionData.h | 12 ++-- .../podio/detail/AssociationCollectionImpl.h | 54 +++++++++----- .../detail/AssociationCollectionIterator.h | 5 +- include/podio/detail/AssociationFwd.h | 34 ++------- include/podio/detail/AssociationObj.h | 18 ++--- include/podio/utilities/TypeHelpers.h | 12 ++++ tests/read_test.h | 3 +- 8 files changed, 105 insertions(+), 105 deletions(-) diff --git a/include/podio/detail/Association.h b/include/podio/detail/Association.h index bf77944b9..98cf67020 100644 --- a/include/podio/detail/Association.h +++ b/include/podio/detail/Association.h @@ -3,6 +3,8 @@ #include "podio/detail/AssociationFwd.h" #include "podio/detail/AssociationObj.h" +#include "podio/utilities/MaybeSharedPtr.h" +#include "podio/utilities/TypeHelpers.h" #ifdef PODIO_JSON_OUTPUT #include "nlohmann/json.hpp" @@ -23,44 +25,32 @@ class AssociationT { // The typedefs in AssociationFwd.h should make sure that at this point // Mutable classes are stripped, i.e. the user should never be able to trigger // these! - static_assert(std::is_same_v, FromT>, + static_assert(std::is_same_v, FromT>, "Associations need to be instantiated with the default types!"); - static_assert(std::is_same_v, ToT>, + static_assert(std::is_same_v, ToT>, "Associations need to be instantiated with the default types!"); using AssociationObjT = AssociationObj; friend AssociationCollection; friend AssociationCollectionIteratorT; + friend AssociationT; public: - using MutT = podio::MutableAssociation; - using DefT = podio::Association; - using CollT = podio::AssociationCollection; + using mutable_type = podio::MutableAssociation; + using value_type = podio::Association; + using collection_type = podio::AssociationCollection; /// Constructor - AssociationT() : m_obj(new AssociationObjT()) { - m_obj->acquire(); + AssociationT() : m_obj(new AssociationObjT(), podio::utils::MarkOwned) { } /// Constructor with weight - AssociationT(float weight) : m_obj(new AssociationObjT()) { - m_obj->acquire(); + AssociationT(float weight) : m_obj(new AssociationObjT(), podio::utils::MarkOwned) { m_obj->weight = weight; } - /// Constructor from existing AssociationObj - AssociationT(AssociationObjT* obj) : m_obj(obj) { - if (m_obj) { - m_obj->acquire(); - } - } - /// Copy constructor - AssociationT(const AssociationT& other) : m_obj(other.m_obj) { - if (m_obj) { - m_obj->acquire(); - } - } + AssociationT(const AssociationT& other) = default; /// Assignment operator AssociationT& operator=(AssociationT other) { @@ -82,13 +72,13 @@ class AssociationT { return {new AssociationObjT(*m_obj)}; } - /// Destructor - ~AssociationT() { - if (m_obj) { - m_obj->release(); // NOLINT(clang-analyzer-cplusplus.NewDelete) issue #174 - } + static Association makeEmpty() { + return {nullptr}; } + /// Destructor + ~AssociationT() = default; + /// Get the weight of the association float getWeight() const { return m_obj->weight; @@ -103,31 +93,33 @@ class AssociationT { /// Access the related-from object FromT getFrom() const { if (!m_obj->m_from) { - return FromT(nullptr); + return FromT::makeEmpty(); } return FromT(*(m_obj->m_from)); } /// Set the related-from object - template , FromT>>> + template , FromT>>> void setFrom(FromU value) { delete m_obj->m_from; - m_obj->m_from = new detail::GetDefT(value); + m_obj->m_from = new detail::GetDefaultHandleType(value); } /// Access the related-to object ToT getTo() const { if (!m_obj->m_to) { - return ToT(nullptr); + return ToT::makeEmpty(); } return ToT(*(m_obj->m_to)); } /// Set the related-to object - template , ToT>>> + template , ToT>>> void setTo(ToU value) { delete m_obj->m_to; - m_obj->m_to = new detail::GetDefT(value); + m_obj->m_to = new detail::GetDefaultHandleType(value); } /** @@ -198,7 +190,7 @@ class AssociationT { /// disconnect from Association instance void unlink() { - m_obj = nullptr; + m_obj = podio::utils::MaybeSharedPtr(nullptr); } /// Get the ObjectID @@ -206,11 +198,11 @@ class AssociationT { if (m_obj) { return m_obj->id; } - return podio::ObjectID{podio::ObjectID::invalid, podio::ObjectID::invalid}; + return podio::ObjectID{}; } - unsigned int id() const { - return getObjectID().collectionID * 10000000 + getObjectID().index; + podio::ObjectID id() const { + return getObjectID(); } bool operator==(const AssociationT& other) const { @@ -227,7 +219,13 @@ class AssociationT { } private: - AssociationObjT* m_obj{nullptr}; + /// Constructor from existing AssociationObj + explicit AssociationT(podio::utils::MaybeSharedPtr obj) : m_obj(std::move(obj)) { + } + AssociationT(AssociationObjT* obj) : m_obj(podio::utils::MaybeSharedPtr(obj)) { + } + + podio::utils::MaybeSharedPtr m_obj{nullptr}; }; // namespace podio template diff --git a/include/podio/detail/AssociationCollectionData.h b/include/podio/detail/AssociationCollectionData.h index ba520d5c3..a6c991e72 100644 --- a/include/podio/detail/AssociationCollectionData.h +++ b/include/podio/detail/AssociationCollectionData.h @@ -42,7 +42,7 @@ class AssociationCollectionData { ~AssociationCollectionData() = default; podio::CollectionWriteBuffers getCollectionBuffers(bool isSubsetColl) { - return {isSubsetColl ? nullptr : (void*)&m_data, &m_refCollections, &m_vecInfo}; + return {isSubsetColl ? nullptr : (void*)&m_data, (void*)m_data.get(), &m_refCollections, &m_vecInfo}; } void clear(bool isSubsetColl) { @@ -104,18 +104,18 @@ class AssociationCollectionData { if (obj->m_from) { m_refCollections[0]->emplace_back(obj->m_from->getObjectID()); } else { - m_refCollections[0]->push_back({podio::ObjectID::invalid, podio::ObjectID::invalid}); + m_refCollections[0]->push_back({podio::ObjectID::invalid, 0}); } if (obj->m_to) { m_refCollections[1]->emplace_back(obj->m_to->getObjectID()); } else { - m_refCollections[1]->push_back({podio::ObjectID::invalid, podio::ObjectID::invalid}); + m_refCollections[1]->push_back({podio::ObjectID::invalid, 0}); } } } - void prepareAfterRead(int collectionID) { + void prepareAfterRead(uint32_t collectionID) { int index = 0; for (const auto data : *m_data) { auto obj = new AssociationObj({index++, collectionID}, data); @@ -148,7 +148,7 @@ class AssociationCollectionData { entries[i]->m_from = nullptr; continue; } - auto* tmp_coll = static_cast*>(coll); + auto* tmp_coll = static_cast*>(coll); entries[i]->m_from = new FromT((*tmp_coll)[id.index]); } else { entries[i]->m_from = nullptr; @@ -163,7 +163,7 @@ class AssociationCollectionData { entries[i]->m_to = nullptr; continue; } - auto* tmp_coll = static_cast*>(coll); + auto* tmp_coll = static_cast*>(coll); entries[i]->m_to = new ToT((*tmp_coll)[id.index]); } else { entries[i]->m_to = nullptr; diff --git a/include/podio/detail/AssociationCollectionImpl.h b/include/podio/detail/AssociationCollectionImpl.h index e547a3685..a57865cc6 100644 --- a/include/podio/detail/AssociationCollectionImpl.h +++ b/include/podio/detail/AssociationCollectionImpl.h @@ -30,9 +30,9 @@ namespace podio { template class AssociationCollection : public podio::CollectionBase { - static_assert(std::is_same_v>, + static_assert(std::is_same_v>, "Associations need to be instantiated with the default types!"); - static_assert(std::is_same_v>, + static_assert(std::is_same_v>, "Associations need to be instantiated with the default types!"); // convenience typedefs @@ -91,7 +91,7 @@ class AssociationCollection : public podio::CollectionBase { return MutableAssocT(m_storage.entries.at(index)); } - void push_back(AssocT object) { + void push_back(MutableAssocT object) { // We have to do different things here depending on whether this is a // subset collection or not. A normal collection cannot collect objects // that are already part of another collection, while a subset collection @@ -101,27 +101,38 @@ class AssociationCollection : public podio::CollectionBase { if (obj->id.index == podio::ObjectID::untracked) { const auto size = m_storage.entries.size(); obj->id = {(int)size, m_collectionID}; - m_storage.entries.push_back(obj); + m_storage.entries.push_back(obj.release()); } else { throw std::invalid_argument("Object already in a collection. Cannot add it to a second collection"); } + } else { - const auto obj = object.m_obj; - if (obj->id.index < 0) { - throw std::invalid_argument( - "Objects can only be stored in a subset collection if they are already elements of a collection"); - } - m_storage.entries.push_back(obj); - // No need to handle any relations here, since this is already done by the - // "owning" collection + push_back(AssocT(object)); } } + void push_back(AssocT object) { + if (!m_isSubsetColl) { + throw std::invalid_argument("Can only add immutable objects to subset collections"); + } + auto obj = object.m_obj; + if (obj->id.index < 0) { + throw std::invalid_argument( + "Object needs to be tracked by another collection in order for it to be storable in a subset collection"); + } + m_storage.entries.push_back(obj.release()); + } + /// Number of elements in the collection size_t size() const override { return m_storage.entries.size(); } + /// Is the collection empty + bool empty() const override { + return m_storage.entries.empty(); + } + void clear() override { m_storage.clear(m_isSubsetColl); m_isPrepared = false; @@ -156,15 +167,15 @@ class AssociationCollection : public podio::CollectionBase { return m_storage.getCollectionBuffers(m_isSubsetColl); } - std::string getTypeName() const override { + const std::string_view getTypeName() const override { return podio::detail::associationCollTypeName(); } - std::string getValueTypeName() const override { + const std::string_view getValueTypeName() const override { return podio::detail::associationSIOName(); } - std::string getDataTypeName() const override { + const std::string_view getDataTypeName() const override { return "float"; } @@ -250,7 +261,7 @@ class AssociationCollection : public podio::CollectionBase { bool m_isValid{false}; mutable bool m_isPrepared{false}; bool m_isSubsetColl{false}; - int m_collectionID{0}; + uint32_t m_collectionID{0}; mutable std::unique_ptr m_storageMtx{std::make_unique()}; mutable CollectionDataT m_storage{}; }; @@ -308,6 +319,17 @@ namespace detail { } }; + readBuffers.deleteBuffers = [](podio::CollectionReadBuffers& buffers) { + if (buffers.data) { + // If we have data then we are not a subset collection and we have + // to clean up all type erased buffers by casting them back to + // something that we can delete + delete static_cast(buffers.data); + } + delete buffers.references; + delete buffers.vectorMembers; + }; + return readBuffers; } diff --git a/include/podio/detail/AssociationCollectionIterator.h b/include/podio/detail/AssociationCollectionIterator.h index 3ea23a5ce..4757848d5 100644 --- a/include/podio/detail/AssociationCollectionIterator.h +++ b/include/podio/detail/AssociationCollectionIterator.h @@ -8,6 +8,7 @@ namespace podio { template class AssociationCollectionIteratorT { using AssocT = AssociationT; + using AssociationObjT = AssociationObj; public: AssociationCollectionIteratorT(size_t index, const AssociationObjPointerContainer* coll) : @@ -22,12 +23,12 @@ class AssociationCollectionIteratorT { } AssocT operator*() { - m_object.m_obj = (*m_collection)[m_index]; + m_object.m_obj = podio::utils::MaybeSharedPtr((*m_collection)[m_index]); return m_object; } AssocT* operator->() { - m_object.m_obj = (*m_collection)[m_index]; + m_object.m_obj = podio::utils::MaybeSharedPtr((*m_collection)[m_index]); return &m_object; } diff --git a/include/podio/detail/AssociationFwd.h b/include/podio/detail/AssociationFwd.h index f52ae6bda..54062da03 100644 --- a/include/podio/detail/AssociationFwd.h +++ b/include/podio/detail/AssociationFwd.h @@ -10,33 +10,6 @@ namespace podio { namespace detail { - template - struct GetMutType { - using type = typename T::MutT; - }; - - template - using GetMutT = typename GetMutType::type; - - template - struct GetDefType { - using type = typename T::DefT; - }; - - template - using GetDefT = typename GetDefType::type; - - /** - * Helper template struct and type-alias to retrieve the collection type from - * a given data type - */ - template - struct GetCollType { - using type = typename T::CollT; - }; - - template - using GetCollT = typename GetCollType::type; /** * Variable template to for determining whether T is either FromT or ToT. @@ -50,7 +23,8 @@ namespace detail { * any of their mutable versions. */ template - static constexpr bool isMutableFromOrToT = detail::isInTuple, GetMutT>>; + static constexpr bool isMutableFromOrToT = + detail::isInTuple, GetMutableHandleType>>; /** * Get the collection type name for an AssociationCollection @@ -98,10 +72,10 @@ template class AssociationT; template -using Association = AssociationT, detail::GetDefT, false>; +using Association = AssociationT, detail::GetDefaultHandleType, false>; template -using MutableAssociation = AssociationT, detail::GetDefT, true>; +using MutableAssociation = AssociationT, detail::GetDefaultHandleType, true>; template class AssociationCollection; diff --git a/include/podio/detail/AssociationObj.h b/include/podio/detail/AssociationObj.h index 6f7dadcbd..9f78b408f 100644 --- a/include/podio/detail/AssociationObj.h +++ b/include/podio/detail/AssociationObj.h @@ -3,36 +3,27 @@ #include "podio/detail/AssociationFwd.h" -#include "podio/ObjBase.h" #include "podio/ObjectID.h" namespace podio { template -class AssociationObj : public podio::ObjBase { +class AssociationObj { friend Association; friend MutableAssociation; public: /// Constructor - AssociationObj() : - ObjBase{{podio::ObjectID::untracked, podio::ObjectID::untracked}, 0}, - weight(0.0f), - m_from(nullptr), - m_to(nullptr) { + AssociationObj() : id(), weight(0.0f), m_from(nullptr), m_to(nullptr) { } /// Constructor from ObjectID and weight (does not initialize relations yet!) - AssociationObj(const podio::ObjectID id_, float weight_) : ObjBase{id_, 0}, weight(weight_) { + AssociationObj(const podio::ObjectID id_, float weight_) : id(id_), weight(weight_) { } /// Copy constructor (deep-copy of relations) - AssociationObj(const AssociationObj& other) : - ObjBase{{podio::ObjectID::untracked, podio::ObjectID::untracked}, 0}, - weight(other.weight), - m_from(nullptr), - m_to(nullptr) { + AssociationObj(const AssociationObj& other) : id(), weight(other.weight), m_from(nullptr), m_to(nullptr) { if (other.m_from) { m_from = new FromT(*other.m_from); } @@ -51,6 +42,7 @@ class AssociationObj : public podio::ObjBase { } public: + podio::ObjectID id{}; float weight{1.0f}; FromT* m_from{nullptr}; ToT* m_to{nullptr}; diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index 9b5ff4640..b12f8b3b5 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -45,6 +45,9 @@ namespace det { template typename Op, typename... Args> using detected_or = detail::detector; + + template