Addition of 2D RooParametricHist class

[lcorcodilos]

This adds a 2D version of RooParametricHist (RooParametricHist2D) and the infrastructure to support such a class. I'll be submitting a corresponding PR to the Combine Harvester repo with the code to plot in 2D.

I started working on this about a year ago in an effort to write a simple 2D fit. Splitting the second dimension into different categories/bins in Combine or unrolling the second dimension onto one dimension both seemed like methods that would be error prone or very messy (since the second dimension is 30 bins for my analysis) so I wrote this instead with suggestions from @pmaksim1.

I've done basic tests of the class like feeding in a TH2 and getting the same TH2 back. But, I've also put it through several fit tests with very basic shapes to confirm that Combine interprets it correctly and is able to fit it.

This class has become an integral part of a more generic 2D fitting framework that I've been working on called 2D Alphabet which is a wrapper that builds data driven background estimates using RooParametricHist2D, feeds the workspace/data card to combine, and then plots the result nicely (repo can be found here).

2D Alphabet is currently being used in the b*->tW all-hadronic analysis (my own) which will be entering Object Review any day now and by several HH analysis channels in their early stages. There are several analyses in B2G that could use this code but since it relies on this class, it's been suggested to try including it in the Combine release - or at least get it verified by the experts :-)

Thanks!

[usarica]

Hi Lucas,
There are already existing classes that work for this equivalent purpose (i.e. the FastHisto2D basic class with RooFit wrapper FastHisto2DFunc). I don't think it would be very controversial to make a PR for additional ones if changing several B2G analyses is more difficult, but it might save time in general to check what is available beforehand.

[lcorcodilos]

Hi Ulas,

Correct me if I'm wrong but FastHisto2D is different because it can't take other RooFit objects (RooRealVar, RooFormulaVar, etc) as the bins like RooParametricHist2D can. In other words, if I was performing this method in 1D, I would be using RooParametricHist not FastHisto.

[nucleosynthesis]

Hi Ula, Lucas Indeed, the RooParametericHist is slightly different since the bins are not fixed values with morphing functions attached but rather the bins themselves are any RooFormulaVar or RooRealVar. There is potentially a way to use the FastHisto2D by supplying a different interpolation function but its still less general I think - Ula, is this the case? Generally, It helps people avoid recreating functionality if said functionality is included in the documentation. If you think these objects could be used more generally, it would be nice to include some documentation in the new docs page. This seems like a reasonable PR, but I would also appreciate some additional docs in the section here: http://cms-analysis.github.io/HiggsAnalysis-CombinedLimit/part3/nonstandard/#rooparametrichist-gamman-for-shapes - Can you directly edit the docs also in this PR? Best, Nick

…

On Mon, May 20, 2019 at 10:44 AM lcorcodilos ***@***.***> wrote: Hi Ulas, Correct me if I'm wrong but FastHisto2D is different because it can't take other RooFit objects (RooRealVar, RooFormulaVar, etc) as the bins like RooParametricHist2D can. In other words, if I was performing this method in 1D, I would be using RooParametricHist not FastHisto. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#545?email_source=notifications&email_token=AAMEVWYQ6NV5ALLWOSRYZLLPWLBNNA5CNFSM4HNW7CL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVZH3RQ#issuecomment-494042566>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMEVW5WEJDPSOPO6VRX263PWLBNNANCNFSM4HNW7CLQ> .

[usarica]

Hi Lucas, Nick,
Indeed, I think the use case is quite useful. I haven't really thought about it in terms of having bins to be parametric themselves. One question here though, maybe for the 1D class as well since I see a similar implementation: Do you actually need to pass the TH1/TH2 object? It can still be kept for backward compatibility, but it seems like the binning of x (and y) should already contain this information. Do you think it would be acceptable to make a PR to add a constructor that does not pass a TH1/2 object? I think this way, the use case would also be clearer (e.g. I was confused about the purpose of passing this object the first time I saw this class).
Regards,
Ulascan

[nucleosynthesis]

