Skip to content
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

[v626][RF] Backports of RooFit PRs to v6-26-00-patches: Part 8 #10114

Conversation

lmoneta and others added 13 commits March 14, 2022 14:58
If NDEBUG is defined, the variables are unused and Clang warns about
them.
 - fix an error message (text was misleading)
 - correclty initialize a data member
 - allow initialHesse to be used with Minimizer option in fitTo
In the CPU mode, the order in which the RooFit objects are evaluated is
not dynamic and there is no need to use the code path for heterogeneous
evaluation. This speeds up the pure CPU mode significantly.
The `RooAbsReal::getValues` has already been established as the entry
point for evaluating RooFit objects with the batch mode and it should
not be broken.

In 6.26, the `getValues` function was broken to fall back on the scalar
mode all the time, because the `evaluateSpan` funtions it used got
replaced by `computeBatch`. In this commit, the desired behavior of
using the BatchMode is restored by using the RooFitDriver. To that end, a
new constructor has been added to the RooFitDriver that takes a
`RooBatchCompute::RunContext` directly.

The override of `getValues` in RooAbsPdf was also removed now, because
it's the job of the RooFitDriver to treat pdfs correctly.
The RooPlot constructor from TH1 had an option to disable the bin width
correction, but this argument was not even used when the RooHist was
created with Poisson errors, which is the default case. The Bin width
correction was always done in this case, no matter what was the value of
`correctForBinWith`.

However, when SumW2 errors are used, the `correctForBinWith` parameter
was actually considered! This caused inconsitent behavior, because when
plotting a `RooDataHist` the `correctForBinWith` parameter is hardcoded
to `false, meaning the bin width correction is done for Poisson errors
but not for SumW2 errors.

This commit fixes this behavior by following the precedent set by the
more common Poisson error: the `correctForBinWith` parameter is
consistently ignored, and an exception is thrown when it's false.

Consequently, the `correctForBinWith` parameter is also not hardcoded to
`false` in `RooDataHist::plotOn` anymore.

The correct way to disable the bin width correction is to enable the
interpretation as a density when importing a TH1 to a RooDataHist.

This commit fixes a bug reported on the forum.
Missing when compiled with -Ddev=ON option
The dependencies are changed to linker libraries only, so they are no
dependencies of the dictionary generation anymore. This should also fix
compile errors reported on the forum.
* the `JSONTool::Export` function should not be part of the public
  interface, which should only provide high-level JSON printing and
  writing functionality

* the signature of the constructor was changed to take a reference
  instead of a pointer, because it can't be `nullptr`
In case of named arguments, the RooFormula will replace the argument
names with` x[0]` to `x[n]`. There are two things that can go wrong if
RooFormula is not implemented right. First, if there is a variable named
"x" it should only be substituted if the matching substring is not
followed by "[", to not replace existing x[i]. Second, variables with
integer names like "0" should only be substituted if the match is not
followed by a "]", again to avoid replacing x[i]. This test checks that
these cases are handled correctly.

The second case was so far not dealt with correctly, but with this
commit it is. A corresponding unit test was also implemented.

The preprocessor commands in `RooFormula` were also reorganized
slightly, such that one can test the `TPRegexp` backend simply by
commenting out the `define ROOFORMULA_HAVE_STD_REGEX`.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

@guitargeek guitargeek merged commit 7424d4c into root-project:v6-26-00-patches Mar 14, 2022
@guitargeek guitargeek deleted the v6-26-00-patches_roofit_backports_1 branch March 14, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants