cms-analysis / HiggsAnalysis-CombinedLimit

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

Force updateGrid in expectedFromGrid HybridNew.cc #941

Closed nucleosynthesis closed 4 months ago

nucleosynthesis commented 4 months ago

if using --noUpdateGrid with --expectedFromGrid and the grid file was not produced using the same value for expectedFromGrid, then the limit will be identical to the result without --expectedFromGrid.

With this change, we now force an update if --expectedFromGrid option set. Usual workflow is to produce the grid (without this option) and then calculate expected results after, so this works with this workflow if user adds --noUpdateGrid

nucleosynthesis commented 4 months ago

Added the needs work label since there could be a smarter way to do this, without having to resort to a full update at various points

nucleosynthesis commented 4 months ago

Note, have also added a warning about this in the docs : https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/commit/21ad9b09b9e30809c8e692c75b9bdf207adcd062#diff-e1cf5d8257ea3082032f066028c25c4e5134426b497d6f3f40042e955177759dR585-R587

nucleosynthesis commented 4 months ago

Slightly smarter now in that the update will only happen, even if user specifies --noUpdateGrid if the quantileExpected value from the grid file doesn't match the one being asked for (if expected, check against value in the file, if observed just compare with -1)

Not bullet proof but for normal workflows, should catch most issues

nucleosynthesis commented 4 months ago

I think this one can be merged now, so if no objections, i'll do so soon