cms-nanoAOD / correctionlib

A generic correction library
https://cms-nanoaod.github.io/correctionlib/
BSD 3-Clause "New" or "Revised" License
16 stars 22 forks source link

Unified interface for Correction and CompoundCorrection #132

Open swertz opened 2 years ago

swertz commented 2 years ago

While testing the jetmet SFs for JES, I realized that working with CompoundCorrections can be a bit awkward. The issue is that depending on the correction level(s) that is (are) needed (simple or multiple), one has to work with either a Correction or a CompoundCorrection object. When writing helper code that loads the corrections, using as input the correction level requested by the user, one doesn't know in advance what will be used. Essentially I need to do e.g.:

// first load all objects
auto cset = CorrectionSet::from_file(".../jet_jerc.json.gz");
std::string key = "Summer19UL18_V5_MC_L2Relative_AK4PFchs"; // or could be "Summer19UL18_V5_MC_L1L2L3Res_AK4PFchs"
auto maybe_corr = std::find_if(cset->begin(), cset->end(), [key](const auto& elem){ return elem.first == key; } );
std::variant<Correction::Ref, CompoundCorrection::Ref> jesSF;
if (maybe_corr !=  cset->end())
    jesSF = maybe_corr->second;
else
    jesSF = cset->compound()[key];
// ...
// then, in the event loop:
float sf;
if (auto corr = std::get_if<Correction::Ref>(&jesSF)) {
    sf = (*corr)->evaluate({eta, pt});
} else {
    sf =  std::get<CompoundCorrection::Ref>(jesSF->evaluate({area,eta,pt,rho});
}

Perhaps it would be easier if both Correction and CompoundCorrection inherited from the same abstract class, that declares the evaluate method? Then, CorrectionSet::at could automatically return either the correction or the compound correction, depending on the presence of the key in corrections_, dropping the extra compound method?

nsmith- commented 2 years ago

Hm I wasn't aware there would commonly be a need to switch between simple and compound objects. I don't see an obvious issue with making them polymorphic. (the interior nodes however are purposely not polymorphic)

swertz commented 2 years ago

I would simply argue that whether a correction is compound or not is an implementation detail, that the user does not necessarily need to know about.

FHead commented 2 years ago

I second the suggestion. Whether the correction is compound or not is not of interest to the vast majority of users. Internally we can design them as separate, but having a unified user-facing interface would be a very nice enhancement. (Incidentally, this is also one of the points I noticed while working on the JECViewer and mentioned in the Cross-POG meeting last week)