Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AllowToModifyEvent warning misleading #52

Open
Zehvogel opened this issue Nov 28, 2023 · 0 comments
Open

AllowToModifyEvent warning misleading #52

Zehvogel opened this issue Nov 28, 2023 · 0 comments

Comments

@Zehvogel
Copy link

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:

Marlin/source/src/Marlin.cc

Lines 395 to 409 in 4fff9cc

bool modify = ( Global::parameters->getStringVal("AllowToModifyEvent") == "true" ) ;
if( modify ) {
streamlog_out( WARNING ) << " ******************************************************************************* \n"
<< " * AllowToModifyEvent is set to 'true' * \n"
<< " * => all processors can modify the input event in processEvent() !! * \n"
<< " * consider setting this flag to 'false' * \n"
<< " * unless you really need it... * \n"
<< " * - if you need a processor that modifies the input event * \n"
<< " * please implement the EventModifier interface and use the modifyEvent() * \n"
<< " * method for this * \n"
<< " ******************************************************************************* \n"
<< std::endl ;
}

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant