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.
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
Alternative variable binning approach #849
Alternative variable binning approach #849
Changes from 12 commits
fcae83c
8c20429
35dcf0e
81c308b
eaf6235
3215d55
87f21fb
854e356
eb78c69
0ee0429
ba8c4a7
e9b0b5b
9f078c6
92409cf
94a0529
7837426
f7a53ad
b0eabba
1a3dd76
5c2d85f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do we need to restrict user to use different MultidimBinning for each selection? I feel like it would be more convenient to allow using duplicate binning. For example one can think of a situation where we have three PID bins, but only middle bin has different binning. If one can not use duplicate binning, they will have to define a selection for lower+higher PID bins and then a 3d binning in (E, coszen, PID).
This of course would not affect the calculation, but, in my opinion, it could just be more convenient to be able to assign same binning to different selections.
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 only enforce this restriction for arbitrary selections defined in a list of strings. You can use the OneDimBinning selection option for the case of three PID bins where the middle bin should get a different binning in E and coszen. The reason for not allowing it for the arbitrary selections is that this would split your statistic (MC and real event counts) and increase your uncertainty in each bin. In general, if the variable used for the split is also part of the binning, the OneDimBinning option should be used.
Or do you want to have other cuts additional to the PID split that are different for each PID 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.
Yes, this was just an example. I agree that having duplicate binning will split your statistic, but if the selection variable carries some information useful for oscillation fit, it could also be balanced out by its contribution to sensitivity. Also like you suggested, it would be useful in the case when there multiple variables used in selection and PID (with different binning for middle bin) is just one of them
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.
Hmm, how about making it a warning instead? I think we should also warn that the selection variables should not be part of the MultiDimBinning(s), because the selection itself already effectively acts as binning here
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.
Yeah, I think warning would be better. And I think it's a good idea to have a warning about not mixing selection and binding variables.
I also just thought that the same statistic split can be achieved if a variable which has no impact on the fit is used in regular multidim binning. I don't think there is a way to have a warning there though, so it is up to the analyzer to make sure binning variables carry some information
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.
yeah, in the end, we have to rely on the analyzers to know what they are doing. 😅