[RF] Make RooAbsReal::getValues use the batch mode via RooFitDriver #9986
Conversation
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.
guitargeek
force-pushed
the
getValues_1
branch
from
ab474f7 to
3c3f9c2
Compare
guitargeek
force-pushed
the
getValues_1
branch
from
5476f11 to
b6c332f
Compare
guitargeek
force-pushed
the
getValues_1
branch
from
b6c332f to
7e2263d
Compare
|
Starting build on |
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
lmoneta
left a comment
lmoneta
left a comment
There was a problem hiding this comment.
LGTM ! Just a small comment.
Thank you for this fix
| RooSpan<double> RooRealSumPdf::evaluateSpan(RooBatchCompute::RunContext& evalData, const RooArgSet* /*normSet*/) const { | ||
| // Do running sum of coef/func pairs, calculate lastCoef. | ||
| RooSpan<double> values; | ||
| for (unsigned int j = 0; j < nEvents; ++j) { |
There was a problem hiding this comment.
It would be better to use size_t instead of unsigned int here, and same also below, for the case nEvents>2^32.
There was a problem hiding this comment.
Ok! I'll change this
|
Starting build on |
3 participants
The
RooAbsReal::getValueshas already been established as the entrypoint for evaluating RooFit objects with the batch mode and it should
not be broken.
In 6.26, the
getValuesfunction was broken to fall back on the scalarmode all the time, because the
evaluateSpanfuntions it used gotreplaced by
computeBatch. In this commit, the desired behavior ofusing the BatchMode is restored by using the RooFitDriver. To that end, a
new constructor has been added to the RooFitDriver that takes a
RooBatchCompute::RunContextdirectly.The override of
getValuesin RooAbsPdf was also removed now, becauseit's the job of the RooFitDriver to treat pdfs correctly.
This PR fixes the performance regression that was observed in the vectorized pdf tests in
roottest. To fix the performance regression completely, this PR also includes a commit to avoid some overhead in the pure CPU batch mode with RooFitDriver.This bugfix should also be backported to 6.26 as a bugfix for the patch release.