-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[RF] Allow RooUniform to set lower and upper bounds (support) #7880
Comments
Implement analytical integration capability for RooStepFunction, because this class is the easiest way to implement a bounded uniform distribution without adding serialized class members to the RooUniform (see root-project#7880). Add a unit test that verifies that wrapping a `RooStepFunction` in a `RooWrapperPdf` doesn't result in any numerical integrals.
Hi @ynikitenko, sorry for coming back to this issue so late! But I think I have now a compromise solution. The problem is that adding new parameters to the RooUniform would mean adding new serializable class members, which might have unpleasant implications, for example of RooWorkspace size and would also need to be discussed with the HS3 serialization guys, which also have specified a uniform distribution.
Well, that's your opinion and it might be the right claim. But one might also argue that the parameters are already encoded in the observable range. And if you introduce additional parameters within the full range, the pdf is not uniform within the full observable range anymore, which can be confusing given the name "RooUniform".
Why not? There is an alternative. You can wrap a step function ("RooStepFunction") with only one interval in a RooWrapper pdf, and you have a uniform pdf as you describe: RooRealVar x{"x", "x", 5.0, 0.0, 10.0};
RooRealVar a{"a", "a", 3.0, 0.0, 10.0};
RooRealVar b{"b", "b", 7.0, 0.0, 10.0};
RooStepFunction stepFunc{"step", "", x, RooArgList{1.0}, {a, b}};
RooWrapperPdf stepPdf{"pdf", "", stepFunc}; The non-boundary related parameter can be an arbitrary constant since it is normalized out. The only problem with this approach is that right now RooFit doesn't know how to analytically normalize this pdf, but I have opened a PR to make this work: Is this an acceptable compromise to you towards closing the issue? |
Is your feature request related to a problem? Please describe.
I wanted to fit my data with a uniform distribution (I know the simple analytical answer), but it turned out that it's not possible to set bounds for it. RooUniform creates a uniform distribution for the whole range of its variable (or many variables in the case of many dimensions).
On the forum @lmoneta writes in 2015: "You cannot really assign weights to a RooUniformPdf." (but probably this has already changed, because I see that it is a subclass of RooAbsPdf, like RooGaussian).
Describe the solution you'd like
Change the constructor to
One- and multidimensional uniform distributions should become separate (like RooGaussian and RooMultiVarGaussian (note also a spurious line break in its docs)). Probably there is even no need for the multidimensional uniform distribution, since it's just a product of one-dimensional pdfs (if you don't model complicated support shapes).
The uniform (rectangular) distribution is just one of pdfs (see Wikipedia, Continuous uniform distribution). It is a normal pdf, and its parameters are its lower and upper bounds. They should be set like other pdf parameters in RooFit.
Describe alternatives you've considered
I think that there are no alternatives. No reason to model this with step functions or histograms. The uniform distribution is a simple and pretty basic probability distribution.
Additional context
I asked about this on the forum, but now as I better understand RooFit and as I see no answers for RooUniform, I think this is the only solution.
P.S. There is a spurious line break in RooUniform class description.
The text was updated successfully, but these errors were encountered: