-
Notifications
You must be signed in to change notification settings - Fork 32
Run track-cluster matching separately for each calorimeter #1906
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
Conversation
9eb01f5
to
0ea528f
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
0ea528f
to
3818632
Compare
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.
Very nice! This is a much appreciated improvement! The diffs for the relevant collections look like I'd expect them to, but I am seeing some diffs for a few collections I'd expect to be unaffected by this PR...
This seems to be similar to what we saw in #1919 ... So maybe the artifacts capybara are using out of date?
I just noticed that the EcalEndcapPInsert has been removed between starting this and now (#1852), so I removed its track cluster match factory |
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.
LGTM
@ruse-traveler There are somehow irreproducibilities in B0 calorimetry, and they are not yet understood. |
Ahhhh gotcha! |
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.
Still looks good!
### Briefly, what does this PR introduce? A suggested in #1885, the track-cluster matching should be done separately for each calorimeter. Later algorithms can then manage the merging and splitting of clusters and integration across calorimeters. This also makes it simpler to tune the matching criteria separately for each subsystem. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [X] New feature (issue #1885 ) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? There is no longer a single TrackClusterMatches collection output, instead is is broken into separate collections for each calorimeter ``` EcalEndcapPTrackClusterMatches EcalEndcapPInsertTrackClusterMatches LFHCALTrackClusterMatches HcalEndcapPInsertClusterMatches EcalBarrelTrackClusterMatches HcalBarrelTrackClusterMatches EcalEndcapNTrackClusterMatches HcalEndcapNTrackClusterMatches ``` ### Does this PR change default behavior? See above --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <[email protected]>
Briefly, what does this PR introduce?
A suggested in #1885, the track-cluster matching should be done separately for each calorimeter. Later algorithms can then manage the merging and splitting of clusters and integration across calorimeters. This also makes it simpler to tune the matching criteria separately for each subsystem.
What kind of change does this PR introduce?
TrackClusterMatch
#1885 )Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
There is no longer a single TrackClusterMatches collection output, instead is is broken into separate collections for each calorimeter
Does this PR change default behavior?
See above