iLCSoft / Marlin

Modular Analysis and Reconstruction for the LINear Collider
GNU General Public License v3.0
11 stars 16 forks source link

AllowToModifyEvent warning misleading #52

Open Zehvogel opened 12 months ago

Zehvogel commented 12 months ago

tl; dr: EventModifiers are not drop-in replacements for Processors but a warning suggest otherwise.

Marlin has an AllowToModifyEvent parameter that allows processors to modify the event in their processEvent() method. Setting this to true emits the following message:

https://github.com/iLCSoft/Marlin/blob/4fff9cc9bae11b1030a81b516a48e274527c53b8/source/src/Marlin.cc#L395-L409

urging one to instead implement the processor as an EventModifier instead.

However, the EventModifiers and the Processors are collected into two different lists and executed one after the other. Therefore, the order that one defines Processors (not knowing that some of them are EventModifiers) in in the steering file is no longer the same as the order of execution. If one wants to create the inputs to the EventModifier and run it in the same steering file this is not possible and one needs to go back to a "regular" Processor and set AllowToModifyEvent...

Unfortunately, this strict separation is already fixed on the lowest layer by the LCReader implementation cf.:

  void SIOReader::processEvent( std::shared_ptr<EVENT::LCEvent> event ) {
    auto eventImpl = dynamic_cast<IOIMPL::LCEventIOImpl*>( event.get() ) ;
    std::set<IO::LCEventListener*>::iterator iter = _evtListeners.begin() ;
    while( iter != _evtListeners.end() ) {
      eventImpl->setAccessMode( EVENT::LCIO::UPDATE ) ;
      (*iter)->modifyEvent( eventImpl ) ;
      eventImpl->setAccessMode( EVENT::LCIO::READ_ONLY ) ; // set the proper acces mode
      (*iter)->processEvent( eventImpl ) ;
      iter++ ;
    }
  }

Therefore, I would suggest to document this behavior better and to modify the warning message.