Move PID look up tables into global pid file #1745
Conversation
github-actions
bot
added
topic: PID
simonge
requested review from
a team and
wdconinc
and removed request for
a team
| @@ -3,5 +3,4 @@ cmake_minimum_required(VERSION 3.16) | |||
| add_subdirectory(tracking) | |||
| add_subdirectory(reco) | |||
| add_subdirectory(pid) | |||
| add_subdirectory(pid_lut) | |||
There was a problem hiding this comment.
Having pid_lut allows to indicate that the codes in there are not supposed to do any actual PID. If you still prefer to merge those into PID, please do so for src/algorithms/pid_lut as well.
src/global/pid/pid.cc
Outdated
| @@ -7,8 +7,11 @@ | |||
|
|
|||
There was a problem hiding this comment.
Moving everything to the same file clarifies data flow for LUT PID. Actual implementations for detectors can not be as uniform and probably not going to co-exist as well. That was my original reasoning.
There was a problem hiding this comment.
Is there ever going to be a situation where the PID from the LUT from some detectors are mixed with real PID from others?
I don't know if it's always possible but it certainly feels nicer not to jump back and forth between plugins.
There was a problem hiding this comment.
I'd say, yes, as soon as any PID reconstruction is production ready (works on full DIS events), we then don't keep its LUT around.
There was a problem hiding this comment.
What decade do you estimate that happening in? ;-)
Capybara summary for PR 1745
|
Briefly, what does this PR introduce?
Moves all of the PID LUT factories out of their detector plugin into the global pid plugin. Also moves PID factory files into the factories directory.
Maybe a matter of personal taste but this would make the data flow through the PID algorithms more transparent.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No
Does this PR change default behavior?
No