Hi, Indeed, the RooParametricHist (the 1D version) is already is use for example in the H->invisible analyses - the use case there is exactly to map from some signal region to a control region to make in-situ constraints of backgrounds (CF HIG-17-023). I think the idea for this analysis (alphabet) is similar. I think a constructor without a TH1/2 would be fine since indeed the binning is all thats extracted and this could also be taken from the RooRealVar Observable. Best, Nick

…

On Mon, May 20, 2019 at 2:02 PM Ulascan Sarica ***@***.***> wrote: Hi Lucas, Nick, Indeed, I think the use case is quite useful. I haven't really thought about it in terms of having bins to be parametric themselves. One question here though, maybe for the 1D class as well since I see a similar implementation: Do you actually need to pass the TH1/TH2 object? It can still be kept for backward compatibility, but it seems like the binning of x (and y) should already contain this information. Do you think it would be acceptable to make a PR to add a constructor that does not pass a TH1/2 object? I think this way, the use case would also be clearer (e.g. I was confused about the purpose of passing this object the first time I saw this class). Regards, Ulascan — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#545?email_source=notifications&email_token=AAMEVWYT2GJZRATTQCYZXQTPWLYVNA5CNFSM4HNW7CL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVZYYIA#issuecomment-494111776>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMEVW75XEVIDQCC5MLILDLPWLYVNANCNFSM4HNW7CLQ> .

[lcorcodilos]

Hi all,

I'm happy to edit the docs to accommodate the new class.

I think the discussion on the TH1/2 implementation is interesting. Is it possible to do variable binning with a RooRealVar? I imagine you'd have to build a RooAbsBinning object and then feed it to the RooRealVar via setBinning. Then internal to RooParametricHist(2D), we'd have to open it up as we do with the TH1(2).

I think from a user standpoint, it's easier to give the TH1/2 since that's what most users start from when building the RooParametricHist object and so it's readily available. From a software development point of view, it's cleaner to drop a redundant argument. Would you want the change in this PR or a separate one since it should also change the 1D RooParametricHist?

Lucas

[usarica]

Hi Lucas, Nick,
It is indeed possible, and in some cases desirable, to do variable binning. I would leave that implementation to a separate PR (I can make that PR if you agree); I think I would also want to cover the scenario of integration code where the parameters depend on x or y (in which case code=0 should be returned even if matchArgs is true).

[nucleosynthesis]

Hi Ula There is already a class which covers the case of a dynamic binning of a pdf - ie the bin contents are the integral of a specified PDF over fixed intervals ( this is documented on the section about parametric workspaces) so I would not also add that functionality here Nick

…

On Mon, 20 May 2019, 19:26 Ulascan Sarica, ***@***.***> wrote: Hi Lucas, Nick, It is indeed possible, and in some cases desirable, to do variable binning. I would leave that implementation to a separate PR (I can make that PR if you agree); I think I would also want to cover the scenario of integration code where the parameters depend on x or y (in which case code=0 should be returned even if matchArgs is true). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#545?email_source=notifications&email_token=AAMEVW7TFJ7QYCNCFGTJFALPWM6T7A5CNFSM4HNW7CL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV2NFZI#issuecomment-494195429>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMEVWYEQMPVMK2PR5QHLODPWM6T7ANCNFSM4HNW7CLQ> .

[nucleosynthesis]

I just realised maybe this isn't what you meant ... On Mon, 20 May 2019, 20:13 Nicholas Wardle, <n.c.k.wardle@googlemail.com> wrote:

…

Hi Ula There is already a class which covers the case of a dynamic binning of a pdf - ie the bin contents are the integral of a specified PDF over fixed intervals ( this is documented on the section about parametric workspaces) so I would not also add that functionality here Nick On Mon, 20 May 2019, 19:26 Ulascan Sarica, ***@***.***> wrote: > Hi Lucas, Nick, > It is indeed possible, and in some cases desirable, to do variable > binning. I would leave that implementation to a separate PR (I can make > that PR if you agree); I think I would also want to cover the scenario of > integration code where the parameters depend on x or y (in which case > code=0 should be returned even if matchArgs is true). > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#545?email_source=notifications&email_token=AAMEVW7TFJ7QYCNCFGTJFALPWM6T7A5CNFSM4HNW7CL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV2NFZI#issuecomment-494195429>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAMEVWYEQMPVMK2PR5QHLODPWM6T7ANCNFSM4HNW7CLQ> > . >

