key4hep / k4FWCore

Core Components for the Gaudi-based Key4hep Framework
Apache License 2.0
10 stars 26 forks source link

Don't use GaudiAlg #172

Closed jmcarcell closed 6 months ago

jmcarcell commented 8 months ago

BEGINRELEASENOTES

ENDRELEASENOTES

This can also serve as example for other repos, basically adding a const EventContext& in the arguments to execute and making execute const and changing GaudiAlgorithm to Gaudi::Algorithm. Headers change too: GaudiAlg/GaudiAlgorithm.h to Gaudi/Algorithm.h

jmcarcell commented 8 months ago

Alternative proposal (changes in the last commit): Remove the UniquePtr test which isn't doing anything than the other tests are not doing and set Cardinality to 1 which is how algorithms that can't run in parallel are set, for example: https://gitlab.cern.ch/gaudi/Gaudi/-/blob/master/GaudiKernel/include/GaudiKernel/Algorithm.h#L24

Having some algorithms around is good because not everything will be functional and it also allows to test if they can be mixed together.

jmcarcell commented 6 months ago

If there aren't any more complaints I'll merge this today. I think the plan is to remove GaudiAlg for the version 40 of Gaudi so there is time but there are a few repos that have to be changed

tmadlener commented 6 months ago

I think this looks good. One thing that is not entirely clear to me: Is the EventContext usable in any meaningful way for a "normal" algorithm? Or is that just used by the scheduler to make sure to use the right inputs?

jmcarcell commented 6 months ago

I'm not sure I have seen that being used by an algorithm. There is an event number that could be used for seeding I guess but we have that information in the headers