Stop looking for primaries for simple radiative photon-like geometries #2178
Conversation
…like radiative photons from a primary electrons
| // we stop looking if we find the parent has status 1 but with only 2 daughters | ||
| // so we don't merge radiative photons with the primary electron as this prevents us | ||
| // from properly linking the clusters back to the event geometry | ||
| // @TODO this should be a shared utiliy function in the edm4xxx |
There was a problem hiding this comment.
Can you file this as an issue in edm4eic?
There was a problem hiding this comment.
Will do - there are multiple copies of this function across multiple algorithms that we could get rid of.
There was a problem hiding this comment.
Been meaning to ask copilot for code duplication... And enable that as a pre-commit or clang-tidy check...
There was a problem hiding this comment.
We should modify other copies too?
There was a problem hiding this comment.
I'm unsure as I don't know how they are used - I could see a case where this function could be somewhat configurable depending on use case - it wouldn't be too hard to add a parameter (could be templated or not) to steer the behavior. And/or have multiple versions with explicit different names that should be called by the user for clarity.
There was a problem hiding this comment.
It also doesn't make sense to have different definitions under the same name.
I would just make the change and @ruse-traveler can confirm about other use cases.
mhkim-anl
left a comment
mhkim-anl
left a comment
There was a problem hiding this comment.
Looks sensible to me as well
sly2j
dismissed stale reviews from mhkim-anl, ruse-traveler, and wdconinc
via
c1133e1
Briefly, what does this PR introduce?
Currently, SimCalorimeterHitProcessor links primary generated particles with the detector hits, which takes care of early showering particles leading to calorimeter hits being associated with a large number of particles. However, this inadvertently also gets rid of e.g. single photons radiated in the detector. This PR changes the primary particle lookup so it stops when it encounters a particle where the parent is a primary and the primary only has two daughters. This should be gentle enough to not re-introduce the previous issue we dealt with when introducing the primary lookup while adding most of the crucial information related to the originator of calorimeter clusters back in.
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?
Does this PR change default behavior?