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

[RF] New RooFit batch mode with GPU support #9004

Merged
merged 53 commits into from
Dec 11, 2021

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Sep 15, 2021

More detailed description will follow, for now let's see what the CI is doing.

Corresponding roottest PR: root-project/roottest#778,

@guitargeek guitargeek force-pushed the roofit_batchmode_dev branch 3 times, most recently from 1bb15dc to e002db0 Compare October 20, 2021 17:30
@guitargeek guitargeek force-pushed the roofit_batchmode_dev branch 3 times, most recently from 8339ed0 to eb20234 Compare October 24, 2021 22:19
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
@root-project root-project deleted a comment from phsft-bot Oct 24, 2021
The new RooFitDriver now also prints some info which is fine, so we
can't just expect the mesagge log to be empty if all RooAbsArgs are
implemented in the batch mode.
* implment `RooAbsCachedPdf::computeBatch`

* consistently recompute cached normalization integral in RooAbsPdf if
  parameters change

* unrelated: compute only value servers in `RooFitDriver`
There is also a new helper class `RooFitDriver::Dataset` to manage the
necessary information from the dataset that is used for the fit.
This is to support RooFit pdfs that use the category index for value
computations, such as `RooEfficiency`.
It was not such a good idea to write to standard output every time a
computation library is loaded. This is quite verbose and some unit tests
fail because of the unexpected output.

This commit makes the RooFitDriver write about the loaded computation
libraries in its constructor, but only if the `FastEvaluations` message
topic is switched on at the Info level (not the case by default).
Some tests still fail with this implementation, we will rely on the
fallback mode until a solution is found.
This makes it easier to also handle different buffer sizes later on, as
required by the RooSimulaneous fits.
With the `fast` flag enabled in `RooDataHist::calcTreeIndex`, the caller
promises that layout of `coords` is identical to our internal `vars`.
Previously, this was verified with an assert in debug mode like this:

   assert(!fast || coords.hasSameLayout(_vars));

However, there are usecases where the externally provided `coords` have
different names than the internal variables, even though they correspond
to each other. For example, if the observables in the computation graph
are renamed with `redirectServers`, which happens in a RooSimultaneous
fit with the new RooFit batch mode. Hence, we can't do a meaningful
assert here.
This excludes the pdfs that are implemented in the BatchComputation
library directy, for which we have to find a solution later.

The problem is that in CUDA evaluations, the division by the
normalization is done on the device, but we have not implemented
division with NaN packing in CUDA yet.

However, we support now NaN-packing for all pdfs that don't have a
`computeBatch` function implemented. In particular, this makes the
`testNaNPacker` pass (it involves a RooGenericPdf).
The `DISABLED_IntegrateBins_SubRange` test covers the minimization of a
`RooBinSamplingPdf` in a sub-range.

This test does not work with the new RooFit batch mode yet, and there
was no time to fix this issue before the ROOT v6.26 branch point.
@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

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.

None yet

5 participants