-
Notifications
You must be signed in to change notification settings - Fork 628
PWGHF: Implement efficiency computation macro for D2H #3616
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
Conversation
PWGHF/D2H/Macros/efficiency.py
Outdated
| for i_bin in range(1, n_bins+1): | ||
| h_eff.SetBinContent(i_bin, teff.GetEfficiency(i_bin)) | ||
| # TH1 can't handle asymmetric errors so we take the max | ||
| err_max = max(teff.GetEfficiencyErrorLow(i_bin), teff.GetEfficiencyErrorUp(i_bin)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering in which case it can happen that the efficiency has an asymmetric error. If I am not wrong, the binomial uncertainty is always symmetric right?
Just to be sure that I'm not overlooking anything, the code looks fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Mattia, thanks for the reply. Indeed, I must say I could not think of a case where this would happen, but I put it as extra safety after reading about it in a ROOT forum. But, in our use case, I don't think that we truly need it, if I am not misssing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Mattia, thanks for the reply. Indeed, I must say I could not think of a case where this would happen, but I put it as extra safety after reading about it in a ROOT forum. But, in our use case, I don't think that we truly need it, if I am not misssing something.
Hi @AlexBigO , exactly! This is my point.
What about adding a warning when this happens? Something like:
if abs(teff.GetEfficiencyErrorLow(i_bin) - teff.GetEfficiencyErrorUp(i_bin))/teff.GetEfficiencyErrorLow(i_bin) > 1e-05: print a warning
Something similar to this. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this is better indeed! Thanks, I implemented it, and will push it along with future comments.
I just put a "sensitivity" of 1e-02, below this, I get a warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
mfaggin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot @AlexBigO , good that this macro can become available for everybody! For me it looks fine, but I leave the time to @fcatalan92 to have a look.
I just have 1 question for my understanding, see the dedicated comment.
Best regards.
Mattia
…en converting TEfficiency to TH1, and add pt binning option
|
Some comments on my latest commit:
|
|
It looks good to me. @fcatalan92 can you have a look as well? |
…ds, try solving MegaLinter errors
|
Sorry for the many attempts to solve the MegaLinter issues, the |
fcatalan92
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
) * Implement efficiency computation macro for D2H * Add new line at the end of config file * Fix bug in TH2 projection, add safety for asymmetric error setting when converting TEfficiency to TH1, and add pt binning option * Add missing loop on all pt bins after rebinning * Remove scaling after rebinning (not nedded), re-organise helper methods, try solving MegaLinter errors * Add safety for canvas file extension and try to solve mypy errors * New attempt to solve mypy errors * Try to fix type of save_canvas() last argument + import-error on modules * Add copyright + attempt to solve mypy error * Attempt to solve pyright error #1 * Second attempt to solve pyright error * Apply linters automatic fixes * Apply Vit's comments * Remove empty line between ROOT and style_formatter imports * Fix typo found by Fabio --------- Co-authored-by: Alexandre Bigot <abigot@sbgat402.in2p3.fr>
) * Implement efficiency computation macro for D2H * Add new line at the end of config file * Fix bug in TH2 projection, add safety for asymmetric error setting when converting TEfficiency to TH1, and add pt binning option * Add missing loop on all pt bins after rebinning * Remove scaling after rebinning (not nedded), re-organise helper methods, try solving MegaLinter errors * Add safety for canvas file extension and try to solve mypy errors * New attempt to solve mypy errors * Try to fix type of save_canvas() last argument + import-error on modules * Add copyright + attempt to solve mypy error * Attempt to solve pyright error #1 * Second attempt to solve pyright error * Apply linters automatic fixes * Apply Vit's comments * Remove empty line between ROOT and style_formatter imports * Fix typo found by Fabio --------- Co-authored-by: Alexandre Bigot <abigot@sbgat402.in2p3.fr>
Hello @fcatalan92 @mfaggin !
Following our discussion via email, here is a proposition of macro for efficiency computation.