search

ess-dmsc / event-formation-unit

Implementation of neutron event formation pipeline for ESS
BSD 2-Clause "Simplified" License
9 stars 6 forks source link

Namespaces for pipelines #132

Closed martukas closed 5 years ago

martukas commented 5 years ago

There are increasingly more potential name clashes across the pipelines for concepts like "Hit" or "Event" or "Readout". To keep things clean, we should likely put most of the classes for each pipeline into dedicated namespaces. This has already been done for Multigrid, and I see this becoming necessary for NMX. Should we just do this for everything? Thoughts @SkyToGround @mortenjc ?

mortenjc commented 5 years ago

Good point. There is certainly some potential clashes and need for cleaning up.

Maybe the first thing is to make sure we are using the right names to begin with? Event is a fairly well defined entity at ESS: (time, pixel)-tuple. If we use this for other purposes, then maybe we should consider renaming? Readout is probably too generic and should have MG, GdGEM and MB prefixed ? Hit is actually a readout and should probably not be used? I have nothing against namespaces, but should we use them to fix bad naming?

SkyToGround commented 5 years ago

@martukas, I agree. Though it is not that much of a problem right now, it will likely become a bigger one as time goes by. Especially if we start doing static linking of detector modules. Regardless, namespaces are simply a good idea (as long as they are used wisely).

martukas commented 5 years ago

@mortenjc when you say "should have MG, GdGEM and MB prefixed" that is exactly the case for namespace. In the case of NMX there is a distinction between Readout and Hit as different levels of abstraction, latter one being the post- geometry and time disambiguation representation. Anyway, then we also have possible increased complexity in clustering, so we may end up with more "Sorters" and "Clusterers" and the like. It's better to do this now to avoid technical debt later. I am willing to do this if we have some tentative consensus.

SkyToGround commented 5 years ago

@martukas Although I do not mind if namespaces are implemented relatively soon, I do think that we should have a face to face meeting (between the three of us) to discuss names before we do it. How about some time on Thursday?