kdlong / cmssw Goto Github PK

Especially the buildGroups function is too long and too stream-of conscious. It should be split into multiple functions

Putting this in one place, though there should also be individual issues

https://docs.google.com/spreadsheets/d/1Qlg1c6qrMGY0q111Va2Vl0fJQUz15ZTjKEvRaV8rHaQ/edit#gid=0

https://www.dropbox.com/s/c17g859p15xpf42/200820_shKo.pdf?dl=0

Finalize functionality and set defaults for the number of weights stored for each type. Also likely need to introduce functionality to store a maximum number of weights from a group.

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?

Select specific PS weights and store in the desired order

Some info, such as the weight sums, of the current NanoAOD GenWeightTablesProducer are not yet in the new producer.

This should also be captured under the failIfInvalidXML

Extend parsing to detect MEParam weights. Most typical use case is a sample with MG reweighting, where the group is labed "mg_reweight," I believe.

Would also be good to capture explicit ME weights that are sometimes used in POWHEG. For the mW samples, these include "mass_variation," "width_variation," and "sthw2_variation"

We can discuss this, but the PS weights follow a distinct patter that may not be clear. This could allow for just a few helper functions rather than the current get weight by label approach. The weights are:

• FSR or ISR
• Conservative, Default, or Reduced variation (variation of underlying param by 4, 2, sqrt(2))
  • OR: variations per parton splitting:
    • G2QQ (gluon to quarks), G2GG (gluon to gluons), XToXG (anything emitting a gluon, I think...)

So it might make sense to have

indexOfFullVariation(isISR, variationType, up/down)
indexOfSplittingVariation(splittingType, variation)

or something along these lines

Some samples with MadGraph and matching have an emission scale variation.

I think these can just be labeled as PartonShower weights (so LHEPartonShowerWeight, seems contradictory, but I think it's ok).

Example in slide 5: https://www.dropbox.com/s/c17g859p15xpf42/200820_shKo.pdf?dl=0

Need to make sure that parton shower weightgroup features support this.