From f4bacda54fa7f7912398b861c2516f9fde6db3da Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 6 May 2024 11:06:36 -0400 Subject: [PATCH 01/24] Implement JCollection, JBasicCollection, JPodioCollection --- .../JANA/Components/JBasicCollection.h | 111 ++++++++++++++++++ src/libraries/JANA/Components/JCollection.h | 93 +++++++++++++++ .../JANA/Components/JPodioCollection.h | 53 +++++++++ 3 files changed, 257 insertions(+) create mode 100644 src/libraries/JANA/Components/JBasicCollection.h create mode 100644 src/libraries/JANA/Components/JCollection.h create mode 100644 src/libraries/JANA/Components/JPodioCollection.h diff --git a/src/libraries/JANA/Components/JBasicCollection.h b/src/libraries/JANA/Components/JBasicCollection.h new file mode 100644 index 000000000..d61128c27 --- /dev/null +++ b/src/libraries/JANA/Components/JBasicCollection.h @@ -0,0 +1,111 @@ + +#pragma once + +#include +#include +#include + +#ifdef JANA2_HAVE_ROOT +#include +#endif + +class JBasicCollection : public JCollection { + bool m_is_persistent = false; + bool m_not_object_owner = false; + bool m_regenerate = false; + bool m_write_to_output = true; + +public: + JBasicCollection() = default; + ~JBasicCollection() override = default; + void SetPersistentFlag(bool persistent) { m_is_persistent = persistent; } + void SetNotOwnerFlag(bool not_owner) { m_not_object_owner = not_owner; } + void SetRegenerateFlag(bool regenerate) { m_regenerate = regenerate; } + void SetWriteToOutputFlag(bool write_to_output) { m_write_to_output = write_to_output; } + + bool GetPersistentFlag() { return m_is_persistent; } + bool GetNotOwnerFlag() { return m_not_object_owner; } + bool GetRegenerateFlag() { return m_regenerate; } + bool GetWriteToOutputFlag() { return m_write_to_output; } +}; + + + +template +class JBasicCollectionT : public JBasicCollection { +private: + std::vector m_data; + +public: + JBasicCollectionT(); + void ClearData() override; + size_t GetSize() const override { return m_data.size();} + + std::vector& GetData() { return m_data; } + + /// EnableGetAs generates a vtable entry so that users may extract the + /// contents of this JFactoryT from the type-erased JFactory. The user has to manually specify which upcasts + /// to allow, and they have to do so for each instance. It is recommended to do so in the constructor. + /// Note that EnableGetAs() is called automatically. + template void EnableGetAs (); + + // The following specializations allow automatically adding standard types (e.g. JObject) using things like + // std::is_convertible(). The std::true_type version defers to the standard EnableGetAs(). + template void EnableGetAs(std::true_type) { EnableGetAs(); } + template void EnableGetAs(std::false_type) {} +}; + +// Template definitions + +template +JBasicCollectionT::JBasicCollectionT() { + SetTypeName(JTypeInfo::demangle()); + EnableGetAs(); + EnableGetAs( std::is_convertible() ); // Automatically add JObject if this can be converted to it +#ifdef JANA2_HAVE_ROOT + EnableGetAs( std::is_convertible() ); // Automatically add TObject if this can be converted to it +#endif +} + +template +void JBasicCollectionT::ClearData() { + + // ClearData won't do anything if Init() hasn't been called + if (GetCreationStatus() == CreationStatus::NotCreatedYet) { + return; + } + // ClearData() does nothing if persistent flag is set. + // User must manually recycle data, e.g. during ChangeRun() + if (GetPersistentFlag()) { + return; + } + + // Assuming we _are_ the object owner, delete the underlying jobjects + if (!GetNotOwnerFlag()) { + for (auto p : m_data) delete p; + } + m_data.clear(); + SetCreationStatus(CreationStatus::NotCreatedYet); +} + +template +template +void JBasicCollectionT::EnableGetAs() { + + auto upcast_lambda = [this]() { + std::vector results; + for (auto t : m_data) { + results.push_back(static_cast(t)); + } + return results; + }; + + auto key = std::type_index(typeid(S)); + using upcast_fn_t = std::function()>; + mUpcastVTable[key] = std::unique_ptr(new JAnyT(std::move(upcast_lambda))); +} + + + + + diff --git a/src/libraries/JANA/Components/JCollection.h b/src/libraries/JANA/Components/JCollection.h new file mode 100644 index 000000000..ed8901731 --- /dev/null +++ b/src/libraries/JANA/Components/JCollection.h @@ -0,0 +1,93 @@ +// Copyright 2024, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#pragma once + +#include +#include +#include + +#include +#include +#include +#include + + +class JCollection { +public: + // Typedefs + enum class CreationStatus { NotCreatedYet, Created, Inserted, InsertedViaGetObjects, NeverCreated }; + +private: + // Fields + CreationStatus m_creation_status = CreationStatus::NotCreatedYet; + std::string m_collection_name; + std::string m_collection_tag; + std::string m_type_name; + mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; + +protected: + std::unordered_map> mUpcastVTable; + +public: + // Interface + JCollection() = default; + virtual ~JCollection() = default; + virtual size_t GetSize() const = 0; + virtual void ClearData() = 0; + + // Getters + CreationStatus GetCreationStatus() { return m_creation_status; } + std::string GetCollectionName() { return m_collection_name; } + std::string GetCollectionTag() { return m_collection_tag; } + std::string GetTypeName() { return m_type_name; } + JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. + + // Setters + void SetCreationStatus(CreationStatus s) { m_creation_status = s;} + void SetCollectionName(std::string collection_name) { m_collection_name = collection_name; } + void SetCollectionTag(std::string collection_tag) { m_collection_tag = collection_tag; } + void SetTypeName(std::string type_name) { m_type_name = type_name; } + void SetInsertOrigin(JCallGraphRecorder::JDataOrigin origin) { m_insert_origin = origin; } ///< Called automatically by JEvent::Insert() to records whether that call was made by a source or factory. + /// + // Templates + // + /// Generically access the encapsulated data, performing an upcast if necessary. This is useful for extracting data from + /// all JFactories where T extends a parent class S, such as JObject or TObject, in contexts where T is not known + /// or it would introduce an unwanted coupling. The main application is for building DSTs. + /// + /// Be aware of the following caveats: + /// - The factory's object type must not use virtual inheritance. + /// - If JFactory::Process hasn't already been called, this will return an empty vector. This will NOT call JFactory::Process. + /// - Someone must call JFactoryT::EnableGetAs, preferably the constructor. Otherwise, this will return an empty vector. + /// - If S isn't a base class of T, this will return an empty vector. + template + std::vector GetAs(); + +}; + + + +// Because C++ doesn't support templated virtual functions, we implement our own dispatch table, mUpcastVTable. +// This means that the JFactoryT is forced to manually populate this table by calling JFactoryT::EnableGetAs. +// We have the option to make the vtable be a static member of JFactoryT, but we have chosen not to because: +// +// 1. It would be inconsistent with the fact that the user is supposed to call EnableGetAs in the ctor +// 2. People in the future may want to generalize GetAs to support user-defined S* -> T* conversions (which I don't recommend) +// 3. The size of the vtable is expected to be very small (<10 elements, most likely 2) + +template +std::vector JCollection::GetAs() { + std::vector results; + auto ti = std::type_index(typeid(S)); + auto search = mUpcastVTable.find(ti); + if (search != mUpcastVTable.end()) { + using upcast_fn_t = std::function()>; + auto temp = static_cast*>(&(*search->second)); + upcast_fn_t upcast_fn = temp->t; + results = upcast_fn(); + } + return results; +} + + diff --git a/src/libraries/JANA/Components/JPodioCollection.h b/src/libraries/JANA/Components/JPodioCollection.h new file mode 100644 index 000000000..60a6df7da --- /dev/null +++ b/src/libraries/JANA/Components/JPodioCollection.h @@ -0,0 +1,53 @@ +// Copyright 2024, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#pragma once + +#include +#include +#include + + +class JPodioCollection : public JCollection { + + // Fields + const podio::CollectionBase* m_collection = nullptr; + podio::Frame* m_frame = nullptr; + + // Getters + template + const T* GetCollection(); + + // Setters + void SetFrame(podio::Frame* frame) { m_frame = frame; } + + template + void SetCollection(std::unique_ptr collection); + + template + void SetCollectionAlreadyInFrame(const CollectionT* collection); +}; + + +template +void JPodioCollection::SetCollection(std::unique_ptr collection) { + /// Provide a PODIO collection. Note that PODIO assumes ownership of this collection, and the + /// collection pointer should be assumed to be invalid after this call + + if (this->m_frame == nullptr) { + throw JException("JPodioCollection: Unable to add collection to frame as frame is missing!"); + } + this->m_frame->put(std::move(collection), this->GetCollectionName()); + const auto* moved = &this->m_frame->template get(this->GetCollectionName()); + this->m_collection = moved; + + this->SetCreationStatus(JCollection::CreationStatus::Inserted); +} + +template +void JPodioCollection::SetCollectionAlreadyInFrame(const CollectionT* collection) { + m_collection = collection; + SetCreationStatus(JPodioCollection::CreationStatus::Inserted); +} + + From afa5e9a841359615e23021d12a8924d4eae429a7 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 23 Aug 2024 00:49:54 -0400 Subject: [PATCH 02/24] JFactory, JFactorySet, JEvent can store and retrieve JCollections --- .../JANA/Components/JBasicCollection.h | 2 +- src/libraries/JANA/Components/JCollection.h | 15 +++-- .../JANA/Components/JHasFactoryOutputs.h | 23 ++++++++ .../JANA/Components/JPodioCollection.h | 38 ++++++++---- src/libraries/JANA/JEvent.h | 58 ++++++++++++++++--- src/libraries/JANA/JFactory.h | 30 +++++----- src/libraries/JANA/JFactorySet.cc | 39 +++++++++++++ src/libraries/JANA/JFactorySet.h | 18 +++--- 8 files changed, 176 insertions(+), 47 deletions(-) create mode 100644 src/libraries/JANA/Components/JHasFactoryOutputs.h diff --git a/src/libraries/JANA/Components/JBasicCollection.h b/src/libraries/JANA/Components/JBasicCollection.h index d61128c27..2ee4f4bac 100644 --- a/src/libraries/JANA/Components/JBasicCollection.h +++ b/src/libraries/JANA/Components/JBasicCollection.h @@ -1,7 +1,7 @@ #pragma once -#include +#include #include #include diff --git a/src/libraries/JANA/Components/JCollection.h b/src/libraries/JANA/Components/JCollection.h index ed8901731..a7e077fcb 100644 --- a/src/libraries/JANA/Components/JCollection.h +++ b/src/libraries/JANA/Components/JCollection.h @@ -13,6 +13,8 @@ #include +class JFactory; + class JCollection { public: // Typedefs @@ -24,6 +26,7 @@ class JCollection { std::string m_collection_name; std::string m_collection_tag; std::string m_type_name; + JFactory* m_factory = nullptr; mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; protected: @@ -37,11 +40,12 @@ class JCollection { virtual void ClearData() = 0; // Getters - CreationStatus GetCreationStatus() { return m_creation_status; } - std::string GetCollectionName() { return m_collection_name; } - std::string GetCollectionTag() { return m_collection_tag; } - std::string GetTypeName() { return m_type_name; } + CreationStatus GetCreationStatus() const { return m_creation_status; } + std::string GetCollectionName() const { return m_collection_name; } + std::string GetCollectionTag() const { return m_collection_tag; } + std::string GetTypeName() const { return m_type_name; } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. + JFactory* GetFactory() const { return m_factory; } // Setters void SetCreationStatus(CreationStatus s) { m_creation_status = s;} @@ -49,7 +53,8 @@ class JCollection { void SetCollectionTag(std::string collection_tag) { m_collection_tag = collection_tag; } void SetTypeName(std::string type_name) { m_type_name = type_name; } void SetInsertOrigin(JCallGraphRecorder::JDataOrigin origin) { m_insert_origin = origin; } ///< Called automatically by JEvent::Insert() to records whether that call was made by a source or factory. - /// + void SetFactory(JFactory* fac) { m_factory = fac; } + // Templates // /// Generically access the encapsulated data, performing an upcast if necessary. This is useful for extracting data from diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h new file mode 100644 index 000000000..163dc825c --- /dev/null +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -0,0 +1,23 @@ + +#pragma once +#include + +namespace jana::components { + +struct JHasFactoryOutputs { + +private: + std::vector> m_output_collections; + +public: + const std::vector>& GetOutputCollections() { + return m_output_collections; + } + +}; + + + +} // namespace jana::components + + diff --git a/src/libraries/JANA/Components/JPodioCollection.h b/src/libraries/JANA/Components/JPodioCollection.h index 60a6df7da..3000c1de4 100644 --- a/src/libraries/JANA/Components/JPodioCollection.h +++ b/src/libraries/JANA/Components/JPodioCollection.h @@ -3,34 +3,46 @@ #pragma once -#include +#include "JANA/Utils/JTypeInfo.h" +#include +#include #include #include class JPodioCollection : public JCollection { +private: // Fields const podio::CollectionBase* m_collection = nullptr; podio::Frame* m_frame = nullptr; +public: // Getters - template - const T* GetCollection(); + const podio::CollectionBase* GetCollection() const { return m_collection; } + + template const typename T::collection_type* GetCollection(); + // Setters void SetFrame(podio::Frame* frame) { m_frame = frame; } - template - void SetCollection(std::unique_ptr collection); + template + void SetCollection(std::unique_ptr collection); - template - void SetCollectionAlreadyInFrame(const CollectionT* collection); + template + void SetCollectionAlreadyInFrame(const typename T::collection_type* collection); }; +template +const typename T::collection_type* JPodioCollection::GetCollection() { + assert(JTypeInfo::demangle() == this->GetTypeName()); + return dynamic_cast(m_collection); +} + -template -void JPodioCollection::SetCollection(std::unique_ptr collection) { +template +void JPodioCollection::SetCollection(std::unique_ptr collection) { /// Provide a PODIO collection. Note that PODIO assumes ownership of this collection, and the /// collection pointer should be assumed to be invalid after this call @@ -38,15 +50,17 @@ void JPodioCollection::SetCollection(std::unique_ptr collection) { throw JException("JPodioCollection: Unable to add collection to frame as frame is missing!"); } this->m_frame->put(std::move(collection), this->GetCollectionName()); - const auto* moved = &this->m_frame->template get(this->GetCollectionName()); + const auto* moved = &this->m_frame->template get(this->GetCollectionName()); this->m_collection = moved; + this->SetTypeName(JTypeInfo::demangle()); this->SetCreationStatus(JCollection::CreationStatus::Inserted); } -template -void JPodioCollection::SetCollectionAlreadyInFrame(const CollectionT* collection) { +template +void JPodioCollection::SetCollectionAlreadyInFrame(const typename T::collection_type* collection) { m_collection = collection; + this->SetTypeName(JTypeInfo::demangle()); SetCreationStatus(JPodioCollection::CreationStatus::Inserted); } diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index e60e7cbf7..9d209957b 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -9,9 +9,7 @@ #include #include #include - #include - #include #include #include @@ -22,15 +20,11 @@ #include #include #include -#include #include -#include #if JANA2_HAVE_PODIO #include -namespace podio { -class CollectionBase; -} +#include #endif class JApplication; @@ -107,6 +101,9 @@ class JEvent : public std::enable_shared_from_this template JFactoryPodioT* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); #endif + // EXPERIMENTAL NEW THING + JCollection* CreateAndGetCollection(std::string name, bool throw_on_missing) const; + //SETTERS void SetRunNumber(int32_t aRunNumber){mRunNumber = aRunNumber;} void SetEventNumber(uint64_t aEventNumber){mEventNumber = aEventNumber;} @@ -507,9 +504,30 @@ JFactoryT* JEvent::GetSingle(const T* &t, const char *tag, bool exception_if_ return fac; } +inline JCollection* JEvent::CreateAndGetCollection(std::string name, bool throw_on_missing) const { + + auto* coll = mFactorySet->GetCollection(name); + if (coll == nullptr) { + if (throw_on_missing) { + throw JException("No collection with tag '%s' found", name.c_str()); + } + else { + return nullptr; + } + } + if (coll->GetFactory() != nullptr) { + // If this was inserted, there would be no factory to run + // fac->Create() will short-circuit if something was already inserted + JCallGraphEntryMaker cg_entry(mCallGraph, coll->GetFactory()); // times execution until this goes out of scope + coll->GetFactory()->Create(this->shared_from_this()); + } + return coll; +} + #if JANA2_HAVE_PODIO inline std::vector JEvent::GetAllCollectionNames() const { + // TODO: Obtain from JFactorySet std::vector keys; for (auto pair : mPodioFactories) { keys.push_back(pair.first); @@ -518,6 +536,19 @@ inline std::vector JEvent::GetAllCollectionNames() const { } inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { + // First try finding it using the new collection model + auto* coll = CreateAndGetCollection(name, throw_on_missing); + if (coll != nullptr) { + auto* podio_coll = dynamic_cast(coll); + if (podio_coll == nullptr) { + throw JException("Not a podio collection: %s", name.c_str()); + } + else { + return podio_coll->GetCollection(); + } + } + + // We couldn't find it using the new collection model, so now we search mPodioFactories auto it = mPodioFactories.find(name); if (it == mPodioFactories.end()) { if (throw_on_missing) { @@ -540,6 +571,19 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, template const typename JFactoryPodioT::CollectionT* JEvent::GetCollection(std::string name, bool throw_on_missing) const { + // First try finding it using the new collection model + auto* coll = CreateAndGetCollection(name, throw_on_missing); + if (coll != nullptr) { + auto* podio_coll = dynamic_cast(coll); + if (podio_coll == nullptr) { + throw JException("Not a podio collection: %s", name.c_str()); + } + else { + return podio_coll->GetCollection(); + } + } + + // Next try finding it using the old way JFactoryT* factory = GetFactory(name, throw_on_missing); if (factory == nullptr) { return nullptr; diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 823c324e7..4fcf27391 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -22,12 +23,12 @@ class JEvent; class JObject; class JApplication; -class JFactory : public jana::components::JComponent { +class JFactory : public jana::components::JComponent, + public jana::components::JHasFactoryOutputs { public: enum class Status {Uninitialized, Unprocessed, Processed, Inserted}; enum class CreationStatus { NotCreatedYet, Created, Inserted, InsertedViaGetObjects, NeverCreated }; - enum JFactory_Flags_t { JFACTORY_NULL = 0x00, // Not used anywhere PERSISTENT = 0x01, // Used heavily. Possibly better served by JServices, hierarchical events, or event groups. @@ -36,6 +37,19 @@ class JFactory : public jana::components::JComponent { REGENERATE = 0x08 // Replaces JANA1 JFactory_base::use_factory and JFactory::GetCheckSourceFirst() }; +protected: + std::string mObjectName; + std::string mTag; + uint32_t mFlags = WRITE_TO_OUTPUT; + int32_t mPreviousRunNumber = -1; + std::unordered_map> mUpcastVTable; + + mutable Status mStatus = Status::Uninitialized; + CreationStatus mCreationStatus = CreationStatus::NotCreatedYet; + mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) + + +public: JFactory(std::string aName, std::string aTag = "") : mObjectName(std::move(aName)), mTag(std::move(aTag)), @@ -178,18 +192,6 @@ class JFactory : public jana::components::JComponent { virtual void Insert(JObject *data) = 0; -protected: - - std::string mObjectName; - std::string mTag; - uint32_t mFlags = WRITE_TO_OUTPUT; - int32_t mPreviousRunNumber = -1; - std::unordered_map> mUpcastVTable; - - mutable Status mStatus = Status::Uninitialized; - mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) - - CreationStatus mCreationStatus = CreationStatus::NotCreatedYet; }; // Because C++ doesn't support templated virtual functions, we implement our own dispatch table, mUpcastVTable. diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 3b29258dc..e7ad2a750 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -6,6 +6,7 @@ #include #include "JFactorySet.h" +#include "JANA/Components/JCollection.h" #include "JFactory.h" #include "JMultifactory.h" #include "JFactoryGenerator.h" @@ -82,6 +83,28 @@ bool JFactorySet::Add(JFactory* aFactory) mFactories[typed_key] = aFactory; mFactoriesFromString[untyped_key] = aFactory; + + // Also add any enclosed collections to the collection map + for (const auto& coll : aFactory->GetOutputCollections()) { + auto named_result = mCollectionsFromName.find(coll->GetCollectionName()); + if (named_result != std::end(mCollectionsFromName)) { + // Collection is duplicate. Since this almost certainly indicates a user error, and + // the caller will not be able to do anything about it anyway, throw an exception. + // We show the user which factory is causing this problem, including both plugin names + + // TODO: I haven't thought through insert vs process for collections yet + assert(named_result->second->GetFactory() != nullptr); + + std::string other_plugin_name = named_result->second->GetFactory()->GetPluginName(); + auto ex = JException("Attempted to add duplicate factories"); + ex.function_name = "JFactorySet::Add"; + ex.instance_name = aFactory->GetPrefix(); + ex.type_name = aFactory->GetTypeName(); + ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; + throw ex; + } + mCollectionsFromName[coll->GetCollectionName()] = coll.get(); + } return true; } @@ -103,6 +126,22 @@ bool JFactorySet::Add(JMultifactory *multifactory) { return true; } +//--------------------------------- +// GetCollection +//--------------------------------- +JCollection* JFactorySet::GetCollection(const std::string& collection_name) const { + auto it = mCollectionsFromName.find(collection_name); + if (it != std::end(mCollectionsFromName)) { + assert(it->second->GetFactory() != nullptr); + if (it->second->GetFactory()->GetLevel() != mLevel) { + throw JException("Collection belongs to a different level on the event hierarchy!"); + } + return it->second; + } + return nullptr; +} + + //--------------------------------- // GetFactory //--------------------------------- diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 25ebc50c0..1b3c2e2be 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -19,6 +20,14 @@ class JMultifactory; class JFactorySet { + protected: + std::map, JFactory*> mFactories; // {(typeid, tag) : factory} + std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} + std::map mCollectionsFromName; + std::vector mMultifactories; + bool mIsFactoryOwner = true; + JEventLevel mLevel = JEventLevel::PhysicsEvent; + public: JFactorySet(); JFactorySet(const std::vector& aFactoryGenerators); @@ -29,6 +38,7 @@ class JFactorySet { void Print(void) const; void Release(void); + JCollection* GetCollection(const std::string& collection_name) const; JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; template JFactoryT* GetFactory(const std::string& tag = "") const; std::vector GetAllFactories() const; @@ -37,14 +47,6 @@ class JFactorySet { JEventLevel GetLevel() const { return mLevel; } void SetLevel(JEventLevel level) { mLevel = level; } - - protected: - std::map, JFactory*> mFactories; // {(typeid, tag) : factory} - std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} - std::vector mMultifactories; - bool mIsFactoryOwner = true; - JEventLevel mLevel = JEventLevel::PhysicsEvent; - }; From 4c9e502360bce800f83d343021f63a10ea8cc137 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 23 Aug 2024 13:22:50 -0400 Subject: [PATCH 03/24] Adjust JFactory interface to support JCollections --- .../JANA/Components/JHasFactoryOutputs.h | 5 ++ src/libraries/JANA/JFactory.h | 19 +++-- src/libraries/JANA/JFactorySet.cc | 84 +++++++++++-------- src/libraries/JANA/JFactoryT.h | 2 +- src/libraries/JANA/Podio/JFactoryPodioT.h | 2 +- src/programs/unit_tests/CMakeLists.txt | 1 + .../unit_tests/Components/JCollectionTests.cc | 55 ++++++++++++ 7 files changed, 126 insertions(+), 42 deletions(-) create mode 100644 src/programs/unit_tests/Components/JCollectionTests.cc diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h index 163dc825c..2adb8291e 100644 --- a/src/libraries/JANA/Components/JHasFactoryOutputs.h +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -1,6 +1,7 @@ #pragma once #include +#include namespace jana::components { @@ -14,6 +15,10 @@ struct JHasFactoryOutputs { return m_output_collections; } + void RegisterCollection(std::unique_ptr&& coll) { + m_output_collections.push_back(std::move(coll)); + } + }; diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 4fcf27391..3369c925d 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -17,6 +17,7 @@ #include #include #include +#include class JEvent; @@ -50,6 +51,9 @@ class JFactory : public jana::components::JComponent, public: + JFactory() : mStatus(Status::Uninitialized) { + } + JFactory(std::string aName, std::string aTag = "") : mObjectName(std::move(aName)), mTag(std::move(aTag)), @@ -149,9 +153,8 @@ class JFactory : public jana::components::JComponent, } // Overloaded by JFactoryT - virtual std::type_index GetObjectType() const = 0; - virtual void ClearData() = 0; + virtual void ClearData() {}; // Overloaded by user Factories @@ -162,8 +165,10 @@ class JFactory : public jana::components::JComponent, virtual void Process(const std::shared_ptr&) {} virtual void Finish() {} + virtual std::optional GetObjectType() const { return std::nullopt; } + virtual std::size_t GetNumObjects() const { - return 0; + throw JException("Not implemented!"); } @@ -187,9 +192,13 @@ class JFactory : public jana::components::JComponent, void DoInit(); void Summarize(JComponentSummary& summary) const override; + virtual void Set(const std::vector &) { + throw JException("Not implemented!"); + }; - virtual void Set(const std::vector &data) = 0; - virtual void Insert(JObject *data) = 0; + virtual void Insert(JObject*) { + throw JException("Not implemented!"); + }; }; diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index e7ad2a750..fa2e761fa 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -55,47 +55,36 @@ bool JFactorySet::Add(JFactory* aFactory) /// throw an exception and let the user figure out what to do. /// This scenario occurs when the user has multiple JFactory producing the /// same T JObject, and is not distinguishing between them via tags. + + // There are two different ways JFactories can work now. In the old way, JFactory must be + // either a JFactoryT or a JFactoryPodioT, and have exactly one output collection. In the + // new way, JFactory has an arbitrary number of output collections which are explicitly + // represented, similar to but better than JMultifactory. We distinguish between + // these two cases by checking whether JFactory::GetObjectType returns an object type vs nullopt. - auto typed_key = std::make_pair( aFactory->GetObjectType(), aFactory->GetTag() ); - auto untyped_key = std::make_pair( aFactory->GetObjectName(), aFactory->GetTag() ); - auto typed_result = mFactories.find(typed_key); - auto untyped_result = mFactoriesFromString.find(untyped_key); + auto object_type = aFactory->GetObjectType(); + if (object_type != std::nullopt) { + + // We have an old-style JFactory! - if (typed_result != std::end(mFactories) || untyped_result != std::end(mFactoriesFromString)) { - // Factory is duplicate. Since this almost certainly indicates a user error, and - // the caller will not be able to do anything about it anyway, throw an exception. - // We show the user which factory is causing this problem, including both plugin names - std::string other_plugin_name; - if (typed_result != std::end(mFactories)) { - other_plugin_name = typed_result->second->GetPluginName(); - } - else { - other_plugin_name = untyped_result->second->GetPluginName(); - } - auto ex = JException("Attempted to add duplicate factories"); - ex.function_name = "JFactorySet::Add"; - ex.instance_name = aFactory->GetPrefix(); - ex.type_name = aFactory->GetTypeName(); - ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; - throw ex; - } + auto typed_key = std::make_pair( *object_type, aFactory->GetTag() ); + auto untyped_key = std::make_pair( aFactory->GetObjectName(), aFactory->GetTag() ); - mFactories[typed_key] = aFactory; - mFactoriesFromString[untyped_key] = aFactory; + auto typed_result = mFactories.find(typed_key); + auto untyped_result = mFactoriesFromString.find(untyped_key); - // Also add any enclosed collections to the collection map - for (const auto& coll : aFactory->GetOutputCollections()) { - auto named_result = mCollectionsFromName.find(coll->GetCollectionName()); - if (named_result != std::end(mCollectionsFromName)) { - // Collection is duplicate. Since this almost certainly indicates a user error, and + if (typed_result != std::end(mFactories) || untyped_result != std::end(mFactoriesFromString)) { + // Factory is duplicate. Since this almost certainly indicates a user error, and // the caller will not be able to do anything about it anyway, throw an exception. // We show the user which factory is causing this problem, including both plugin names - - // TODO: I haven't thought through insert vs process for collections yet - assert(named_result->second->GetFactory() != nullptr); - - std::string other_plugin_name = named_result->second->GetFactory()->GetPluginName(); + std::string other_plugin_name; + if (typed_result != std::end(mFactories)) { + other_plugin_name = typed_result->second->GetPluginName(); + } + else { + other_plugin_name = untyped_result->second->GetPluginName(); + } auto ex = JException("Attempted to add duplicate factories"); ex.function_name = "JFactorySet::Add"; ex.instance_name = aFactory->GetPrefix(); @@ -103,7 +92,32 @@ bool JFactorySet::Add(JFactory* aFactory) ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; throw ex; } - mCollectionsFromName[coll->GetCollectionName()] = coll.get(); + + mFactories[typed_key] = aFactory; + mFactoriesFromString[untyped_key] = aFactory; + } + else { + // We have a new-style JFactory! + for (const auto& coll : aFactory->GetOutputCollections()) { + auto named_result = mCollectionsFromName.find(coll->GetCollectionName()); + if (named_result != std::end(mCollectionsFromName)) { + // Collection is duplicate. Since this almost certainly indicates a user error, and + // the caller will not be able to do anything about it anyway, throw an exception. + // We show the user which factory is causing this problem, including both plugin names + + // TODO: I haven't thought through insert vs process for collections yet + assert(named_result->second->GetFactory() != nullptr); + + std::string other_plugin_name = named_result->second->GetFactory()->GetPluginName(); + auto ex = JException("Attempted to add duplicate factories"); + ex.function_name = "JFactorySet::Add"; + ex.instance_name = aFactory->GetPrefix(); + ex.type_name = aFactory->GetTypeName(); + ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; + throw ex; + } + mCollectionsFromName[coll->GetCollectionName()] = coll.get(); + } } return true; } diff --git a/src/libraries/JANA/JFactoryT.h b/src/libraries/JANA/JFactoryT.h index 46fbccb33..456a63200 100644 --- a/src/libraries/JANA/JFactoryT.h +++ b/src/libraries/JANA/JFactoryT.h @@ -66,7 +66,7 @@ class JFactoryT : public JFactory { void Process(const std::shared_ptr&) override {} - std::type_index GetObjectType(void) const override { + std::optional GetObjectType(void) const override { return std::type_index(typeid(T)); } diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index 9ff5a60ad..0c91ce146 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -54,7 +54,7 @@ class JFactoryPodioT : public JFactoryT, public JFactoryPodio { void Finish() override {} void Create(const std::shared_ptr& event) final; - std::type_index GetObjectType() const final { return std::type_index(typeid(T)); } + std::optional GetObjectType() const final { return std::type_index(typeid(T)); } std::size_t GetNumObjects() const final { return mCollection->size(); } void ClearData() final; diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index 9b34afecd..2096b9d86 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -13,6 +13,7 @@ set(TEST_SOURCES Components/ExactlyOnceTests.cc Components/GetObjectsTests.cc Components/NEventNSkipTests.cc + Components/JCollectionTests.cc Components/JComponentTests.cc Components/JEventGetAllTests.cc Components/JEventProcessorTests.cc diff --git a/src/programs/unit_tests/Components/JCollectionTests.cc b/src/programs/unit_tests/Components/JCollectionTests.cc new file mode 100644 index 000000000..a5cbaeb8b --- /dev/null +++ b/src/programs/unit_tests/Components/JCollectionTests.cc @@ -0,0 +1,55 @@ + + +// Copyright 2023, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#include "JANA/JEventSource.h" +#include "catch.hpp" +#include +#include +#include +#include +#include +#include + +namespace omnifactory_tests { + +struct TestSource : public JEventSource { + Parameter y {this, "y", "asdf", "Does something"}; + TestSource(std::string res_name, JApplication* app) { + SetCallbackStyle(CallbackStyle::ExpertMode); + } + static std::string GetDescription() { + return "Test source"; + } + void Init() { + REQUIRE(y() == "asdf"); + } + Result Emit(JEvent&) { + return Result::Success; + } +}; + +struct TestFactory : public JFactory { + Parameter x {this, "x", 22, "Does something"}; + TestFactory() { + SetCallbackStyle(CallbackStyle::ExpertMode); + } + void Init() { + REQUIRE(x() == 23); + } + void Process(const std::shared_ptr& event) { + } +}; + +TEST_CASE("ParametersTest") { + JApplication app; + app.Add(new JEventSourceGeneratorT); + app.Add(new JFactoryGeneratorT); + app.Add("fake_file.root"); + app.SetParameterValue("jana:nevents", 2); + app.SetParameterValue("autoactivate", ""); + app.Run(); +} + +} From ebd64903b61c295400dfa6dff1d7c9b5b13f4be5 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 23 Aug 2024 22:03:45 -0400 Subject: [PATCH 04/24] JFactory: Add PodioOutput, VariadicPodioOutput --- .../JANA/Components/JHasFactoryOutputs.h | 27 ++++-- .../JANA/Components/JPodioCollection.h | 19 +++- src/libraries/JANA/Components/JPodioOutput.h | 91 +++++++++++++++++++ src/libraries/JANA/JFactory.cc | 3 + src/libraries/JANA/JFactorySet.cc | 39 ++++---- .../unit_tests/Components/JCollectionTests.cc | 32 +++++-- 6 files changed, 178 insertions(+), 33 deletions(-) create mode 100644 src/libraries/JANA/Components/JPodioOutput.h diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h index 2adb8291e..813e29a43 100644 --- a/src/libraries/JANA/Components/JHasFactoryOutputs.h +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -3,22 +3,35 @@ #include #include +class JEvent; + namespace jana::components { -struct JHasFactoryOutputs { + +class JHasFactoryOutputs { +public: + struct OutputBase { + protected: + std::vector m_collection_names; // TODO: Possibly don't need anymore + std::vector> m_collections; + bool m_is_variadic = false; + public: + const std::vector>& GetCollections() const { return m_collections;} + virtual void PutCollections(const JEvent& event) = 0; + virtual void Reset() = 0; + }; private: - std::vector> m_output_collections; + std::vector m_outputs; public: - const std::vector>& GetOutputCollections() { - return m_output_collections; + const std::vector& GetOutputs() { + return m_outputs; } - void RegisterCollection(std::unique_ptr&& coll) { - m_output_collections.push_back(std::move(coll)); + void RegisterOutput(OutputBase* output) { + m_outputs.push_back(output); } - }; diff --git a/src/libraries/JANA/Components/JPodioCollection.h b/src/libraries/JANA/Components/JPodioCollection.h index 3000c1de4..70d2a8cc9 100644 --- a/src/libraries/JANA/Components/JPodioCollection.h +++ b/src/libraries/JANA/Components/JPodioCollection.h @@ -13,11 +13,28 @@ class JPodioCollection : public JCollection { private: - // Fields const podio::CollectionBase* m_collection = nullptr; podio::Frame* m_frame = nullptr; public: + size_t GetSize() const override { + return m_collection->size(); + } + + virtual void ClearData() override { + m_collection = nullptr; + m_frame = nullptr; + // Podio clears the data itself when the frame is destroyed. + // Until then, the collection is immutable. + // + // Consider: Instead of putting the frame in its own JFactory, maybe we + // want to maintain a shared_ptr to the frame here, and delete the + // the reference on ClearData(). Thus, the final call to ClearData() + // for each events deletes the frame and actually frees the data. + // This would let us support multiple frames within one event, though + // it might also prevent the user from accessing frames directly. + } + // Getters const podio::CollectionBase* GetCollection() const { return m_collection; } diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h new file mode 100644 index 000000000..17214093b --- /dev/null +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -0,0 +1,91 @@ +#pragma once +#include "JANA/Components/JPodioCollection.h" +#include "JANA/Utils/JTypeInfo.h" +#include +#include + + +namespace jana::components { + + +template +class PodioOutput : public JHasFactoryOutputs::OutputBase { +private: + std::unique_ptr m_data; + JPodioCollection* m_collection; +public: + PodioOutput(JHasFactoryOutputs* owner, std::string default_collection_name="") { + owner->RegisterOutput(this); + this->m_collection_names.push_back(default_collection_name); + auto coll = std::make_unique(); + coll->SetCollectionName(default_collection_name); + coll->SetTypeName(JTypeInfo::demangle()); + m_collection = coll.get(); + m_collections.push_back(std::move(coll)); + } + + std::unique_ptr& operator()() { return m_data; } + +protected: + //void CreateCollections() override { + //} + + void PutCollections(const JEvent& event) override { + auto frame = const_cast(event.GetSingle()); + frame->put(std::move(m_data), m_collection->GetCollectionName()); + const auto* moved = &frame->template get(m_collection->GetCollectionName()); + m_data = nullptr; + m_collection->SetCollectionAlreadyInFrame(moved); + //m_collection->SetFrame(frame); // We might not need this! + } + void Reset() override { + m_data = std::move(std::make_unique()); + } +}; + + +template +class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { +private: + std::vector> m_data; + +public: + VariadicPodioOutput(JHasFactoryOutputs* owner, std::vector default_collection_names={}) { + owner->RegisterOutput(this); + this->m_collection_names = default_collection_names; + this->m_is_variadic = true; + for (const std::string& name : default_collection_names) { + auto coll = std::make_unique(); + coll->SetCollectionName(name); + coll->SetTypeName(JTypeInfo::demangle()); + m_collections.push_back(std::move(coll)); + } + } + void PutCollections(const JEvent& event) override { + if (m_data.size() != this->collection_names.size()) { + throw JException("VariadicPodioOutput InsertCollection failed: Declared %d collections, but provided %d.", this->collection_names.size(), m_data.size()); + } + auto frame = const_cast(event.GetSingle()); + size_t i = 0; + for (auto& datum : m_data) { + frame->put(std::move(std::move(datum)), m_collections[i]->GetCollectionName()); + const auto* moved = &frame->template get(m_collections[i]->GetCollectionName()); + datum = nullptr; + const auto &coll = dynamic_cast(m_collections[i]); + coll.SetCollectionAlreadyInFrame(moved); + } + } + void Reset() override { + m_data.clear(); + for (auto& coll : this->m_collections) { + coll->ClearData(); + } + for (auto& coll_name : this->collection_names) { + m_data.push_back(std::make_unique()); + } + } +}; + + +} // namespace jana::components + diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 5a1dc7b0f..32f65008d 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -53,6 +53,9 @@ void JFactory::Create(const std::shared_ptr& event) { mPreviousRunNumber = run_number; } CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event); }); + for (auto& output : this->GetOutputs()) { + output->PutCollections(*event); + } mStatus = Status::Processed; mCreationStatus = CreationStatus::Created; } diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index fa2e761fa..2f2cca45e 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -98,25 +98,28 @@ bool JFactorySet::Add(JFactory* aFactory) } else { // We have a new-style JFactory! - for (const auto& coll : aFactory->GetOutputCollections()) { - auto named_result = mCollectionsFromName.find(coll->GetCollectionName()); - if (named_result != std::end(mCollectionsFromName)) { - // Collection is duplicate. Since this almost certainly indicates a user error, and - // the caller will not be able to do anything about it anyway, throw an exception. - // We show the user which factory is causing this problem, including both plugin names - - // TODO: I haven't thought through insert vs process for collections yet - assert(named_result->second->GetFactory() != nullptr); - - std::string other_plugin_name = named_result->second->GetFactory()->GetPluginName(); - auto ex = JException("Attempted to add duplicate factories"); - ex.function_name = "JFactorySet::Add"; - ex.instance_name = aFactory->GetPrefix(); - ex.type_name = aFactory->GetTypeName(); - ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; - throw ex; + for (const auto* output : aFactory->GetOutputs()) { + for (const auto& coll : output->GetCollections()) { + auto named_result = mCollectionsFromName.find(coll->GetCollectionName()); + if (named_result != std::end(mCollectionsFromName)) { + // Collection is duplicate. Since this almost certainly indicates a user error, and + // the caller will not be able to do anything about it anyway, throw an exception. + // We show the user which factory is causing this problem, including both plugin names + + // TODO: I haven't thought through insert vs process for collections yet + assert(named_result->second->GetFactory() != nullptr); + + std::string other_plugin_name = named_result->second->GetFactory()->GetPluginName(); + auto ex = JException("Attempted to add duplicate factories"); + ex.function_name = "JFactorySet::Add"; + ex.instance_name = aFactory->GetPrefix(); + ex.type_name = aFactory->GetTypeName(); + ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; + throw ex; + } + coll->SetFactory(aFactory); + mCollectionsFromName[coll->GetCollectionName()] = coll.get(); } - mCollectionsFromName[coll->GetCollectionName()] = coll.get(); } } return true; diff --git a/src/programs/unit_tests/Components/JCollectionTests.cc b/src/programs/unit_tests/Components/JCollectionTests.cc index a5cbaeb8b..18515a7a8 100644 --- a/src/programs/unit_tests/Components/JCollectionTests.cc +++ b/src/programs/unit_tests/Components/JCollectionTests.cc @@ -3,20 +3,23 @@ // Copyright 2023, Jefferson Science Associates, LLC. // Subject to the terms in the LICENSE file found in the top-level directory. -#include "JANA/JEventSource.h" +#include "JANA/Components/JPodioCollection.h" #include "catch.hpp" #include #include +#include +#include #include #include -#include -#include +#include +#include +#include -namespace omnifactory_tests { +namespace jcollection_tests { struct TestSource : public JEventSource { Parameter y {this, "y", "asdf", "Does something"}; - TestSource(std::string res_name, JApplication* app) { + TestSource(std::string, JApplication*) { SetCallbackStyle(CallbackStyle::ExpertMode); } static std::string GetDescription() { @@ -32,6 +35,8 @@ struct TestSource : public JEventSource { struct TestFactory : public JFactory { Parameter x {this, "x", 22, "Does something"}; + jana::components::PodioOutput m_clusters {this, "my_collection"}; + TestFactory() { SetCallbackStyle(CallbackStyle::ExpertMode); } @@ -39,6 +44,19 @@ struct TestFactory : public JFactory { REQUIRE(x() == 23); } void Process(const std::shared_ptr& event) { + m_clusters()->push_back(MutableExampleCluster(22.2)); + m_clusters()->push_back(MutableExampleCluster(27)); + } +}; + +struct TestProc : public JEventProcessor { + + PodioInput m_clusters_in {this, InputOptions("my_collection")}; + + void Process(const std::shared_ptr& event) { + REQUIRE(m_clusters_in() != nullptr); + REQUIRE(m_clusters_in()->size() == 2); + std::cout << "Proc found data: " << m_clusters_in() << std::endl; } }; @@ -46,10 +64,10 @@ TEST_CASE("ParametersTest") { JApplication app; app.Add(new JEventSourceGeneratorT); app.Add(new JFactoryGeneratorT); + app.Add(new TestProc); app.Add("fake_file.root"); app.SetParameterValue("jana:nevents", 2); - app.SetParameterValue("autoactivate", ""); app.Run(); } -} +} // namespace jcollection_tests From 3df7ddc6922b1f05b22c2367b89a95f63eaf4e0c Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sat, 24 Aug 2024 18:01:15 -0400 Subject: [PATCH 05/24] Test cases pass --- src/libraries/JANA/Components/JPodioOutput.h | 38 +++++++++-- src/libraries/JANA/JEvent.h | 4 +- .../unit_tests/Components/JCollectionTests.cc | 63 ++++++++++++++++++- 3 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h index 17214093b..95f6f7761 100644 --- a/src/libraries/JANA/Components/JPodioOutput.h +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -1,8 +1,9 @@ #pragma once -#include "JANA/Components/JPodioCollection.h" -#include "JANA/Utils/JTypeInfo.h" +#include +#include #include #include +#include namespace jana::components { @@ -22,6 +23,7 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { coll->SetTypeName(JTypeInfo::demangle()); m_collection = coll.get(); m_collections.push_back(std::move(coll)); + m_data = std::move(std::make_unique()); } std::unique_ptr& operator()() { return m_data; } @@ -31,7 +33,19 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { //} void PutCollections(const JEvent& event) override { - auto frame = const_cast(event.GetSingle()); + podio::Frame* frame; + try { + frame = const_cast(event.GetSingle()); + if (frame == nullptr) { + frame = new podio::Frame; + event.Insert(frame); + } + } + catch (...) { + frame = new podio::Frame; + event.Insert(frame); + } + frame->put(std::move(m_data), m_collection->GetCollectionName()); const auto* moved = &frame->template get(m_collection->GetCollectionName()); m_data = nullptr; @@ -60,12 +74,28 @@ class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { coll->SetTypeName(JTypeInfo::demangle()); m_collections.push_back(std::move(coll)); } + for (auto& coll_name : this->collection_names) { + m_data.push_back(std::make_unique()); + } } void PutCollections(const JEvent& event) override { if (m_data.size() != this->collection_names.size()) { throw JException("VariadicPodioOutput InsertCollection failed: Declared %d collections, but provided %d.", this->collection_names.size(), m_data.size()); } - auto frame = const_cast(event.GetSingle()); + + podio::Frame* frame; + try { + frame = const_cast(event.GetSingle()); + if (frame == nullptr) { + frame = new podio::Frame; + event.Insert(frame); + } + } + catch (...) { + frame = new podio::Frame; + event.Insert(frame); + } + size_t i = 0; for (auto& datum : m_data) { frame->put(std::move(std::move(datum)), m_collections[i]->GetCollectionName()); diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 9d209957b..b6ae6c2dc 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -537,7 +537,7 @@ inline std::vector JEvent::GetAllCollectionNames() const { inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { // First try finding it using the new collection model - auto* coll = CreateAndGetCollection(name, throw_on_missing); + auto* coll = CreateAndGetCollection(name, false); if (coll != nullptr) { auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { @@ -572,7 +572,7 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, template const typename JFactoryPodioT::CollectionT* JEvent::GetCollection(std::string name, bool throw_on_missing) const { // First try finding it using the new collection model - auto* coll = CreateAndGetCollection(name, throw_on_missing); + auto* coll = CreateAndGetCollection(name, false); if (coll != nullptr) { auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { diff --git a/src/programs/unit_tests/Components/JCollectionTests.cc b/src/programs/unit_tests/Components/JCollectionTests.cc index 18515a7a8..86bf95b4a 100644 --- a/src/programs/unit_tests/Components/JCollectionTests.cc +++ b/src/programs/unit_tests/Components/JCollectionTests.cc @@ -4,6 +4,8 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #include "JANA/Components/JPodioCollection.h" +#include "JANA/Services/JComponentManager.h" +#include "PodioDatamodel/ExampleCluster.h" #include "catch.hpp" #include #include @@ -41,7 +43,7 @@ struct TestFactory : public JFactory { SetCallbackStyle(CallbackStyle::ExpertMode); } void Init() { - REQUIRE(x() == 23); + REQUIRE(x() == 22); } void Process(const std::shared_ptr& event) { m_clusters()->push_back(MutableExampleCluster(22.2)); @@ -53,14 +55,21 @@ struct TestProc : public JEventProcessor { PodioInput m_clusters_in {this, InputOptions("my_collection")}; - void Process(const std::shared_ptr& event) { + TestProc() { + SetCallbackStyle(CallbackStyle::ExpertMode); + } + + void Process(const JEvent& event) { + auto clusters = event.GetCollection("my_collection", true); + REQUIRE(clusters->size() == 2); + REQUIRE(m_clusters_in() != nullptr); REQUIRE(m_clusters_in()->size() == 2); std::cout << "Proc found data: " << m_clusters_in() << std::endl; } }; -TEST_CASE("ParametersTest") { +TEST_CASE("JCollectionEndToEndTest") { JApplication app; app.Add(new JEventSourceGeneratorT); app.Add(new JFactoryGeneratorT); @@ -70,4 +79,52 @@ TEST_CASE("ParametersTest") { app.Run(); } +TEST_CASE("JCollectionTests_GetCollectionBase") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.Initialize(); + + auto event = std::make_shared(); + app.GetService()->configure_event(*event); + + auto coll = event->GetCollectionBase("my_collection"); + REQUIRE(coll != nullptr); + REQUIRE(coll->size() == 2); + auto typed_coll = dynamic_cast(coll); + REQUIRE(typed_coll->at(1).energy() == 27); +} + +TEST_CASE("JCollectionTests_GetCollection") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.Initialize(); + + auto event = std::make_shared(); + app.GetService()->configure_event(*event); + + auto coll = event->GetCollection("my_collection"); + REQUIRE(coll != nullptr); + REQUIRE(coll->size() == 2); + REQUIRE(coll->at(1).energy() == 27); +} + +TEST_CASE("JCollectionTests_InsertCollection") { + JApplication app; + app.Initialize(); + + auto event = std::make_shared(); + app.GetService()->configure_event(*event); + + ExampleClusterCollection coll_in; + coll_in.create(15.0); + coll_in.create(30.0); + coll_in.create(88.0); + event->InsertCollection(std::move(coll_in), "sillyclusters"); + + auto coll_out = event->GetCollection("sillyclusters"); + REQUIRE(coll_out != nullptr); + REQUIRE(coll_out->size() == 3); + REQUIRE(coll_out->at(1).energy() == 30); +} + } // namespace jcollection_tests From afc8f893222391fdcb962aa63a361c3ba8549090 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 25 Aug 2024 00:31:09 -0400 Subject: [PATCH 06/24] JEvent::InsertCollection uses JCollection --- src/libraries/JANA/JEvent.h | 81 +++++++++++++++---------------- src/libraries/JANA/JFactorySet.cc | 53 +++++++++++--------- src/libraries/JANA/JFactorySet.h | 1 + 3 files changed, 70 insertions(+), 65 deletions(-) diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index b6ae6c2dc..cd9733db1 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -96,9 +97,9 @@ class JEvent : public std::enable_shared_from_this #if JANA2_HAVE_PODIO std::vector GetAllCollectionNames() const; const podio::CollectionBase* GetCollectionBase(std::string name, bool throw_on_missing=true) const; - template const typename JFactoryPodioT::CollectionT* GetCollection(std::string name, bool throw_on_missing=true) const; - template JFactoryPodioT* InsertCollection(typename JFactoryPodioT::CollectionT&& collection, std::string name); - template JFactoryPodioT* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); + template const typename PodioT::collection_type* GetCollection(std::string name, bool throw_on_missing=true) const; + template JPodioCollection* InsertCollection(typename PodioT::collection_type&& collection, std::string name); + template JPodioCollection* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); #endif // EXPERIMENTAL NEW THING @@ -570,7 +571,7 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, } template -const typename JFactoryPodioT::CollectionT* JEvent::GetCollection(std::string name, bool throw_on_missing) const { +const typename T::collection_type* JEvent::GetCollection(std::string name, bool throw_on_missing) const { // First try finding it using the new collection model auto* coll = CreateAndGetCollection(name, false); if (coll != nullptr) { @@ -598,66 +599,60 @@ const typename JFactoryPodioT::CollectionT* JEvent::GetCollection(std::string } -template -JFactoryPodioT* JEvent::InsertCollection(typename JFactoryPodioT::CollectionT&& collection, std::string name) { +template +JPodioCollection* JEvent::InsertCollection(typename PodioT::collection_type&& collection, std::string name) { /// InsertCollection inserts the provided PODIO collection into both the podio::Frame and then a JFactoryPodioT auto frame = GetOrCreateFrame(shared_from_this()); const auto& owned_collection = frame->put(std::move(collection), name); - return InsertCollectionAlreadyInFrame(&owned_collection, name); + return InsertCollectionAlreadyInFrame(&owned_collection, name); } -template -JFactoryPodioT* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) { - /// InsertCollection inserts the provided PODIO collection into a JFactoryPodioT. It assumes that the collection pointer +template +JPodioCollection* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) { + /// InsertCollection inserts the provided PODIO collection into a JPodioCollection. It assumes that the collection pointer /// is _already_ owned by the podio::Frame corresponding to this JEvent. This is meant to be used if you are starting out /// with a PODIO frame (e.g. a JEventSource that uses podio::ROOTFrameReader). - const auto* typed_collection = dynamic_cast(collection); + const auto* typed_collection = dynamic_cast(collection); if (typed_collection == nullptr) { throw JException("Attempted to insert a collection of the wrong type! name='%s', expected type='%s', actual type='%s'", - name.c_str(), JTypeInfo::demangle().c_str(), collection->getDataTypeName().data()); + name.c_str(), JTypeInfo::demangle().c_str(), collection->getDataTypeName().data()); } // Users are allowed to Insert with tag="" if and only if that tag gets resolved by default tags. if (mUseDefaultTags && name.empty()) { - auto defaultTag = mDefaultTags.find(JTypeInfo::demangle()); + auto defaultTag = mDefaultTags.find(JTypeInfo::demangle()); if (defaultTag != mDefaultTags.end()) name = defaultTag->second; } - // Retrieve factory if it already exists, else create it - JFactoryT* factory = mFactorySet->GetFactory(name); - if (factory == nullptr) { - factory = new JFactoryPodioT(); - factory->SetTag(name); - factory->SetLevel(GetLevel()); - mFactorySet->Add(factory); - - auto it = mPodioFactories.find(name); - if (it != mPodioFactories.end()) { - throw JException("InsertCollection failed because tag '%s' is not unique", name.c_str()); - } - mPodioFactories[name] = factory; + // Retrieve storage if it already exists, else create it + auto storage = mFactorySet->GetCollection(name); + + if (storage == nullptr) { + // No factories already registered this! E.g. from an event source + auto coll = new JPodioCollection; + coll->SetCollectionName(name); + coll->SetTypeName(JTypeInfo::demangle()); + coll->SetCreationStatus(JCollection::CreationStatus::Inserted); + coll->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); + coll->SetCollectionAlreadyInFrame(typed_collection); + mFactorySet->Add(coll); + return coll; } - - // PODIO collections can only be inserted once, unlike regular JANA factories. - if (factory->GetStatus() == JFactory::Status::Inserted || - factory->GetStatus() == JFactory::Status::Processed) { - - throw JException("PODIO collections can only be inserted once, but factory with tag '%s' already has data", name.c_str()); - } - - // There's a chance that some user already added to the event's JFactorySet a - // JFactoryT which ISN'T a JFactoryPodioT. In this case, we cannot set the collection. - JFactoryPodioT* typed_factory = dynamic_cast*>(factory); - if (typed_factory == nullptr) { - throw JException("Factory must inherit from JFactoryPodioT in order to use JEvent::GetCollection()"); + else { + // This is overriding a factory + // Check that we only inserted this collection once + if (storage->GetCreationStatus() != JCollection::CreationStatus::NotCreatedYet) { + throw JException("Collections can only be inserted once!"); + } + auto typed_storage = dynamic_cast(storage); + typed_storage->SetCollectionAlreadyInFrame(typed_collection); + typed_storage->SetCreationStatus(JPodioCollection::CreationStatus::Inserted); + typed_storage->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); + return typed_storage; } - - typed_factory->SetCollectionAlreadyInFrame(typed_collection); - typed_factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); - return typed_factory; } #endif // JANA2_HAVE_PODIO diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 2f2cca45e..cb30eabd1 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -45,6 +45,34 @@ JFactorySet::~JFactorySet() for (auto* mf : mMultifactories) { delete mf; } } +//--------------------------------- +// Add +//--------------------------------- +void JFactorySet::Add(JCollection* collection) { + auto named_result = mCollectionsFromName.find(collection->GetCollectionName()); + if (named_result != std::end(mCollectionsFromName)) { + // Collection is duplicate. Since this almost certainly indicates a user error, and + // the caller will not be able to do anything about it anyway, throw an exception. + // We show the user which factory is causing this problem, including both plugin names + + auto ex = JException("Attempted to add duplicate collections"); + ex.function_name = "JFactorySet::Add"; + ex.instance_name = collection->GetCollectionName(); + + auto fac = collection->GetFactory(); + if (fac != nullptr) { + ex.type_name = fac->GetTypeName(); + ex.plugin_name = fac->GetPluginName(); + if (named_result->second->GetFactory() != nullptr) { + ex.plugin_name += ", " + named_result->second->GetFactory()->GetPluginName(); + } + } + throw ex; + } + // Note that this is agnostic to event level. We may decide to change this. + mCollectionsFromName[collection->GetCollectionName()] = collection; +} + //--------------------------------- // Add //--------------------------------- @@ -65,7 +93,6 @@ bool JFactorySet::Add(JFactory* aFactory) auto object_type = aFactory->GetObjectType(); if (object_type != std::nullopt) { - // We have an old-style JFactory! auto typed_key = std::make_pair( *object_type, aFactory->GetTag() ); @@ -100,25 +127,7 @@ bool JFactorySet::Add(JFactory* aFactory) // We have a new-style JFactory! for (const auto* output : aFactory->GetOutputs()) { for (const auto& coll : output->GetCollections()) { - auto named_result = mCollectionsFromName.find(coll->GetCollectionName()); - if (named_result != std::end(mCollectionsFromName)) { - // Collection is duplicate. Since this almost certainly indicates a user error, and - // the caller will not be able to do anything about it anyway, throw an exception. - // We show the user which factory is causing this problem, including both plugin names - - // TODO: I haven't thought through insert vs process for collections yet - assert(named_result->second->GetFactory() != nullptr); - - std::string other_plugin_name = named_result->second->GetFactory()->GetPluginName(); - auto ex = JException("Attempted to add duplicate factories"); - ex.function_name = "JFactorySet::Add"; - ex.instance_name = aFactory->GetPrefix(); - ex.type_name = aFactory->GetTypeName(); - ex.plugin_name = aFactory->GetPluginName() + ", " + other_plugin_name; - throw ex; - } - coll->SetFactory(aFactory); - mCollectionsFromName[coll->GetCollectionName()] = coll.get(); + Add(coll.get()); } } } @@ -149,8 +158,8 @@ bool JFactorySet::Add(JMultifactory *multifactory) { JCollection* JFactorySet::GetCollection(const std::string& collection_name) const { auto it = mCollectionsFromName.find(collection_name); if (it != std::end(mCollectionsFromName)) { - assert(it->second->GetFactory() != nullptr); - if (it->second->GetFactory()->GetLevel() != mLevel) { + auto fac = it->second->GetFactory(); + if (fac != nullptr && fac->GetLevel() != mLevel) { throw JException("Collection belongs to a different level on the event hierarchy!"); } return it->second; diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 1b3c2e2be..8c1d0d87a 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -35,6 +35,7 @@ class JFactorySet { bool Add(JFactory* aFactory); bool Add(JMultifactory* multifactory); + void Add(JCollection* collection); void Print(void) const; void Release(void); From 940b231d81258bf81a536a72b6263e8c3cf5ab8a Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 2 Sep 2024 20:09:50 -0400 Subject: [PATCH 07/24] JFactoryPodioT uses JPodioCollection --- src/libraries/JANA/Components/JPodioOutput.h | 3 + src/libraries/JANA/JEvent.h | 68 ++------ src/libraries/JANA/JMultifactory.cc | 5 - src/libraries/JANA/JMultifactory.h | 18 +-- src/libraries/JANA/Podio/JFactoryPodioT.cc | 23 --- src/libraries/JANA/Podio/JFactoryPodioT.h | 148 ++---------------- .../unit_tests/Components/PodioTests.cc | 1 + 7 files changed, 36 insertions(+), 230 deletions(-) delete mode 100644 src/libraries/JANA/Podio/JFactoryPodioT.cc diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h index 95f6f7761..a0c26bb8b 100644 --- a/src/libraries/JANA/Components/JPodioOutput.h +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -28,6 +28,9 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { std::unique_ptr& operator()() { return m_data; } + const JCollection* GetCollection() const { return m_collection; } + + protected: //void CreateCollections() override { //} diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index cd9733db1..06a868949 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -24,7 +24,6 @@ #include #if JANA2_HAVE_PODIO -#include #include #endif @@ -48,19 +47,6 @@ class JEvent : public std::enable_shared_from_this void SetFactorySet(JFactorySet* aFactorySet) { delete mFactorySet; mFactorySet = aFactorySet; -#if JANA2_HAVE_PODIO - // Maintain the index of PODIO factories - for (JFactory* factory : mFactorySet->GetAllFactories()) { - if (dynamic_cast(factory) != nullptr) { - auto tag = factory->GetTag(); - auto it = mPodioFactories.find(tag); - if (it != mPodioFactories.end()) { - throw JException("SetFactorySet failed because PODIO factory tag '%s' is not unique", tag.c_str()); - } - mPodioFactories[tag] = factory; - } - } -#endif } JFactorySet* GetFactorySet() const { return mFactorySet; } @@ -537,8 +523,7 @@ inline std::vector JEvent::GetAllCollectionNames() const { } inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { - // First try finding it using the new collection model - auto* coll = CreateAndGetCollection(name, false); + auto* coll = CreateAndGetCollection(name, throw_on_missing); if (coll != nullptr) { auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { @@ -548,32 +533,12 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, return podio_coll->GetCollection(); } } - - // We couldn't find it using the new collection model, so now we search mPodioFactories - auto it = mPodioFactories.find(name); - if (it == mPodioFactories.end()) { - if (throw_on_missing) { - throw JException("No factory with tag '%s' found", name.c_str()); - } - else { - return nullptr; - } - } - JFactoryPodio* factory = dynamic_cast(it->second); - if (factory == nullptr) { - // Should be no way to get here if we encapsulate mPodioFactories correctly - throw JException("Factory with tag '%s' does not inherit from JFactoryPodio!", name.c_str()); - } - JCallGraphEntryMaker cg_entry(mCallGraph, it->second); // times execution until this goes out of scope - it->second->Create(this->shared_from_this()); - return factory->GetCollection(); - // TODO: Might be cheaper/simpler to obtain factory from mPodioFactories instead of mFactorySet + return nullptr; } template const typename T::collection_type* JEvent::GetCollection(std::string name, bool throw_on_missing) const { - // First try finding it using the new collection model - auto* coll = CreateAndGetCollection(name, false); + auto* coll = CreateAndGetCollection(name, throw_on_missing); if (coll != nullptr) { auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { @@ -583,19 +548,7 @@ const typename T::collection_type* JEvent::GetCollection(std::string name, bool return podio_coll->GetCollection(); } } - - // Next try finding it using the old way - JFactoryT* factory = GetFactory(name, throw_on_missing); - if (factory == nullptr) { - return nullptr; - } - JFactoryPodioT* typed_factory = dynamic_cast*>(factory); - if (typed_factory == nullptr) { - throw JException("Factory must inherit from JFactoryPodioT in order to use JEvent::GetCollection()"); - } - JCallGraphEntryMaker cg_entry(mCallGraph, typed_factory); // times execution until this goes out of scope - typed_factory->Create(this->shared_from_this()); - return static_cast::CollectionT*>(typed_factory->GetCollection()); + return nullptr; } @@ -603,7 +556,18 @@ template JPodioCollection* JEvent::InsertCollection(typename PodioT::collection_type&& collection, std::string name) { /// InsertCollection inserts the provided PODIO collection into both the podio::Frame and then a JFactoryPodioT - auto frame = GetOrCreateFrame(shared_from_this()); + podio::Frame* frame = nullptr; + try { + frame = const_cast(GetSingle("")); + if (frame == nullptr) { + frame = new podio::Frame; + Insert(frame); + } + } + catch (...) { + frame = new podio::Frame; + Insert(frame); + } const auto& owned_collection = frame->put(std::move(collection), name); return InsertCollectionAlreadyInFrame(&owned_collection, name); } diff --git a/src/libraries/JANA/JMultifactory.cc b/src/libraries/JANA/JMultifactory.cc index e6da04fc1..389085792 100644 --- a/src/libraries/JANA/JMultifactory.cc +++ b/src/libraries/JANA/JMultifactory.cc @@ -10,11 +10,6 @@ void JMultifactory::Execute(const std::shared_ptr& event) { std::lock_guard lock(m_mutex); -#if JANA2_HAVE_PODIO - if (mNeedPodio) { - mPodioFrame = GetOrCreateFrame(event); - } -#endif if (m_status == Status::Uninitialized) { CallWithJExceptionWrapper("JMultifactory::Init", [&](){ diff --git a/src/libraries/JANA/JMultifactory.h b/src/libraries/JANA/JMultifactory.h index d41dc1442..2e9ce2125 100644 --- a/src/libraries/JANA/JMultifactory.h +++ b/src/libraries/JANA/JMultifactory.h @@ -74,10 +74,6 @@ class JMultifactory : public jana::components::JComponent, // However, don't worry about a Status variable. Every time Execute() gets called, so does Process(). // The JMultifactoryHelpers will control calls to Execute(). -#if JANA2_HAVE_PODIO - bool mNeedPodio = false; // Whether we need to retrieve the podio::Frame - podio::Frame* mPodioFrame = nullptr; // To provide the podio::Frame to SetPodioData, SetCollection -#endif public: JMultifactory() = default; @@ -165,13 +161,7 @@ void JMultifactory::SetData(std::string tag, std::vector data) { ex.plugin_name = m_plugin_name; throw ex; } -#if JANA2_HAVE_PODIO - // This may or may not be a Podio factory. We find out if it is, and if so, set the frame before calling Set(). - auto* typed = dynamic_cast(helper); - if (typed != nullptr) { - typed->SetFrame(mPodioFrame); // Needs to be called before helper->Set(), otherwise Set() excepts - } -#endif + // This will except if helper is a JFactoryPodioT. User should use SetPodioData() instead for PODIO data. helper->Set(data); } @@ -179,10 +169,9 @@ void JMultifactory::SetData(std::string tag, std::vector data) { #if JANA2_HAVE_PODIO template -void JMultifactory::DeclarePodioOutput(std::string tag, bool owns_data) { +void JMultifactory::DeclarePodioOutput(std::string tag, bool) { // TODO: Decouple tag name from collection name auto* helper = new JMultifactoryHelperPodio(this); - if (!owns_data) helper->SetSubsetCollection(true); helper->SetTag(std::move(tag)); helper->SetPluginName(m_plugin_name); @@ -190,7 +179,6 @@ void JMultifactory::DeclarePodioOutput(std::string tag, bool owns_data) { helper->SetLevel(GetLevel()); mHelpers.SetLevel(GetLevel()); mHelpers.Add(helper); - mNeedPodio = true; } template @@ -214,7 +202,6 @@ void JMultifactory::SetCollection(std::string tag, typename JFactoryPodioT::C throw ex; } - typed->SetFrame(mPodioFrame); typed->SetCollection(std::move(collection)); } @@ -239,7 +226,6 @@ void JMultifactory::SetCollection(std::string tag, std::unique_ptrSetFrame(mPodioFrame); typed->SetCollection(std::move(collection)); } diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.cc b/src/libraries/JANA/Podio/JFactoryPodioT.cc deleted file mode 100644 index 506948502..000000000 --- a/src/libraries/JANA/Podio/JFactoryPodioT.cc +++ /dev/null @@ -1,23 +0,0 @@ - -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - -#include "JFactoryPodioT.h" -#include - -podio::Frame* GetOrCreateFrame(const std::shared_ptr& event) { - podio::Frame* result = nullptr; - try { - result = const_cast(event->GetSingle("")); - if (result == nullptr) { - result = new podio::Frame; - event->Insert(result); - } - } - catch (...) { - result = new podio::Frame; - event->Insert(result); - } - return result; -} - diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index 0c91ce146..2b33172ed 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -6,41 +6,16 @@ #pragma once #include -#include - -/// The point of this additional base class is to allow us _untyped_ access to the underlying PODIO collection, -/// at the cost of some weird multiple inheritance. The JEvent can trigger the untyped factory using Create(), then -/// -class JFactoryPodio { -protected: - const podio::CollectionBase* mCollection = nullptr; - bool mIsSubsetCollection = false; - podio::Frame* mFrame = nullptr; - -private: - // Meant to be called internally, from JMultifactory - friend class JMultifactory; - void SetFrame(podio::Frame* frame) { mFrame = frame; } - - // Meant to be called internally, from JEvent: - friend class JEvent; - const podio::CollectionBase* GetCollection() { return mCollection; } - -public: - // Meant to be called from ctor, or externally, if we are creating a dummy factory such as a multifactory helper - void SetSubsetCollection(bool isSubsetCollection=true) { mIsSubsetCollection = isSubsetCollection; } -}; +#include template -class JFactoryPodioT : public JFactoryT, public JFactoryPodio { +class JFactoryPodioT : public JFactoryT { public: using CollectionT = typename T::collection_type; + private: - // mCollection is owned by the frame. - // mFrame is owned by the JFactoryT. - // mData holds lightweight value objects which hold a pointer into mCollection. - // This factory owns these value objects. + jana::omni::PodioOutput m_output {this}; public: explicit JFactoryPodioT(); @@ -53,9 +28,8 @@ class JFactoryPodioT : public JFactoryT, public JFactoryPodio { void EndRun() override {} void Finish() override {} - void Create(const std::shared_ptr& event) final; std::optional GetObjectType() const final { return std::type_index(typeid(T)); } - std::size_t GetNumObjects() const final { return mCollection->size(); } + std::size_t GetNumObjects() const final { return m_output.GetCollection()->GetSize(); } void ClearData() final; void SetCollection(CollectionT&& collection); @@ -64,13 +38,6 @@ class JFactoryPodioT : public JFactoryT, public JFactoryPodio { void Set(std::vector&& aData) final; void Insert(T* aDatum) final; - - -private: - // This is meant to be called by JEvent::Insert - friend class JEvent; - void SetCollectionAlreadyInFrame(const CollectionT* collection); - }; @@ -88,18 +55,7 @@ void JFactoryPodioT::SetCollection(CollectionT&& collection) { /// Provide a PODIO collection. Note that PODIO assumes ownership of this collection, and the /// collection pointer should be assumed to be invalid after this call - if (this->mFrame == nullptr) { - throw JException("JFactoryPodioT: Unable to add collection to frame as frame is missing!"); - } - const auto& moved = this->mFrame->put(std::move(collection), this->GetTag()); - this->mCollection = &moved; - - for (const T& item : moved) { - T* clone = new T(item); - this->mData.push_back(clone); - } - this->mStatus = JFactory::Status::Inserted; - this->mCreationStatus = JFactory::CreationStatus::Inserted; + m_output() = std::make_unique(std::move(collection)); } @@ -108,103 +64,27 @@ void JFactoryPodioT::SetCollection(std::unique_ptr collection) { /// Provide a PODIO collection. Note that PODIO assumes ownership of this collection, and the /// collection pointer should be assumed to be invalid after this call - if (this->mFrame == nullptr) { - throw JException("JFactoryPodioT: Unable to add collection to frame as frame is missing!"); - } - this->mFrame->put(std::move(collection), this->GetTag()); - const auto* moved = &this->mFrame->template get(this->GetTag()); - this->mCollection = moved; - - for (const T& item : *moved) { - T* clone = new T(item); - this->mData.push_back(clone); - } - this->mStatus = JFactory::Status::Inserted; - this->mCreationStatus = JFactory::CreationStatus::Inserted; + m_output() = std::move(collection); } template void JFactoryPodioT::ClearData() { - if (this->mStatus == JFactory::Status::Uninitialized) { - return; - } - for (auto p : this->mData) { - // Avoid potentially invalid call to ObjBase::release(). The frame and - // all the collections and all Obj may have been deallocated at this point. - p->unlink(); - delete p; - } - this->mData.clear(); - this->mCollection = nullptr; // Collection is owned by the Frame, so we ignore here - this->mFrame = nullptr; // Frame is owned by the JEvent, so we ignore here - this->mStatus = JFactory::Status::Unprocessed; - this->mCreationStatus = JFactory::CreationStatus::NotCreatedYet; -} - -template -void JFactoryPodioT::SetCollectionAlreadyInFrame(const CollectionT* collection) { - for (const T& item : *collection) { - T* clone = new T(item); - this->mData.push_back(clone); - } - this->mCollection = collection; - this->mStatus = JFactory::Status::Inserted; - this->mCreationStatus = JFactory::CreationStatus::Inserted; -} - -// This free function is used to break the dependency loop between JFactoryPodioT and JEvent. -podio::Frame* GetOrCreateFrame(const std::shared_ptr& event); - -template -void JFactoryPodioT::Create(const std::shared_ptr& event) { - mFrame = GetOrCreateFrame(event); - try { - JFactory::Create(event); - } - catch (...) { - if (mCollection == nullptr) { - // If calling Create() excepts, we still create an empty collection - // so that podio::ROOTFrameWriter doesn't segfault on the null mCollection pointer - SetCollection(CollectionT()); - } - throw; - } - if (mCollection == nullptr) { - SetCollection(CollectionT()); - // If calling Process() didn't result in a call to Set() or SetCollection(), we create an empty collection - // so that podio::ROOTFrameWriter doesn't segfault on the null mCollection pointer - } + // Happens automatically now } template -void JFactoryPodioT::Set(const std::vector& aData) { - CollectionT collection; - if (mIsSubsetCollection) collection.setSubsetCollection(true); - for (T* item : aData) { - collection.push_back(*item); - delete item; - } - SetCollection(std::move(collection)); +void JFactoryPodioT::Set(const std::vector&) { + throw JException("Not supported!"); } template -void JFactoryPodioT::Set(std::vector&& aData) { - CollectionT collection; - if (mIsSubsetCollection) collection.setSubsetCollection(true); - for (T* item : aData) { - collection.push_back(*item); - delete item; - } - SetCollection(std::move(collection)); +void JFactoryPodioT::Set(std::vector&&) { + throw JException("Not supported!"); } template -void JFactoryPodioT::Insert(T* aDatum) { - CollectionT collection; - if (mIsSubsetCollection) collection->setSubsetCollection(true); - collection->push_back(*aDatum); - delete aDatum; - SetCollection(std::move(collection)); +void JFactoryPodioT::Insert(T*) { + throw JException("Not supported!"); } diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index c0bedb560..7d45ebada 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -4,6 +4,7 @@ #include #include #include +#include namespace podiotests { From 738374ecc06acfaa0a5ecb6894a12f5a2dee34d3 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 5 Sep 2024 22:51:47 -0400 Subject: [PATCH 08/24] Rename JCollection to JStorage --- src/libraries/JANA/CMakeLists.txt | 5 --- .../{JBasicCollection.h => JBasicStorage.h} | 0 .../JANA/Components/JHasFactoryOutputs.h | 6 ++-- src/libraries/JANA/Components/JPodioOutput.h | 12 +++---- .../{JPodioCollection.h => JPodioStorage.h} | 16 +++++----- .../Components/{JCollection.h => JStorage.h} | 8 ++--- src/libraries/JANA/JEvent.h | 32 +++++++++---------- src/libraries/JANA/JFactorySet.cc | 6 ++-- src/libraries/JANA/JFactorySet.h | 8 ++--- src/libraries/JANA/Podio/JFactoryPodioT.h | 4 +-- src/programs/unit_tests/CMakeLists.txt | 2 +- .../{JCollectionTests.cc => JStorageTests.cc} | 12 +++---- 12 files changed, 53 insertions(+), 58 deletions(-) rename src/libraries/JANA/Components/{JBasicCollection.h => JBasicStorage.h} (100%) rename src/libraries/JANA/Components/{JPodioCollection.h => JPodioStorage.h} (80%) rename src/libraries/JANA/Components/{JCollection.h => JStorage.h} (97%) rename src/programs/unit_tests/Components/{JCollectionTests.cc => JStorageTests.cc} (93%) diff --git a/src/libraries/JANA/CMakeLists.txt b/src/libraries/JANA/CMakeLists.txt index ffb7ea743..375d1f3e2 100644 --- a/src/libraries/JANA/CMakeLists.txt +++ b/src/libraries/JANA/CMakeLists.txt @@ -53,11 +53,6 @@ set(JANA2_SOURCES Compatibility/md5.c ) -if (${USE_PODIO}) - list(APPEND JANA2_SOURCES - Podio/JFactoryPodioT.cc - ) -endif() if (NOT ${USE_XERCES}) message(STATUS "Skipping support for libJANA's JGeometryXML because USE_XERCES=Off") diff --git a/src/libraries/JANA/Components/JBasicCollection.h b/src/libraries/JANA/Components/JBasicStorage.h similarity index 100% rename from src/libraries/JANA/Components/JBasicCollection.h rename to src/libraries/JANA/Components/JBasicStorage.h diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h index 813e29a43..5dfbd93fc 100644 --- a/src/libraries/JANA/Components/JHasFactoryOutputs.h +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include class JEvent; @@ -13,10 +13,10 @@ class JHasFactoryOutputs { struct OutputBase { protected: std::vector m_collection_names; // TODO: Possibly don't need anymore - std::vector> m_collections; + std::vector> m_collections; bool m_is_variadic = false; public: - const std::vector>& GetCollections() const { return m_collections;} + const std::vector>& GetCollections() const { return m_collections;} virtual void PutCollections(const JEvent& event) = 0; virtual void Reset() = 0; }; diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h index a0c26bb8b..113a9c3eb 100644 --- a/src/libraries/JANA/Components/JPodioOutput.h +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -1,5 +1,5 @@ #pragma once -#include +#include #include #include #include @@ -13,12 +13,12 @@ template class PodioOutput : public JHasFactoryOutputs::OutputBase { private: std::unique_ptr m_data; - JPodioCollection* m_collection; + JPodioStorage* m_collection; public: PodioOutput(JHasFactoryOutputs* owner, std::string default_collection_name="") { owner->RegisterOutput(this); this->m_collection_names.push_back(default_collection_name); - auto coll = std::make_unique(); + auto coll = std::make_unique(); coll->SetCollectionName(default_collection_name); coll->SetTypeName(JTypeInfo::demangle()); m_collection = coll.get(); @@ -28,7 +28,7 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { std::unique_ptr& operator()() { return m_data; } - const JCollection* GetCollection() const { return m_collection; } + const JStorage* GetCollection() const { return m_collection; } protected: @@ -72,7 +72,7 @@ class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { this->m_collection_names = default_collection_names; this->m_is_variadic = true; for (const std::string& name : default_collection_names) { - auto coll = std::make_unique(); + auto coll = std::make_unique(); coll->SetCollectionName(name); coll->SetTypeName(JTypeInfo::demangle()); m_collections.push_back(std::move(coll)); @@ -104,7 +104,7 @@ class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { frame->put(std::move(std::move(datum)), m_collections[i]->GetCollectionName()); const auto* moved = &frame->template get(m_collections[i]->GetCollectionName()); datum = nullptr; - const auto &coll = dynamic_cast(m_collections[i]); + const auto &coll = dynamic_cast(m_collections[i]); coll.SetCollectionAlreadyInFrame(moved); } } diff --git a/src/libraries/JANA/Components/JPodioCollection.h b/src/libraries/JANA/Components/JPodioStorage.h similarity index 80% rename from src/libraries/JANA/Components/JPodioCollection.h rename to src/libraries/JANA/Components/JPodioStorage.h index 70d2a8cc9..9fa07fb07 100644 --- a/src/libraries/JANA/Components/JPodioCollection.h +++ b/src/libraries/JANA/Components/JPodioStorage.h @@ -4,13 +4,13 @@ #pragma once #include "JANA/Utils/JTypeInfo.h" -#include +#include #include #include #include -class JPodioCollection : public JCollection { +class JPodioStorage : public JStorage { private: const podio::CollectionBase* m_collection = nullptr; @@ -52,33 +52,33 @@ class JPodioCollection : public JCollection { }; template -const typename T::collection_type* JPodioCollection::GetCollection() { +const typename T::collection_type* JPodioStorage::GetCollection() { assert(JTypeInfo::demangle() == this->GetTypeName()); return dynamic_cast(m_collection); } template -void JPodioCollection::SetCollection(std::unique_ptr collection) { +void JPodioStorage::SetCollection(std::unique_ptr collection) { /// Provide a PODIO collection. Note that PODIO assumes ownership of this collection, and the /// collection pointer should be assumed to be invalid after this call if (this->m_frame == nullptr) { - throw JException("JPodioCollection: Unable to add collection to frame as frame is missing!"); + throw JException("JPodioStorage: Unable to add collection to frame as frame is missing!"); } this->m_frame->put(std::move(collection), this->GetCollectionName()); const auto* moved = &this->m_frame->template get(this->GetCollectionName()); this->m_collection = moved; this->SetTypeName(JTypeInfo::demangle()); - this->SetCreationStatus(JCollection::CreationStatus::Inserted); + this->SetCreationStatus(JStorage::CreationStatus::Inserted); } template -void JPodioCollection::SetCollectionAlreadyInFrame(const typename T::collection_type* collection) { +void JPodioStorage::SetCollectionAlreadyInFrame(const typename T::collection_type* collection) { m_collection = collection; this->SetTypeName(JTypeInfo::demangle()); - SetCreationStatus(JPodioCollection::CreationStatus::Inserted); + SetCreationStatus(JPodioStorage::CreationStatus::Inserted); } diff --git a/src/libraries/JANA/Components/JCollection.h b/src/libraries/JANA/Components/JStorage.h similarity index 97% rename from src/libraries/JANA/Components/JCollection.h rename to src/libraries/JANA/Components/JStorage.h index a7e077fcb..ceea07189 100644 --- a/src/libraries/JANA/Components/JCollection.h +++ b/src/libraries/JANA/Components/JStorage.h @@ -15,7 +15,7 @@ class JFactory; -class JCollection { +class JStorage { public: // Typedefs enum class CreationStatus { NotCreatedYet, Created, Inserted, InsertedViaGetObjects, NeverCreated }; @@ -34,8 +34,8 @@ class JCollection { public: // Interface - JCollection() = default; - virtual ~JCollection() = default; + JStorage() = default; + virtual ~JStorage() = default; virtual size_t GetSize() const = 0; virtual void ClearData() = 0; @@ -82,7 +82,7 @@ class JCollection { // 3. The size of the vtable is expected to be very small (<10 elements, most likely 2) template -std::vector JCollection::GetAs() { +std::vector JStorage::GetAs() { std::vector results; auto ti = std::type_index(typeid(S)); auto search = mUpcastVTable.find(ti); diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 06a868949..7ba4cff2f 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include @@ -24,7 +24,7 @@ #include #if JANA2_HAVE_PODIO -#include +#include #endif class JApplication; @@ -84,12 +84,12 @@ class JEvent : public std::enable_shared_from_this std::vector GetAllCollectionNames() const; const podio::CollectionBase* GetCollectionBase(std::string name, bool throw_on_missing=true) const; template const typename PodioT::collection_type* GetCollection(std::string name, bool throw_on_missing=true) const; - template JPodioCollection* InsertCollection(typename PodioT::collection_type&& collection, std::string name); - template JPodioCollection* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); + template JPodioStorage* InsertCollection(typename PodioT::collection_type&& collection, std::string name); + template JPodioStorage* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); #endif // EXPERIMENTAL NEW THING - JCollection* CreateAndGetCollection(std::string name, bool throw_on_missing) const; + JStorage* CreateAndGetCollection(std::string name, bool throw_on_missing) const; //SETTERS void SetRunNumber(int32_t aRunNumber){mRunNumber = aRunNumber;} @@ -491,7 +491,7 @@ JFactoryT* JEvent::GetSingle(const T* &t, const char *tag, bool exception_if_ return fac; } -inline JCollection* JEvent::CreateAndGetCollection(std::string name, bool throw_on_missing) const { +inline JStorage* JEvent::CreateAndGetCollection(std::string name, bool throw_on_missing) const { auto* coll = mFactorySet->GetCollection(name); if (coll == nullptr) { @@ -525,7 +525,7 @@ inline std::vector JEvent::GetAllCollectionNames() const { inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { auto* coll = CreateAndGetCollection(name, throw_on_missing); if (coll != nullptr) { - auto* podio_coll = dynamic_cast(coll); + auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { throw JException("Not a podio collection: %s", name.c_str()); } @@ -540,7 +540,7 @@ template const typename T::collection_type* JEvent::GetCollection(std::string name, bool throw_on_missing) const { auto* coll = CreateAndGetCollection(name, throw_on_missing); if (coll != nullptr) { - auto* podio_coll = dynamic_cast(coll); + auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { throw JException("Not a podio collection: %s", name.c_str()); } @@ -553,7 +553,7 @@ const typename T::collection_type* JEvent::GetCollection(std::string name, bool template -JPodioCollection* JEvent::InsertCollection(typename PodioT::collection_type&& collection, std::string name) { +JPodioStorage* JEvent::InsertCollection(typename PodioT::collection_type&& collection, std::string name) { /// InsertCollection inserts the provided PODIO collection into both the podio::Frame and then a JFactoryPodioT podio::Frame* frame = nullptr; @@ -574,8 +574,8 @@ JPodioCollection* JEvent::InsertCollection(typename PodioT::collection_type&& co template -JPodioCollection* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) { - /// InsertCollection inserts the provided PODIO collection into a JPodioCollection. It assumes that the collection pointer +JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) { + /// InsertCollection inserts the provided PODIO collection into a JPodioStorage. It assumes that the collection pointer /// is _already_ owned by the podio::Frame corresponding to this JEvent. This is meant to be used if you are starting out /// with a PODIO frame (e.g. a JEventSource that uses podio::ROOTFrameReader). @@ -596,10 +596,10 @@ JPodioCollection* JEvent::InsertCollectionAlreadyInFrame(const podio::Collection if (storage == nullptr) { // No factories already registered this! E.g. from an event source - auto coll = new JPodioCollection; + auto coll = new JPodioStorage; coll->SetCollectionName(name); coll->SetTypeName(JTypeInfo::demangle()); - coll->SetCreationStatus(JCollection::CreationStatus::Inserted); + coll->SetCreationStatus(JStorage::CreationStatus::Inserted); coll->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); coll->SetCollectionAlreadyInFrame(typed_collection); mFactorySet->Add(coll); @@ -608,12 +608,12 @@ JPodioCollection* JEvent::InsertCollectionAlreadyInFrame(const podio::Collection else { // This is overriding a factory // Check that we only inserted this collection once - if (storage->GetCreationStatus() != JCollection::CreationStatus::NotCreatedYet) { + if (storage->GetCreationStatus() != JStorage::CreationStatus::NotCreatedYet) { throw JException("Collections can only be inserted once!"); } - auto typed_storage = dynamic_cast(storage); + auto typed_storage = dynamic_cast(storage); typed_storage->SetCollectionAlreadyInFrame(typed_collection); - typed_storage->SetCreationStatus(JPodioCollection::CreationStatus::Inserted); + typed_storage->SetCreationStatus(JStorage::CreationStatus::Inserted); typed_storage->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); return typed_storage; } diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index cb30eabd1..d73376caf 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -6,7 +6,7 @@ #include #include "JFactorySet.h" -#include "JANA/Components/JCollection.h" +#include "JANA/Components/JStorage.h" #include "JFactory.h" #include "JMultifactory.h" #include "JFactoryGenerator.h" @@ -48,7 +48,7 @@ JFactorySet::~JFactorySet() //--------------------------------- // Add //--------------------------------- -void JFactorySet::Add(JCollection* collection) { +void JFactorySet::Add(JStorage* collection) { auto named_result = mCollectionsFromName.find(collection->GetCollectionName()); if (named_result != std::end(mCollectionsFromName)) { // Collection is duplicate. Since this almost certainly indicates a user error, and @@ -155,7 +155,7 @@ bool JFactorySet::Add(JMultifactory *multifactory) { //--------------------------------- // GetCollection //--------------------------------- -JCollection* JFactorySet::GetCollection(const std::string& collection_name) const { +JStorage* JFactorySet::GetCollection(const std::string& collection_name) const { auto it = mCollectionsFromName.find(collection_name); if (it != std::end(mCollectionsFromName)) { auto fac = it->second->GetFactory(); diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 8c1d0d87a..7b26e8124 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -9,7 +9,7 @@ #include #include -#include +#include #include #include @@ -23,7 +23,7 @@ class JFactorySet { protected: std::map, JFactory*> mFactories; // {(typeid, tag) : factory} std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} - std::map mCollectionsFromName; + std::map mCollectionsFromName; std::vector mMultifactories; bool mIsFactoryOwner = true; JEventLevel mLevel = JEventLevel::PhysicsEvent; @@ -35,11 +35,11 @@ class JFactorySet { bool Add(JFactory* aFactory); bool Add(JMultifactory* multifactory); - void Add(JCollection* collection); + void Add(JStorage* storage); void Print(void) const; void Release(void); - JCollection* GetCollection(const std::string& collection_name) const; + JStorage* GetCollection(const std::string& collection_name) const; JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; template JFactoryT* GetFactory(const std::string& tag = "") const; std::vector GetAllFactories() const; diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index 2b33172ed..4e907cc40 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -6,7 +6,7 @@ #pragma once #include -#include +#include template @@ -15,7 +15,7 @@ class JFactoryPodioT : public JFactoryT { using CollectionT = typename T::collection_type; private: - jana::omni::PodioOutput m_output {this}; + jana::components::PodioOutput m_output {this}; public: explicit JFactoryPodioT(); diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index 2096b9d86..8f40bf68d 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -13,7 +13,7 @@ set(TEST_SOURCES Components/ExactlyOnceTests.cc Components/GetObjectsTests.cc Components/NEventNSkipTests.cc - Components/JCollectionTests.cc + Components/JStorageTests.cc Components/JComponentTests.cc Components/JEventGetAllTests.cc Components/JEventProcessorTests.cc diff --git a/src/programs/unit_tests/Components/JCollectionTests.cc b/src/programs/unit_tests/Components/JStorageTests.cc similarity index 93% rename from src/programs/unit_tests/Components/JCollectionTests.cc rename to src/programs/unit_tests/Components/JStorageTests.cc index 86bf95b4a..d4734a0ef 100644 --- a/src/programs/unit_tests/Components/JCollectionTests.cc +++ b/src/programs/unit_tests/Components/JStorageTests.cc @@ -3,7 +3,7 @@ // Copyright 2023, Jefferson Science Associates, LLC. // Subject to the terms in the LICENSE file found in the top-level directory. -#include "JANA/Components/JPodioCollection.h" +#include "JANA/Components/JPodioStorage.h" #include "JANA/Services/JComponentManager.h" #include "PodioDatamodel/ExampleCluster.h" #include "catch.hpp" @@ -17,7 +17,7 @@ #include #include -namespace jcollection_tests { +namespace jstorage_tests { struct TestSource : public JEventSource { Parameter y {this, "y", "asdf", "Does something"}; @@ -69,7 +69,7 @@ struct TestProc : public JEventProcessor { } }; -TEST_CASE("JCollectionEndToEndTest") { +TEST_CASE("JStorageEndToEndTest") { JApplication app; app.Add(new JEventSourceGeneratorT); app.Add(new JFactoryGeneratorT); @@ -79,7 +79,7 @@ TEST_CASE("JCollectionEndToEndTest") { app.Run(); } -TEST_CASE("JCollectionTests_GetCollectionBase") { +TEST_CASE("JStorageTests_GetCollectionBase") { JApplication app; app.Add(new JFactoryGeneratorT); app.Initialize(); @@ -94,7 +94,7 @@ TEST_CASE("JCollectionTests_GetCollectionBase") { REQUIRE(typed_coll->at(1).energy() == 27); } -TEST_CASE("JCollectionTests_GetCollection") { +TEST_CASE("JStorageTests_GetCollection") { JApplication app; app.Add(new JFactoryGeneratorT); app.Initialize(); @@ -108,7 +108,7 @@ TEST_CASE("JCollectionTests_GetCollection") { REQUIRE(coll->at(1).energy() == 27); } -TEST_CASE("JCollectionTests_InsertCollection") { +TEST_CASE("JStorageTests_InsertCollection") { JApplication app; app.Initialize(); From c8136a23f65bd0e3cc03cccb890236d9780b069e Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 6 Sep 2024 10:25:59 -0400 Subject: [PATCH 09/24] JPodioStorage test cases --- src/libraries/JANA/Components/JPodioStorage.h | 4 +- src/libraries/JANA/Components/JStorage.h | 8 +- src/libraries/JANA/JEvent.h | 27 ++-- src/libraries/JANA/JFactorySet.cc | 1 + .../unit_tests/Components/JStorageTests.cc | 134 +++++++++++++----- 5 files changed, 123 insertions(+), 51 deletions(-) diff --git a/src/libraries/JANA/Components/JPodioStorage.h b/src/libraries/JANA/Components/JPodioStorage.h index 9fa07fb07..90a29ca86 100644 --- a/src/libraries/JANA/Components/JPodioStorage.h +++ b/src/libraries/JANA/Components/JPodioStorage.h @@ -71,14 +71,14 @@ void JPodioStorage::SetCollection(std::unique_ptr c this->m_collection = moved; this->SetTypeName(JTypeInfo::demangle()); - this->SetCreationStatus(JStorage::CreationStatus::Inserted); + this->SetStatus(Status::Inserted); } template void JPodioStorage::SetCollectionAlreadyInFrame(const typename T::collection_type* collection) { m_collection = collection; this->SetTypeName(JTypeInfo::demangle()); - SetCreationStatus(JPodioStorage::CreationStatus::Inserted); + SetStatus(Status::Inserted); } diff --git a/src/libraries/JANA/Components/JStorage.h b/src/libraries/JANA/Components/JStorage.h index ceea07189..04e8c03d5 100644 --- a/src/libraries/JANA/Components/JStorage.h +++ b/src/libraries/JANA/Components/JStorage.h @@ -18,11 +18,11 @@ class JFactory; class JStorage { public: // Typedefs - enum class CreationStatus { NotCreatedYet, Created, Inserted, InsertedViaGetObjects, NeverCreated }; + enum class Status { Empty, Created, Inserted, InsertedViaGetObjects }; private: // Fields - CreationStatus m_creation_status = CreationStatus::NotCreatedYet; + Status m_status = Status::Empty; std::string m_collection_name; std::string m_collection_tag; std::string m_type_name; @@ -40,7 +40,7 @@ class JStorage { virtual void ClearData() = 0; // Getters - CreationStatus GetCreationStatus() const { return m_creation_status; } + Status GetStatus() const { return m_status; } std::string GetCollectionName() const { return m_collection_name; } std::string GetCollectionTag() const { return m_collection_tag; } std::string GetTypeName() const { return m_type_name; } @@ -48,7 +48,7 @@ class JStorage { JFactory* GetFactory() const { return m_factory; } // Setters - void SetCreationStatus(CreationStatus s) { m_creation_status = s;} + void SetStatus(Status s) { m_status = s;} void SetCollectionName(std::string collection_name) { m_collection_name = collection_name; } void SetCollectionTag(std::string collection_tag) { m_collection_tag = collection_tag; } void SetTypeName(std::string type_name) { m_type_name = type_name; } diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 7ba4cff2f..014ca914c 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -493,8 +493,8 @@ JFactoryT* JEvent::GetSingle(const T* &t, const char *tag, bool exception_if_ inline JStorage* JEvent::CreateAndGetCollection(std::string name, bool throw_on_missing) const { - auto* coll = mFactorySet->GetCollection(name); - if (coll == nullptr) { + auto* storage = mFactorySet->GetCollection(name); + if (storage == nullptr) { if (throw_on_missing) { throw JException("No collection with tag '%s' found", name.c_str()); } @@ -502,13 +502,18 @@ inline JStorage* JEvent::CreateAndGetCollection(std::string name, bool throw_on_ return nullptr; } } - if (coll->GetFactory() != nullptr) { - // If this was inserted, there would be no factory to run - // fac->Create() will short-circuit if something was already inserted - JCallGraphEntryMaker cg_entry(mCallGraph, coll->GetFactory()); // times execution until this goes out of scope - coll->GetFactory()->Create(this->shared_from_this()); + auto fac = storage->GetFactory(); + if (fac != nullptr) { + if ((storage->GetStatus() == JStorage::Status::Empty) || + (fac->TestFactoryFlag(JFactory::JFactory_Flags_t::REGENERATE))) { + + // If this was inserted, there would be no factory to run + // fac->Create() will short-circuit if something was already inserted + JCallGraphEntryMaker cg_entry(mCallGraph, fac); // times execution until this goes out of scope + fac->Create(this->shared_from_this()); + } } - return coll; + return storage; } #if JANA2_HAVE_PODIO @@ -599,7 +604,7 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas auto coll = new JPodioStorage; coll->SetCollectionName(name); coll->SetTypeName(JTypeInfo::demangle()); - coll->SetCreationStatus(JStorage::CreationStatus::Inserted); + coll->SetStatus(JStorage::Status::Inserted); coll->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); coll->SetCollectionAlreadyInFrame(typed_collection); mFactorySet->Add(coll); @@ -608,12 +613,12 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas else { // This is overriding a factory // Check that we only inserted this collection once - if (storage->GetCreationStatus() != JStorage::CreationStatus::NotCreatedYet) { + if (storage->GetStatus() != JStorage::Status::Empty) { throw JException("Collections can only be inserted once!"); } auto typed_storage = dynamic_cast(storage); typed_storage->SetCollectionAlreadyInFrame(typed_collection); - typed_storage->SetCreationStatus(JStorage::CreationStatus::Inserted); + typed_storage->SetStatus(JStorage::Status::Inserted); typed_storage->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); return typed_storage; } diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index d73376caf..e30b1ff41 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -127,6 +127,7 @@ bool JFactorySet::Add(JFactory* aFactory) // We have a new-style JFactory! for (const auto* output : aFactory->GetOutputs()) { for (const auto& coll : output->GetCollections()) { + coll->SetFactory(aFactory); Add(coll.get()); } } diff --git a/src/programs/unit_tests/Components/JStorageTests.cc b/src/programs/unit_tests/Components/JStorageTests.cc index d4734a0ef..763eda949 100644 --- a/src/programs/unit_tests/Components/JStorageTests.cc +++ b/src/programs/unit_tests/Components/JStorageTests.cc @@ -3,16 +3,16 @@ // Copyright 2023, Jefferson Science Associates, LLC. // Subject to the terms in the LICENSE file found in the top-level directory. -#include "JANA/Components/JPodioStorage.h" -#include "JANA/Services/JComponentManager.h" -#include "PodioDatamodel/ExampleCluster.h" #include "catch.hpp" + #include #include #include #include #include +#include #include +#include #include #include #include @@ -21,7 +21,7 @@ namespace jstorage_tests { struct TestSource : public JEventSource { Parameter y {this, "y", "asdf", "Does something"}; - TestSource(std::string, JApplication*) { + TestSource() { SetCallbackStyle(CallbackStyle::ExpertMode); } static std::string GetDescription() { @@ -36,16 +36,32 @@ struct TestSource : public JEventSource { }; struct TestFactory : public JFactory { - Parameter x {this, "x", 22, "Does something"}; jana::components::PodioOutput m_clusters {this, "my_collection"}; TestFactory() { SetCallbackStyle(CallbackStyle::ExpertMode); } void Init() { - REQUIRE(x() == 22); } void Process(const std::shared_ptr& event) { + LOG_WARN(GetLogger()) << "Calling TestFactory::Process" << LOG_END; + m_clusters()->push_back(MutableExampleCluster(22.2)); + m_clusters()->push_back(MutableExampleCluster(27)); + } +}; + +struct RegeneratingTestFactory : public JFactory { + + jana::components::PodioOutput m_clusters {this, "my_collection"}; + + RegeneratingTestFactory() { + SetCallbackStyle(CallbackStyle::ExpertMode); + SetFactoryFlag(JFactory_Flags_t::REGENERATE); + } + void Init() { + } + void Process(const std::shared_ptr& event) { + LOG_WARN(GetLogger()) << "Calling TestFactory::Process" << LOG_END; m_clusters()->push_back(MutableExampleCluster(22.2)); m_clusters()->push_back(MutableExampleCluster(27)); } @@ -69,17 +85,28 @@ struct TestProc : public JEventProcessor { } }; -TEST_CASE("JStorageEndToEndTest") { +TEST_CASE("JStorageTests_EventInsertAndRetrieve") { JApplication app; - app.Add(new JEventSourceGeneratorT); - app.Add(new JFactoryGeneratorT); - app.Add(new TestProc); - app.Add("fake_file.root"); - app.SetParameterValue("jana:nevents", 2); - app.Run(); + app.Initialize(); + + auto event = std::make_shared(); + app.GetService()->configure_event(*event); + + auto coll_in = std::make_unique(); + coll_in->create().energy(7.6); + coll_in->create().energy(28); + coll_in->create().energy(42); + + event->InsertCollection(std::move(*coll_in), "my_collection"); + + auto coll_out = event->GetCollectionBase("my_collection"); + REQUIRE(coll_out != nullptr); + REQUIRE(coll_out->size() == 3); + auto typed_coll = dynamic_cast(coll_out); + REQUIRE(typed_coll->at(1).energy() == 28); } -TEST_CASE("JStorageTests_GetCollectionBase") { +TEST_CASE("JStorageTests_InsertOverridesFactory") { JApplication app; app.Add(new JFactoryGeneratorT); app.Initialize(); @@ -87,14 +114,47 @@ TEST_CASE("JStorageTests_GetCollectionBase") { auto event = std::make_shared(); app.GetService()->configure_event(*event); - auto coll = event->GetCollectionBase("my_collection"); - REQUIRE(coll != nullptr); - REQUIRE(coll->size() == 2); - auto typed_coll = dynamic_cast(coll); - REQUIRE(typed_coll->at(1).energy() == 27); + auto coll_in = std::make_unique(); + coll_in->create().energy(7.6); + coll_in->create().energy(28); + coll_in->create().energy(42); + + event->InsertCollection(std::move(*coll_in), "my_collection"); + + auto coll_out = event->GetCollectionBase("my_collection"); + REQUIRE(coll_out != nullptr); + REQUIRE(coll_out->size() == 3); + auto typed_coll = dynamic_cast(coll_out); + REQUIRE(typed_coll->at(1).energy() == 28); + +} +TEST_CASE("JStorageTests_RegenerateOverridesInsert") { + JApplication app; + app.SetParameterValue("enable_regenerate", true); + app.Add(new JFactoryGeneratorT); + app.Initialize(); + + auto event = std::make_shared(); + app.GetService()->configure_event(*event); + + auto coll_in = std::make_unique(); + coll_in->create().energy(7.6); + coll_in->create().energy(28); + coll_in->create().energy(42); + + event->InsertCollection(std::move(*coll_in), "my_collection"); + + try { + event->GetCollectionBase("my_collection"); + REQUIRE(1==0); + + } + catch (const std::exception& e) { + std::cout << e.what() << std::endl; + } } -TEST_CASE("JStorageTests_GetCollection") { +TEST_CASE("JStorageTests_FactoryProcessAndRetrieveUntyped") { JApplication app; app.Add(new JFactoryGeneratorT); app.Initialize(); @@ -102,29 +162,35 @@ TEST_CASE("JStorageTests_GetCollection") { auto event = std::make_shared(); app.GetService()->configure_event(*event); - auto coll = event->GetCollection("my_collection"); - REQUIRE(coll != nullptr); - REQUIRE(coll->size() == 2); - REQUIRE(coll->at(1).energy() == 27); + auto coll_out = event->GetCollectionBase("my_collection"); + REQUIRE(coll_out != nullptr); + REQUIRE(coll_out->size() == 2); + auto typed_coll = dynamic_cast(coll_out); + REQUIRE(typed_coll->at(1).energy() == 27); } -TEST_CASE("JStorageTests_InsertCollection") { +TEST_CASE("JStorageTests_FactoryProcessAndRetrieveTyped") { JApplication app; + app.Add(new JFactoryGeneratorT); app.Initialize(); auto event = std::make_shared(); app.GetService()->configure_event(*event); - ExampleClusterCollection coll_in; - coll_in.create(15.0); - coll_in.create(30.0); - coll_in.create(88.0); - event->InsertCollection(std::move(coll_in), "sillyclusters"); + auto coll = event->GetCollection("my_collection"); + REQUIRE(coll != nullptr); + REQUIRE(coll->size() == 2); + REQUIRE(coll->at(1).energy() == 27); +} - auto coll_out = event->GetCollection("sillyclusters"); - REQUIRE(coll_out != nullptr); - REQUIRE(coll_out->size() == 3); - REQUIRE(coll_out->at(1).energy() == 30); +TEST_CASE("JStorageEndToEndTest") { + JApplication app; + app.Add(new JEventSourceGeneratorT); + app.Add(new JFactoryGeneratorT); + app.Add(new TestProc); + app.Add("fake_file.root"); + app.SetParameterValue("jana:nevents", 2); + app.Run(); } } // namespace jcollection_tests From 580f7020ac4f73e457055b4d54355037453002d2 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 6 Sep 2024 12:27:24 -0400 Subject: [PATCH 10/24] More renaming --- src/libraries/JANA/JEvent.h | 39 ++++++++++++++++--------------- src/libraries/JANA/JFactorySet.cc | 2 +- src/libraries/JANA/JFactorySet.h | 2 +- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 014ca914c..8bf57f480 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -89,7 +89,7 @@ class JEvent : public std::enable_shared_from_this #endif // EXPERIMENTAL NEW THING - JStorage* CreateAndGetCollection(std::string name, bool throw_on_missing) const; + JStorage* GetStorage(const std::string& name, bool create) const; //SETTERS void SetRunNumber(int32_t aRunNumber){mRunNumber = aRunNumber;} @@ -491,19 +491,14 @@ JFactoryT* JEvent::GetSingle(const T* &t, const char *tag, bool exception_if_ return fac; } -inline JStorage* JEvent::CreateAndGetCollection(std::string name, bool throw_on_missing) const { +inline JStorage* JEvent::GetStorage(const std::string& name, bool create) const { - auto* storage = mFactorySet->GetCollection(name); - if (storage == nullptr) { - if (throw_on_missing) { - throw JException("No collection with tag '%s' found", name.c_str()); - } - else { - return nullptr; - } - } + auto* storage = mFactorySet->GetStorage(name); auto fac = storage->GetFactory(); - if (fac != nullptr) { + + if (fac != nullptr && create) { + + // The regenerate logic lives out here now if ((storage->GetStatus() == JStorage::Status::Empty) || (fac->TestFactoryFlag(JFactory::JFactory_Flags_t::REGENERATE))) { @@ -528,22 +523,25 @@ inline std::vector JEvent::GetAllCollectionNames() const { } inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { - auto* coll = CreateAndGetCollection(name, throw_on_missing); - if (coll != nullptr) { - auto* podio_coll = dynamic_cast(coll); - if (podio_coll == nullptr) { + auto* storage = GetStorage(name, true); + if (storage != nullptr) { + auto* podio_storage = dynamic_cast(storage); + if (podio_storage == nullptr) { throw JException("Not a podio collection: %s", name.c_str()); } else { - return podio_coll->GetCollection(); + return podio_storage->GetCollection(); } } + else if (throw_on_missing) { + throw JException("Collection not found: '%s'", name.c_str()); + } return nullptr; } template const typename T::collection_type* JEvent::GetCollection(std::string name, bool throw_on_missing) const { - auto* coll = CreateAndGetCollection(name, throw_on_missing); + auto* coll = GetStorage(name, true); if (coll != nullptr) { auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { @@ -553,6 +551,9 @@ const typename T::collection_type* JEvent::GetCollection(std::string name, bool return podio_coll->GetCollection(); } } + else if (throw_on_missing) { + throw JException("Collection not found: '%s'", name.c_str()); + } return nullptr; } @@ -597,7 +598,7 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas } // Retrieve storage if it already exists, else create it - auto storage = mFactorySet->GetCollection(name); + auto storage = mFactorySet->GetStorage(name); if (storage == nullptr) { // No factories already registered this! E.g. from an event source diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index e30b1ff41..9d4d2f91c 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -156,7 +156,7 @@ bool JFactorySet::Add(JMultifactory *multifactory) { //--------------------------------- // GetCollection //--------------------------------- -JStorage* JFactorySet::GetCollection(const std::string& collection_name) const { +JStorage* JFactorySet::GetStorage(const std::string& collection_name) const { auto it = mCollectionsFromName.find(collection_name); if (it != std::end(mCollectionsFromName)) { auto fac = it->second->GetFactory(); diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 7b26e8124..400f6ca80 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -39,7 +39,7 @@ class JFactorySet { void Print(void) const; void Release(void); - JStorage* GetCollection(const std::string& collection_name) const; + JStorage* GetStorage(const std::string& collection_name) const; JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; template JFactoryT* GetFactory(const std::string& tag = "") const; std::vector GetAllFactories() const; From e6662e05680d11ce0056c58535179da77f58f703 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 6 Sep 2024 20:35:42 -0400 Subject: [PATCH 11/24] Add BasicStorage and BasicOutput --- src/libraries/JANA/Components/JBasicOutput.h | 27 +++++++++++++++++++ src/libraries/JANA/Components/JBasicStorage.h | 25 ++++++++--------- 2 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 src/libraries/JANA/Components/JBasicOutput.h diff --git a/src/libraries/JANA/Components/JBasicOutput.h b/src/libraries/JANA/Components/JBasicOutput.h new file mode 100644 index 000000000..7465e0bf9 --- /dev/null +++ b/src/libraries/JANA/Components/JBasicOutput.h @@ -0,0 +1,27 @@ +#pragma once +#include +#include + +namespace jana::components { + +template +class Output : public JHasFactoryOutputs::OutputBase { + std::vector m_data; + +public: + Output(JHasFactoryOutputs* owner, std::string default_tag_name="") { + owner->RegisterOutput(this); + this->collection_names.push_back(default_tag_name); + this->type_name = JTypeInfo::demangle(); + } + + std::vector& operator()() { return m_data; } + +protected: + void PutCollections(const JEvent& event) override { + event.Insert(m_data, this->collection_names[0]); + } + void Reset() override { } +}; + +} // jana::components diff --git a/src/libraries/JANA/Components/JBasicStorage.h b/src/libraries/JANA/Components/JBasicStorage.h index 2ee4f4bac..c0f8d4da1 100644 --- a/src/libraries/JANA/Components/JBasicStorage.h +++ b/src/libraries/JANA/Components/JBasicStorage.h @@ -1,7 +1,7 @@ #pragma once -#include +#include #include #include @@ -9,35 +9,32 @@ #include #endif -class JBasicCollection : public JCollection { +class JBasicStorage : public JStorage { bool m_is_persistent = false; bool m_not_object_owner = false; - bool m_regenerate = false; bool m_write_to_output = true; public: - JBasicCollection() = default; - ~JBasicCollection() override = default; + JBasicStorage() = default; + ~JBasicStorage() override = default; void SetPersistentFlag(bool persistent) { m_is_persistent = persistent; } void SetNotOwnerFlag(bool not_owner) { m_not_object_owner = not_owner; } - void SetRegenerateFlag(bool regenerate) { m_regenerate = regenerate; } void SetWriteToOutputFlag(bool write_to_output) { m_write_to_output = write_to_output; } bool GetPersistentFlag() { return m_is_persistent; } bool GetNotOwnerFlag() { return m_not_object_owner; } - bool GetRegenerateFlag() { return m_regenerate; } bool GetWriteToOutputFlag() { return m_write_to_output; } }; template -class JBasicCollectionT : public JBasicCollection { +class JBasicStorageT : public JBasicStorage { private: std::vector m_data; public: - JBasicCollectionT(); + JBasicStorageT(); void ClearData() override; size_t GetSize() const override { return m_data.size();} @@ -58,7 +55,7 @@ class JBasicCollectionT : public JBasicCollection { // Template definitions template -JBasicCollectionT::JBasicCollectionT() { +JBasicStorageT::JBasicStorageT() { SetTypeName(JTypeInfo::demangle()); EnableGetAs(); EnableGetAs( std::is_convertible() ); // Automatically add JObject if this can be converted to it @@ -68,10 +65,10 @@ JBasicCollectionT::JBasicCollectionT() { } template -void JBasicCollectionT::ClearData() { +void JBasicStorageT::ClearData() { // ClearData won't do anything if Init() hasn't been called - if (GetCreationStatus() == CreationStatus::NotCreatedYet) { + if (GetStatus() == Status::Empty) { return; } // ClearData() does nothing if persistent flag is set. @@ -85,12 +82,12 @@ void JBasicCollectionT::ClearData() { for (auto p : m_data) delete p; } m_data.clear(); - SetCreationStatus(CreationStatus::NotCreatedYet); + SetStatus(Status::Empty); } template template -void JBasicCollectionT::EnableGetAs() { +void JBasicStorageT::EnableGetAs() { auto upcast_lambda = [this]() { std::vector results; From 763a3faf19b7b44283ff808d5d0bea763bac57fd Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 18 Sep 2024 22:04:28 -0400 Subject: [PATCH 12/24] Fully replace JEvent podio machinery with JStorage equivalent --- src/libraries/JANA/Components/JHasFactoryOutputs.h | 2 +- src/libraries/JANA/Components/JStorage.h | 10 +++++++--- src/libraries/JANA/JEvent.h | 12 ++---------- src/libraries/JANA/JFactorySet.cc | 12 ++++++++++++ src/libraries/JANA/JFactorySet.h | 1 + 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h index 5dfbd93fc..7865d124e 100644 --- a/src/libraries/JANA/Components/JHasFactoryOutputs.h +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -25,7 +25,7 @@ class JHasFactoryOutputs { std::vector m_outputs; public: - const std::vector& GetOutputs() { + const std::vector& GetOutputs() const { return m_outputs; } diff --git a/src/libraries/JANA/Components/JStorage.h b/src/libraries/JANA/Components/JStorage.h index 04e8c03d5..08b366995 100644 --- a/src/libraries/JANA/Components/JStorage.h +++ b/src/libraries/JANA/Components/JStorage.h @@ -9,8 +9,10 @@ #include #include +#include #include -#include +#include +#include class JFactory; @@ -24,9 +26,10 @@ class JStorage { // Fields Status m_status = Status::Empty; std::string m_collection_name; - std::string m_collection_tag; + std::optional m_collection_tag = std::nullopt; std::string m_type_name; JFactory* m_factory = nullptr; + std::optional m_inner_type_index = std::nullopt; // e.g. Hit, Cluster mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; protected: @@ -42,8 +45,9 @@ class JStorage { // Getters Status GetStatus() const { return m_status; } std::string GetCollectionName() const { return m_collection_name; } - std::string GetCollectionTag() const { return m_collection_tag; } + std::optional GetCollectionTag() const { return m_collection_tag; } std::string GetTypeName() const { return m_type_name; } + std::optional GetTypeIndex() const { return m_inner_type_index; } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. JFactory* GetFactory() const { return m_factory; } diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 8bf57f480..18c77d895 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -195,10 +195,6 @@ class JEvent : public std::enable_shared_from_this int64_t mEventIndex = -1; - -#if JANA2_HAVE_PODIO - std::map mPodioFactories; -#endif }; /// Insert() allows an EventSource to insert items directly into the JEvent, @@ -494,6 +490,7 @@ JFactoryT* JEvent::GetSingle(const T* &t, const char *tag, bool exception_if_ inline JStorage* JEvent::GetStorage(const std::string& name, bool create) const { auto* storage = mFactorySet->GetStorage(name); + if (storage == nullptr) return nullptr; auto fac = storage->GetFactory(); if (fac != nullptr && create) { @@ -514,12 +511,7 @@ inline JStorage* JEvent::GetStorage(const std::string& name, bool create) const #if JANA2_HAVE_PODIO inline std::vector JEvent::GetAllCollectionNames() const { - // TODO: Obtain from JFactorySet - std::vector keys; - for (auto pair : mPodioFactories) { - keys.push_back(pair.first); - } - return keys; + return mFactorySet->GetAllCollectionNames(); } inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 9d4d2f91c..07fb59a4c 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -4,6 +4,7 @@ #include #include +#include #include "JFactorySet.h" #include "JANA/Components/JStorage.h" @@ -207,6 +208,17 @@ std::vector JFactorySet::GetAllMultifactories() const { return results; } +//--------------------------------- +// GetAllCollectionNames +//--------------------------------- +std::vector JFactorySet::GetAllCollectionNames() const { + std::vector results; + for (const auto& it : mCollectionsFromName) { + results.push_back(it.first); + } + return results; +} + //--------------------------------- // Print //--------------------------------- diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 400f6ca80..26065701c 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -39,6 +39,7 @@ class JFactorySet { void Print(void) const; void Release(void); + std::vector GetAllCollectionNames() const; JStorage* GetStorage(const std::string& collection_name) const; JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; template JFactoryT* GetFactory(const std::string& tag = "") const; From b9c4e1d61a2e5c13ff358a495385d26a4d050fd8 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 19 Sep 2024 01:06:58 -0400 Subject: [PATCH 13/24] JFactoryPodioT inherits from JFactory, not JFactoryT --- .../JANA/Components/JHasFactoryOutputs.h | 1 - src/libraries/JANA/Components/JPodioOutput.h | 2 -- src/libraries/JANA/JFactorySet.cc | 8 +++-- src/libraries/JANA/JMultifactory.h | 2 +- src/libraries/JANA/Podio/JFactoryPodioT.h | 33 +++++++------------ .../unit_tests/Components/PodioTests.cc | 11 +++---- 6 files changed, 22 insertions(+), 35 deletions(-) diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h index 7865d124e..b6b85f5fd 100644 --- a/src/libraries/JANA/Components/JHasFactoryOutputs.h +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -12,7 +12,6 @@ class JHasFactoryOutputs { public: struct OutputBase { protected: - std::vector m_collection_names; // TODO: Possibly don't need anymore std::vector> m_collections; bool m_is_variadic = false; public: diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h index 113a9c3eb..9106dedcf 100644 --- a/src/libraries/JANA/Components/JPodioOutput.h +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -17,7 +17,6 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { public: PodioOutput(JHasFactoryOutputs* owner, std::string default_collection_name="") { owner->RegisterOutput(this); - this->m_collection_names.push_back(default_collection_name); auto coll = std::make_unique(); coll->SetCollectionName(default_collection_name); coll->SetTypeName(JTypeInfo::demangle()); @@ -69,7 +68,6 @@ class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { public: VariadicPodioOutput(JHasFactoryOutputs* owner, std::vector default_collection_names={}) { owner->RegisterOutput(this); - this->m_collection_names = default_collection_names; this->m_is_variadic = true; for (const std::string& name : default_collection_names) { auto coll = std::make_unique(); diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 07fb59a4c..c035fe5fd 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -50,6 +50,10 @@ JFactorySet::~JFactorySet() // Add //--------------------------------- void JFactorySet::Add(JStorage* collection) { + + if (collection->GetCollectionName().empty()) { + throw JException("Attempted to add a collection with no name"); + } auto named_result = mCollectionsFromName.find(collection->GetCollectionName()); if (named_result != std::end(mCollectionsFromName)) { // Collection is duplicate. Since this almost certainly indicates a user error, and @@ -86,8 +90,8 @@ bool JFactorySet::Add(JFactory* aFactory) /// same T JObject, and is not distinguishing between them via tags. // There are two different ways JFactories can work now. In the old way, JFactory must be - // either a JFactoryT or a JFactoryPodioT, and have exactly one output collection. In the - // new way, JFactory has an arbitrary number of output collections which are explicitly + // a JFactoryT, and have exactly one output collection. In the new way (which includes JFactoryPodioT), + // JFactory has an arbitrary number of output collections which are explicitly // represented, similar to but better than JMultifactory. We distinguish between // these two cases by checking whether JFactory::GetObjectType returns an object type vs nullopt. diff --git a/src/libraries/JANA/JMultifactory.h b/src/libraries/JANA/JMultifactory.h index 2e9ce2125..2eebf631c 100644 --- a/src/libraries/JANA/JMultifactory.h +++ b/src/libraries/JANA/JMultifactory.h @@ -183,7 +183,7 @@ void JMultifactory::DeclarePodioOutput(std::string tag, bool) { template void JMultifactory::SetCollection(std::string tag, typename JFactoryPodioT::CollectionT&& collection) { - JFactoryT* helper = mHelpers.GetFactory(tag); + JFactory* helper = mHelpers.GetStorage(tag)->GetFactory(); if (helper == nullptr) { auto ex = JException("JMultifactory: Attempting to SetData() without corresponding DeclareOutput()"); ex.function_name = "JMultifactory::SetCollection"; diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index 4e907cc40..792da15b4 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -5,12 +5,13 @@ #pragma once -#include +#include "JANA/Utils/JTypeInfo.h" +#include #include template -class JFactoryPodioT : public JFactoryT { +class JFactoryPodioT : public JFactory { public: using CollectionT = typename T::collection_type; @@ -21,6 +22,11 @@ class JFactoryPodioT : public JFactoryT { explicit JFactoryPodioT(); ~JFactoryPodioT() override; + void SetTag(std::string tag) { + mTag = tag; + m_output.GetCollections().at(0)->SetCollectionName(tag); + } + void Init() override {} void BeginRun(const std::shared_ptr&) override {} void ChangeRun(const std::shared_ptr&) override {} @@ -28,21 +34,18 @@ class JFactoryPodioT : public JFactoryT { void EndRun() override {} void Finish() override {} - std::optional GetObjectType() const final { return std::type_index(typeid(T)); } std::size_t GetNumObjects() const final { return m_output.GetCollection()->GetSize(); } void ClearData() final; void SetCollection(CollectionT&& collection); void SetCollection(std::unique_ptr collection); - void Set(const std::vector& aData) final; - void Set(std::vector&& aData) final; - void Insert(T* aDatum) final; - }; template -JFactoryPodioT::JFactoryPodioT() = default; +JFactoryPodioT::JFactoryPodioT() { + mObjectName = JTypeInfo::demangle(); +} template JFactoryPodioT::~JFactoryPodioT() { @@ -73,18 +76,4 @@ void JFactoryPodioT::ClearData() { // Happens automatically now } -template -void JFactoryPodioT::Set(const std::vector&) { - throw JException("Not supported!"); -} - -template -void JFactoryPodioT::Set(std::vector&&) { - throw JException("Not supported!"); -} - -template -void JFactoryPodioT::Insert(T*) { - throw JException("Not supported!"); -} diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 7d45ebada..c2441b9f4 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -31,12 +31,6 @@ TEST_CASE("PodioTestsInsertAndRetrieve") { REQUIRE((*collection_retrieved)[0].energy() == 16.0); } - SECTION("Retrieve using JEvent::Get()") { - std::vector clusters_retrieved = event->Get("clusters"); - REQUIRE(clusters_retrieved.size() == 2); - REQUIRE(clusters_retrieved[0]->energy() == 16.0); - } - SECTION("Retrieve directly from podio::Frame") { auto frame = event->GetSingle(); auto* collection_retrieved = dynamic_cast(frame->get("clusters")); @@ -162,7 +156,10 @@ TEST_CASE("JFactoryPodioT::Init gets called") { const auto* res = dynamic_cast(r); REQUIRE(res != nullptr); REQUIRE((*res)[0].energy() == 16.0); - auto fac = dynamic_cast(event->GetFactory("clusters")); + + auto fac_untyped = event->GetStorage("clusters", false)->GetFactory(); + REQUIRE(fac_untyped != nullptr); + auto fac = dynamic_cast(fac_untyped); REQUIRE(fac != nullptr); REQUIRE(fac->init_called == true); } From f2551920c2f0703aa9056fbbe1880df9cb5babf5 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 19 Sep 2024 03:35:45 -0400 Subject: [PATCH 14/24] JMultifactory no longer uses JPodioFactoryT --- src/libraries/JANA/JMultifactory.h | 47 ++++++++++++++++++------------ 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/libraries/JANA/JMultifactory.h b/src/libraries/JANA/JMultifactory.h index 2eebf631c..07acd1a5f 100644 --- a/src/libraries/JANA/JMultifactory.h +++ b/src/libraries/JANA/JMultifactory.h @@ -12,7 +12,7 @@ #include #if JANA2_HAVE_PODIO -#include "JANA/Podio/JFactoryPodioT.h" +#include #endif class JMultifactory; @@ -40,14 +40,18 @@ class JMultifactoryHelper : public JFactoryT{ #if JANA2_HAVE_PODIO -// TODO: This redundancy goes away if we merge JFactoryPodioT with JFactoryT template -class JMultifactoryHelperPodio : public JFactoryPodioT{ +class JMultifactoryHelperPodio : public JFactory { + jana::components::PodioOutput m_output {this}; JMultifactory* mMultiFactory; public: - JMultifactoryHelperPodio(JMultifactory* parent) : mMultiFactory(parent) {} + JMultifactoryHelperPodio(JMultifactory* parent, std::string collection_name) : mMultiFactory(parent) { + mObjectName = JTypeInfo::demangle(); + mTag = collection_name; + m_output.GetCollections().at(0)->SetCollectionName(collection_name); + } virtual ~JMultifactoryHelperPodio() = default; // This does NOT own mMultiFactory; the enclosing JFactorySet does @@ -61,6 +65,13 @@ class JMultifactoryHelperPodio : public JFactoryPodioT{ // Helpers do not produce any summary information void Summarize(JComponentSummary&) const override { } + + void SetCollection(typename T::collection_type&& collection) { + m_output() = std::make_unique(std::move(collection)); + } + void SetCollection(std::unique_ptr collection) { + m_output() = std::move(collection); + } }; #endif // JANA2_HAVE_PODIO @@ -103,10 +114,10 @@ class JMultifactory : public jana::components::JComponent, void DeclarePodioOutput(std::string tag, bool owns_data=true); template - void SetCollection(std::string tag, typename JFactoryPodioT::CollectionT&& collection); + void SetCollection(std::string tag, typename T::collection_type&& collection); template - void SetCollection(std::string tag, std::unique_ptr::CollectionT> collection); + void SetCollection(std::string tag, std::unique_ptr collection); #endif @@ -161,7 +172,7 @@ void JMultifactory::SetData(std::string tag, std::vector data) { ex.plugin_name = m_plugin_name; throw ex; } - // This will except if helper is a JFactoryPodioT. User should use SetPodioData() instead for PODIO data. + // This will except if helper is a JMultifactoryHelperPodio. User should use SetPodioData() instead for PODIO data. helper->Set(data); } @@ -169,11 +180,9 @@ void JMultifactory::SetData(std::string tag, std::vector data) { #if JANA2_HAVE_PODIO template -void JMultifactory::DeclarePodioOutput(std::string tag, bool) { - // TODO: Decouple tag name from collection name - auto* helper = new JMultifactoryHelperPodio(this); - - helper->SetTag(std::move(tag)); +void JMultifactory::DeclarePodioOutput(std::string coll_name, bool) { + auto* helper = new JMultifactoryHelperPodio(this, coll_name); + helper->SetTag(std::move(coll_name)); helper->SetPluginName(m_plugin_name); helper->SetFactoryName(GetTypeName() + "::Helper<" + JTypeInfo::demangle() + ">"); helper->SetLevel(GetLevel()); @@ -182,7 +191,7 @@ void JMultifactory::DeclarePodioOutput(std::string tag, bool) { } template -void JMultifactory::SetCollection(std::string tag, typename JFactoryPodioT::CollectionT&& collection) { +void JMultifactory::SetCollection(std::string tag, typename T::collection_type&& collection) { JFactory* helper = mHelpers.GetStorage(tag)->GetFactory(); if (helper == nullptr) { auto ex = JException("JMultifactory: Attempting to SetData() without corresponding DeclareOutput()"); @@ -192,9 +201,9 @@ void JMultifactory::SetCollection(std::string tag, typename JFactoryPodioT::C ex.plugin_name = m_plugin_name; throw ex; } - auto* typed = dynamic_cast*>(helper); + auto* typed = dynamic_cast*>(helper); if (typed == nullptr) { - auto ex = JException("JMultifactory: Helper needs to be a JFactoryPodioT (this shouldn't be reachable)"); + auto ex = JException("JMultifactory: Helper needs to be a JMultifactoryHelperPodio (this shouldn't be reachable)"); ex.function_name = "JMultifactory::SetCollection"; ex.type_name = m_type_name; ex.instance_name = m_prefix; @@ -206,8 +215,8 @@ void JMultifactory::SetCollection(std::string tag, typename JFactoryPodioT::C } template -void JMultifactory::SetCollection(std::string tag, std::unique_ptr::CollectionT> collection) { - JFactoryT* helper = mHelpers.GetFactory(tag); +void JMultifactory::SetCollection(std::string tag, std::unique_ptr collection) { + JFactory* helper = mHelpers.GetStorage(tag)->GetFactory(); if (helper == nullptr) { auto ex = JException("JMultifactory: Attempting to SetData() without corresponding DeclareOutput()"); ex.function_name = "JMultifactory::SetCollection"; @@ -216,9 +225,9 @@ void JMultifactory::SetCollection(std::string tag, std::unique_ptr*>(helper); + auto* typed = dynamic_cast*>(helper); if (typed == nullptr) { - auto ex = JException("JMultifactory: Helper needs to be a JFactoryPodioT (this shouldn't be reachable)"); + auto ex = JException("JMultifactory: Helper needs to be a JMultifactoryHelperPodio (this shouldn't be reachable)"); ex.function_name = "JMultifactory::SetCollection"; ex.type_name = m_type_name; ex.instance_name = m_prefix; From ee60443025806fd9eb3de1c02b320ce5d039bba6 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 19 Sep 2024 14:18:40 -0400 Subject: [PATCH 15/24] Fix JFactorySet ownership confusion --- src/libraries/JANA/JFactorySet.cc | 26 ++++++------ src/libraries/JANA/JFactorySet.h | 5 ++- .../unit_tests/Components/PodioTests.cc | 42 +++++++++++++++++++ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index c035fe5fd..4c77c08ea 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -40,10 +40,17 @@ JFactorySet::~JFactorySet() /// The only time mIsFactoryOwner should/can be set false is when a JMultifactory is using a JFactorySet internally /// to manage its JMultifactoryHelpers. if (mIsFactoryOwner) { - for (auto& f : mFactories) delete f.second; + for (auto& s : mCollectionsFromName) { + // Only delete _inserted_ collections. Collections are otherwise owned by their factories + if (s.second->GetFactory() == nullptr) { + delete s.second; + } + } + for (auto f : mAllFactories) delete f; + // Now that the factories are deleted, nothing can call the multifactories so it is safe to delete them as well + + for (auto* mf : mMultifactories) { delete mf; } } - // Now that the factories are deleted, nothing can call the multifactories so it is safe to delete them as well - for (auto* mf : mMultifactories) { delete mf; } } //--------------------------------- @@ -96,6 +103,7 @@ bool JFactorySet::Add(JFactory* aFactory) // these two cases by checking whether JFactory::GetObjectType returns an object type vs nullopt. + mAllFactories.push_back(aFactory); auto object_type = aFactory->GetObjectType(); if (object_type != std::nullopt) { // We have an old-style JFactory! @@ -194,22 +202,14 @@ JFactory* JFactorySet::GetFactory(const std::string& object_name, const std::str // GetAllFactories //--------------------------------- std::vector JFactorySet::GetAllFactories() const { - std::vector results; - for (auto p : mFactories) { - results.push_back(p.second); - } - return results; + return mAllFactories; } //--------------------------------- // GetAllMultifactories //--------------------------------- std::vector JFactorySet::GetAllMultifactories() const { - std::vector results; - for (auto f : mMultifactories) { - results.push_back(f); - } - return results; + return mMultifactories; } //--------------------------------- diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 26065701c..73b1cf217 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -21,6 +21,7 @@ class JMultifactory; class JFactorySet { protected: + std::vector mAllFactories; std::map, JFactory*> mFactories; // {(typeid, tag) : factory} std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} std::map mCollectionsFromName; @@ -45,7 +46,7 @@ class JFactorySet { template JFactoryT* GetFactory(const std::string& tag = "") const; std::vector GetAllFactories() const; std::vector GetAllMultifactories() const; - template std::vector*> GetAllFactories() const; + template std::vector*> GetAllFactoriesT() const; JEventLevel GetLevel() const { return mLevel; } void SetLevel(JEventLevel level) { mLevel = level; } @@ -78,7 +79,7 @@ JFactoryT* JFactorySet::GetFactory(const std::string& tag) const { } template -std::vector*> JFactorySet::GetAllFactories() const { +std::vector*> JFactorySet::GetAllFactoriesT() const { auto sKey = std::type_index(typeid(T)); std::vector*> data; for (auto it=std::begin(mFactories);it!=std::end(mFactories);it++){ diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index c2441b9f4..920d2842d 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include namespace podiotests { @@ -163,5 +164,46 @@ TEST_CASE("JFactoryPodioT::Init gets called") { REQUIRE(fac != nullptr); REQUIRE(fac->init_called == true); } + + +namespace multifactory { + +struct TestMultiFac : public JMultifactory { + TestMultiFac() { + DeclarePodioOutput("sillyclusters"); + } + bool init_called = false; + void Init() override { + init_called = true; + } + void Process(const std::shared_ptr&) override { + ExampleClusterCollection c; + c.push_back(MutableExampleCluster(16.0)); + SetCollection("sillyclusters", std::move(c)); + } +}; + +TEST_CASE("Podio JMultifactory::Init gets called") { + + JApplication app; + auto event = std::make_shared(&app); + auto fs = new JFactorySet; + fs->Add(new TestMultiFac); + event->SetFactorySet(fs); + event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool + + auto r = event->GetCollectionBase("sillyclusters"); + REQUIRE(r != nullptr); + const auto* res = dynamic_cast(r); + REQUIRE(res != nullptr); + REQUIRE((*res)[0].energy() == 16.0); + + auto multifac = event->GetFactorySet()->GetAllMultifactories().at(0); + REQUIRE(multifac != nullptr); + auto multifac_typed = dynamic_cast(multifac); + REQUIRE(multifac_typed != nullptr); + REQUIRE(multifac_typed->init_called == true); +} +} // namespace multifactory } // namespace podiotests From c93938a733094a2c6dd1e6a5e5a3aef06f98acb6 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 19 Sep 2024 16:14:37 -0400 Subject: [PATCH 16/24] JFactorySet no longer depends on JFactoryT --- src/libraries/JANA/JEvent.h | 98 +++++++++++----- src/libraries/JANA/JFactoryGenerator.h | 3 +- src/libraries/JANA/JFactorySet.cc | 44 ++++++- src/libraries/JANA/JFactorySet.h | 110 ++++++------------ src/libraries/JANA/JMultifactory.h | 6 +- .../unit_tests/Components/JComponentTests.cc | 3 +- 6 files changed, 155 insertions(+), 109 deletions(-) diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 18c77d895..e54c3bf00 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -204,51 +205,73 @@ class JEvent : public std::enable_shared_from_this template inline JFactoryT* JEvent::Insert(T* item, const std::string& tag) const { + std::string object_name = JTypeInfo::demangle(); + std::string resolved_tag = tag; if (mUseDefaultTags && tag.empty()) { - auto defaultTag = mDefaultTags.find(JTypeInfo::demangle()); + auto defaultTag = mDefaultTags.find(object_name); if (defaultTag != mDefaultTags.end()) resolved_tag = defaultTag->second; } - auto factory = mFactorySet->GetFactory(resolved_tag); - if (factory == nullptr) { - factory = new JFactoryT; - factory->SetTag(tag); - factory->SetLevel(mFactorySet->GetLevel()); - mFactorySet->Add(factory); + auto untyped_factory = mFactorySet->GetFactory(std::type_index(typeid(T)), object_name, resolved_tag); + JFactoryT* typed_factory; + if (untyped_factory == nullptr) { + typed_factory = new JFactoryT; + typed_factory->SetTag(tag); + typed_factory->SetLevel(mFactorySet->GetLevel()); + mFactorySet->Add(typed_factory); } - factory->Insert(item); - factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); // (see note at top of JCallGraphRecorder.h) - return factory; + else { + typed_factory = dynamic_cast*>(untyped_factory); + if (typed_factory == nullptr) { + throw JException("Retrieved factory is not a JFactoryT!"); + } + } + typed_factory->Insert(item); + typed_factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); // (see note at top of JCallGraphRecorder.h) + return typed_factory; } template inline JFactoryT* JEvent::Insert(const std::vector& items, const std::string& tag) const { + std::string object_name = JTypeInfo::demangle(); std::string resolved_tag = tag; if (mUseDefaultTags && tag.empty()) { - auto defaultTag = mDefaultTags.find(JTypeInfo::demangle()); + auto defaultTag = mDefaultTags.find(object_name); if (defaultTag != mDefaultTags.end()) resolved_tag = defaultTag->second; } - auto factory = mFactorySet->GetFactory(resolved_tag); - if (factory == nullptr) { - factory = new JFactoryT; - factory->SetTag(tag); - factory->SetLevel(mFactorySet->GetLevel()); - mFactorySet->Add(factory); + auto untyped_factory = mFactorySet->GetFactory(std::type_index(typeid(T)), object_name, resolved_tag); + JFactoryT* typed_factory; + if (untyped_factory == nullptr) { + typed_factory = new JFactoryT; + typed_factory->SetTag(tag); + typed_factory->SetLevel(mFactorySet->GetLevel()); + mFactorySet->Add(typed_factory); + } + else { + typed_factory = dynamic_cast*>(untyped_factory); + if (typed_factory == nullptr) { + throw JException("Retrieved factory is not a JFactoryT!"); + } } for (T* item : items) { - factory->Insert(item); + typed_factory->Insert(item); } - factory->SetStatus(JFactory::Status::Inserted); // for when items is empty - factory->SetCreationStatus(JFactory::CreationStatus::Inserted); // for when items is empty - factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); // (see note at top of JCallGraphRecorder.h) - return factory; + typed_factory->SetStatus(JFactory::Status::Inserted); // for when items is empty + typed_factory->SetCreationStatus(JFactory::CreationStatus::Inserted); // for when items is empty + typed_factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); // (see note at top of JCallGraphRecorder.h) + return typed_factory; } /// GetFactory() should be used with extreme care because it subverts the JEvent abstraction. /// Most historical uses of GetFactory are far better served by JMultifactory inline JFactory* JEvent::GetFactory(const std::string& object_name, const std::string& tag) const { - return mFactorySet->GetFactory(object_name, tag); + std::string resolved_tag = tag; + if (mUseDefaultTags && tag.empty()) { + auto defaultTag = mDefaultTags.find(object_name); + if (defaultTag != mDefaultTags.end()) resolved_tag = defaultTag->second; + } + return mFactorySet->GetFactory(object_name, resolved_tag); } /// GetAllFactories() should be used with extreme care because it subverts the JEvent abstraction. @@ -262,20 +285,27 @@ inline std::vector JEvent::GetAllFactories() const { template inline JFactoryT* JEvent::GetFactory(const std::string& tag, bool throw_on_missing) const { + std::string object_name = JTypeInfo::demangle(); std::string resolved_tag = tag; if (mUseDefaultTags && tag.empty()) { - auto defaultTag = mDefaultTags.find(JTypeInfo::demangle()); + auto defaultTag = mDefaultTags.find(object_name); if (defaultTag != mDefaultTags.end()) resolved_tag = defaultTag->second; } - auto factory = mFactorySet->GetFactory(resolved_tag); + auto factory = mFactorySet->GetFactory(std::type_index(typeid(T)), object_name, resolved_tag); if (factory == nullptr) { if (throw_on_missing) { - JException ex("Could not find JFactoryT<" + JTypeInfo::demangle() + "> with tag=" + tag); + JException ex("Could not find JFactory producing '%s' with tag '%s'", object_name.c_str(), resolved_tag.c_str()); ex.show_stacktrace = false; throw ex; } + return nullptr; }; - return factory; + auto typed_factory = dynamic_cast*>(factory); + if (typed_factory == nullptr) { + JException ex("JFactory producing '%s' with tag '%s' is not a JFactoryT!", object_name.c_str(), resolved_tag.c_str()); + throw ex; + }; + return typed_factory; } @@ -392,15 +422,23 @@ std::vector JEvent::Get(const std::string& tag, bool strict) const { /// wishes to examine them all together. template inline std::vector*> JEvent::GetFactoryAll(bool throw_on_missing) const { - auto factories = mFactorySet->GetAllFactories(); + std::vector*> results; + std::string object_name = JTypeInfo::demangle(); + auto factories = mFactorySet->GetAllFactories(std::type_index(typeid(T)), object_name); if (factories.size() == 0) { if (throw_on_missing) { - JException ex("Could not find any JFactoryT<" + JTypeInfo::demangle() + "> (from any tag)"); + JException ex("Could not find any JFactoryT<%s> (from any tag)", object_name.c_str()); ex.show_stacktrace = false; throw ex; } }; - return factories; + for (auto* fac : factories) { + auto fac_typed = dynamic_cast*>(fac); + if (fac_typed != nullptr) { + results.push_back(fac_typed); + } + } + return results; } /// GetAll returns all JObjects of (child) type T, regardless of tag. diff --git a/src/libraries/JANA/JFactoryGenerator.h b/src/libraries/JANA/JFactoryGenerator.h index e774801ee..62eb6eec2 100644 --- a/src/libraries/JANA/JFactoryGenerator.h +++ b/src/libraries/JANA/JFactoryGenerator.h @@ -4,8 +4,9 @@ #pragma once -#include #include +#include + class JApplication; diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 4c77c08ea..7ff1dfbd2 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -4,10 +4,11 @@ #include #include +#include #include +#include #include "JFactorySet.h" -#include "JANA/Components/JStorage.h" #include "JFactory.h" #include "JMultifactory.h" #include "JFactoryGenerator.h" @@ -198,13 +199,54 @@ JFactory* JFactorySet::GetFactory(const std::string& object_name, const std::str return nullptr; } +//--------------------------------- +// GetFactory +//--------------------------------- +JFactory* JFactorySet::GetFactory(std::type_index object_type, const std::string& object_name, const std::string& tag) const { + + auto typed_key = std::make_pair(object_type, tag); + auto typed_iter = mFactories.find(typed_key); + if (typed_iter != std::end(mFactories)) { + JEventLevel found_level = typed_iter->second->GetLevel(); + if (found_level != mLevel) { + throw JException("Factory belongs to a different level on the event hierarchy. Expected: %s, Found: %s", toString(mLevel).c_str(), toString(found_level).c_str()); + } + return typed_iter->second; + } + return GetFactory(object_name, tag); +} + //--------------------------------- // GetAllFactories //--------------------------------- std::vector JFactorySet::GetAllFactories() const { + + // This returns both old-style (JFactoryT) and new-style (JFactory+JStorage) factories, unlike + // GetAllFactories(object_type, object_name) below. This is because we use this method in + // JEventProcessors to activate factories in a generic way, particularly when working with Podio data. + return mAllFactories; } +//--------------------------------- +// GetAllFactories +//--------------------------------- +std::vector JFactorySet::GetAllFactories(std::type_index object_type, const std::string& object_name) const { + + // This returns all factories which _directly_ produce objects of type object_type, i.e. they don't use a JStorage. + // This is what all of its callers already expect anyhow. Obviously we'd like to migrate everything over to JStorage + // eventually. Rather than updating this to also check mCollectionsFromName, it probably makes more sense to create + // a JFactorySet::GetAllStorages(type_index, object_name) instead, and migrate all callers to use that. + + std::vector results; + for (auto& it : mFactories) { + if (it.second->GetObjectType() == object_type || it.second->GetObjectName() == object_name) { + results.push_back(it.second); + } + } + return results; +} + //--------------------------------- // GetAllMultifactories //--------------------------------- diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index 73b1cf217..f4f80499e 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -4,93 +4,55 @@ #pragma once +#include + +#include #include #include #include -#include -#include -#include -#include - class JFactoryGenerator; class JFactory; class JMultifactory; +class JStorage; class JFactorySet { - protected: - std::vector mAllFactories; - std::map, JFactory*> mFactories; // {(typeid, tag) : factory} - std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} - std::map mCollectionsFromName; - std::vector mMultifactories; - bool mIsFactoryOwner = true; - JEventLevel mLevel = JEventLevel::PhysicsEvent; - - public: - JFactorySet(); - JFactorySet(const std::vector& aFactoryGenerators); - virtual ~JFactorySet(); - - bool Add(JFactory* aFactory); - bool Add(JMultifactory* multifactory); - void Add(JStorage* storage); - void Print(void) const; - void Release(void); - - std::vector GetAllCollectionNames() const; - JStorage* GetStorage(const std::string& collection_name) const; - JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; - template JFactoryT* GetFactory(const std::string& tag = "") const; - std::vector GetAllFactories() const; - std::vector GetAllMultifactories() const; - template std::vector*> GetAllFactoriesT() const; - - JEventLevel GetLevel() const { return mLevel; } - void SetLevel(JEventLevel level) { mLevel = level; } -}; +private: + std::vector mAllFactories; + std::map, JFactory*> mFactories; // {(typeid, tag) : factory} + std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} + std::map mCollectionsFromName; + std::vector mMultifactories; + bool mIsFactoryOwner = true; + JEventLevel mLevel = JEventLevel::PhysicsEvent; + +public: + JFactorySet(); + JFactorySet(const std::vector& aFactoryGenerators); + virtual ~JFactorySet(); + bool Add(JFactory* aFactory); + bool Add(JMultifactory* multifactory); + void Add(JStorage* storage); + void Print() const; + void Release(); -template -JFactoryT* JFactorySet::GetFactory(const std::string& tag) const { - - auto typed_key = std::make_pair(std::type_index(typeid(T)), tag); - auto typed_iter = mFactories.find(typed_key); - if (typed_iter != std::end(mFactories)) { - JEventLevel found_level = typed_iter->second->GetLevel(); - if (found_level != mLevel) { - throw JException("Factory belongs to a different level on the event hierarchy. Expected: %s, Found: %s", toString(mLevel).c_str(), toString(found_level).c_str()); - } - return static_cast*>(typed_iter->second); - } - - auto untyped_key = std::make_pair(JTypeInfo::demangle(), tag); - auto untyped_iter = mFactoriesFromString.find(untyped_key); - if (untyped_iter != std::end(mFactoriesFromString)) { - JEventLevel found_level = untyped_iter->second->GetLevel(); - if (found_level != mLevel) { - throw JException("Factory belongs to a different level on the event hierarchy. Expected: %s, Found: %s", toString(mLevel).c_str(), toString(found_level).c_str()); - } - return static_cast*>(untyped_iter->second); - } - return nullptr; -} - -template -std::vector*> JFactorySet::GetAllFactoriesT() const { - auto sKey = std::type_index(typeid(T)); - std::vector*> data; - for (auto it=std::begin(mFactories);it!=std::end(mFactories);it++){ - if (it->first.first==sKey){ - if (it->second->GetLevel() == mLevel) { - data.push_back(static_cast*>(it->second)); - } - } - } - return data; -} + std::vector GetAllCollectionNames() const; + JStorage* GetStorage(const std::string& collection_name) const; + + JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; + JFactory* GetFactory(std::type_index object_type, const std::string& object_name, const std::string& tag = "") const; + + std::vector GetAllFactories() const; + std::vector GetAllFactories(std::type_index object_type, const std::string& object_name) const; + + std::vector GetAllMultifactories() const; + + JEventLevel GetLevel() const { return mLevel; } + void SetLevel(JEventLevel level) { mLevel = level; } +}; diff --git a/src/libraries/JANA/JMultifactory.h b/src/libraries/JANA/JMultifactory.h index 07acd1a5f..2569a56df 100644 --- a/src/libraries/JANA/JMultifactory.h +++ b/src/libraries/JANA/JMultifactory.h @@ -10,6 +10,7 @@ #include #include #include +#include #if JANA2_HAVE_PODIO #include @@ -163,8 +164,8 @@ void JMultifactory::DeclareOutput(std::string tag, bool owns_data) { template void JMultifactory::SetData(std::string tag, std::vector data) { - JFactoryT* helper = mHelpers.GetFactory(tag); - if (helper == nullptr) { + JFactory* helper_untyped = mHelpers.GetFactory(std::type_index(typeid(T)), JTypeInfo::demangle(), tag); + if (helper_untyped == nullptr) { auto ex = JException("JMultifactory: Attempting to SetData() without corresponding DeclareOutput()"); ex.function_name = "JMultifactory::SetData"; ex.type_name = m_type_name; @@ -172,6 +173,7 @@ void JMultifactory::SetData(std::string tag, std::vector data) { ex.plugin_name = m_plugin_name; throw ex; } + JFactoryT* helper = static_cast*>(helper_untyped); // This will except if helper is a JMultifactoryHelperPodio. User should use SetPodioData() instead for PODIO data. helper->Set(data); } diff --git a/src/programs/unit_tests/Components/JComponentTests.cc b/src/programs/unit_tests/Components/JComponentTests.cc index 0f20cb6a8..fba6574f6 100644 --- a/src/programs/unit_tests/Components/JComponentTests.cc +++ b/src/programs/unit_tests/Components/JComponentTests.cc @@ -5,12 +5,13 @@ #include #include #include +#include namespace jana { template MultifactoryT* RetrieveMultifactory(JFactorySet* facset, std::string output_collection_name) { - auto fac = facset->GetFactory(output_collection_name); + auto fac = facset->GetFactory(std::type_index(typeid(OutputCollectionT)), JTypeInfo::demangle(), output_collection_name); REQUIRE(fac != nullptr); auto helper = dynamic_cast*>(fac); REQUIRE(helper != nullptr); From c94ac6f6eddc201ea76edcda8a2613c1d845fb18 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 19 Sep 2024 16:25:42 -0400 Subject: [PATCH 17/24] Only run StorageTests when USE_PODIO=1 --- src/programs/unit_tests/CMakeLists.txt | 3 +-- src/programs/unit_tests/Components/JStorageTests.cc | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index 8f40bf68d..6c6f8b662 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -13,7 +13,6 @@ set(TEST_SOURCES Components/ExactlyOnceTests.cc Components/GetObjectsTests.cc Components/NEventNSkipTests.cc - Components/JStorageTests.cc Components/JComponentTests.cc Components/JEventGetAllTests.cc Components/JEventProcessorTests.cc @@ -45,7 +44,7 @@ set(TEST_SOURCES ) if (${USE_PODIO}) - list(APPEND TEST_SOURCES Components/PodioTests.cc) + list(APPEND TEST_SOURCES Components/PodioTests.cc Components/JStorageTests.cc) endif() add_jana_test(jana-unit-tests SOURCES ${TEST_SOURCES}) diff --git a/src/programs/unit_tests/Components/JStorageTests.cc b/src/programs/unit_tests/Components/JStorageTests.cc index 763eda949..73835f8b6 100644 --- a/src/programs/unit_tests/Components/JStorageTests.cc +++ b/src/programs/unit_tests/Components/JStorageTests.cc @@ -13,9 +13,10 @@ #include #include #include +#include + #include #include -#include namespace jstorage_tests { From 34088d06980b6a94aa3ff6ae76e63c24bba5dea5 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 20 Sep 2024 14:16:35 -0400 Subject: [PATCH 18/24] Fix test case --- src/programs/unit_tests/Components/JStorageTests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/programs/unit_tests/Components/JStorageTests.cc b/src/programs/unit_tests/Components/JStorageTests.cc index 73835f8b6..178e5fed2 100644 --- a/src/programs/unit_tests/Components/JStorageTests.cc +++ b/src/programs/unit_tests/Components/JStorageTests.cc @@ -70,7 +70,7 @@ struct RegeneratingTestFactory : public JFactory { struct TestProc : public JEventProcessor { - PodioInput m_clusters_in {this, InputOptions("my_collection")}; + PodioInput m_clusters_in {this, InputOptions{.name="my_collection"}}; TestProc() { SetCallbackStyle(CallbackStyle::ExpertMode); From d29d6aae378a55f01d364a6c080ea52f5dde3498 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 20 Sep 2024 14:16:44 -0400 Subject: [PATCH 19/24] Replace std::optional with JOptional --- src/libraries/JANA/Components/JStorage.h | 9 ++-- src/libraries/JANA/JFactory.h | 3 +- src/libraries/JANA/JFactorySet.cc | 17 +++--- src/libraries/JANA/JFactoryT.h | 2 +- src/libraries/JANA/Utils/JAny.h | 68 ++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 15 deletions(-) diff --git a/src/libraries/JANA/Components/JStorage.h b/src/libraries/JANA/Components/JStorage.h index 08b366995..1b812ddc0 100644 --- a/src/libraries/JANA/Components/JStorage.h +++ b/src/libraries/JANA/Components/JStorage.h @@ -12,7 +12,6 @@ #include #include #include -#include class JFactory; @@ -26,10 +25,10 @@ class JStorage { // Fields Status m_status = Status::Empty; std::string m_collection_name; - std::optional m_collection_tag = std::nullopt; + JOptional m_collection_tag; std::string m_type_name; JFactory* m_factory = nullptr; - std::optional m_inner_type_index = std::nullopt; // e.g. Hit, Cluster + JOptional m_inner_type_index; mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; protected: @@ -45,9 +44,9 @@ class JStorage { // Getters Status GetStatus() const { return m_status; } std::string GetCollectionName() const { return m_collection_name; } - std::optional GetCollectionTag() const { return m_collection_tag; } + JOptional GetCollectionTag() const { return m_collection_tag; } std::string GetTypeName() const { return m_type_name; } - std::optional GetTypeIndex() const { return m_inner_type_index; } + JOptional GetTypeIndex() const { return m_inner_type_index; } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. JFactory* GetFactory() const { return m_factory; } diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 3369c925d..7db60f2a2 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -17,7 +17,6 @@ #include #include #include -#include class JEvent; @@ -165,7 +164,7 @@ class JFactory : public jana::components::JComponent, virtual void Process(const std::shared_ptr&) {} virtual void Finish() {} - virtual std::optional GetObjectType() const { return std::nullopt; } + virtual std::type_index GetObjectType() const { throw JException("GetObjectType not supported for non-JFactoryT's"); } virtual std::size_t GetNumObjects() const { throw JException("Not implemented!"); diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index 7ff1dfbd2..ccf99d6f7 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -105,11 +105,11 @@ bool JFactorySet::Add(JFactory* aFactory) mAllFactories.push_back(aFactory); - auto object_type = aFactory->GetObjectType(); - if (object_type != std::nullopt) { + + if (aFactory->GetOutputs().empty()) { // We have an old-style JFactory! - auto typed_key = std::make_pair( *object_type, aFactory->GetTag() ); + auto typed_key = std::make_pair( aFactory->GetObjectType(), aFactory->GetTag() ); auto untyped_key = std::make_pair( aFactory->GetObjectName(), aFactory->GetTag() ); auto typed_result = mFactories.find(typed_key); @@ -291,10 +291,13 @@ void JFactorySet::Print() const /// Release() loops over all contained factories, clearing their data void JFactorySet::Release() { - - for (const auto& sFactoryPair : mFactories) { - auto sFactory = sFactoryPair.second; - sFactory->ClearData(); + for (auto* fac : mAllFactories) { + fac->ClearData(); + } + for (auto& it : mCollectionsFromName) { + if (it.second->GetFactory() == nullptr) { + it.second->ClearData(); + } } } diff --git a/src/libraries/JANA/JFactoryT.h b/src/libraries/JANA/JFactoryT.h index 456a63200..46fbccb33 100644 --- a/src/libraries/JANA/JFactoryT.h +++ b/src/libraries/JANA/JFactoryT.h @@ -66,7 +66,7 @@ class JFactoryT : public JFactory { void Process(const std::shared_ptr&) override {} - std::optional GetObjectType(void) const override { + std::type_index GetObjectType(void) const override { return std::type_index(typeid(T)); } diff --git a/src/libraries/JANA/Utils/JAny.h b/src/libraries/JANA/Utils/JAny.h index ad244afaf..68e343af1 100644 --- a/src/libraries/JANA/Utils/JAny.h +++ b/src/libraries/JANA/Utils/JAny.h @@ -3,6 +3,9 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #pragma once +#include +#include +#include /// Ideally we'd just use std::any, but we are restricted to C++14 for the time being struct JAny { @@ -17,3 +20,68 @@ struct JAnyT : JAny { ~JAnyT() override = default; // deletes the t }; + +template +class JOptional { +private: + using StorageT = typename std::aligned_storage::type; + bool has_value; + StorageT storage; + +public: + JOptional() : has_value(false) {} + + JOptional(const T& val) : has_value(true) { + new (&storage) T(val); + } + + JOptional(T&& val) : has_value(true) { + new (&storage) T(std::move(val)); + } + + ~JOptional() { + reset(); + } + + // Checks if there is a value + bool hasValue() const { return has_value; } + + // Accesses the value, throws if not present + T& get() { + if (!has_value) { + throw std::runtime_error("No value present"); + } + return *reinterpret_cast(&storage); // Access without launder (C++14) + } + + const T& get() const { + if (!has_value) { + throw std::runtime_error("No value present"); + } + return *reinterpret_cast(&storage); // Access without launder (C++14) + } + + // Resets the optional (removes the value) + void reset() { + if (has_value) { + reinterpret_cast(&storage)->~T(); // Explicitly call destructor + has_value = false; + } + } + + // Set the value + void set(const T& val) { + reset(); + new (&storage) T(val); // Placement new + has_value = true; + } + + // Set using move semantics + void set(T&& val) { + reset(); + new (&storage) T(std::move(val)); // Placement new + has_value = true; + } +}; + + From 2463f2699e58195cb86a2d2d3503632dfa05aa27 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 20 Sep 2024 15:54:20 -0400 Subject: [PATCH 20/24] Clear PodioStorage correctly --- src/libraries/JANA/Components/JPodioStorage.h | 4 ++ src/libraries/JANA/Components/JStorage.h | 1 + src/libraries/JANA/JFactory.h | 1 + .../unit_tests/Components/JStorageTests.cc | 4 +- .../unit_tests/Components/PodioTests.cc | 43 +++++++++++++++++++ 5 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/libraries/JANA/Components/JPodioStorage.h b/src/libraries/JANA/Components/JPodioStorage.h index 90a29ca86..ec5097c1f 100644 --- a/src/libraries/JANA/Components/JPodioStorage.h +++ b/src/libraries/JANA/Components/JPodioStorage.h @@ -18,12 +18,16 @@ class JPodioStorage : public JStorage { public: size_t GetSize() const override { + if (m_collection == nullptr) { + return 0; + } return m_collection->size(); } virtual void ClearData() override { m_collection = nullptr; m_frame = nullptr; + SetStatus(JStorage::Status::Empty); // Podio clears the data itself when the frame is destroyed. // Until then, the collection is immutable. // diff --git a/src/libraries/JANA/Components/JStorage.h b/src/libraries/JANA/Components/JStorage.h index 1b812ddc0..2d76574db 100644 --- a/src/libraries/JANA/Components/JStorage.h +++ b/src/libraries/JANA/Components/JStorage.h @@ -12,6 +12,7 @@ #include #include #include +#include class JFactory; diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 7db60f2a2..d9fb08912 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -57,6 +57,7 @@ class JFactory : public jana::components::JComponent, : mObjectName(std::move(aName)), mTag(std::move(aTag)), mStatus(Status::Uninitialized) { + SetTypeName(mObjectName); SetPrefix(aTag.empty() ? mObjectName : mObjectName + ":" + mTag); }; diff --git a/src/programs/unit_tests/Components/JStorageTests.cc b/src/programs/unit_tests/Components/JStorageTests.cc index 178e5fed2..8a0cec486 100644 --- a/src/programs/unit_tests/Components/JStorageTests.cc +++ b/src/programs/unit_tests/Components/JStorageTests.cc @@ -44,7 +44,7 @@ struct TestFactory : public JFactory { } void Init() { } - void Process(const std::shared_ptr& event) { + void Process(const std::shared_ptr&) { LOG_WARN(GetLogger()) << "Calling TestFactory::Process" << LOG_END; m_clusters()->push_back(MutableExampleCluster(22.2)); m_clusters()->push_back(MutableExampleCluster(27)); @@ -61,7 +61,7 @@ struct RegeneratingTestFactory : public JFactory { } void Init() { } - void Process(const std::shared_ptr& event) { + void Process(const std::shared_ptr&) { LOG_WARN(GetLogger()) << "Calling TestFactory::Process" << LOG_END; m_clusters()->push_back(MutableExampleCluster(22.2)); m_clusters()->push_back(MutableExampleCluster(27)); diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 920d2842d..ef8e3c15b 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -204,6 +204,49 @@ TEST_CASE("Podio JMultifactory::Init gets called") { REQUIRE(multifac_typed != nullptr); REQUIRE(multifac_typed->init_called == true); } +TEST_CASE("PodioTests_InsertMultiple") { + + JApplication app; + auto event = std::make_shared(&app); + + // Insert a cluster + + auto coll1 = ExampleClusterCollection(); + auto cluster1 = coll1.create(22.0); + auto storage = event->InsertCollection(std::move(coll1), "clusters"); + + REQUIRE(storage->GetSize() == 1); + REQUIRE(storage->GetStatus() == JStorage::Status::Inserted); + + // Retrieve and validate cluster + + auto cluster1_retrieved = event->GetCollection("clusters"); + REQUIRE(cluster1_retrieved->at(0).energy() == 22.0); + + // Clear event + + event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool + + // After clearing, the JStorage will still exist, but it will be empty + auto storage2 = event->GetStorage("clusters", false); + REQUIRE(storage2->GetStatus() == JStorage::Status::Empty); + REQUIRE(storage2->GetSize() == 0); + + // Insert a cluster. If event isn't being cleared correctly, this will throw + + auto coll2 = ExampleClusterCollection(); + auto cluster2 = coll2.create(33.0); + auto storage3 = event->InsertCollection(std::move(coll2), "clusters"); + REQUIRE(storage3->GetStatus() == JStorage::Status::Inserted); + REQUIRE(storage3->GetSize() == 1); + + // Retrieve and validate cluster + + auto cluster2_retrieved = event->GetCollection("clusters"); + REQUIRE(cluster2_retrieved->at(0).energy() == 33.0); +} + + } // namespace multifactory } // namespace podiotests From 41ae312622cd5ad3f0570dd8f7d463447462d768 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 20 Sep 2024 21:07:55 -0400 Subject: [PATCH 21/24] Further JFactory::ClearData fixes --- src/libraries/JANA/JFactory.h | 7 +- src/libraries/JANA/JFactorySet.cc | 8 +- src/libraries/JANA/Podio/JFactoryPodioT.h | 6 - .../unit_tests/Components/PodioTests.cc | 105 +++++++++++++++++- 4 files changed, 111 insertions(+), 15 deletions(-) diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index d9fb08912..5de3aee08 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -154,7 +154,12 @@ class JFactory : public jana::components::JComponent, // Overloaded by JFactoryT - virtual void ClearData() {}; + virtual void ClearData() { + if (mStatus == Status::Processed) { + mStatus = Status::Unprocessed; + mCreationStatus = CreationStatus::NotCreatedYet; + } + }; // Overloaded by user Factories diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index ccf99d6f7..a85a41590 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -295,9 +295,11 @@ void JFactorySet::Release() { fac->ClearData(); } for (auto& it : mCollectionsFromName) { - if (it.second->GetFactory() == nullptr) { - it.second->ClearData(); - } + // fac->ClearData() only clears JFactoryT's, because that's how it always worked. + // Clearing is fundamentally an operation on the data bundle, not on the factory itself. + // Furthermore, "clearing" the factory is misleading because factories can cache arbitrary + // state inside member variables, and there's no way to clear that. + it.second->ClearData(); } } diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index 792da15b4..cbd198dc7 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -35,7 +35,6 @@ class JFactoryPodioT : public JFactory { void Finish() override {} std::size_t GetNumObjects() const final { return m_output.GetCollection()->GetSize(); } - void ClearData() final; void SetCollection(CollectionT&& collection); void SetCollection(std::unique_ptr collection); @@ -71,9 +70,4 @@ void JFactoryPodioT::SetCollection(std::unique_ptr collection) { } -template -void JFactoryPodioT::ClearData() { - // Happens automatically now -} - diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index ef8e3c15b..6aae53718 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -2,10 +2,15 @@ #include #include + #include #include #include #include +#include +#include +#include +#include namespace podiotests { @@ -176,21 +181,26 @@ struct TestMultiFac : public JMultifactory { void Init() override { init_called = true; } - void Process(const std::shared_ptr&) override { + void Process(const std::shared_ptr& event) override { ExampleClusterCollection c; - c.push_back(MutableExampleCluster(16.0)); + c.push_back(MutableExampleCluster(16.0 + event->GetEventNumber())); SetCollection("sillyclusters", std::move(c)); } }; -TEST_CASE("Podio JMultifactory::Init gets called") { +TEST_CASE("PodioTests_JMultifactoryInit") { JApplication app; auto event = std::make_shared(&app); + event->SetEventNumber(0); auto fs = new JFactorySet; fs->Add(new TestMultiFac); event->SetFactorySet(fs); - event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool + + // Simulate a trip to the event pool _before_ calling JFactory::Process() + // This is important because a badly designed JFactory::ClearData() could + // mangle the Unprocessed status and consequently skip Init(). + event->GetFactorySet()->Release(); // In theory this shouldn't hurt auto r = event->GetCollectionBase("sillyclusters"); REQUIRE(r != nullptr); @@ -204,6 +214,30 @@ TEST_CASE("Podio JMultifactory::Init gets called") { REQUIRE(multifac_typed != nullptr); REQUIRE(multifac_typed->init_called == true); } + + +TEST_CASE("PodioTests_MultifacMultiple") { + + JApplication app; + auto event = std::make_shared(&app); + event->SetEventNumber(0); + auto fs = new JFactorySet; + fs->Add(new TestMultiFac); + event->SetFactorySet(fs); + + auto r = event->GetCollection("sillyclusters"); + REQUIRE(r->at(0).energy() == 16.0); + + event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool + + event->SetEventNumber(4); + r = event->GetCollection("sillyclusters"); + REQUIRE(r->at(0).energy() == 20.0); + +} +} // namespace multifactory + + TEST_CASE("PodioTests_InsertMultiple") { JApplication app; @@ -247,6 +281,67 @@ TEST_CASE("PodioTests_InsertMultiple") { } -} // namespace multifactory +namespace omnifacmultiple { + +struct MyClusterFactory : public JOmniFactory { + + PodioOutput m_clusters_out{this, "clusters"}; + + void Configure() { + } + + void ChangeRun(int32_t /*run_nr*/) { + } + + void Execute(int32_t /*run_nr*/, uint64_t evt_nr) { + + auto cs = std::make_unique(); + auto cluster = MutableExampleCluster(101.0 + evt_nr); + cs->push_back(cluster); + m_clusters_out() = std::move(cs); + } +}; + +TEST_CASE("PodioTests_OmniFacMultiple") { + + JApplication app; + app.SetParameterValue("jana:loglevel", "error"); + app.Add(new JOmniFactoryGeneratorT("cluster_fac", {}, {"clusters"})); + app.Initialize(); + auto event = std::make_shared(&app); + app.GetService()->configure_event(*event); + event->SetEventNumber(22); + + // Check that storage is already present + auto storage = event->GetStorage("clusters", false); + REQUIRE(storage != nullptr); + REQUIRE(storage->GetStatus() == JStorage::Status::Empty); + + // Retrieve triggers factory + auto coll = event->GetCollection("clusters"); + REQUIRE(coll->size() == 1); + REQUIRE(coll->at(0).energy() == 123.0); + + // Clear everything + event->GetFactorySet()->Release(); + event->SetEventNumber(1010); + + // Check that storage has been reset + storage = event->GetStorage("clusters", false); + REQUIRE(storage != nullptr); + REQUIRE(storage->GetStatus() == JStorage::Status::Empty); + + REQUIRE(storage->GetFactory() != nullptr); + + // Retrieve triggers factory + auto coll2 = event->GetCollection("clusters"); + REQUIRE(coll2->size() == 1); + REQUIRE(coll2->at(0).energy() == 1111.0); +} + +} // namespace omnifacmultiple + + + } // namespace podiotests From 4b5c0a7cc6473ac278787f0471c188470d729bc6 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sat, 21 Sep 2024 00:25:19 -0400 Subject: [PATCH 22/24] Bring back JFactorySet::GetFactory We bring it back only in a minimal, deprecated form. We only do this because of some test cases in EICrecon that use this feature. --- src/libraries/JANA/JFactorySet.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index f4f80499e..ce96db52d 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include @@ -52,6 +53,13 @@ class JFactorySet { JEventLevel GetLevel() const { return mLevel; } void SetLevel(JEventLevel level) { mLevel = level; } + + template + [[deprecated]] + JFactory* GetFactory(const std::string& tag) const { + auto object_name = JTypeInfo::demangle(); + return GetFactory(object_name, tag); + } }; From da316c7e03e31115b97152106b0657993ada2a33 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 11:44:05 -0400 Subject: [PATCH 23/24] Remove templating from JPodioStorage --- src/libraries/JANA/Components/JPodioOutput.h | 25 +++++----- src/libraries/JANA/Components/JPodioStorage.h | 50 +------------------ src/libraries/JANA/JEvent.h | 12 +++-- 3 files changed, 24 insertions(+), 63 deletions(-) diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h index 9106dedcf..5ccef6524 100644 --- a/src/libraries/JANA/Components/JPodioOutput.h +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -13,21 +14,21 @@ template class PodioOutput : public JHasFactoryOutputs::OutputBase { private: std::unique_ptr m_data; - JPodioStorage* m_collection; + JPodioStorage* m_podio_storage; public: PodioOutput(JHasFactoryOutputs* owner, std::string default_collection_name="") { owner->RegisterOutput(this); - auto coll = std::make_unique(); - coll->SetCollectionName(default_collection_name); - coll->SetTypeName(JTypeInfo::demangle()); - m_collection = coll.get(); - m_collections.push_back(std::move(coll)); + auto storage = std::make_unique(); + storage->SetCollectionName(default_collection_name); + storage->SetTypeName(JTypeInfo::demangle()); + m_podio_storage = storage.get(); + m_collections.push_back(std::move(storage)); m_data = std::move(std::make_unique()); } std::unique_ptr& operator()() { return m_data; } - const JStorage* GetCollection() const { return m_collection; } + const JStorage* GetCollection() const { return m_podio_storage; } protected: @@ -48,11 +49,10 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { event.Insert(frame); } - frame->put(std::move(m_data), m_collection->GetCollectionName()); - const auto* moved = &frame->template get(m_collection->GetCollectionName()); + frame->put(std::move(m_data), m_podio_storage->GetCollectionName()); + const auto* moved = &frame->template get(m_podio_storage->GetCollectionName()); m_data = nullptr; - m_collection->SetCollectionAlreadyInFrame(moved); - //m_collection->SetFrame(frame); // We might not need this! + m_podio_storage->SetCollection(moved); } void Reset() override { m_data = std::move(std::make_unique()); @@ -103,7 +103,8 @@ class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { const auto* moved = &frame->template get(m_collections[i]->GetCollectionName()); datum = nullptr; const auto &coll = dynamic_cast(m_collections[i]); - coll.SetCollectionAlreadyInFrame(moved); + coll.SetCollection(moved); + i += 1; } } void Reset() override { diff --git a/src/libraries/JANA/Components/JPodioStorage.h b/src/libraries/JANA/Components/JPodioStorage.h index ec5097c1f..ae16c0a03 100644 --- a/src/libraries/JANA/Components/JPodioStorage.h +++ b/src/libraries/JANA/Components/JPodioStorage.h @@ -3,18 +3,15 @@ #pragma once -#include "JANA/Utils/JTypeInfo.h" #include #include #include -#include -class JPodioStorage : public JStorage { +class JPodioStorage : public JStorage { private: const podio::CollectionBase* m_collection = nullptr; - podio::Frame* m_frame = nullptr; public: size_t GetSize() const override { @@ -26,7 +23,6 @@ class JPodioStorage : public JStorage { virtual void ClearData() override { m_collection = nullptr; - m_frame = nullptr; SetStatus(JStorage::Status::Empty); // Podio clears the data itself when the frame is destroyed. // Until then, the collection is immutable. @@ -39,50 +35,8 @@ class JPodioStorage : public JStorage { // it might also prevent the user from accessing frames directly. } - // Getters const podio::CollectionBase* GetCollection() const { return m_collection; } - - template const typename T::collection_type* GetCollection(); - - - // Setters - void SetFrame(podio::Frame* frame) { m_frame = frame; } - - template - void SetCollection(std::unique_ptr collection); - - template - void SetCollectionAlreadyInFrame(const typename T::collection_type* collection); + void SetCollection(const podio::CollectionBase* collection) { m_collection = collection; } }; -template -const typename T::collection_type* JPodioStorage::GetCollection() { - assert(JTypeInfo::demangle() == this->GetTypeName()); - return dynamic_cast(m_collection); -} - - -template -void JPodioStorage::SetCollection(std::unique_ptr collection) { - /// Provide a PODIO collection. Note that PODIO assumes ownership of this collection, and the - /// collection pointer should be assumed to be invalid after this call - - if (this->m_frame == nullptr) { - throw JException("JPodioStorage: Unable to add collection to frame as frame is missing!"); - } - this->m_frame->put(std::move(collection), this->GetCollectionName()); - const auto* moved = &this->m_frame->template get(this->GetCollectionName()); - this->m_collection = moved; - - this->SetTypeName(JTypeInfo::demangle()); - this->SetStatus(Status::Inserted); -} - -template -void JPodioStorage::SetCollectionAlreadyInFrame(const typename T::collection_type* collection) { - m_collection = collection; - this->SetTypeName(JTypeInfo::demangle()); - SetStatus(Status::Inserted); -} - diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index e54c3bf00..4b5593df7 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -26,6 +26,7 @@ #if JANA2_HAVE_PODIO #include +#include #endif class JApplication; @@ -578,7 +579,12 @@ const typename T::collection_type* JEvent::GetCollection(std::string name, bool throw JException("Not a podio collection: %s", name.c_str()); } else { - return podio_coll->GetCollection(); + auto coll = podio_coll->GetCollection(); + auto typed_coll = dynamic_cast(coll); + if (typed_coll == nullptr) { + throw JException("Unable to cast Podio collection to %s", JTypeInfo::demangle().c_str()); + } + return typed_coll; } } else if (throw_on_missing) { @@ -637,7 +643,7 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas coll->SetTypeName(JTypeInfo::demangle()); coll->SetStatus(JStorage::Status::Inserted); coll->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); - coll->SetCollectionAlreadyInFrame(typed_collection); + coll->SetCollection(typed_collection); mFactorySet->Add(coll); return coll; } @@ -648,7 +654,7 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas throw JException("Collections can only be inserted once!"); } auto typed_storage = dynamic_cast(storage); - typed_storage->SetCollectionAlreadyInFrame(typed_collection); + typed_storage->SetCollection(typed_collection); typed_storage->SetStatus(JStorage::Status::Inserted); typed_storage->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); return typed_storage; From 36ebc7e0d924a8261844371957eed49c48f12483 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 13:30:12 -0400 Subject: [PATCH 24/24] Deep terminology cleanup --- .../{JBasicStorage.h => JBasicDataBundle.h} | 18 ++--- .../Components/{JStorage.h => JDataBundle.h} | 22 +++--- .../JANA/Components/JHasFactoryOutputs.h | 8 +-- .../{JPodioStorage.h => JPodioDataBundle.h} | 6 +- src/libraries/JANA/Components/JPodioOutput.h | 69 +++++++++---------- src/libraries/JANA/JEvent.h | 44 ++++++------ src/libraries/JANA/JFactory.cc | 2 +- src/libraries/JANA/JFactorySet.cc | 49 +++++++------ src/libraries/JANA/JFactorySet.h | 10 +-- src/libraries/JANA/JMultifactory.h | 10 +-- src/libraries/JANA/Podio/JFactoryPodioT.h | 4 +- .../unit_tests/Components/PodioTests.cc | 67 ++++++++++++++---- 12 files changed, 175 insertions(+), 134 deletions(-) rename src/libraries/JANA/Components/{JBasicStorage.h => JBasicDataBundle.h} (89%) rename src/libraries/JANA/Components/{JStorage.h => JDataBundle.h} (87%) rename src/libraries/JANA/Components/{JPodioStorage.h => JPodioDataBundle.h} (90%) diff --git a/src/libraries/JANA/Components/JBasicStorage.h b/src/libraries/JANA/Components/JBasicDataBundle.h similarity index 89% rename from src/libraries/JANA/Components/JBasicStorage.h rename to src/libraries/JANA/Components/JBasicDataBundle.h index c0f8d4da1..fe5053bc0 100644 --- a/src/libraries/JANA/Components/JBasicStorage.h +++ b/src/libraries/JANA/Components/JBasicDataBundle.h @@ -1,7 +1,7 @@ #pragma once -#include +#include #include #include @@ -9,14 +9,14 @@ #include #endif -class JBasicStorage : public JStorage { +class JBasicDataBundle : public JDataBundle { bool m_is_persistent = false; bool m_not_object_owner = false; bool m_write_to_output = true; public: - JBasicStorage() = default; - ~JBasicStorage() override = default; + JBasicDataBundle() = default; + ~JBasicDataBundle() override = default; void SetPersistentFlag(bool persistent) { m_is_persistent = persistent; } void SetNotOwnerFlag(bool not_owner) { m_not_object_owner = not_owner; } void SetWriteToOutputFlag(bool write_to_output) { m_write_to_output = write_to_output; } @@ -29,12 +29,12 @@ class JBasicStorage : public JStorage { template -class JBasicStorageT : public JBasicStorage { +class JBasicDataBundleT : public JBasicDataBundle { private: std::vector m_data; public: - JBasicStorageT(); + JBasicDataBundleT(); void ClearData() override; size_t GetSize() const override { return m_data.size();} @@ -55,7 +55,7 @@ class JBasicStorageT : public JBasicStorage { // Template definitions template -JBasicStorageT::JBasicStorageT() { +JBasicDataBundleT::JBasicDataBundleT() { SetTypeName(JTypeInfo::demangle()); EnableGetAs(); EnableGetAs( std::is_convertible() ); // Automatically add JObject if this can be converted to it @@ -65,7 +65,7 @@ JBasicStorageT::JBasicStorageT() { } template -void JBasicStorageT::ClearData() { +void JBasicDataBundleT::ClearData() { // ClearData won't do anything if Init() hasn't been called if (GetStatus() == Status::Empty) { @@ -87,7 +87,7 @@ void JBasicStorageT::ClearData() { template template -void JBasicStorageT::EnableGetAs() { +void JBasicDataBundleT::EnableGetAs() { auto upcast_lambda = [this]() { std::vector results; diff --git a/src/libraries/JANA/Components/JStorage.h b/src/libraries/JANA/Components/JDataBundle.h similarity index 87% rename from src/libraries/JANA/Components/JStorage.h rename to src/libraries/JANA/Components/JDataBundle.h index 2d76574db..c370076c0 100644 --- a/src/libraries/JANA/Components/JStorage.h +++ b/src/libraries/JANA/Components/JDataBundle.h @@ -17,7 +17,7 @@ class JFactory; -class JStorage { +class JDataBundle { public: // Typedefs enum class Status { Empty, Created, Inserted, InsertedViaGetObjects }; @@ -25,8 +25,8 @@ class JStorage { private: // Fields Status m_status = Status::Empty; - std::string m_collection_name; - JOptional m_collection_tag; + std::string m_unique_name; + JOptional m_short_name; std::string m_type_name; JFactory* m_factory = nullptr; JOptional m_inner_type_index; @@ -37,15 +37,15 @@ class JStorage { public: // Interface - JStorage() = default; - virtual ~JStorage() = default; + JDataBundle() = default; + virtual ~JDataBundle() = default; virtual size_t GetSize() const = 0; virtual void ClearData() = 0; // Getters Status GetStatus() const { return m_status; } - std::string GetCollectionName() const { return m_collection_name; } - JOptional GetCollectionTag() const { return m_collection_tag; } + std::string GetUniqueName() const { return m_unique_name; } + JOptional GetShortName() const { return m_short_name; } std::string GetTypeName() const { return m_type_name; } JOptional GetTypeIndex() const { return m_inner_type_index; } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. @@ -53,12 +53,12 @@ class JStorage { // Setters void SetStatus(Status s) { m_status = s;} - void SetCollectionName(std::string collection_name) { m_collection_name = collection_name; } - void SetCollectionTag(std::string collection_tag) { m_collection_tag = collection_tag; } + void SetUniqueName(std::string unique_name) { m_unique_name = unique_name; } + void SetShortName(std::string short_name) { m_short_name = short_name; } void SetTypeName(std::string type_name) { m_type_name = type_name; } void SetInsertOrigin(JCallGraphRecorder::JDataOrigin origin) { m_insert_origin = origin; } ///< Called automatically by JEvent::Insert() to records whether that call was made by a source or factory. void SetFactory(JFactory* fac) { m_factory = fac; } - + // Templates // /// Generically access the encapsulated data, performing an upcast if necessary. This is useful for extracting data from @@ -86,7 +86,7 @@ class JStorage { // 3. The size of the vtable is expected to be very small (<10 elements, most likely 2) template -std::vector JStorage::GetAs() { +std::vector JDataBundle::GetAs() { std::vector results; auto ti = std::type_index(typeid(S)); auto search = mUpcastVTable.find(ti); diff --git a/src/libraries/JANA/Components/JHasFactoryOutputs.h b/src/libraries/JANA/Components/JHasFactoryOutputs.h index b6b85f5fd..c266ea773 100644 --- a/src/libraries/JANA/Components/JHasFactoryOutputs.h +++ b/src/libraries/JANA/Components/JHasFactoryOutputs.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include class JEvent; @@ -12,11 +12,11 @@ class JHasFactoryOutputs { public: struct OutputBase { protected: - std::vector> m_collections; + std::vector> m_databundles; bool m_is_variadic = false; public: - const std::vector>& GetCollections() const { return m_collections;} - virtual void PutCollections(const JEvent& event) = 0; + const std::vector>& GetDataBundles() const { return m_databundles; } + virtual void StoreData(const JEvent& event) = 0; virtual void Reset() = 0; }; diff --git a/src/libraries/JANA/Components/JPodioStorage.h b/src/libraries/JANA/Components/JPodioDataBundle.h similarity index 90% rename from src/libraries/JANA/Components/JPodioStorage.h rename to src/libraries/JANA/Components/JPodioDataBundle.h index ae16c0a03..d27e20de7 100644 --- a/src/libraries/JANA/Components/JPodioStorage.h +++ b/src/libraries/JANA/Components/JPodioDataBundle.h @@ -3,12 +3,12 @@ #pragma once -#include +#include #include #include -class JPodioStorage : public JStorage { +class JPodioDataBundle : public JDataBundle { private: const podio::CollectionBase* m_collection = nullptr; @@ -23,7 +23,7 @@ class JPodioStorage : public JStorage { virtual void ClearData() override { m_collection = nullptr; - SetStatus(JStorage::Status::Empty); + SetStatus(JDataBundle::Status::Empty); // Podio clears the data itself when the frame is destroyed. // Until then, the collection is immutable. // diff --git a/src/libraries/JANA/Components/JPodioOutput.h b/src/libraries/JANA/Components/JPodioOutput.h index 5ccef6524..de4d6d476 100644 --- a/src/libraries/JANA/Components/JPodioOutput.h +++ b/src/libraries/JANA/Components/JPodioOutput.h @@ -1,5 +1,5 @@ #pragma once -#include +#include #include #include #include @@ -13,29 +13,27 @@ namespace jana::components { template class PodioOutput : public JHasFactoryOutputs::OutputBase { private: - std::unique_ptr m_data; - JPodioStorage* m_podio_storage; + std::unique_ptr m_transient_collection; + JPodioDataBundle* m_podio_databundle; public: PodioOutput(JHasFactoryOutputs* owner, std::string default_collection_name="") { owner->RegisterOutput(this); - auto storage = std::make_unique(); - storage->SetCollectionName(default_collection_name); - storage->SetTypeName(JTypeInfo::demangle()); - m_podio_storage = storage.get(); - m_collections.push_back(std::move(storage)); - m_data = std::move(std::make_unique()); + auto bundle = std::make_unique(); + bundle->SetUniqueName(default_collection_name); + bundle->SetTypeName(JTypeInfo::demangle()); + m_podio_databundle = bundle.get(); + m_databundles.push_back(std::move(bundle)); + m_transient_collection = std::move(std::make_unique()); } - std::unique_ptr& operator()() { return m_data; } + std::unique_ptr& operator()() { return m_transient_collection; } - const JStorage* GetCollection() const { return m_podio_storage; } + JPodioDataBundle* GetDataBundle() const { return m_podio_databundle; } protected: - //void CreateCollections() override { - //} - void PutCollections(const JEvent& event) override { + void StoreData(const JEvent& event) override { podio::Frame* frame; try { frame = const_cast(event.GetSingle()); @@ -49,13 +47,13 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { event.Insert(frame); } - frame->put(std::move(m_data), m_podio_storage->GetCollectionName()); - const auto* moved = &frame->template get(m_podio_storage->GetCollectionName()); - m_data = nullptr; - m_podio_storage->SetCollection(moved); + frame->put(std::move(m_transient_collection), m_podio_databundle->GetUniqueName()); + const auto* moved = &frame->template get(m_podio_databundle->GetUniqueName()); + m_transient_collection = nullptr; + m_podio_databundle->SetCollection(moved); } void Reset() override { - m_data = std::move(std::make_unique()); + m_transient_collection = std::move(std::make_unique()); } }; @@ -63,25 +61,26 @@ class PodioOutput : public JHasFactoryOutputs::OutputBase { template class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { private: - std::vector> m_data; + std::vector> m_collections; + std::vector m_databundles; public: VariadicPodioOutput(JHasFactoryOutputs* owner, std::vector default_collection_names={}) { owner->RegisterOutput(this); this->m_is_variadic = true; for (const std::string& name : default_collection_names) { - auto coll = std::make_unique(); - coll->SetCollectionName(name); + auto coll = std::make_unique(); + coll->SetUniqueName(name); coll->SetTypeName(JTypeInfo::demangle()); m_collections.push_back(std::move(coll)); } for (auto& coll_name : this->collection_names) { - m_data.push_back(std::make_unique()); + m_collections.push_back(std::make_unique()); } } - void PutCollections(const JEvent& event) override { - if (m_data.size() != this->collection_names.size()) { - throw JException("VariadicPodioOutput InsertCollection failed: Declared %d collections, but provided %d.", this->collection_names.size(), m_data.size()); + void StoreData(const JEvent& event) override { + if (m_collections.size() != this->collection_names.size()) { + throw JException("VariadicPodioOutput InsertCollection failed: Declared %d collections, but provided %d.", this->collection_names.size(), m_collections.size()); } podio::Frame* frame; @@ -98,22 +97,22 @@ class VariadicPodioOutput : public JHasFactoryOutputs::OutputBase { } size_t i = 0; - for (auto& datum : m_data) { - frame->put(std::move(std::move(datum)), m_collections[i]->GetCollectionName()); - const auto* moved = &frame->template get(m_collections[i]->GetCollectionName()); - datum = nullptr; - const auto &coll = dynamic_cast(m_collections[i]); - coll.SetCollection(moved); + for (auto& collection : m_collections) { + frame->put(std::move(std::move(collection)), m_databundles[i]->GetUniqueName()); + const auto* moved = &frame->template get(m_databundles[i]->GetUniqueName()); + collection = nullptr; + const auto &databundle = dynamic_cast(m_databundles[i]); + databundle->SetCollection(moved); i += 1; } } void Reset() override { - m_data.clear(); - for (auto& coll : this->m_collections) { + m_collections.clear(); + for (auto& coll : this->m_databundles) { coll->ClearData(); } for (auto& coll_name : this->collection_names) { - m_data.push_back(std::make_unique()); + m_collections.push_back(std::make_unique()); } } }; diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 4b5593df7..f4699be3e 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include @@ -25,7 +25,7 @@ #include #if JANA2_HAVE_PODIO -#include +#include #include #endif @@ -86,12 +86,12 @@ class JEvent : public std::enable_shared_from_this std::vector GetAllCollectionNames() const; const podio::CollectionBase* GetCollectionBase(std::string name, bool throw_on_missing=true) const; template const typename PodioT::collection_type* GetCollection(std::string name, bool throw_on_missing=true) const; - template JPodioStorage* InsertCollection(typename PodioT::collection_type&& collection, std::string name); - template JPodioStorage* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); + template JPodioDataBundle* InsertCollection(typename PodioT::collection_type&& collection, std::string name); + template JPodioDataBundle* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); #endif // EXPERIMENTAL NEW THING - JStorage* GetStorage(const std::string& name, bool create) const; + JDataBundle* GetDataBundle(const std::string& name, bool create) const; //SETTERS void SetRunNumber(int32_t aRunNumber){mRunNumber = aRunNumber;} @@ -526,16 +526,16 @@ JFactoryT* JEvent::GetSingle(const T* &t, const char *tag, bool exception_if_ return fac; } -inline JStorage* JEvent::GetStorage(const std::string& name, bool create) const { +inline JDataBundle* JEvent::GetDataBundle(const std::string& name, bool create) const { - auto* storage = mFactorySet->GetStorage(name); + auto* storage = mFactorySet->GetDataBundle(name); if (storage == nullptr) return nullptr; auto fac = storage->GetFactory(); if (fac != nullptr && create) { // The regenerate logic lives out here now - if ((storage->GetStatus() == JStorage::Status::Empty) || + if ((storage->GetStatus() == JDataBundle::Status::Empty) || (fac->TestFactoryFlag(JFactory::JFactory_Flags_t::REGENERATE))) { // If this was inserted, there would be no factory to run @@ -550,13 +550,13 @@ inline JStorage* JEvent::GetStorage(const std::string& name, bool create) const #if JANA2_HAVE_PODIO inline std::vector JEvent::GetAllCollectionNames() const { - return mFactorySet->GetAllCollectionNames(); + return mFactorySet->GetAllDataBundleNames(); } inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, bool throw_on_missing) const { - auto* storage = GetStorage(name, true); + auto* storage = GetDataBundle(name, true); if (storage != nullptr) { - auto* podio_storage = dynamic_cast(storage); + auto* podio_storage = dynamic_cast(storage); if (podio_storage == nullptr) { throw JException("Not a podio collection: %s", name.c_str()); } @@ -572,9 +572,9 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string name, template const typename T::collection_type* JEvent::GetCollection(std::string name, bool throw_on_missing) const { - auto* coll = GetStorage(name, true); + auto* coll = GetDataBundle(name, true); if (coll != nullptr) { - auto* podio_coll = dynamic_cast(coll); + auto* podio_coll = dynamic_cast(coll); if (podio_coll == nullptr) { throw JException("Not a podio collection: %s", name.c_str()); } @@ -595,7 +595,7 @@ const typename T::collection_type* JEvent::GetCollection(std::string name, bool template -JPodioStorage* JEvent::InsertCollection(typename PodioT::collection_type&& collection, std::string name) { +JPodioDataBundle* JEvent::InsertCollection(typename PodioT::collection_type&& collection, std::string name) { /// InsertCollection inserts the provided PODIO collection into both the podio::Frame and then a JFactoryPodioT podio::Frame* frame = nullptr; @@ -616,7 +616,7 @@ JPodioStorage* JEvent::InsertCollection(typename PodioT::collection_type&& colle template -JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) { +JPodioDataBundle* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) { /// InsertCollection inserts the provided PODIO collection into a JPodioStorage. It assumes that the collection pointer /// is _already_ owned by the podio::Frame corresponding to this JEvent. This is meant to be used if you are starting out /// with a PODIO frame (e.g. a JEventSource that uses podio::ROOTFrameReader). @@ -634,14 +634,14 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas } // Retrieve storage if it already exists, else create it - auto storage = mFactorySet->GetStorage(name); + auto storage = mFactorySet->GetDataBundle(name); if (storage == nullptr) { // No factories already registered this! E.g. from an event source - auto coll = new JPodioStorage; - coll->SetCollectionName(name); + auto coll = new JPodioDataBundle; + coll->SetUniqueName(name); coll->SetTypeName(JTypeInfo::demangle()); - coll->SetStatus(JStorage::Status::Inserted); + coll->SetStatus(JDataBundle::Status::Inserted); coll->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); coll->SetCollection(typed_collection); mFactorySet->Add(coll); @@ -650,12 +650,12 @@ JPodioStorage* JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBas else { // This is overriding a factory // Check that we only inserted this collection once - if (storage->GetStatus() != JStorage::Status::Empty) { + if (storage->GetStatus() != JDataBundle::Status::Empty) { throw JException("Collections can only be inserted once!"); } - auto typed_storage = dynamic_cast(storage); + auto typed_storage = dynamic_cast(storage); typed_storage->SetCollection(typed_collection); - typed_storage->SetStatus(JStorage::Status::Inserted); + typed_storage->SetStatus(JDataBundle::Status::Inserted); typed_storage->SetInsertOrigin(mCallGraph.GetInsertDataOrigin()); return typed_storage; } diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 32f65008d..759591121 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -54,7 +54,7 @@ void JFactory::Create(const std::shared_ptr& event) { } CallWithJExceptionWrapper("JFactory::Process", [&](){ Process(event); }); for (auto& output : this->GetOutputs()) { - output->PutCollections(*event); + output->StoreData(*event); } mStatus = Status::Processed; mCreationStatus = CreationStatus::Created; diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index a85a41590..8f6053c71 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -3,11 +3,10 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #include -#include #include #include -#include +#include #include "JFactorySet.h" #include "JFactory.h" #include "JMultifactory.h" @@ -41,7 +40,7 @@ JFactorySet::~JFactorySet() /// The only time mIsFactoryOwner should/can be set false is when a JMultifactory is using a JFactorySet internally /// to manage its JMultifactoryHelpers. if (mIsFactoryOwner) { - for (auto& s : mCollectionsFromName) { + for (auto& s : mDataBundlesFromName) { // Only delete _inserted_ collections. Collections are otherwise owned by their factories if (s.second->GetFactory() == nullptr) { delete s.second; @@ -57,22 +56,22 @@ JFactorySet::~JFactorySet() //--------------------------------- // Add //--------------------------------- -void JFactorySet::Add(JStorage* collection) { +void JFactorySet::Add(JDataBundle* databundle) { - if (collection->GetCollectionName().empty()) { - throw JException("Attempted to add a collection with no name"); + if (databundle->GetUniqueName().empty()) { + throw JException("Attempted to add a databundle with no unique_name"); } - auto named_result = mCollectionsFromName.find(collection->GetCollectionName()); - if (named_result != std::end(mCollectionsFromName)) { + auto named_result = mDataBundlesFromName.find(databundle->GetUniqueName()); + if (named_result != std::end(mDataBundlesFromName)) { // Collection is duplicate. Since this almost certainly indicates a user error, and // the caller will not be able to do anything about it anyway, throw an exception. // We show the user which factory is causing this problem, including both plugin names - auto ex = JException("Attempted to add duplicate collections"); + auto ex = JException("Attempted to add duplicate databundles"); ex.function_name = "JFactorySet::Add"; - ex.instance_name = collection->GetCollectionName(); + ex.instance_name = databundle->GetUniqueName(); - auto fac = collection->GetFactory(); + auto fac = databundle->GetFactory(); if (fac != nullptr) { ex.type_name = fac->GetTypeName(); ex.plugin_name = fac->GetPluginName(); @@ -83,7 +82,7 @@ void JFactorySet::Add(JStorage* collection) { throw ex; } // Note that this is agnostic to event level. We may decide to change this. - mCollectionsFromName[collection->GetCollectionName()] = collection; + mDataBundlesFromName[databundle->GetUniqueName()] = databundle; } //--------------------------------- @@ -140,9 +139,9 @@ bool JFactorySet::Add(JFactory* aFactory) else { // We have a new-style JFactory! for (const auto* output : aFactory->GetOutputs()) { - for (const auto& coll : output->GetCollections()) { - coll->SetFactory(aFactory); - Add(coll.get()); + for (const auto& bundle : output->GetDataBundles()) { + bundle->SetFactory(aFactory); + Add(bundle.get()); } } } @@ -168,14 +167,14 @@ bool JFactorySet::Add(JMultifactory *multifactory) { } //--------------------------------- -// GetCollection +// GetDataBundle //--------------------------------- -JStorage* JFactorySet::GetStorage(const std::string& collection_name) const { - auto it = mCollectionsFromName.find(collection_name); - if (it != std::end(mCollectionsFromName)) { +JDataBundle* JFactorySet::GetDataBundle(const std::string& name) const { + auto it = mDataBundlesFromName.find(name); + if (it != std::end(mDataBundlesFromName)) { auto fac = it->second->GetFactory(); if (fac != nullptr && fac->GetLevel() != mLevel) { - throw JException("Collection belongs to a different level on the event hierarchy!"); + throw JException("Data bundle belongs to a different level on the event hierarchy!"); } return it->second; } @@ -235,7 +234,7 @@ std::vector JFactorySet::GetAllFactories(std::type_index object_type, // This returns all factories which _directly_ produce objects of type object_type, i.e. they don't use a JStorage. // This is what all of its callers already expect anyhow. Obviously we'd like to migrate everything over to JStorage - // eventually. Rather than updating this to also check mCollectionsFromName, it probably makes more sense to create + // eventually. Rather than updating this to also check mDataBundlesFromName, it probably makes more sense to create // a JFactorySet::GetAllStorages(type_index, object_name) instead, and migrate all callers to use that. std::vector results; @@ -255,11 +254,11 @@ std::vector JFactorySet::GetAllMultifactories() const { } //--------------------------------- -// GetAllCollectionNames +// GetAllDataBundleNames //--------------------------------- -std::vector JFactorySet::GetAllCollectionNames() const { +std::vector JFactorySet::GetAllDataBundleNames() const { std::vector results; - for (const auto& it : mCollectionsFromName) { + for (const auto& it : mDataBundlesFromName) { results.push_back(it.first); } return results; @@ -294,7 +293,7 @@ void JFactorySet::Release() { for (auto* fac : mAllFactories) { fac->ClearData(); } - for (auto& it : mCollectionsFromName) { + for (auto& it : mDataBundlesFromName) { // fac->ClearData() only clears JFactoryT's, because that's how it always worked. // Clearing is fundamentally an operation on the data bundle, not on the factory itself. // Furthermore, "clearing" the factory is misleading because factories can cache arbitrary diff --git a/src/libraries/JANA/JFactorySet.h b/src/libraries/JANA/JFactorySet.h index ce96db52d..46b58d178 100644 --- a/src/libraries/JANA/JFactorySet.h +++ b/src/libraries/JANA/JFactorySet.h @@ -15,7 +15,7 @@ class JFactoryGenerator; class JFactory; class JMultifactory; -class JStorage; +class JDataBundle; class JFactorySet { @@ -24,7 +24,7 @@ class JFactorySet { std::vector mAllFactories; std::map, JFactory*> mFactories; // {(typeid, tag) : factory} std::map, JFactory*> mFactoriesFromString; // {(objname, tag) : factory} - std::map mCollectionsFromName; + std::map mDataBundlesFromName; std::vector mMultifactories; bool mIsFactoryOwner = true; JEventLevel mLevel = JEventLevel::PhysicsEvent; @@ -36,12 +36,12 @@ class JFactorySet { bool Add(JFactory* aFactory); bool Add(JMultifactory* multifactory); - void Add(JStorage* storage); + void Add(JDataBundle* storage); void Print() const; void Release(); - std::vector GetAllCollectionNames() const; - JStorage* GetStorage(const std::string& collection_name) const; + std::vector GetAllDataBundleNames() const; + JDataBundle* GetDataBundle(const std::string& collection_name) const; JFactory* GetFactory(const std::string& object_name, const std::string& tag="") const; JFactory* GetFactory(std::type_index object_type, const std::string& object_name, const std::string& tag = "") const; diff --git a/src/libraries/JANA/JMultifactory.h b/src/libraries/JANA/JMultifactory.h index 2569a56df..763fca62e 100644 --- a/src/libraries/JANA/JMultifactory.h +++ b/src/libraries/JANA/JMultifactory.h @@ -51,7 +51,7 @@ class JMultifactoryHelperPodio : public JFactory { JMultifactoryHelperPodio(JMultifactory* parent, std::string collection_name) : mMultiFactory(parent) { mObjectName = JTypeInfo::demangle(); mTag = collection_name; - m_output.GetCollections().at(0)->SetCollectionName(collection_name); + m_output.GetDataBundle()->SetUniqueName(collection_name); } virtual ~JMultifactoryHelperPodio() = default; @@ -193,8 +193,8 @@ void JMultifactory::DeclarePodioOutput(std::string coll_name, bool) { } template -void JMultifactory::SetCollection(std::string tag, typename T::collection_type&& collection) { - JFactory* helper = mHelpers.GetStorage(tag)->GetFactory(); +void JMultifactory::SetCollection(std::string name, typename T::collection_type&& collection) { + JFactory* helper = mHelpers.GetDataBundle(name)->GetFactory(); if (helper == nullptr) { auto ex = JException("JMultifactory: Attempting to SetData() without corresponding DeclareOutput()"); ex.function_name = "JMultifactory::SetCollection"; @@ -217,8 +217,8 @@ void JMultifactory::SetCollection(std::string tag, typename T::collection_type&& } template -void JMultifactory::SetCollection(std::string tag, std::unique_ptr collection) { - JFactory* helper = mHelpers.GetStorage(tag)->GetFactory(); +void JMultifactory::SetCollection(std::string name, std::unique_ptr collection) { + JFactory* helper = mHelpers.GetDataBundle(name)->GetFactory(); if (helper == nullptr) { auto ex = JException("JMultifactory: Attempting to SetData() without corresponding DeclareOutput()"); ex.function_name = "JMultifactory::SetCollection"; diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index cbd198dc7..7db5ad5cf 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -24,7 +24,7 @@ class JFactoryPodioT : public JFactory { void SetTag(std::string tag) { mTag = tag; - m_output.GetCollections().at(0)->SetCollectionName(tag); + m_output.GetDataBundle()->SetUniqueName(tag); } void Init() override {} @@ -34,7 +34,7 @@ class JFactoryPodioT : public JFactory { void EndRun() override {} void Finish() override {} - std::size_t GetNumObjects() const final { return m_output.GetCollection()->GetSize(); } + std::size_t GetNumObjects() const final { return m_output.GetDataBundle()->GetSize(); } void SetCollection(CollectionT&& collection); void SetCollection(std::unique_ptr collection); diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 6aae53718..faa0f54c6 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -3,15 +3,16 @@ #include -#include #include #include #include #include -#include +#include #include #include +#include +#include namespace podiotests { @@ -163,7 +164,7 @@ TEST_CASE("JFactoryPodioT::Init gets called") { REQUIRE(res != nullptr); REQUIRE((*res)[0].energy() == 16.0); - auto fac_untyped = event->GetStorage("clusters", false)->GetFactory(); + auto fac_untyped = event->GetDataBundle("clusters", false)->GetFactory(); REQUIRE(fac_untyped != nullptr); auto fac = dynamic_cast(fac_untyped); REQUIRE(fac != nullptr); @@ -250,7 +251,7 @@ TEST_CASE("PodioTests_InsertMultiple") { auto storage = event->InsertCollection(std::move(coll1), "clusters"); REQUIRE(storage->GetSize() == 1); - REQUIRE(storage->GetStatus() == JStorage::Status::Inserted); + REQUIRE(storage->GetStatus() == JDataBundle::Status::Inserted); // Retrieve and validate cluster @@ -261,9 +262,9 @@ TEST_CASE("PodioTests_InsertMultiple") { event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool - // After clearing, the JStorage will still exist, but it will be empty - auto storage2 = event->GetStorage("clusters", false); - REQUIRE(storage2->GetStatus() == JStorage::Status::Empty); + // After clearing, the JDataBundle will still exist, but it will be empty + auto storage2 = event->GetDataBundle("clusters", false); + REQUIRE(storage2->GetStatus() == JDataBundle::Status::Empty); REQUIRE(storage2->GetSize() == 0); // Insert a cluster. If event isn't being cleared correctly, this will throw @@ -271,7 +272,7 @@ TEST_CASE("PodioTests_InsertMultiple") { auto coll2 = ExampleClusterCollection(); auto cluster2 = coll2.create(33.0); auto storage3 = event->InsertCollection(std::move(coll2), "clusters"); - REQUIRE(storage3->GetStatus() == JStorage::Status::Inserted); + REQUIRE(storage3->GetStatus() == JDataBundle::Status::Inserted); REQUIRE(storage3->GetSize() == 1); // Retrieve and validate cluster @@ -313,9 +314,9 @@ TEST_CASE("PodioTests_OmniFacMultiple") { event->SetEventNumber(22); // Check that storage is already present - auto storage = event->GetStorage("clusters", false); + auto storage = event->GetDataBundle("clusters", false); REQUIRE(storage != nullptr); - REQUIRE(storage->GetStatus() == JStorage::Status::Empty); + REQUIRE(storage->GetStatus() == JDataBundle::Status::Empty); // Retrieve triggers factory auto coll = event->GetCollection("clusters"); @@ -327,9 +328,9 @@ TEST_CASE("PodioTests_OmniFacMultiple") { event->SetEventNumber(1010); // Check that storage has been reset - storage = event->GetStorage("clusters", false); + storage = event->GetDataBundle("clusters", false); REQUIRE(storage != nullptr); - REQUIRE(storage->GetStatus() == JStorage::Status::Empty); + REQUIRE(storage->GetStatus() == JDataBundle::Status::Empty); REQUIRE(storage->GetFactory() != nullptr); @@ -342,6 +343,48 @@ TEST_CASE("PodioTests_OmniFacMultiple") { } // namespace omnifacmultiple +namespace omnifacreadinsert { + +struct RWClusterFac : public JOmniFactory { + + PodioInput m_clusters_in{this}; + PodioOutput m_clusters_out{this}; + + void Configure() {} + void ChangeRun(int32_t /*run_nr*/) {} + void Execute(int32_t /*run_nr*/, uint64_t evt_nr) { + + auto cs = std::make_unique(); + for (const auto& cluster_in : *m_clusters_in()) { + auto cluster = MutableExampleCluster(1.0 + cluster_in.energy()); + cs->push_back(cluster); + } + m_clusters_out() = std::move(cs); + } +}; + +TEST_CASE("PodioTests_OmniFacReadInsert") { + + JApplication app; + app.SetParameterValue("jana:loglevel", "error"); + app.Add(new JOmniFactoryGeneratorT("cluster_fac", {"protoclusters"}, {"clusters"})); + app.Initialize(); + auto event = std::make_shared(&app); + app.GetService()->configure_event(*event); + + auto coll1 = ExampleClusterCollection(); + auto cluster1 = coll1.create(22.0); + auto storage = event->InsertCollection(std::move(coll1), "protoclusters"); + + REQUIRE(storage->GetSize() == 1); + REQUIRE(storage->GetStatus() == JDataBundle::Status::Inserted); + // Retrieve triggers factory + auto coll = event->GetCollection("clusters"); + REQUIRE(coll->size() == 1); + REQUIRE(coll->at(0).energy() == 23.0); + +} +} // namespace omnifacreadinsert } // namespace podiotests