[RF] Document rho and Mirror options of RooKeysPdf - #22994
[RF] Document rho and Mirror options of RooKeysPdf#22994ArneshBanerjee wants to merge 3 commits into
Conversation
Add Doxygen documentation for the rho bandwidth scale factor and the Mirror boundary-correction options, and document each constructor parameter. Also point to RooNDKeysPdf for the multi-dimensional case. Closes root-project#8507.
ferdymercury
left a comment
ferdymercury
left a comment
There was a problem hiding this comment.
Thanks for this! Some comments from my side
|
|
||
| Cranmer KS, Kernel Estimation in High-Energy Physics. | ||
| Computer Physics Communications 136:198-207,2001 - e-Print Archive: hep ex/0011057 | ||
|
|
There was a problem hiding this comment.
There was some discussion wrt the sqrt(12) in the code, the original author said:
My memory of this is that in some cases instead of processing a set of unbinned data the input was a finely binned histogram. In this case, data that might be uniformly spread in a histogram bin would then be aggregated into a single sample with no variance. I believe that is why the kernel has Alpha/(2.0*Sqrt(3.0)) corresponding to the std. deviation of a uniform / rectangular distribution corresponding to the histogram bin.
maybe worth adding a note on this regard ?
| @@ -28,6 +28,24 @@ | |||
|
|
|||
| Cranmer KS, Kernel Estimation in High-Energy Physics. | |||
| Computer Physics Communications 136:198-207,2001 - e-Print Archive: hep ex/0011057 | |||
There was a problem hiding this comment.
maybe add hyperlink: https://doi.org/10.1016/S0010-4655(00)00243-5
|
|
||
| /** \class RooKeysPdf | ||
| \ingroup Roofit | ||
|
|
There was a problem hiding this comment.
Maybe mention that this was inspired by https://web.archive.org/web/20020705034344/https://www-wisconsin.cern.ch/~cranmer/keys.html
| * GR, Gerhard Raven, UC San Diego, raven@slac.stanford.edu * | ||
| * DK, David Kirkby, UC Irvine, dkirkby@uci.edu * WV, Wouter Verkerke, UC Santa Barbara, verkerke@slac.stanford.edu * | ||
| * * |
There was a problem hiding this comment.
| * GR, Gerhard Raven, UC San Diego, raven@slac.stanford.edu * | |
| * DK, David Kirkby, UC Irvine, dkirkby@uci.edu * | |
| * WV, Wouter Verkerke, UC Santa Barbara, verkerke@slac.stanford.edu * | |
| * * |
| * * | ||
| * Redistribution and use in source and binary forms, * | ||
| * with or without modification, are permitted according to the terms * | ||
| * listed in LICENSE (http://roofit.sourceforge.net/license.txt) * |
There was a problem hiding this comment.
this line seems outdated?
| Class RooKeysPdf implements a one-dimensional kernel estimation p.d.f which model the distribution | ||
| of an arbitrary input dataset as a superposition of Gaussian kernels, one for each data point, | ||
| each contributing 1/N to the total integral of the pdf. | ||
| If the 'adaptive mode' is enabled, the width of the Gaussian is adaptively calculated from the |
There was a problem hiding this comment.
| \note KEYS stands for Kernel Estimating Your Shapes, see https://web.archive.org/web/20010604031632/http://www-wisconsin.cern.ch/~cranmer/KEYS.pdf | |
| If the 'adaptive mode' is enabled, the width of the Gaussian is adaptively calculated from the |
Add references to Cranmer's KEYS package and the paper DOI, note the uniform-distribution origin of the 2*sqrt(3) factor in the kernel width, and fix the author line formatting in the header.
|
Thanks a lot for the suggestions @ferdymercury, they have been done in the latest commit:
On the |
| double h=std::pow(double(4)/double(3),0.2)*std::pow(_sumWgt,-0.2)*_rho; | ||
| double hmin=h*sigmav*std::sqrt(2.)/10; | ||
| // The 2*sqrt(3) factor comes from the standard deviation of a uniform | ||
| // distribution, sqrt(12)/2. This accounts for the case where the input is a |
There was a problem hiding this comment.
| // distribution, sqrt(12)/2. This accounts for the case where the input is a | |
| // distribution, 1/sqrt(12). This accounts for the case where the input is a |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
done |
3 participants
This adds documentation for RooKeysPdf, addressing #8507.
The
rhoandMirrorconstructor parameters were not documented, andthe Mirror enum values had cryptic names with no explanation. This PR:
rhoas the overall scale factor for the kernel width.correct it, with a one-line description for each Mirror enum value.
The descriptions are based on the existing implementation: how the
mirror flags are set in the constructor and how the reflected events
are added (symmetric) or subtracted (asymmetric) in LoadDataSet. This
is a documentation-only change, so no test is added.