[v628][RF] Backports of RooFit PRs to v6-28-00-patches: Part 3
#11993
Conversation
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
There is a problem with the usage of `abs` and not `std::abs`, because `abs` has no overload for doubles. By passing a double to `abs` without `std::`, it gets rounded to an `int`, which is a bug.
As noted in d008860, using `abs()` without `std::` can be very dangerous because there might be no overload for floating point numbers. To make sure that no bugs associated to this can happen, I suggest to avoid using `abs()` without `std::` all the way in RooFit. Also, to go with modern C++ all the way, the occurences of `fabs()` and `std::fabs()` are also replaced with `std::abs()`.
This is a followup to commit 8679898. In that commit, the RooPlot was changed to not create an internal clone of the plot variables anymore. That change was very effective in speeding up RooFit, because it resulted in huge speedups when plotting complicated pdfs that are expensive to clone. However, in rare cases like for the RooUnitTest, it is necessary that the RooPlot owns the plot plot variables to avoid dangling pointers when the RooPlot lives longer than the original plot variables. To support these cases, a new member function `RooPlot::createInternalPlotVarClone()` is introduced. It can be used to convert the RooPlot to a plot that owns a clone of the plot variable at any point in time of the RooPlot lifetime.
In the `rf205` tutorial and the associated stressRooFit test, the `bkg` RooAddPdf was created using the `sig1frac`, while it is clearly the idea to use the `bkg1frac` parameter. This is fixed in this commit, also fixing the name of the `bkg1frac` parameter, which was accidentally set to `sig1frac` (probably a copy-paste error). However, to not change the output of the tutorial and the stressRooFit test, the value of `bkg1frac` was set to the same value as `sig1frac`, such that this commit only fixes the model but doesn't change the tutorial and test output.
As the file will be anyway touched and significantly changed in the nexts commit.
Since ROOT 6.28, we have a `RooPower` class to represent power laws. The unit tests in `testTestStatistics` used power laws implemented as RooGenericPdf everywhere, and we can now use the `RooPower` class to avoid numeric integrals, which also helps to reduce the logging output.
This is done to avoid some unnecessary info printout.
I recently learned about this trick to make the formulation of large test matrices more concise.
Add optional `doOffset` option to `RooAbsPdf::extendedTerm()`.
The offsetting adds a counterterm where the expected number of events
equals the observed number of events. This constant shift results in a
term closer to zero that is approximately chi-square distributed. It is
useful to do this also when summing multiple NLL terms to avoid numeric
precision loss that happens if you sum multiple terms of different
orders of magnitude.
It is one of the necessary ingredients to completely implement the new
`Offset("bin")` option.
This commit does the necessary changes to implement the `Offset("bin")`
feature for fits to RooDataHists with both the BatchMode and the old
RooFit.
Some variables and enums are also renamed to be more clear or concise
(for example, the term `Off` to denote no offsetting can be misleading
and was replaced with `None`).
Test the new `Offset("bin")` feature for fits to RooDataHists over the
test matrix that is the tensor product of `BatchMode()`, doing and
extended fit, and SumW2 correction. The test should compute the
likelihood for a template PDF created from the data, and it should be
numerically compatible with zero.
guitargeek
requested review from
Axel-Naumann,
couet,
hageboeck and
lmoneta
as code owners
|
Starting build on |
52 tasks
|
Build failed on ROOT-ubuntu2004/python3. Errors:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This is a backport of all the relevant RooFit PRs that were recently merged to master to v6-28-00-patches (in the right order, to not have the commit history diverge too much).
doubletointin xroofit #11984std::absall the way in RooFit #11986RooPlot::createInternalPlotVarClone()function for RooUnitTest #11841rf205tutorial and related stressRooFit test #11985Offset("bin")feature for fits to RooDataHist #11988Related to #11856.