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 ScaleWeightGroup class #31

Open
kdlong opened this issue Oct 4, 2020 · 0 comments
Open

Clean up ScaleWeightGroup class #31

kdlong opened this issue Oct 4, 2020 · 0 comments

Comments

@kdlong
Copy link
Owner

kdlong commented Oct 4, 2020

The different variations are stored in a vector in a predetermined order, but that order isn't advertised anywhere, and is accessed with unexplained "magic numbers." There should either be an enum explaining the indices, or the vector should be an unordered_map. There should also be a check in the getXIndex function that the entry exists.

Talking about these lines: https://github.com/kdlong/cmssw/blob/ImprovedParsing/SimDataFormats/GeneratorProducts/interface/ScaleWeightGroupInfo.h#L46-L64

There are also some unexplained "magic numbers" here: https://github.com/kdlong/cmssw/blob/ImprovedParsing/SimDataFormats/GeneratorProducts/src/ScaleWeightGroupInfo.cc#L42

Another small point is that the official CMSSW naming convention recommends not using "get" in the name of getting funcitons. So getDynNames, getScaleIndex --> dynNames, scaleIndex.

https://cms-sw.github.io/cms_coding_rules.html#2----naming-rules-1

@dteague are you able to implement this?

@kdlong kdlong changed the title Clean up ScaleWeightGroup calss Clean up ScaleWeightGroup class Oct 4, 2020
kdlong added a commit that referenced this issue Sep 28, 2022
[WIP] new pileup safe simcluster merger and PF truth skeleton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant