-
Notifications
You must be signed in to change notification settings - Fork 17
PseudoLayer plugin for ALLEGRO (initial version) #25
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
base: master
Are you sure you want to change the base?
PseudoLayer plugin for ALLEGRO (initial version) #25
Conversation
src/LCContent.cc
Outdated
| PANDORA_RETURN_RESULT_IF(pandora::STATUS_CODE_SUCCESS, !=, PandoraApi::SetPseudoLayerPlugin(pandora, new lc_content::LCPseudoLayerPlugin)); | ||
| // GM: can we decide the pseudolayer plugin to use at runtime via the xml pandora config? | ||
| // PANDORA_RETURN_RESULT_IF(pandora::STATUS_CODE_SUCCESS, !=, PandoraApi::SetPseudoLayerPlugin(pandora, new lc_content::LCPseudoLayerPlugin)); | ||
| PANDORA_RETURN_RESULT_IF(pandora::STATUS_CODE_SUCCESS, !=, PandoraApi::SetPseudoLayerPlugin(pandora, new allegro_content::ALLEGROPseudoLayerPlugin)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andresailer - I marked this PR as WIP for the moment because this line of code of course can't be merged as is.
Would you know how I can register the ALLEGRO plugin without interfering with the LC one (leaving the LC one as default) and choosing at run time via the xml scripts the ALLEGRO pseudolayer plugin instead of the LC one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@giovannimarchiori , @andresailer - To understand the question correctly: you'd like to choose between these two different plugin registrations, but not via the the settings that configure your Pandora application (DDMarlinPandora or similar)? If configuring your application is an option, you can just move the plugin registration directly into your application, rather than calling the RegisterBasicPlugins function here. You could then have some simple logic to switch (based off config) which plugin is actually created and registered.
If you actually wanted to make the decision via the PandoraSettings XML, though... An ALLEGRO registration could still be performed, but directly (rather than via RegisterBasicPlugins) as above. A pragmatic first idea might be for the ALLEGRO plugin to inherit from the LCPseudoLayerPlugin. It could then be configured, via XML and its ReadSettings callback, as to whether to run in ALLEGRO mode or LC mode. E.g. set a member variable, used throughout to decide whether to run the new ALLEGRO functionality or to call the appropriate LC functionality from the (now) base class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @PandoraPFA
for the moment until we find out how many things we have to change from LC to ALLEGRO to have a good p-flow base d on Pandora for our detector I wanted to write as little code as possible and the easiest I found out is to add a detectorName string argument to RegisterBasicPlugins (or in alternative one could add a string property for LCContent). This is because the RegisterBasicPlugins does a lot of things and I just want to change for the moment the pseudo layer plugin. This is currently invoked in DDMarlinPandora here: https://github.com/iLCSoft/DDMarlinPandora/blob/10481ed2ef0c2b4ee0f914fdd0efb438256c665b/src/DDPandoraPFANewProcessor.cc#L297
We could follow a different path and as you say modify DDMarlinPandora but then I would have to make there an if (ALLEGRO) then {..} in which I do not just set the ALLEGRO pseudo layer plugin but also register all other components currently registered in LCContent::RegisterBasicPlugins
Probably for ALLEGRO we'll need to go for some custom set of algorithms anyway and we'll have to write a significant amount of code but with this PR (and some related ones) we wanted to provide to our developer community an initial working version of the code from which to start.
Anyway, if it is preferred that we do not modify the existing LCContent code, I can do the change in DDMarlinPandora - where we already have an open PR iLCSoft/DDMarlinPandora#31
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @giovannimarchiori - thanks for the update. Strictly, the right place to start putting in detector-specific options would be the application (DDMarlinPandora here), rather than in the supposedly more generic algorithm library. That would fit better with the original design. But, given that LCContent isn't under much development pressure at the moment, a couple of simple, clear and well-understood tweaks could be made. If the number of points of modification started to increase, however, then it would be important to switch back to the original design before the situation became rather too difficult to maintain. [The application's where all the detector/use-case specific details are supposed to have been factored out; the algorithm library is supposed to be rather generic, but is linked to a specific algorithm strategy and problem to solve; the SDK is supposed to be so generic that you can use the same library to run e.g. neutrino event reconstruction and collider event reconstruction without modification (as is currently the case!).] Thanks.
|
Hi @johnmarshall80 , is there a message streaming service that can be used in pandora to printout info/warning/error/debug messages and choose the desired verbosity? To debug the performance of the algorithms I would like to have some verbose out and be able to turn this on/off at runtime. |
Hi @giovannimarchiori - there's not a fully-fledged message service, but there is one centralised option that we've used before in algorithms and algorithm tools. See e.g. here or here, with the XML option to switch such "Algorithm info" on/off being set e.g. here or here. Thanks. |
|
Hi @johnmarshall80 On a slightly unrelated note: is there a forum where one can asks questions about Pandora (for instance I have some doubts about some modifications needed for our specific calorimeter geometry), or should one create an Issue in this project, or else? |
Hi @giovannimarchiori - if you send me an e-mail to confirm, we can get you added as a guest (in the first instance) of the Pandora slack workspace. Thanks. |
|
giovanni.marchiori AT cern.ch |
Provides an initial version of a pseudo layer plugin for ALLEGRO
More work needed for endcap (deferred to later development)
Marking it as WIP till I find out out to use the plugin instead of the LC version from xml