-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix norm uncertainties #562
base: main
Are you sure you want to change the base?
Conversation
…onstruct uncertainty in setupCombine
Since I believe this is the first PR which changes the nominal result we should think about whether we want to make a branch or such before merging it. |
A branch for where we keep the nominal result unchanged or a branch where changes go? |
for right this minute (until we finish with the supplemental plots with helicity xsecs etc) it would be better to leave the nominal analysis unchanged. The easiest way is probably to set propagateToFakes=False for these particular systematics such that the old behaviour is preserved |
…for now, to keep synchronized with unblinding conditions
Is it understood why there are still numerical differences with respect to the reference? The constructed shape uncertainties should be really exactly equivalent to the lnN no? |
Yes there are small numerical differences because previously a histogram was filled in the histmaker with the event weights scaled up and down according to the luminosity uncertainty. Now the nominal histogram is used and scaled up and mirrored at the setupCombine.py step. |
Can this be merged? |
Did you test the impact on the result with high stats? If we're going to do additional studies based on reviewer comments, we probably don't want the central value to change. |
I also don't really understand why this change should induce even visible numerical differences, since the shape variation should just be a simple scaling of the histogram. |
Here are the results from running the workflow dispatch: |
d94bf50
to
f285e3c
Compare
I was able to trace back the small differences. In the W case previously there was a histogram provided with the lumi varied up and down by multiplying the event weight w_up = w * factor and w_down = w / factor. Then the variations were symmetrized by "average" in the log space. The PR gives now consistent results with the reference CI and is from my side ready to be merged. |
d100e4a
to
cfe4768
Compare
Previously norm uncertainties were added via "addLnNSystematic" but not propagated into the sideband regions for the fake estimation.
The LnN systematics are now removed and norm uncertainties are instead added as shape uncertainties (that was already the case before once converted to hdf5 tensor). Now the same interface is used, via "addSystematic". I validated that it is equivalent for the Z, some differences are expected for the W.
The luminosity histogram for the W is removed as well and this variation is added in the same way with "addSystematic" in setupCombine on the fly.
Fixing the combined W and Z fit in the CI