cms-analysis / HiggsAnalysis-CombinedLimit

CMS Higgs Combination toolkit.
https://cms-analysis.github.io/HiggsAnalysis-CombinedLimit/latest
Apache License 2.0
75 stars 389 forks source link

More standard implementation of RooMultiPdf #1024

Closed guitargeek closed 6 days ago

guitargeek commented 6 days ago

The people that implemented RooMultiPdf had a doubt: they didn't dare to implement RooMultiPdf::evaluate() because they were not sure how the normalization set information is propagated.

The anser is: it's propagated in the base implementation of getValV() via the proxies, like the RooListProxy with the pdf in this case. The propagated normalization set can be queried from the proxy with nset().

Hence, this PR suggests to implement evaluate() accordingly.

Furthermore, the selfNormalized() method needs to be overridden, to indicate that the base implementation of RooAbsPdf::getValV() doesn't need to create any normalization integrals implicitly, which would be a huge performance hit.

This more standard implementation makes sure that the class is more compatible with regular RooFit, which expects that evaluate() is implemented.

It was validated with the following command that results and performance remain unaffected:

text2workspace.py data/ci/datacard_RooMultiPdf.txt.gz -o multipdf_ws.root
combine -M MultiDimFit -m 125.38  --setParameters pdf_index_ggh=2 --freezeParameters MH --cminDefaultMinimizerStrategy 0 --X-rtd FAST_VERTICAL_MORPH --X-rtd MINIMIZER_freezeDisassociatedParams --X-rtd MINIMIZER_multiMin_maskChannels=2 --algo singles ws_RooMultiPdf.root

Two additional commits in this PR fix a memory leak and refactor the code to be less verbose respectively.