From 6a8061bb35dc190aff620bbc946feb777ba88f10 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 3 Sep 2024 23:46:37 -0400 Subject: [PATCH 01/10] Separate PodioDatamodel from PodioExample We stop using DatamodelGlue.h everywhere while we are at it. This is because the DatamodelGlue concept noticeably slows down compilation when the data model gets large. We still need a datamodel visitor for event sources, but shouldn't need it anywhere else. --- src/examples/CMakeLists.txt | 1 + src/examples/PodioDatamodel/CMakeLists.txt | 35 +++++++++++++++++++ src/examples/PodioDatamodel/README.md | 11 ++++++ .../datamodel.yaml} | 14 ++++---- src/examples/PodioExample/CMakeLists.txt | 27 +------------- src/examples/PodioExample/DatamodelGlue.h | 8 ++--- .../PodioExample/ExampleClusterFactory.cc | 4 +-- .../PodioExample/ExampleClusterFactory.h | 3 +- .../PodioExample/ExampleMultifactory.cc | 3 +- src/examples/PodioExample/PodioExample.cc | 4 +-- .../PodioExample/PodioExampleProcessor.cc | 2 ++ .../PodioExample/PodioExampleProcessor.h | 1 - .../PodioExample/PodioExampleSource.cc | 2 +- src/examples/TimesliceExample/CMakeLists.txt | 2 +- .../TimesliceExample/CollectionTabulators.h | 3 +- .../TimesliceExample/MyClusterFactory.h | 2 +- src/examples/TimesliceExample/MyFileReader.h | 3 +- src/examples/TimesliceExample/MyFileWriter.h | 1 - .../TimesliceExample/MyProtoclusterFactory.h | 3 +- .../TimesliceExample/MyTimesliceSplitter.h | 3 +- src/programs/perf_tests/CMakeLists.txt | 2 +- src/programs/perf_tests/PodioStressTest.h | 4 +-- src/programs/unit_tests/CMakeLists.txt | 2 +- .../unit_tests/Components/PodioTests.cc | 3 +- 24 files changed, 82 insertions(+), 61 deletions(-) create mode 100644 src/examples/PodioDatamodel/CMakeLists.txt create mode 100644 src/examples/PodioDatamodel/README.md rename src/examples/{PodioExample/layout.yaml => PodioDatamodel/datamodel.yaml} (74%) diff --git a/src/examples/CMakeLists.txt b/src/examples/CMakeLists.txt index 197d183f3..32c315c46 100644 --- a/src/examples/CMakeLists.txt +++ b/src/examples/CMakeLists.txt @@ -7,6 +7,7 @@ add_subdirectory(EventGroupExample) add_subdirectory(SubeventExample) add_subdirectory(SubeventCUDAExample) add_subdirectory(UnitTestingExample) +add_subdirectory(PodioDatamodel) add_subdirectory(PodioExample) add_subdirectory(TimesliceExample) add_subdirectory(RootDatamodelExample) diff --git a/src/examples/PodioDatamodel/CMakeLists.txt b/src/examples/PodioDatamodel/CMakeLists.txt new file mode 100644 index 000000000..eb6bab7cf --- /dev/null +++ b/src/examples/PodioDatamodel/CMakeLists.txt @@ -0,0 +1,35 @@ + +if (USE_PODIO) + + PODIO_GENERATE_DATAMODEL(PodioDatamodel datamodel.yaml headers sources + IO_BACKEND_HANDLERS ROOT + OUTPUT_FOLDER ${CMAKE_CURRENT_BINARY_DIR} + ) + + PODIO_ADD_DATAMODEL_CORE_LIB(PodioDatamodel "${headers}" "${sources}" + OUTPUT_FOLDER ${CMAKE_CURRENT_BINARY_DIR}) + + PODIO_ADD_ROOT_IO_DICT(PodioDatamodelDict PodioDatamodel "${headers}" + ${CMAKE_CURRENT_BINARY_DIR}/src/selection.xml) + + install(TARGETS PodioDatamodel + EXPORT jana2_targets + LIBRARY DESTINATION lib + PUBLIC_HEADER DESTINATION include/JANA/examples/PodioDatamodel + ) + + install(TARGETS PodioDatamodelDict + EXPORT jana2_targets + DESTINATION lib + ) + + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/PodioDatamodelDictDict.rootmap DESTINATION lib) + install(FILES ${CMAKE_CURRENT_BINARY_DIR}/libPodioDatamodelDict_rdict.pcm DESTINATION lib) + +else() + message(STATUS "Skipping examples/PodioDatamodel because USE_PODIO=Off") + +endif() + + + diff --git a/src/examples/PodioDatamodel/README.md b/src/examples/PodioDatamodel/README.md new file mode 100644 index 000000000..1bf56d5be --- /dev/null +++ b/src/examples/PodioDatamodel/README.md @@ -0,0 +1,11 @@ + +PodioDatamodel +============== + +This example shows how to define and integrate a PODIO data model within a JANA project. +The entire data model is defined within a single YAML file. The generated files can be found +in the build directory. The headers are installed to `$CMAKE_INSTALL_PREFIX/include/examples/PodioDatamodel` +and the shared library is installed to `$CMAKE_INSTALL_PREFIX/lib`. + +This data model is used by `PodioExample`, `TimesliceExample`, and more. + diff --git a/src/examples/PodioExample/layout.yaml b/src/examples/PodioDatamodel/datamodel.yaml similarity index 74% rename from src/examples/PodioExample/layout.yaml rename to src/examples/PodioDatamodel/datamodel.yaml index 6542f6805..8a05c2cdf 100644 --- a/src/examples/PodioExample/layout.yaml +++ b/src/examples/PodioDatamodel/datamodel.yaml @@ -1,6 +1,4 @@ --- -# A more complete example is provided by podio under tests/datalayout.yaml - schema_version: 1 options : # should getters / setters be prefixed with get / set? @@ -29,12 +27,12 @@ datatypes : Description : "Example Hit" Author : "B. Hegner" Members: - - unsigned long long cellID // cellID - - double x // x-coordinate - - double y // y-coordinate - - double z // z-coordinate - - double energy // measured energy deposit - - uint64_t time // ticks since start of timeframe + - uint64_t cellID // cellID + - double x // x-coordinate + - double y // y-coordinate + - double z // z-coordinate + - double energy // measured energy deposit + - uint64_t time // ticks since start of timeframe ExampleCluster : Description : "Cluster" diff --git a/src/examples/PodioExample/CMakeLists.txt b/src/examples/PodioExample/CMakeLists.txt index f42a34e09..94c2bc56a 100644 --- a/src/examples/PodioExample/CMakeLists.txt +++ b/src/examples/PodioExample/CMakeLists.txt @@ -9,39 +9,14 @@ set(PodioExample_SOURCES if (USE_PODIO) - PODIO_GENERATE_DATAMODEL(PodioExampleDatamodel layout.yaml headers sources - IO_BACKEND_HANDLERS ROOT - OUTPUT_FOLDER ${CMAKE_CURRENT_BINARY_DIR} - ) - - PODIO_ADD_DATAMODEL_CORE_LIB(PodioExampleDatamodel "${headers}" "${sources}" - OUTPUT_FOLDER ${CMAKE_CURRENT_BINARY_DIR}) - - PODIO_ADD_ROOT_IO_DICT(PodioExampleDatamodelDict PodioExampleDatamodel "${headers}" - ${CMAKE_CURRENT_BINARY_DIR}/src/selection.xml) - - find_package(podio REQUIRED) add_executable(PodioExample ${PodioExample_SOURCES}) target_link_libraries(PodioExample - jana2 PodioExampleDatamodel PodioExampleDatamodelDict podio::podioRootIO) + PUBLIC jana2 PodioDatamodel PodioDatamodelDict podio::podioRootIO) set_target_properties(PodioExample PROPERTIES INSTALL_RPATH_USE_LINK_PATH TRUE) install(TARGETS PodioExample DESTINATION bin) - install(TARGETS PodioExampleDatamodel - EXPORT jana2_targets - LIBRARY DESTINATION lib/JANA/plugins - PUBLIC_HEADER DESTINATION include/JANA/plugins/PodioExampleDatamodel - ) - - install(TARGETS PodioExampleDatamodelDict - EXPORT jana2_targets - DESTINATION lib/JANA/plugins) - - install(FILES ${CMAKE_CURRENT_BINARY_DIR}/PodioExampleDatamodelDictDict.rootmap DESTINATION lib/JANA/plugins) - install(FILES ${CMAKE_CURRENT_BINARY_DIR}/libPodioExampleDatamodelDict_rdict.pcm DESTINATION lib/JANA/plugins) - add_test(NAME jana-example-podio-tests COMMAND PodioExample) diff --git a/src/examples/PodioExample/DatamodelGlue.h b/src/examples/PodioExample/DatamodelGlue.h index a96ac9789..f7ff17bf3 100644 --- a/src/examples/PodioExample/DatamodelGlue.h +++ b/src/examples/PodioExample/DatamodelGlue.h @@ -6,10 +6,10 @@ #ifndef JANA2_DATAMODELGLUE_H #define JANA2_DATAMODELGLUE_H -#include -#include -#include -#include +#include +#include +#include +#include template diff --git a/src/examples/PodioExample/ExampleClusterFactory.cc b/src/examples/PodioExample/ExampleClusterFactory.cc index 9e91ea83a..2b5e4d251 100644 --- a/src/examples/PodioExample/ExampleClusterFactory.cc +++ b/src/examples/PodioExample/ExampleClusterFactory.cc @@ -4,7 +4,8 @@ #include "ExampleClusterFactory.h" -#include "PodioExampleDatamodel/ExampleHit.h" +#include "PodioDatamodel/ExampleClusterCollection.h" +#include "PodioDatamodel/ExampleHitCollection.h" #include ExampleClusterFactory::ExampleClusterFactory() { @@ -54,4 +55,3 @@ void ExampleClusterFactory::Process(const std::shared_ptr &event) } -// TODO: Expose collections as refs, not ptrs? diff --git a/src/examples/PodioExample/ExampleClusterFactory.h b/src/examples/PodioExample/ExampleClusterFactory.h index 39a979490..0f5cb6ea0 100644 --- a/src/examples/PodioExample/ExampleClusterFactory.h +++ b/src/examples/PodioExample/ExampleClusterFactory.h @@ -7,8 +7,7 @@ #define JANA2_EXAMPLECLUSTERFACTORY_H #include -#include "PodioExampleDatamodel/ExampleCluster.h" -#include "DatamodelGlue.h" +#include "PodioDatamodel/ExampleCluster.h" class ExampleClusterFactory : public JFactoryPodioT { public: diff --git a/src/examples/PodioExample/ExampleMultifactory.cc b/src/examples/PodioExample/ExampleMultifactory.cc index 8305e8951..ef6631449 100644 --- a/src/examples/PodioExample/ExampleMultifactory.cc +++ b/src/examples/PodioExample/ExampleMultifactory.cc @@ -5,7 +5,8 @@ #include "ExampleMultifactory.h" #include -#include "DatamodelGlue.h" +#include +#include ExampleMultifactory::ExampleMultifactory() { diff --git a/src/examples/PodioExample/PodioExample.cc b/src/examples/PodioExample/PodioExample.cc index 4b9c88bc8..49acd8045 100644 --- a/src/examples/PodioExample/PodioExample.cc +++ b/src/examples/PodioExample/PodioExample.cc @@ -3,8 +3,8 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #include #include -#include "PodioExampleDatamodel/MutableExampleHit.h" -#include "PodioExampleDatamodel/ExampleHitCollection.h" +#include "PodioDatamodel/MutableExampleHit.h" +#include "PodioDatamodel/ExampleHitCollection.h" #include #include diff --git a/src/examples/PodioExample/PodioExampleProcessor.cc b/src/examples/PodioExample/PodioExampleProcessor.cc index f09ec290c..564eed490 100644 --- a/src/examples/PodioExample/PodioExampleProcessor.cc +++ b/src/examples/PodioExample/PodioExampleProcessor.cc @@ -4,6 +4,8 @@ #include "PodioExampleProcessor.h" +#include +#include /* struct PrintingVisitor { diff --git a/src/examples/PodioExample/PodioExampleProcessor.h b/src/examples/PodioExample/PodioExampleProcessor.h index f14b33672..5a19f9897 100644 --- a/src/examples/PodioExample/PodioExampleProcessor.h +++ b/src/examples/PodioExample/PodioExampleProcessor.h @@ -7,7 +7,6 @@ #define JANA2_PODIOEXAMPLEPROCESSOR_H #include -#include "DatamodelGlue.h" class PodioExampleProcessor : public JEventProcessor { public: diff --git a/src/examples/PodioExample/PodioExampleSource.cc b/src/examples/PodioExample/PodioExampleSource.cc index 66c5f5e4f..7ea286fd3 100644 --- a/src/examples/PodioExample/PodioExampleSource.cc +++ b/src/examples/PodioExample/PodioExampleSource.cc @@ -4,7 +4,7 @@ #include "PodioExampleSource.h" -#include +#include std::unique_ptr PodioExampleSource::NextFrame(int event_index, int &event_number, int &run_number) { diff --git a/src/examples/TimesliceExample/CMakeLists.txt b/src/examples/TimesliceExample/CMakeLists.txt index e3bc209b4..efd750b94 100644 --- a/src/examples/TimesliceExample/CMakeLists.txt +++ b/src/examples/TimesliceExample/CMakeLists.txt @@ -4,7 +4,7 @@ if (USE_PODIO) add_jana_plugin(TimesliceExample) - target_link_libraries(TimesliceExample PUBLIC PodioExampleDatamodel PodioExampleDatamodelDict podio::podioRootIO) + target_link_libraries(TimesliceExample PUBLIC PodioDatamodel PodioDatamodelDict podio::podioRootIO) add_test(NAME jana-example-timeslices-simple-tests COMMAND ${CMAKE_INSTALL_PREFIX}/bin/jana -Pplugins=TimesliceExample -Pjana:nevents=10 events.root) diff --git a/src/examples/TimesliceExample/CollectionTabulators.h b/src/examples/TimesliceExample/CollectionTabulators.h index d8446fd62..4b8705adb 100644 --- a/src/examples/TimesliceExample/CollectionTabulators.h +++ b/src/examples/TimesliceExample/CollectionTabulators.h @@ -1,6 +1,7 @@ #pragma once -#include "DatamodelGlue.h" +#include +#include #include JTablePrinter TabulateClusters(const ExampleClusterCollection* c); diff --git a/src/examples/TimesliceExample/MyClusterFactory.h b/src/examples/TimesliceExample/MyClusterFactory.h index b123ae9d2..054347f3d 100644 --- a/src/examples/TimesliceExample/MyClusterFactory.h +++ b/src/examples/TimesliceExample/MyClusterFactory.h @@ -3,8 +3,8 @@ #pragma once -#include #include +#include struct MyClusterFactory : public JOmniFactory { diff --git a/src/examples/TimesliceExample/MyFileReader.h b/src/examples/TimesliceExample/MyFileReader.h index 7c372d442..7ad404051 100644 --- a/src/examples/TimesliceExample/MyFileReader.h +++ b/src/examples/TimesliceExample/MyFileReader.h @@ -3,8 +3,9 @@ #pragma once -#include #include +#include +#include #include "CollectionTabulators.h" diff --git a/src/examples/TimesliceExample/MyFileWriter.h b/src/examples/TimesliceExample/MyFileWriter.h index 110f823e7..109e13f4d 100644 --- a/src/examples/TimesliceExample/MyFileWriter.h +++ b/src/examples/TimesliceExample/MyFileWriter.h @@ -4,7 +4,6 @@ #pragma once -#include #include #include "CollectionTabulators.h" #include diff --git a/src/examples/TimesliceExample/MyProtoclusterFactory.h b/src/examples/TimesliceExample/MyProtoclusterFactory.h index 099d99081..4d491ca9f 100644 --- a/src/examples/TimesliceExample/MyProtoclusterFactory.h +++ b/src/examples/TimesliceExample/MyProtoclusterFactory.h @@ -3,8 +3,9 @@ #pragma once -#include #include +#include +#include struct MyProtoclusterFactory : public JOmniFactory { diff --git a/src/examples/TimesliceExample/MyTimesliceSplitter.h b/src/examples/TimesliceExample/MyTimesliceSplitter.h index 96b658a9f..f43e016ca 100644 --- a/src/examples/TimesliceExample/MyTimesliceSplitter.h +++ b/src/examples/TimesliceExample/MyTimesliceSplitter.h @@ -3,9 +3,8 @@ #pragma once -#include - #include +#include #include "CollectionTabulators.h" struct MyTimesliceSplitter : public JEventUnfolder { diff --git a/src/programs/perf_tests/CMakeLists.txt b/src/programs/perf_tests/CMakeLists.txt index 558516bbd..de6ebde60 100644 --- a/src/programs/perf_tests/CMakeLists.txt +++ b/src/programs/perf_tests/CMakeLists.txt @@ -4,7 +4,7 @@ add_jana_test(jana-perf-tests) if (USE_PODIO) find_package(podio REQUIRED) - target_link_libraries(jana-perf-tests PRIVATE podio::podio PodioExampleDatamodel PodioExampleDatamodelDict podio::podioRootIO) + target_link_libraries(jana-perf-tests PRIVATE PodioDatamodel PodioDatamodelDict) else() message(STATUS "jana-perf-tests compiled without PODIO stress test because USE_PODIO=Off") diff --git a/src/programs/perf_tests/PodioStressTest.h b/src/programs/perf_tests/PodioStressTest.h index 8ad2ee9df..7df870772 100644 --- a/src/programs/perf_tests/PodioStressTest.h +++ b/src/programs/perf_tests/PodioStressTest.h @@ -1,7 +1,7 @@ #pragma once -#include -#include +#include +#include diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index 7ae7f6915..a45524d3a 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -51,7 +51,7 @@ add_jana_test(jana-unit-tests SOURCES ${TEST_SOURCES}) if (${USE_PODIO}) # Pull in the data model from examples/PodioExample. # We don't want to have multiple separate toy data models in the JANA codebase - target_link_libraries(jana-unit-tests PRIVATE PodioExampleDatamodel PodioExampleDatamodelDict) + target_link_libraries(jana-unit-tests PRIVATE PodioDatamodel PodioDatamodelDict) endif() diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 9fdcd4099..c0bedb560 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -2,8 +2,7 @@ #include #include -#include -#include // Hopefully this won't be necessary in the future +#include #include namespace podiotests { From 93041e341448720394cfd8aa82c96b4bd4871242 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 4 Sep 2024 12:51:11 -0400 Subject: [PATCH 02/10] JEventSourceGenerator uses default ctor when possible This gets us out of the mess where user classes are required to have certain constructor arguments in order to forward them to the parent constructor. --- src/libraries/JANA/JEventSourceGeneratorT.h | 55 ++++++++++++++++++--- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/src/libraries/JANA/JEventSourceGeneratorT.h b/src/libraries/JANA/JEventSourceGeneratorT.h index f13c3ed48..8b6608fa1 100644 --- a/src/libraries/JANA/JEventSourceGeneratorT.h +++ b/src/libraries/JANA/JEventSourceGeneratorT.h @@ -8,6 +8,7 @@ #include #include +#include class JApplication; @@ -45,7 +46,7 @@ class JApplication; /// -template +template class JEventSourceGeneratorT:public JEventSourceGenerator{ public: @@ -62,10 +63,13 @@ class JEventSourceGeneratorT:public JEventSourceGenerator{ /// Create an instance of the source type this generates JEventSource* MakeJEventSource( std::string resource_name ){ + auto source = new T( resource_name, mApplication ); if (mLevel != JEventLevel::None) { source->SetLevel(mLevel); } + source->SetTypeName(JTypeInfo::demangle()); + source->SetPluginName(mPluginName); return source; } @@ -79,13 +83,52 @@ class JEventSourceGeneratorT:public JEventSourceGenerator{ /// template<> double JEventSourceGeneratorT::CheckOpenable(std::string source) { ... } /// double CheckOpenable( std::string /* source */ ){ return 0.01; } +}; + + + + +// Specialization for default-constructible event sources +template +class JEventSourceGeneratorT>> : public JEventSourceGenerator{ + public: + + JEventSourceGeneratorT() : JEventSourceGenerator() {} + virtual ~JEventSourceGeneratorT() {} + + /// Return name of the source type this will generate + std::string GetType(void) const { + return JTypeInfo::demangle(); + } + + /// Return description of the source type this will generate + std::string GetDescription(void) const { return T::GetDescription(); } - protected: + /// Create an instance of the source type this generates + JEventSource* MakeJEventSource(std::string resource_name) { - /// This is called by JEventSourceManager::AddJEventSourceGenerator which - /// itself is called by JApplication::Add(JEventSourceGenerator*). There - /// should be no need to call it from anywhere else. - void SetJApplication(JApplication *app){ mApplication = app; } + auto source = new T; + source->SetTypeName(JTypeInfo::demangle()); + source->SetResourceName(resource_name); + source->SetApplication(mApplication); + source->SetPluginName(mPluginName); + if (mLevel != JEventLevel::None) { + source->SetLevel(mLevel); + } + return source; + } + + /// Check how likely a source of the type this generates is to read + /// the specified source. This mechanism is to allow a single executable + /// to read from multiple file types, each corresponding to a different + /// JEventSource subclass. If you use only a single source type, then + /// there is no need to override this. If you do need this functionality + /// however, then override this in your code with something like: + /// + /// template<> double JEventSourceGeneratorT::CheckOpenable(std::string source) { ... } + /// + double CheckOpenable( std::string /* source */ ){ return 0.01; } }; + From 0f22ff242e3a56a0ff41a284cac1c0019a347fd9 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 4 Sep 2024 13:29:31 -0400 Subject: [PATCH 03/10] Relax type requirements on JEvent::InsertCollectionAlreadyInFrame --- src/libraries/JANA/JEvent.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 194b7f7f2..e60e7cbf7 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -104,7 +104,7 @@ class JEvent : public std::enable_shared_from_this 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 typename JFactoryPodioT::CollectionT* collection, std::string name); + template JFactoryPodioT* InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name); #endif //SETTERS @@ -565,10 +565,16 @@ JFactoryPodioT* JEvent::InsertCollection(typename JFactoryPodioT::Collecti template -JFactoryPodioT* JEvent::InsertCollectionAlreadyInFrame(const typename JFactoryPodioT::CollectionT* collection, std::string name) { +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 /// 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); + 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()); + } // Users are allowed to Insert with tag="" if and only if that tag gets resolved by default tags. if (mUseDefaultTags && name.empty()) { @@ -605,7 +611,7 @@ JFactoryPodioT* JEvent::InsertCollectionAlreadyInFrame(const typename JFactor throw JException("Factory must inherit from JFactoryPodioT in order to use JEvent::GetCollection()"); } - typed_factory->SetCollectionAlreadyInFrame(collection); + typed_factory->SetCollectionAlreadyInFrame(typed_collection); typed_factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); return typed_factory; } From 5214edb5894b2ecf24f439c29089048a01e482be Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 4 Sep 2024 13:53:15 -0400 Subject: [PATCH 04/10] Add PodioFileReader example --- src/examples/PodioFileReader/CMakeLists.txt | 15 +++ .../PodioFileReader/PodioFileReader.cc | 105 ++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 src/examples/PodioFileReader/CMakeLists.txt create mode 100644 src/examples/PodioFileReader/PodioFileReader.cc diff --git a/src/examples/PodioFileReader/CMakeLists.txt b/src/examples/PodioFileReader/CMakeLists.txt new file mode 100644 index 000000000..c2792c7fe --- /dev/null +++ b/src/examples/PodioFileReader/CMakeLists.txt @@ -0,0 +1,15 @@ + +if (USE_PODIO) + + add_jana_plugin(PodioFileReader) + + target_link_libraries(PodioFileReader + PUBLIC PodioDatamodel PodioDatamodelDict podio::podioRootIO) + +else() + message(STATUS "Skipping examples/PodioFileReader because USE_PODIO=Off") + +endif() + + + diff --git a/src/examples/PodioFileReader/PodioFileReader.cc b/src/examples/PodioFileReader/PodioFileReader.cc new file mode 100644 index 000000000..671fb78c3 --- /dev/null +++ b/src/examples/PodioFileReader/PodioFileReader.cc @@ -0,0 +1,105 @@ + +// Copyright 2023, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + + +#include +#include + +#include +#include +#include +#include + +#include + + + +class PodioFileReader : public JEventSource { + +private: + uint64_t m_entry_count = 0; + podio::ROOTFrameReader m_reader; + // ROOTFrameReader emits a lot of deprecation warnings, but the supposed replacement + // won't actually be a replacement until the next version + +public: + PodioFileReader() { + SetCallbackStyle(CallbackStyle::ExpertMode); + } + + void Open() final { + m_reader.openFile(GetResourceName()); + m_entry_count = m_reader.getEntries("events"); + } + + void Close() final { + // ROOT(Frame)Reader doesn't support closing the file (!). + // Maybe we can do this via ROOT. + } + + Result Emit(JEvent& event) final { + + // Figure out which entry to read + + uint64_t event_index = event.GetEventNumber(); // Event number starts from zero by default + if (event_index >= m_entry_count) return Result::FailureFinished; + + // Read entry + + auto frame_data = m_reader.readEntry("events", event_index); + auto frame = std::make_unique(std::move(frame_data)); + + // Extract event key (event nr, run nr, timeslice nr, etc) + + auto& eventinfo = frame->get("eventinfos"); + if (eventinfo.size() != 1) throw JException("Bad eventinfo: Entry %d contains %d items, 1 expected.", event_index, eventinfo.size()); + event.SetEventNumber(eventinfo[0].EventNumber()); + event.SetRunNumber(eventinfo[0].RunNumber()); + + // Make all PODIO collections available to JANA + + for (const std::string& coll_name : frame->getAvailableCollections()) { + + // It is possible to only put a subset of the collections into the JEvent. + // However this is risky because PODIO tracks inter-object associations but + // doesn't track the resulting inter-collection associations. Thus if the user + // omits any collections when they read a file, they risk introducing dangling + // pointers. This can silently corrupt the data in the output file. + + const podio::CollectionBase* coll = frame->get(coll_name); + const auto& coll_type = coll->getDataTypeName(); + + if (coll_type == "EventInfo") { + event.InsertCollectionAlreadyInFrame(coll, coll_name); + } + else if (coll_type == "TimesliceInfo") { + event.InsertCollectionAlreadyInFrame(coll, coll_name); + } + else if (coll_type == "ExampleHit") { + event.InsertCollectionAlreadyInFrame(coll, coll_name); + } + else if (coll_type == "ExampleCluster") { + event.InsertCollectionAlreadyInFrame(coll, coll_name); + } + else { + throw JException("Collection '%s' has typename '%' which is not known to PodioFileReader", coll_name.c_str(), coll_type.data()); + } + } + + // Put the frame in the events as well, tying their lifetimes together + + event.Insert(frame.release()); // Transfer ownership from unique_ptr to JFactoryT + return Result::Success; + } +}; + + + +extern "C" { +void InitPlugin(JApplication* app) { + InitJANAPlugin(app); + app->Add(new JEventSourceGeneratorT); +} +} + From 264df2f9320286a6723b80b1618f0f79364b6c12 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 4 Sep 2024 14:24:45 -0400 Subject: [PATCH 05/10] PodioExample uses PodioFileReader --- src/examples/CMakeLists.txt | 1 + src/examples/PodioExample/CMakeLists.txt | 6 +- src/examples/PodioExample/DatamodelGlue.h | 72 ------------------- src/examples/PodioExample/PodioExample.cc | 7 +- .../PodioExample/PodioExampleSource.cc | 19 ----- .../PodioExample/PodioExampleSource.h | 21 ------ .../PodioFileReader/PodioFileReader.cc | 8 ++- 7 files changed, 12 insertions(+), 122 deletions(-) delete mode 100644 src/examples/PodioExample/DatamodelGlue.h delete mode 100644 src/examples/PodioExample/PodioExampleSource.cc delete mode 100644 src/examples/PodioExample/PodioExampleSource.h diff --git a/src/examples/CMakeLists.txt b/src/examples/CMakeLists.txt index 32c315c46..a49906aa1 100644 --- a/src/examples/CMakeLists.txt +++ b/src/examples/CMakeLists.txt @@ -8,6 +8,7 @@ add_subdirectory(SubeventExample) add_subdirectory(SubeventCUDAExample) add_subdirectory(UnitTestingExample) add_subdirectory(PodioDatamodel) +add_subdirectory(PodioFileReader) add_subdirectory(PodioExample) add_subdirectory(TimesliceExample) add_subdirectory(RootDatamodelExample) diff --git a/src/examples/PodioExample/CMakeLists.txt b/src/examples/PodioExample/CMakeLists.txt index 94c2bc56a..244144747 100644 --- a/src/examples/PodioExample/CMakeLists.txt +++ b/src/examples/PodioExample/CMakeLists.txt @@ -2,7 +2,6 @@ set(PodioExample_SOURCES PodioExample.cc PodioExampleProcessor.cc - PodioExampleSource.cc ExampleClusterFactory.cc ExampleMultifactory.cc ) @@ -17,10 +16,7 @@ if (USE_PODIO) install(TARGETS PodioExample DESTINATION bin) - add_test(NAME jana-example-podio-tests - COMMAND PodioExample) - - set_tests_properties(jana-example-podio-tests PROPERTIES DISABLED TRUE) + add_test(NAME jana-example-podio-tests COMMAND PodioExample) else() message(STATUS "Skipping examples/PodioExample because USE_PODIO=Off") diff --git a/src/examples/PodioExample/DatamodelGlue.h b/src/examples/PodioExample/DatamodelGlue.h deleted file mode 100644 index f7ff17bf3..000000000 --- a/src/examples/PodioExample/DatamodelGlue.h +++ /dev/null @@ -1,72 +0,0 @@ - -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - - -#ifndef JANA2_DATAMODELGLUE_H -#define JANA2_DATAMODELGLUE_H - -#include -#include -#include -#include - - -template -struct Overload : Ts ... { - using Ts::operator() ...; -}; -template Overload(Ts...) -> Overload; - - -template -void visitPodioType(const std::string& podio_typename, F& helper, ArgsT... args) { - if (podio_typename == "EventInfo") { - return helper.template operator()(std::forward(args)...); - } - if (podio_typename == "TimesliceInfo") { - return helper.template operator()(std::forward(args)...); - } - else if (podio_typename == "ExampleHit") { - return helper.template operator()(std::forward(args)...); - } - else if (podio_typename == "ExampleCluster") { - return helper.template operator()(std::forward(args)...); - } - throw std::runtime_error("Not a podio typename!"); -} - -// If you are using C++20, you can use templated lambdas to write your visitor completely inline like so: -/* - visitPodioType(coll->getValueTypeName(), - [&](const podio::CollectionBase* coll, std::string name) { - using CollT = const typename T::collection_type; - CollT* typed_col = static_cast(coll); - - std::cout << name << std::endl; - for (const T& object : *typed_col) { - std::cout << coll->getValueTypeName() << std::endl; - std::cout << object << std::endl; - } - }, coll, coll_name); - */ - -template -struct VisitExampleDatamodel { - void operator()(Visitor& visitor, const podio::CollectionBase &collection) { - const auto podio_typename = collection.getTypeName(); - if (podio_typename == "EventInfoCollection") { - return visitor(static_cast(collection)); - } else if (podio_typename == "TimesliceInfoCollection") { - return visitor(static_cast(collection)); - } else if (podio_typename == "ExampleHitCollection") { - return visitor(static_cast(collection)); - } else if (podio_typename == "ExampleClusterCollection") { - return visitor(static_cast(collection)); - } - throw std::runtime_error("Unrecognized podio typename!"); - } -}; - - -#endif //JANA2_DATAMODELGLUE_H diff --git a/src/examples/PodioExample/PodioExample.cc b/src/examples/PodioExample/PodioExample.cc index 49acd8045..c716ab895 100644 --- a/src/examples/PodioExample/PodioExample.cc +++ b/src/examples/PodioExample/PodioExample.cc @@ -3,7 +3,8 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #include #include -#include "PodioDatamodel/MutableExampleHit.h" + +#include "PodioDatamodel/EventInfoCollection.h" #include "PodioDatamodel/ExampleHitCollection.h" #include #include @@ -12,7 +13,6 @@ #include #include -#include "PodioExampleSource.h" #include "PodioExampleProcessor.h" #include "ExampleClusterFactory.h" #include "ExampleMultifactory.h" @@ -82,7 +82,8 @@ int main() { app.Add(new JEventProcessorPodio); app.Add(new JFactoryGeneratorT()); app.Add(new JFactoryGeneratorT()); - app.Add(new PodioExampleSource("hits.root")); + app.Add("hits.root"); + app.AddPlugin("PodioFileReader"); app.Run(); verify_clusters_file(); diff --git a/src/examples/PodioExample/PodioExampleSource.cc b/src/examples/PodioExample/PodioExampleSource.cc deleted file mode 100644 index 7ea286fd3..000000000 --- a/src/examples/PodioExample/PodioExampleSource.cc +++ /dev/null @@ -1,19 +0,0 @@ - -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - - -#include "PodioExampleSource.h" -#include - -std::unique_ptr PodioExampleSource::NextFrame(int event_index, int &event_number, int &run_number) { - - auto frame_data = m_reader.readEntry("events", event_index); - auto frame = std::make_unique(std::move(frame_data)); - auto& eventinfo = frame->get("eventinfos"); - if (eventinfo.size() != 1) throw JException("Bad eventinfo: Entry %d contains %d items, 1 expected.", event_index, eventinfo.size()); - - event_number = eventinfo[0].EventNumber(); - run_number = eventinfo[0].RunNumber(); - return frame; -} diff --git a/src/examples/PodioExample/PodioExampleSource.h b/src/examples/PodioExample/PodioExampleSource.h deleted file mode 100644 index 5d3e53301..000000000 --- a/src/examples/PodioExample/PodioExampleSource.h +++ /dev/null @@ -1,21 +0,0 @@ - -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - - -#ifndef JANA2_PODIOEXAMPLESOURCE_H -#define JANA2_PODIOEXAMPLESOURCE_H - -#include "DatamodelGlue.h" // Has to come BEFORE JEventSourcePodio.h -#include - -class PodioExampleSource : public JEventSourcePodio { -public: - explicit PodioExampleSource(std::string filename) : JEventSourcePodio(std::move(filename)) { - } - std::unique_ptr NextFrame(int event_index, int &event_number, int &run_number) override; - -}; - - -#endif //JANA2_PODIOEXAMPLESOURCE_H diff --git a/src/examples/PodioFileReader/PodioFileReader.cc b/src/examples/PodioFileReader/PodioFileReader.cc index 671fb78c3..588f3c8ac 100644 --- a/src/examples/PodioFileReader/PodioFileReader.cc +++ b/src/examples/PodioFileReader/PodioFileReader.cc @@ -28,6 +28,10 @@ class PodioFileReader : public JEventSource { SetCallbackStyle(CallbackStyle::ExpertMode); } + static std::string GetDescription() { + return "Example that reads a PODIO file into JANA. This example uses `PodioDatamodel`, but it is trivial to adapt it to any data model. For now this only works on files containing events, not timeslices or any other levels."; + } + void Open() final { m_reader.openFile(GetResourceName()); m_entry_count = m_reader.getEntries("events"); @@ -68,7 +72,7 @@ class PodioFileReader : public JEventSource { // pointers. This can silently corrupt the data in the output file. const podio::CollectionBase* coll = frame->get(coll_name); - const auto& coll_type = coll->getDataTypeName(); + const auto& coll_type = coll->getValueTypeName(); if (coll_type == "EventInfo") { event.InsertCollectionAlreadyInFrame(coll, coll_name); @@ -83,7 +87,7 @@ class PodioFileReader : public JEventSource { event.InsertCollectionAlreadyInFrame(coll, coll_name); } else { - throw JException("Collection '%s' has typename '%' which is not known to PodioFileReader", coll_name.c_str(), coll_type.data()); + throw JException("Collection '%s' has typename '%s' which is not known to PodioFileReader", coll_name.c_str(), coll_type.data()); } } From 3a3ec1d78ca7a5ab9e876e08b5b404253e58cfea Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 4 Sep 2024 14:50:06 -0400 Subject: [PATCH 06/10] Remove JEventSourcePodio This was a bad abstraction. The use of the templated visitor pattern obscured some very simple code. It is far better for users to copy-paste PodioFileReader and adapt for their needs. --- .../PodioFileReader/PodioFileReader.cc | 2 +- src/libraries/JANA/Podio/JEventSourcePodio.h | 108 ------------------ 2 files changed, 1 insertion(+), 109 deletions(-) delete mode 100644 src/libraries/JANA/Podio/JEventSourcePodio.h diff --git a/src/examples/PodioFileReader/PodioFileReader.cc b/src/examples/PodioFileReader/PodioFileReader.cc index 588f3c8ac..a8a7cc17d 100644 --- a/src/examples/PodioFileReader/PodioFileReader.cc +++ b/src/examples/PodioFileReader/PodioFileReader.cc @@ -69,7 +69,7 @@ class PodioFileReader : public JEventSource { // However this is risky because PODIO tracks inter-object associations but // doesn't track the resulting inter-collection associations. Thus if the user // omits any collections when they read a file, they risk introducing dangling - // pointers. This can silently corrupt the data in the output file. + // pointers. This can segfault or silently corrupt the data in the output file. const podio::CollectionBase* coll = frame->get(coll_name); const auto& coll_type = coll->getValueTypeName(); diff --git a/src/libraries/JANA/Podio/JEventSourcePodio.h b/src/libraries/JANA/Podio/JEventSourcePodio.h deleted file mode 100644 index 7954d24eb..000000000 --- a/src/libraries/JANA/Podio/JEventSourcePodio.h +++ /dev/null @@ -1,108 +0,0 @@ - -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - - -#pragma once -#include -#include - -template