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.

Sign up for GitHub

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

Jump to bottom

[RF] Make RooBatchCompute thread safe #14877

Merged
merged 4 commits into from
Mar 5, 2024
Merged

[RF] Make RooBatchCompute thread safe #14877

merged 4 commits into from
Mar 5, 2024

Commits on Mar 4, 2024

  1. [RF] Avoid std::vector allocations when calling RooBatchCompute 

    This can be quite a big overhead if the dataset is small.
    @guitargeek
    guitargeek committed Mar 4, 2024
    Configuration menu
    Copy the full SHA
    d952790 View commit details
    Browse the repository at this point in the history

  2. [RF] Avoid using static std::vector<double> buffer in RooBatchCompute 

    In the RooBatchCompute CPU library, all scalar inputs have to be copied
n times into a buffer that is as long as the SIMD registers, to allow
for vectorization in all cases.

To avoid frequent memory allocations, this buffer was made a `static`
variable in the original implementation of the batchcompute library,
which of course made it non-threadsafe.

This is now hitting us, because RooFit needs to be able to do multiple
fits concurrently. This is a requirement for CMSSW, and a blocker for
ROOT master adoption in CMSSW since the new CPU backend is the default:
cms-sw/cmsdist#9034

This commit fixes the concurrency problem by doing the buffering in the
DataMaps that are used in the `RooFit::Evaluator`. Like this, multiple
computation graphs can be concurrently evaluated.

It was tested with the ATLAS benchmarks in `rootbench` that the fitting
performance remains the same.
    @guitargeek
    guitargeek committed Mar 4, 2024
    Configuration menu
    Copy the full SHA
    2ba8f99 View commit details
    Browse the repository at this point in the history

  3. [RF] Refactor RooBatchCompute to simplify Batches classes 

    The Batches classes in RooBatchCompute have only public data members
anyway. It's not necessary to have extra member functions for changing
them.
    @guitargeek
    guitargeek committed Mar 4, 2024
    Configuration menu
    Copy the full SHA
    14b5570 View commit details
    Browse the repository at this point in the history

  4. [RF] Mention in the docs that the new CPU eval backend is the default

    @guitargeek
    guitargeek committed Mar 4, 2024
    Configuration menu
    Copy the full SHA
    2abf7e5 View commit details
    Browse the repository at this point in the history