fcs-proj / FastCaloSim

An experiment-independent library for fast calorimeter simulation
Apache License 2.0
3 stars 1 forks source link

Redesign messaging #49

Open jbeirer opened 2 weeks ago

jbeirer commented 2 weeks ago

To comply with messaging in Athena, our messaging currently relies on preprocessor macros. As those are identical with what is used in some Athena classes, we'll rename the ATH_MSG to FCS_MSG for now. Eventually, we should re-design the messaging to not use macros at all - see also https://github.com/fcs-proj/FastCaloSim/pull/48

TheRealRobinHood commented 2 weeks ago

Would you like to retain the logging architecture? So that inheritance is used or would you like to have the logging class detached, e.g. through a singleton or dependency injection?

jbeirer commented 2 weeks ago

I fear that to remain consistent with our input files e.g. https://github.com/fcs-proj/FastCaloSim/tree/main/test/data/param/barrel we will need to keep the logging structure as is for now. The plan is to port the code responsible for generating the input files to this repo soon, but this is still work in progress

TheRealRobinHood commented 2 weeks ago

Thank you very much for your feedback. I am currently working on the feature, but it may take some time as I don't have time every day after work.

TheRealRobinHood commented 1 week ago

I am afraid that it is not possible to improve the structure without regenerating the input files. This is because changing the MLogging class also changes the binary structure of the class. I have rewritten the MLogging class so that preprocessor statements are no longer necessary. So far, however, I have only implemented the class in the header file. The header and source file still need to be split up.

https://github.com/TheRealRobinHood/FastCaloSim/redesign-messaging

I suggest waiting until the repository responsible for generating the input files has been ported to this repository before finally re-implementing the MLogging class.

I believe this approach would provide the most performant solution.

jbeirer commented 1 week ago

Yes, agreed. There are some more urgent things on my list before I can focus on porting that part of the code, but we will try to get this in asap. In any case, thanks for this already!