Skip to content
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

Cluster Shape Parameter Calculation Could Be Factorized #1733

Closed
ruse-traveler opened this issue Feb 11, 2025 · 0 comments
Closed

Cluster Shape Parameter Calculation Could Be Factorized #1733

ruse-traveler opened this issue Feb 11, 2025 · 0 comments
Assignees

Comments

@ruse-traveler
Copy link
Contributor

Is your feature request related to a problem? Please describe.

While working on EICrecon#1699, the cluster shape parameter calculation from CalorimeterClusterRecoCoG was copied-and-pasted into the updated TrackClusterMergeSplitter algorithm. As was pointed out in the discussion, this suggests that this calculation might factorize nicely into a separate algorithm which could, in turn, improve modularity and save us from having to copy code in multiple cluster reconstruction/re-reconstruction algorithms.

Describe the solution you'd like

It would be straightforward to move the calculation to a new algorithm, and remove it from CalorimeterClusterRecoCoG. The output of the existing RecoCoG factories would be clusters without shape parameters (e.g. LFHCALClustersWithoutShapeParameters), and then the new ClusterShapeCalculator algorithm would process these clusters and produce clusters with shape parameters (i.e. LHFCALClusters).

Since the without-shape-parameter clusters would be an intermediate step, it would likely be preferable to not save them to output.

Describe alternatives you've considered

An alternative could be to move the calculation to either some sort of utils namespace, or possibly create an algorithms service in the vein of ParticleSvc that could be called in multiple places.

@ruse-traveler ruse-traveler self-assigned this Feb 11, 2025
@ruse-traveler ruse-traveler moved this from Todo to In Progress in ePIC Software and Computing Feb 11, 2025
github-merge-queue bot pushed a commit that referenced this issue Mar 9, 2025
### Briefly, what does this PR introduce?

This PR moves the cluster shape parameter calculation from
`CalorimeterClusterRecoCoG` to its own algorithm.

### What kind of change does this PR introduce?
- [ ] Bug fix (issue #__)
- [x] New feature (issue #1733 )
- [ ] Documentation update
- [ ] Other: __

### Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [x] Changes have been communicated to collaborators

### Does this PR introduce breaking changes? What changes might users
need to make to their code?

No.

### Does this PR change default behavior?

No.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Dmitry Kalinkin <[email protected]>
@ruse-traveler ruse-traveler moved this from In Progress to Done in ePIC Software and Computing Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

1 participant