Make sure that reading of LCIO events stops early enough (#210)

* Make sure that spuriously missing ParticleID info fails CI

* Make the LcioEventAlgo stop the run before running out of events

Effectively stop the run when processing the last event even if it is at
the beginning of the event loop. Also make it throw a proper exception
to immediately abort when the reading fails.

* Add a warning output for potential issues

* Add some documentation about the current limitation

---------

Co-authored-by: Mateusz Jakub Fila <37295697+m-fila@users.noreply.github.com>

Author: tmadlener

doc/MarlinWrapperIntroduction.md

@@ -220,3 +220,25 @@ collection in the process, you would configure the tool like so
 lcio2edm4hepConv = Lcio2EDM4hepTool("Lcio2EDM4hep")
 lcio2edm4hepConv.collNameMapping = {"MCParticle": "MCParticles"}
 ```
+
+## Potential pitfalls when using other Gaudi Algorithms
+
+Although mixing wrapped Marlin Processors with other Gaudi Algorithms is working
+for most cases, there are a few conceptual differences that have not yet been
+completely mapped. This might lead to unexpected or different results when
+running in Gaudi vs. running the same processors via Marlin (as far as
+possible). This list aims to collect them and where possible also tries to point
+out ways to work around them.
+
+- In Marlin processors can use a `marlin::SkipEventException` to skip the
+  processing of the rest of the event.
+- In Marlin a `marlin::StopProcessingException` will immediately stop the
+  processing of the event loop. However, in Gaudi the current event will still
+  finish processing.
+
+For the `MarlinProcessorWrapper` algorithm we have put in checks and effectively
+skip the execution of the actual wrapped processor in these cases. However,
+other Gaudi algorithms will not have this check. Hence, execution chains that
+**only consist of wrapped Marlin processors will work as expected** here.
+However, **mixed chains will most likely not work as expected** (e.g. algorithms
+will not find expected inputs, etc.).

k4MarlinWrapper/examples/clicRec_e4h_input.py

@@ -2433,7 +2433,7 @@
 from Configurables import PodioOutput
 
 out = PodioOutput("PodioOutput", filename="my_output.root")
-out.outputCommands = ["keep *"]
+out.outputCommands = ["keep *", "drop RefinedVertexJets_PID_RefinedVertex"]
 
 
 algList.append(inp)

k4MarlinWrapper/k4MarlinWrapper/LcioEventAlgo.h

@@ -41,6 +41,8 @@ class LcioEvent : public Gaudi::Algorithm {
 private:
   Gaudi::Property<std::vector<std::string>> m_fileNames{this, "Files", {}};
   MT::LCReader*                             m_reader = nullptr;
+  int                                       m_numberOfEvents{};
+  mutable int                               m_currentEvent{};  // No atomicity necessary since not re-entrant
   bool                                      isReEntrant() const final { return false; }
 };
 

k4MarlinWrapper/src/components/LcioEventAlgo.cpp

@@ -16,19 +16,18 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-#include <GaudiKernel/IEventProcessor.h>
-
-#include "k4MarlinWrapper/LCEventWrapper.h"
 #include "k4MarlinWrapper/LcioEventAlgo.h"
+#include "k4MarlinWrapper/LCEventWrapper.h"
 
 #include "marlin/Global.h"
 #include "marlin/StringParameters.h"
 
 #include <EVENT/LCIO.h>
 #include <MT/LCReader.h>
 
-#include <memory>
+#include <GaudiKernel/IEventProcessor.h>
+
+#include <stdexcept>
 #include <string>
 
 DECLARE_COMPONENT(LcioEvent)
@@ -44,54 +43,42 @@ StatusCode LcioEvent::initialize() {
 
   m_reader = new MT::LCReader(0);
   m_reader->open(m_fileNames);
-  info() << "Initialized the LcioEvent Algo: " << m_fileNames[0] << endmsg;
+  m_numberOfEvents = m_reader->getNumberOfEvents();
+  info() << "Initialized the LcioEvent Algo. Reading from " << m_fileNames.size() << " with " << m_numberOfEvents
+         << " events in total." << endmsg;
   return StatusCode::SUCCESS;
 }
 
 StatusCode LcioEvent::execute(const EventContext&) const {
+  debug() << "Reading event " << ++m_currentEvent << " / " << m_numberOfEvents << endmsg;
   auto theEvent = m_reader->readNextEvent(EVENT::LCIO::UPDATE);
 
-  if (!theEvent) {
-    // Store flag to indicate there was NOT a LCEvent
-    auto             pStatus  = std::make_unique<LCEventWrapperStatus>(false);
-    const StatusCode scStatus = eventSvc()->registerObject("/Event/LCEventStatus", pStatus.release());
-    if (scStatus.isFailure()) {
-      error() << "Failed to store flag for underlying LCEvent: MarlinProcessorWrapper may try to run over non existing "
-                 "event"
-              << endmsg;
-      return scStatus;
-    }
+  if (theEvent == nullptr) {
+    fatal() << "Failed to read event " << m_currentEvent << endmsg;
+    throw std::runtime_error("LCEvent could not be read");
+  }
 
-    auto svc = service<IEventProcessor>("ApplicationMgr");
-    if (svc) {
-      svc->stopRun().ignore();
-      svc->release();
-    } else {
-      abort();
+  // Since this is presumably the first algorithm to run we have to check here
+  // to see if we need to stop the run. Events in flight will still be
+  // processed, so technically this signals that we want to end the run after
+  // all of the algorithms for this event have finished.
+  if (m_currentEvent >= m_numberOfEvents) {
+    info() << "This is the last event in the input files. Stopping the run" << endmsg;
+    auto evtProcService = service<IEventProcessor>("ApplicationMgr", false);
+    if (!evtProcService) {
+      fatal() << "Could not get the ApplicationMgr for stopping the run" << endmsg;
     }
-  } else {
-    // pass theEvent to the DataStore, so we can access them in our processor
-    // wrappers
-    info() << "Reading from file: " << m_fileNames[0] << endmsg;
-
-    auto             myEvWr = new LCEventWrapper(std::move(theEvent));
-    const StatusCode sc     = eventSvc()->registerObject("/Event/LCEvent", myEvWr);
-    if (sc.isFailure()) {
-      error() << "Failed to store the LCEvent" << endmsg;
-      return sc;
-    } else {
-      // Store flag to indicate there was a LCEvent
-      auto pStatus = std::make_unique<LCEventWrapperStatus>(true);
-      std::cout << "Saving status: " << pStatus->hasLCEvent << std::endl;
-      const StatusCode scStatus = eventSvc()->registerObject("/Event/LCEventStatus", pStatus.release());
-      if (scStatus.isFailure()) {
-        error() << "Failed to store flag for underlying LCEvent: MarlinProcessorWrapper may try to run over non "
-                   "existing event"
-                << endmsg;
-        return scStatus;
-      }
+    if (evtProcService->stopRun().isFailure()) {
+      error() << "Out of events, but could not signal to stop the run" << endmsg;
     }
   }
 
+  auto             myEvWr = new LCEventWrapper(std::move(theEvent));
+  const StatusCode sc     = eventSvc()->registerObject("/Event/LCEvent", myEvWr);
+  if (sc.isFailure()) {
+    error() << "Failed to store the LCEvent" << endmsg;
+    return sc;
+  }
+
   return StatusCode::SUCCESS;
 }

k4MarlinWrapper/src/components/MarlinProcessorWrapper.cpp

@@ -214,7 +214,7 @@ StatusCode MarlinProcessorWrapper::initialize() {
 }
 
 StatusCode MarlinProcessorWrapper::execute(const EventContext&) const {
-  // Get flag to know if there was an underlying LCEvent
+  // Get flag to check if this processor should be skipped or not
   DataObject* pStatus  = nullptr;
   StatusCode  scStatus = eventSvc()->retrieveObject("/Event/LCEventStatus", pStatus);
   if (scStatus.isSuccess()) {
@@ -279,6 +279,9 @@ StatusCode MarlinProcessorWrapper::execute(const EventContext&) const {
 
   // Handle exceptions that may come from Marlin
   catch (marlin::SkipEventException& e) {
+    warning() << "Caught marlin::SkipEventException. Skipping the wrapped Processors, but Gaudi Algorithms will still "
+                 "execute and may fail"
+              << endmsg;
     // Store flag to prevent the rest of the event from processing
     auto             upStatus = std::make_unique<LCEventWrapperStatus>(false);
     const StatusCode code     = eventSvc()->registerObject("/Event/LCEventStatus", upStatus.release());