diff --git a/.github/workflows/test_integration_epic.yml b/.github/workflows/test_integration_epic.yml index e28882092..8b9ded597 100644 --- a/.github/workflows/test_integration_epic.yml +++ b/.github/workflows/test_integration_epic.yml @@ -134,7 +134,7 @@ jobs: echo "ldd bin/eicrecon" ldd bin/eicrecon - - name: Generate EICrecon input data + - name: Generate EICrecon input data for craterlake uses: eic/run-cvmfs-osg-eic-shell@main with: platform-release: "eic_xl:nightly" @@ -146,7 +146,7 @@ jobs: --gun.momentumMax "20*GeV" --gun.distribution "uniform" -N 20 \ --outputFile sim_e_1GeV_20GeV_craterlake.edm4hep.root -v WARNING - - name: Run EICrecon + - name: Run EICrecon for craterlake uses: eic/run-cvmfs-osg-eic-shell@main with: platform-release: "eic_xl:nightly" @@ -157,3 +157,27 @@ jobs: export LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/../EICrecon/lib:${GITHUB_WORKSPACE}/../EICrecon/lib/EICrecon/plugins:${JANA_HOME}/lib:${JANA_HOME}/lib/JANA/plugins:$LD_LIBRARY_PATH ../EICrecon/bin/eicrecon sim_e_1GeV_20GeV_craterlake.edm4hep.root + - name: Generate EICrecon input data for inner detector + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "eic_xl:nightly" + setup: "/opt/detector/epic-main/bin/thisepic.sh" + run: | + echo "--- Generating EICrecon input data ---" + npsim --compactFile ${DETECTOR_PATH}/${DETECTOR}_inner_detector.xml \ + -G --random.seed 1 --gun.particle "e-" --gun.momentumMin "1*GeV" \ + --gun.momentumMax "20*GeV" --gun.distribution "uniform" -N 20 \ + --outputFile sim_e_1GeV_20GeV_inner_detector.edm4hep.root -v WARNING + + - name: Run EICrecon for inner detector + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "eic_xl:nightly" + setup: "/opt/detector/epic-main/bin/thisepic.sh" + run: | + export JANA_HOME=$GITHUB_WORKSPACE + export JANA_PLUGIN_PATH=$GITHUB_WORKSPACE/../EICrecon/lib/EICrecon/plugins + export LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/../EICrecon/lib:${GITHUB_WORKSPACE}/../EICrecon/lib/EICrecon/plugins:${JANA_HOME}/lib:${JANA_HOME}/lib/JANA/plugins:$LD_LIBRARY_PATH + export DETECTOR_CONFIG=epic_inner_detector # not important + ../EICrecon/bin/eicrecon sim_e_1GeV_20GeV_inner_detector.edm4hep.root + diff --git a/docs/behavior.md b/docs/behavior.md index 553db1f76..585f4fac1 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -3,6 +3,118 @@ ## Introduction +## Components + +### JFactories + +#### Callbacks + +The user-defined callbacks are `Init`, `Process`, `BeginRun`, `EndRun`, and `Finish`. + +- `Init` is run at most once, and is used for loading and caching constant data. It is guaranteed to run before any call to `Process`. + If `Process` is never called, e.g. because the data is `Insert`ed instead, then `Init` will not be called. + +- `Finish` only runs if `Init` has ran, and is responsible for cleaning up and closing any state opened by `Init`. + +- `BeginRun` runs after `Init` and before `Process`. It will only be called if the run number has been set, and this will be the first call to `Process` corresponding to that run number. + `BeginRun` is responsible for loading and caching data keyed off of the run number, e.g. conditions, calibrations, lookup tables, machine learning models. + +- `EndRun` runs after a previously set run number gets changed. It runs after the last call to `Process` with that run number, and before the `BeginRun` for the new run number. + `EndRun` is responsible for cleaning up and closing any state opened by the previous call to `BeginRun`. `EndRun` is guaranteed to be called exactly once for each `BeginRun`, as long + as JANA2 is shut down cleanly. + +- `Process` is called exactly once for every JEvent. By the time `Process` is called, JANA2 guarantees that `Init` will have been called, followed by `BeginRun` if the run number has been set. + +#### Activation +JFactories are lazy by design, which means that they won't be activated unless requested by another component. Activation is defined as calling `JFactory::Create` with a given JEvent, which +triggers the running of zero or more factory callbacks. Users are discouraged from calling `JFactory::Create` directly; instead, factories are usually activated via any of the following mechanisms: + +- Declaring an `Input` helper member variable on another JComponent. +- Calling `JEvent::Get*()` or `JEvent::GetCollection()` +- Setting the parameter `jana:autoactivate=$DATABUNDLE_NAME`. This will activate the factory even though its results are never used downstream, which is mainly useful for debugging. + +#### Exception handling +JFactory's user-defined methods are allowed to throw exceptions. Unlike other JANA2 components, throwing an exception here does not immediately terminate processing -- the +user has the opportunity to catch the exception in the caller. For instance, calls to `JEvent::Get*` may be wrapped in a try-catch block. The exception itself is wrapped in a `JException` which preserves +stack trace and component information. If a factory callback excepts, the exception is stored so that it can be re-thrown on future calls. The excepting callback will only be called once. If any +callbacks except, JANA2 will still store the contents of each `Output` helper's transient output buffer. This means that if `Process` excepts, all data inserted prior to the exception will be preserved. + +#### State machine + +The JFactory state machine is defined as follows. The state has two components, `InitStatus` and `Status`. `InitStatus <- {NotRun, Run, Excepted}`, which allows `JFactory::Create` to guarantee that +`Init` gets called at most once, even when some events have the factory data `Insert`ed and others let it be `Processed`. `Status <- {Empty, Processed, Inserted, Excepted}` captures what exactly +is in cache. `Empty` is the only state where no data has been cached/stored; `Excepted` means that an empty or partial collection was stored. This storage operation is guaranteed to happen exactly once +per activated factory per event, a requirement imposed by Podio's write-exactly-once semantics. The following transition diagram shows the relationship between valid states and transitions corresponding +to factory callbacks. + +```mermaid + +stateDiagram-v2 + NE : (InitNotRun, Empty) + XE : (InitExcepted, Empty) + XX : (InitExcepted, Excepted) + RE : (InitRun, Empty) + RI : (InitRun, Inserted) + RP : (InitRun, Processed) + RX : (InitRun, Excepted) + NI : (InitNotRun, Inserted) + XI : (InitExcepted, Inserted) + + [*] --> NE + NE --> RE: Init + RE --> RP: Process + RP --> RE: ClearData + RE --> RX: Process + RX --> RE: ClearData + RE --> RI: Insert + RI --> RE: ClearData + NE --> NI: Insert + NI --> NE: ClearData + NE --> XE: Init + XE --> XX: Process + XX --> XE: ClearData + XE --> XI: Insert + XI --> XE: ClearData +``` + +Note that many of these transitions are encapsulated behind `JFactory::Create`. The only operations available to the user are `Create` (i.e. activate), `Insert`, and `ClearData`. +Redrawing the state diagram in terms of these transitions gives us: + +```mermaid +stateDiagram-v2 + NE : (InitNotRun, Empty) + XE : (InitExcepted, Empty) + XX : (InitExcepted, Excepted) + RE : (InitRun, Empty) + RI : (InitRun, Inserted) + RP : (InitRun, Processed) + RX : (InitRun, Excepted) + NI : (InitNotRun, Inserted) + XI : (InitExcepted, Inserted) + + [*] --> NE + NE --> RP: Create + RE --> RP: Create + RP --> RE: ClearData + RE --> RX: Create + RX --> RE: ClearData + RE --> RI: Insert + RI --> RE: ClearData + NE --> NI: Insert + NI --> NE: ClearData + NE --> XX: Create + XE --> XX: Create + XX --> XE: ClearData + XE --> XI: Insert + XI --> XE: ClearData + NE --> RX: Create +``` + +Although not shown for the sake of visual clarity, it is important to note that `Create` operations are idempotent, so all of the `Status:Inserted`, `Status:Processed`, and `Status:Excepted` states +have implicit `Create` transitions pointing back to themselves. Correspondingly, the `Status:Empty` states have `ClearData` transitions pointing back to themselves. However, the `Status:Inserted` states +do _not_ have `Insert` transitions pointing back to themselves. Multiple `Insert` operations are disallowed due to the write-once constraint. + + ## Execution Engine ### Engine initialization diff --git a/src/libraries/JANA/Components/JHasOutputs.cc b/src/libraries/JANA/Components/JHasOutputs.cc index e430f0c45..35ebb559b 100644 --- a/src/libraries/JANA/Components/JHasOutputs.cc +++ b/src/libraries/JANA/Components/JHasOutputs.cc @@ -6,10 +6,8 @@ void jana::components::UpdateFactoryStatusOnEulerianStore(JFactory* fac) { // that the factory doesn't accidentally get re-run. // We do this inside a weird little free function because we need to avoid creating // a circular definition of JFactory in our templates. - // Eventually we will need to refactor JFactory::Status and CreationStatus. fac->SetStatus(JFactory::Status::Inserted); - fac->SetCreationStatus(JFactory::CreationStatus::Inserted); } diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 4ce518fcd..75e1c367c 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -449,7 +449,6 @@ inline JFactoryT* JEvent::Insert(T* item, const std::string& tag) const { auto* factory = databundle->GetFactory(); if (factory != nullptr) { 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 dynamic_cast*>(factory); } @@ -476,7 +475,6 @@ inline JFactoryT* JEvent::Insert(const std::vector& items, const std::str auto* factory = databundle->GetFactory(); if (factory != nullptr) { 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 dynamic_cast*>(factory); } @@ -515,7 +513,7 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string unique return nullptr; } - if (typed_bundle->GetStatus() == JDatabundle::Status::Empty) { + if (typed_bundle->GetStatus() == JDatabundle::Status::Empty || typed_bundle->GetStatus() == JDatabundle::Status::Excepted) { auto* fac = typed_bundle->GetFactory(); if (fac != nullptr) { JCallGraphEntryMaker cg_entry(mCallGraph, fac); // times execution until this goes out of scope diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index d3d800dd4..685e23006 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -41,10 +41,6 @@ void JFactory::Create(const JEvent& event) { m_logger = m_app->GetJParameterManager()->GetLogger(GetLoggerName()); } - if (mStatus == Status::Uninitialized) { - DoInit(); - } - // How do we obtain our data? The priority is as follows: // 1. JFactory::Process() if REGENERATE flag is set // 2. JEvent::Insert() @@ -88,7 +84,6 @@ void JFactory::Create(const JEvent& event) { if (found_data) { mStatus = Status::Inserted; - mCreationStatus = CreationStatus::InsertedViaGetObjects; return; } } @@ -99,26 +94,57 @@ void JFactory::Create(const JEvent& event) { // 4. JFactory::Process() // --------------------------------------------------------------------- - // If the data was Processed (instead of Inserted), it will be in cache, and we can just exit. - // Otherwise we call Process() to create the data in the first place. - // If we already ran Process() but it excepted, we re-run Process() to trigger the same exception, so that every consumer - // is forced to handle it. Otherwise one "fault-tolerant" consumer will swallow the exception for everybody else. - if (mStatus == Status::Unprocessed || mStatus == Status::Excepted) { - auto run_number = event.GetRunNumber(); - if (mPreviousRunNumber != run_number) { - if (m_callback_style == CallbackStyle::LegacyMode) { - if (mPreviousRunNumber != -1) { - CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); - } - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); - CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); - } - else if (m_callback_style == CallbackStyle::ExpertMode) { - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); - } - mPreviousRunNumber = run_number; + // Check if init had _previously_ excepted but the cache was since cleared + if (mInitStatus == InitStatus::InitExcepted && mStatus == Status::Empty) { + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } + mStatus = Status::Excepted; + std::rethrow_exception(mException); + } + + // Make sure Init() ran, which might except... + try { + DoInit(); // This checks mInitStatus internally before calling Init() + } + catch(...) { + // If Init() excepts, we still need to store an empty collection + mStatus = Status::Excepted; + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + std::rethrow_exception(mException); + } + + // At this point, Init() has run and has _not_ excepted + + if (mStatus == Status::Excepted) { + // But Process() might have already excepted! + std::rethrow_exception(mException); + } + else if (mStatus == Status::Empty) { + // Now we know that we need to run Process() to create the data in the first place try { + auto run_number = event.GetRunNumber(); + if (mPreviousRunNumber != run_number) { + if (m_callback_style == CallbackStyle::LegacyMode) { + if (mPreviousRunNumber != -1) { + CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); + } + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); + CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); + } + else if (m_callback_style == CallbackStyle::ExpertMode) { + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); + } + mPreviousRunNumber = run_number; + } for (auto* input : GetInputs()) { input->Populate(event); } @@ -143,7 +169,7 @@ void JFactory::Create(const JEvent& event) { LOG << "Exception in JFactory::Create, prefix=" << GetPrefix(); mStatus = Status::Excepted; - mCreationStatus = CreationStatus::Created; + mException = std::current_exception(); for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } @@ -152,8 +178,9 @@ void JFactory::Create(const JEvent& event) { } throw; } + + // Save the (successfully processed) data mStatus = Status::Processed; - mCreationStatus = CreationStatus::Created; for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); } @@ -164,7 +191,7 @@ void JFactory::Create(const JEvent& event) { } void JFactory::DoInit() { - if (mStatus != Status::Uninitialized) { + if (mInitStatus != InitStatus::InitNotRun) { return; } for (auto* parameter : m_parameters) { @@ -173,17 +200,23 @@ void JFactory::DoInit() { for (auto* service : m_services) { service->Fetch(m_app); } - CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); - mStatus = Status::Unprocessed; + try { + CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); + mInitStatus = InitStatus::InitRun; + } + catch (...) { + mInitStatus = InitStatus::InitExcepted; + mException = std::current_exception(); + throw; + } } void JFactory::DoFinish() { - if (mStatus == Status::Unprocessed || mStatus == Status::Processed) { + if (mInitStatus == InitStatus::InitRun) { if (mPreviousRunNumber != -1) { CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); } CallWithJExceptionWrapper("JFactory::Finish", [&](){ Finish(); }); - mStatus = Status::Finished; } } diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 2eb03aa21..0dc2b6eea 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -34,8 +34,9 @@ class JFactory : public jana::components::JComponent, public jana::components::JHasOutputs { public: - enum class Status {Uninitialized, Unprocessed, Processed, Inserted, Excepted, Finished}; - enum class CreationStatus { NotCreatedYet, Created, Inserted, InsertedViaGetObjects, NeverCreated }; + enum class InitStatus {InitNotRun, InitRun, InitExcepted}; + enum class CreationStatus { NotCreatedYet, Created }; + enum class Status {Empty, Processed, Inserted, Excepted}; enum JFactory_Flags_t { JFACTORY_NULL = 0x00, // Not used anywhere @@ -70,14 +71,18 @@ class JFactory : public jana::components::JComponent, std::string GetFactoryName() const { return m_type_name; } Status GetStatus() const { return mStatus; } - CreationStatus GetCreationStatus() const { return mCreationStatus; } + InitStatus GetInitStatus() const { return mInitStatus; } + [[deprecated]] + CreationStatus GetCreationStatus() const { + if (mStatus == Status::Empty) return CreationStatus::NotCreatedYet; + return CreationStatus::Created; + } 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. uint32_t GetPreviousRunNumber(void) const { return mPreviousRunNumber; } void SetFactoryName(std::string factoryName) { SetTypeName(factoryName); } void SetStatus(Status status){ mStatus = status; } - void SetCreationStatus(CreationStatus status){ mCreationStatus = status; } 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 SetPreviousRunNumber(uint32_t aRunNumber) { mPreviousRunNumber = aRunNumber; } @@ -99,7 +104,7 @@ class JFactory : public jana::components::JComponent, /// Get data source value depending on how objects came to be here. (Used mainly by JEvent::Get() ) inline JCallGraphRecorder::JDataSource GetDataSource() const { JCallGraphRecorder::JDataSource datasource = JCallGraphRecorder::DATA_FROM_FACTORY; - if( mCreationStatus == JFactory::CreationStatus::Inserted ){ + if( mStatus == JFactory::Status::Inserted ){ if( m_insert_origin == JCallGraphRecorder::ORIGIN_FROM_SOURCE ){ datasource = JCallGraphRecorder::DATA_FROM_SOURCE; }else{ @@ -118,7 +123,8 @@ class JFactory : public jana::components::JComponent, } void ClearData() { - if (this->mStatus == JFactory::Status::Uninitialized) { + + if (this->mStatus == JFactory::Status::Empty) { return; } @@ -132,8 +138,7 @@ class JFactory : public jana::components::JComponent, return; } - this->mStatus = JFactory::Status::Unprocessed; - this->mCreationStatus = JFactory::CreationStatus::NotCreatedYet; + this->mStatus = JFactory::Status::Empty; for (auto* output: GetOutputs()) { output->ClearData(); @@ -187,10 +192,10 @@ class JFactory : public jana::components::JComponent, bool mInsideCreate = false; // Use this to detect cycles in factory dependencies std::string mObjectName; - 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; + Status mStatus = Status::Empty; + InitStatus mInitStatus = InitStatus::InitNotRun; + JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) + std::exception_ptr mException; }; // We are moving away from JFactory::GetAs because it only considers the first databundle diff --git a/src/libraries/JANA/JFactoryT.h b/src/libraries/JANA/JFactoryT.h index 64b34cea4..24bb04a74 100644 --- a/src/libraries/JANA/JFactoryT.h +++ b/src/libraries/JANA/JFactoryT.h @@ -88,14 +88,12 @@ class JFactoryT : public JFactory { // from JANA1 and I haven't found a cleaner solution that gives them what they want yet. mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } else { ClearData(); mData = aData; mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } } @@ -104,14 +102,12 @@ class JFactoryT : public JFactory { mData = std::move(aData); mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } virtual void Insert(T* aDatum) { mData.push_back(aDatum); mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } /// Set a flag (or flags) diff --git a/src/libraries/JANA/Utils/JInspector.cc b/src/libraries/JANA/Utils/JInspector.cc index 940d01655..6a593de86 100644 --- a/src/libraries/JANA/Utils/JInspector.cc +++ b/src/libraries/JANA/Utils/JInspector.cc @@ -177,22 +177,21 @@ void JInspector::ToText(const JFactory* fac, bool asJson, std::ostream& out) { auto tag = fac->GetTag(); if (tag.empty()) tag = "(no tag)"; - std::string creationStatus; - switch (fac->GetCreationStatus()) { - case JFactory::CreationStatus::NotCreatedYet: creationStatus = "NotCreatedYet"; break; - case JFactory::CreationStatus::Created: creationStatus = "Created"; break; - case JFactory::CreationStatus::Inserted: creationStatus = "Inserted"; break; - case JFactory::CreationStatus::InsertedViaGetObjects: creationStatus = "InsertedViaGetObjects"; break; - case JFactory::CreationStatus::NeverCreated: creationStatus = "NeverCreated"; break; - default: creationStatus = "Unknown"; + std::string status; + switch (fac->GetStatus()) { + case JFactory::Status::Empty: status = "Empty"; break; + case JFactory::Status::Processed: status = "Processed"; break; + case JFactory::Status::Inserted: status = "Inserted"; break; + case JFactory::Status::Excepted: status = "Excepted"; break; + default: status = "Unknown"; } if (!asJson) { out << "Plugin name " << pluginName << std::endl; - // out << "Factory name " << factoryName << std::endl; + out << "Type name " << factoryName << std::endl; out << "Object name " << fac->GetObjectName() << std::endl; out << "Tag " << tag << std::endl; - out << "Creation status " << creationStatus << std::endl; + out << "Status " << status << std::endl; out << "Object count " << fac->GetNumObjects() << std::endl; } else { @@ -201,7 +200,7 @@ void JInspector::ToText(const JFactory* fac, bool asJson, std::ostream& out) { out << " \"factory_name\": \"" << factoryName << "\"," << std::endl; out << " \"object_name\": \"" << fac->GetObjectName() << "\"," << std::endl; out << " \"tag\": \"" << tag << "\"," << std::endl; - out << " \"creation\": \"" << creationStatus << "\"," << std::endl; + out << " \"status\": \"" << status << "\"," << std::endl; out << " \"object_count\": " << fac->GetNumObjects() << "," << std::endl; out << "}" << std::endl; } @@ -220,27 +219,26 @@ void JInspector::ToText(const std::vector& factories, const std::set< for (auto fac : factories) { auto tag = fac->GetTag(); if (tag.empty()) tag = "(no tag)"; - std::string creationStatus; - switch (fac->GetCreationStatus()) { - case JFactory::CreationStatus::NotCreatedYet: creationStatus = "NotCreatedYet"; break; - case JFactory::CreationStatus::Created: creationStatus = "Created"; break; - case JFactory::CreationStatus::Inserted: creationStatus = "Inserted"; break; - case JFactory::CreationStatus::InsertedViaGetObjects: creationStatus = "InsertedViaGetObjects"; break; - case JFactory::CreationStatus::NeverCreated: creationStatus = "NeverCreated"; break; - default: creationStatus = "Unknown"; + std::string status; + switch (fac->GetStatus()) { + case JFactory::Status::Empty: status = "Empty"; break; + case JFactory::Status::Processed: status = "Processed"; break; + case JFactory::Status::Inserted: status = "Inserted"; break; + case JFactory::Status::Excepted: status = "Excepted"; break; + default: status = "Unknown"; } idx += 1; - if (filterlevel > 0 && (fac->GetCreationStatus()==JFactory::CreationStatus::NeverCreated || - fac->GetCreationStatus()==JFactory::CreationStatus::NotCreatedYet)) continue; + if (filterlevel > 0 && (fac->GetStatus() == JFactory::Status::Empty)) continue; + if (filterlevel > 1 && (fac->GetNumObjects()== 0)) continue; - if (filterlevel > 2 && (fac->GetCreationStatus()==JFactory::CreationStatus::Inserted || - fac->GetCreationStatus()==JFactory::CreationStatus::InsertedViaGetObjects)) continue; + + if (filterlevel > 2 && (fac->GetStatus() == JFactory::Status::Inserted)) continue; char discrepancy = ' '; auto key = MakeFactoryKey(fac->GetObjectName(), fac->GetTag()); if (discrepancies.find(key) != discrepancies.end()) discrepancy = 'x'; - t | idx | fac->GetObjectName() | tag | creationStatus | fac->GetNumObjects() | discrepancy; + t | idx | fac->GetObjectName() | tag | status | fac->GetNumObjects() | discrepancy; } t.Render(out); } diff --git a/src/plugins/janaview/JEventProcessor_janaview.cc b/src/plugins/janaview/JEventProcessor_janaview.cc index 312a2e9dd..9452e8424 100644 --- a/src/plugins/janaview/JEventProcessor_janaview.cc +++ b/src/plugins/janaview/JEventProcessor_janaview.cc @@ -223,7 +223,7 @@ void JEventProcessor_janaview::GetAssociatedTo(JObject *jobj, vectorGetCreationStatus() == JFactory::CreationStatus::NotCreatedYet) continue; + if(factories[i]->GetStatus() == JFactory::Status::Empty) continue; // Get objects for this factory and associated objects for each of those vector vobjs = factories[i]->GetAs(); diff --git a/src/programs/unit_tests/Components/JFactoryTests.cc b/src/programs/unit_tests/Components/JFactoryTests.cc index d55a3ccaf..e489cb938 100644 --- a/src/programs/unit_tests/Components/JFactoryTests.cc +++ b/src/programs/unit_tests/Components/JFactoryTests.cc @@ -168,7 +168,7 @@ TEST_CASE("JFactoryTests") { SECTION("Issue 135: Users modifying mData directly and calling Set() afterwards") { // Unfortunately, we are giving users the ability to access mData directly. // If they do this, they might think they should call Set() directly afterwards so that - // the Status and CreationStatus flags get correctly set. + // the Status flag gets correctly set. // Previously, this would clear mData before setting it, which would make their data // disappear, and they'd have to dig deep into the JANA internals to find out why. // Now, we account for this case. I still think we should clean up this abstraction, though. @@ -220,7 +220,7 @@ TEST_CASE("JFactoryTests") { LOG << "JFactoryTests: Exception in JFactory::Process" << LOG_END; auto event = std::make_shared(); JFactoryTestExceptingFactory fac; - REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized); + REQUIRE(fac.GetStatus() == JFactory::Status::Empty); REQUIRE_THROWS(fac.CreateAndGetData(event)); REQUIRE(fac.GetStatus() == JFactory::Status::Excepted); @@ -231,10 +231,10 @@ TEST_CASE("JFactoryTests") { LOG << "JFactoryTests: Exception in JFactory::Init" << LOG_END; auto event = std::make_shared(); JFactoryTestExceptingInInitFactory fac; - REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized); + REQUIRE(fac.GetStatus() == JFactory::Status::Empty); REQUIRE_THROWS(fac.CreateAndGetData(event)); - REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized); + REQUIRE(fac.GetStatus() == JFactory::Status::Excepted); REQUIRE_THROWS(fac.CreateAndGetData(event)); } } diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 5c10f71ba..91a2439e9 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -1,4 +1,7 @@ +#include "JANA/Components/JDatabundle.h" +#include "JANA/JApplicationFwd.h" +#include "PodioDatamodel/ExampleHit.h" #include #include @@ -353,4 +356,177 @@ TEST_CASE("PodioMultifactoryClearData_Test") { } // TEST_CASE } // namespace +namespace podio_exception_tests { + +class ExceptingPodioFactory : public JFactory { + + PodioOutput hits_out {this}; + Parameter except {this, "except", 0, "0: None, 1: in Init(), 2: start of Process(), 3: end of Process()"}; + +public: + ExceptingPodioFactory() { + SetPrefix("myfac"); + } + void Init() override { + LOG_INFO(GetLogger()) << "except=" << *except; + if (*except == 1) { + throw JException("Exception in Init()"); + } + } + void Process(const JEvent& event) override { + if (*except == 2) { + throw JException("Exception in Process() before some data is written"); + } + MutableExampleHit hit; + hit.x(event.GetEventNumber()); + hits_out->push_back(hit); + if (*except == 3) { + throw JException("Exception in Process() after some data is written"); + } + } +}; + + +TEST_CASE("PodioTests_ExceptionInFactoryInit") { + SECTION("ExceptionInInit") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 1); + app.Initialize(); + + JEvent event(&app); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + auto frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + + event.Clear(true); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + } + SECTION("ExceptionAtStartOfProcess") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 2); + app.Initialize(); + + JEvent event(&app); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + auto frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + + event.Clear(true); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + } + + SECTION("ExceptionAtEndOfProcess") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 3); + app.Initialize(); + + JEvent event(&app); + event.SetEventNumber(22); + + std::cout << "Calling failing factory" << std::endl; + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + std::cout << "Calling failing factory" << std::endl; + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + std::cout << "Retrieving collection from frame" << std::endl; + auto frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 1); + + std::cout << "Clearing event" << std::endl; + event.Clear(true); + event.SetEventNumber(23); + + std::cout << "Calling failing factory" << std::endl; + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + std::cout << "Retrieving collection from frame" << std::endl; + + frame = event.Get().at(0); + for (const auto& hit : frame->get("ExampleHit")) { + std::cout << "Hit x=" << hit.x() << std::endl; + } + REQUIRE(frame->get("ExampleHit").size() == 1); + } +} + +TEST_CASE("PodioTests_InterleavedInsertAndProcess") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 0); // Disable exceptions + app.Initialize(); + + JEvent event(&app); + + auto databundle = event.GetFactorySet()->GetDatabundle("ExampleHit"); + REQUIRE(databundle->GetStatus() == JDatabundle::Status::Empty); + REQUIRE(databundle->GetFactory() != nullptr); + REQUIRE(databundle->GetFactory()->GetStatus() == JFactory::Status::Empty); + REQUIRE(databundle->GetFactory()->GetInitStatus() == JFactory::InitStatus::InitNotRun); + + + // Insert hits. This will NOT trigger Init + ExampleHitCollection hits; + hits.push_back(MutableExampleHit()); + event.InsertCollection(std::move(hits), "ExampleHit"); + + event.GetCollectionBase("ExampleHit"); + REQUIRE(databundle->GetFactory()->GetInitStatus() == JFactory::InitStatus::InitNotRun); + + event.Clear(); + + // Trigger Init and Process + event.GetCollectionBase("ExampleHit"); + REQUIRE(databundle->GetFactory()->GetInitStatus() == JFactory::InitStatus::InitRun); +} + +} // namespace +