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

Clean up HGCal subdetector terminology and usage #26225

Open
kpedro88 opened this issue Mar 20, 2019 · 6 comments
Open

Clean up HGCal subdetector terminology and usage #26225

kpedro88 opened this issue Mar 20, 2019 · 6 comments

Comments

@kpedro88
Copy link
Contributor

From @cseez #26099 (comment):

The correct terminology is that FH refers to fine samplings, BH refers to coarse samplings - FH/BH should not be used to refer to the distinction between Si and mixed cassettes. I don't know how deeply embedded the improper usage is in CMSSW, but we should try to eradicate it... even if it takes a few steps to do so...

Right now, RecHitTools has lastLayerEE() and lastLayerFH() functions. It should be checked that the usage of these functions is correct. In addition, functions like lastLayerFine() and lastLayerCoarse() can be added to present the indicated information in a clear way.

attn: @bsunanda @rovere

@kpedro88
Copy link
Contributor Author

assign upgrade

@cmsbuild
Copy link
Contributor

New categories assigned: upgrade

@kpedro88 you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

A new Issue was created by @kpedro88 Kevin Pedro.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@rovere
Copy link
Contributor

rovere commented Mar 29, 2019

@kpedro88
RecHitTools is a nice utility package, but it needs to be properly configured from the EventSetup. This, IMHO, in a few situations, makes its usage far less trivial, especially in plugins and helper plugins where the price you have to pay is to percolate the EventSetup deep down your code (either that or the rechHitTools instance).
But I agree that we should carefully review the usage of hard-coded constants in the code. Constants declared as such in a commonly included header, with proper naming, do not follow into that category, IMHO. It lacks, of course, the flexibility and "dynamic nature" of geometry derived values from the EventSetup.

@kpedro88
Copy link
Contributor Author

kpedro88 commented Mar 29, 2019

Constants declared as such in a commonly included header, with proper naming, do not follow into that category, IMHO.

The V10 geometry has a different number of layers than V9. This clearly can't be handled well with a hardcoded constant. The approach really needs to be reassessed. I highly doubt V10 will be the last major change before we actually build the HGCal.

@rovere
Copy link
Contributor

rovere commented Mar 29, 2019

HGCal...
I agree, we should review the code and find an optimal way to handle flexibility and usability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants