-
- Downloads
[RF] Remove entry masking feature from RooDataHist
This commit removes some functions from the RooFit data classes: * `RooAbsData::valid()` (virtual method that was overridden in RooDataHist but not RooDataSet) * `RooDataHist::valid(std::size_t i)` and `RooDataHist::valid()` * `RooDataHist::cacheValidEntries()` The `cacheValidEntries` method was originally intended to be used in `RooAbsOptTestStatistic` to mask histogram entries out of the variable range in case of a subrange fit. The reasons why `cacheValidEntries` and the related `valid()` methods should be removed are: 1. It is redundant. In a subrange fit, the `RooAbsOptTestStatistic` is creating a clone of the dataset with the subrange only using `RooAbsData::reduce()`. So all entries are valid by definition. 2. RooDataHist and RooDataSet have inconistent implementations. For the RooDataHist, `valid()` tells you "if bin `i` is considered valid within the current range definitions of all observables" (according to the documentation). For the RooDataSet, it always returns `true`. This inconsistency leaves plenty of room for error, and means that a subrange selection relying on `RooAbsData::vaild()` would be broken anyway because it wouldn't work with RooDataSet. 3. The masking of out-of-range entries unnessecarily increases the `mutable` state of the RooDataHist, which can cause trouble if one updates the observable range but then forgets to call `cacheValidEntries()`. 4. Even the documentation said that `RooDataHist::cacheValidEntries()` was a "shortcut function only for RooAbsOptTestStatistic". Why keep it if `RooAbsOptTestStatistic` doesn't even use it in a meaningful way anymore?
Showing
- README/ReleaseNotes/v628/index.md 6 additions, 1 deletionREADME/ReleaseNotes/v628/index.md
- roofit/roofitcore/inc/RooAbsData.h 0 additions, 1 deletionroofit/roofitcore/inc/RooAbsData.h
- roofit/roofitcore/inc/RooDataHist.h 0 additions, 15 deletionsroofit/roofitcore/inc/RooDataHist.h
- roofit/roofitcore/src/RooAbsOptTestStatistic.cxx 0 additions, 7 deletionsroofit/roofitcore/src/RooAbsOptTestStatistic.cxx
- roofit/roofitcore/src/RooChi2Var.cxx 0 additions, 2 deletionsroofit/roofitcore/src/RooChi2Var.cxx
- roofit/roofitcore/src/RooDataHist.cxx 3 additions, 43 deletionsroofit/roofitcore/src/RooDataHist.cxx
- roofit/roofitcore/src/RooNLLVar.cxx 0 additions, 5 deletionsroofit/roofitcore/src/RooNLLVar.cxx
- roofit/roofitcore/src/RooXYChi2Var.cxx 0 additions, 8 deletionsroofit/roofitcore/src/RooXYChi2Var.cxx
- roofit/roofitcore/src/TestStatistics/RooAbsL.cxx 12 additions, 6 deletionsroofit/roofitcore/src/TestStatistics/RooAbsL.cxx
- roofit/roofitcore/src/TestStatistics/RooBinnedL.cxx 0 additions, 3 deletionsroofit/roofitcore/src/TestStatistics/RooBinnedL.cxx
- roofit/roofitcore/test/testRooDataHist.cxx 0 additions, 56 deletionsroofit/roofitcore/test/testRooDataHist.cxx
Loading
Please register or sign in to comment