[usarica]

Hi Nick,
I was referring to variable binning of a 1D or 2D histogram. In fact the current implementation would already support it (i.e. if the histogram shape has variable binning, it would already be picked up), but a PR would still be needed to allow a constructor without the extra TH1/2. The other thing I had in mind was to prevent analytical integration when at least one of the par's depends on x (and y), so that would require additional checks on getAnalyticalIntegral.

[nucleosynthesis]

Hi, Ah ok so indeed I misunderstood. disregard my last message then. Nick

…

On Mon, 20 May 2019, 20:21 Ulascan Sarica, ***@***.***> wrote: Hi Nick, I was referring to variable binning of a 1D or 2D histogram. In fact the current implementation would already support it (i.e. if the histogram shape has variable binning, it would already be picked up), but a PR would still be needed to allow a constructor without the extra TH1/2. The other thing I had in mind was to prevent analytical integration when at least one of the par's depends on x (and y), so that would require additional checks on getAnalyticalIntegral. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#545?email_source=notifications&email_token=AAMEVW6QJG7JUBIFMK33S23PWNFC7A5CNFSM4HNW7CL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV2PJKA#issuecomment-494204072>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMEVW6LSY3ZM3UICZ7CUFDPWNFC7ANCNFSM4HNW7CLQ> .

[acarvalh]

Hi. Is this PR expected to be merged? I am plan to use the results of the analysis that triggered it in a combination and it would be nice to use the upstream version of combine.

[nsmith-]

@acarvalh where can I find cards for the analysis? Are they in https://gitlab.cern.ch/cms-hcg/cadi ? (if not we can add them)
(this is to help check the PR)

[lcorcodilos]

@acarvalh @nsmith- The cards, workspace files, and FitDiagnostics command for B2G-19-003 (the b*->tW analysis mentioned in the original PR text) can be found on lxplus here [1].

Note that I'm no longer a member of CMS/CERN and I just happen to have access to lxplus still. I don't know how long my public space will last but I figured this would be better than attaching the file publicly. If there are any other files you may need, I can see if I have them sitting somewhere still.

[1] - /afs/cern.ch/user/l/lcorcodi/public/forCombinePR545.tar.xz

[nsmith-]

Thanks @lcorcodilos I was able to copy this file.
Features missing in 2D vs. the 1D implementation:

• Full analytic integral (though most all cases should only need full-range I believe)
• Ability to add morphs

Features desired (for both 1D and 2D):

• Constructor from RooRealVar binning rather than dummy TH1/TH2

I would propose to merge this and leave those features for future work (will make an issue to track)

[hcombbot]

Pull Request Test.
Summary
========
Running options:
* MODE : cmssw
* COMBINE_TAG : 102x
* COMBINE_REPO : cms-analysis
* COMBINE_MERGE : lcorcodilos/PRbranch
* GITHUB_PR : 545


Ratio to reference values:
--------
| comb_2019_hbb_boosted_standalone | comb_2019_hgg | comb_2019_hmm | comb_2019_htt | comb_2019_hww | comb_2019_tth_hbb | comb_2019_tth_hgg | comb_2019_tth_multilepton | comb_2019_vh_htt | comb_2019_vhbb | comb_2019_vhbb2017 |
| ------ | ------ | ------ | ------ | ------ | ------ | ------ | ------ | ------ | ------ | ------ |
| 1.00 | 1.00 | 1.00 | 1.00 | 1.00 | 1.00 | 1.00 | 1.00 | 1.00 | 1.00 | 1.00 |

You can find more detail at https://gitlab.cern.ch/cms-hcg/performances/ci/-/pipelines/3816077