[RooFit] Guard against zero bin errors in RooPlot::chiSquareHandle zero bin error in chiSquare calculation (in: Roofit)#21710
Closed
AyachiMishra wants to merge 1 commit intoroot-project:masterfrom
Closed
Conversation
Added warning for zero bin error and deprecated method usage.
1 task
Contributor
|
Thanks! However, as we discussed in the issue, a warning is not appropriate. Skipping empty bins in a chi-square test is statistically wrong: the chi-square is not defined in that case. So erroring hard by returning The better solution would be to provide an alternative goodness-of-fit tests that is also defined when bins are empty, based on the Poisson likelihood. Not sure why you opened this PR even when this was already discussed in the issue, maybe you used GitHub copilot 🙂 |
Author
|
I appreciate the reality check! I got too eager to patch it up instead of weighing the statistical implications and meaning that was discussed later in that issue. I will keep an eye on the developments in this regard though. Thanks for the guidance! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(closes #21697 )
RooPlot::chiSquare can have zero bin errors (e.g. when using Expected data errors), leading to division by zero in the pull calculation and resulting in NaN values.
I Added a warning for zero bin error and deprecated method usage as discussed in the issue: #21697 .
Until RooPlot::chiSquare gets deprecated in favor of RooAbsReal::createChi2 (as mentioned by @guitargeek ), this guard will be helpful for users who come across this error..