Repository navigation
[RF] Multi-threaded batch evaluation in the CPU backend via NumCPU() - #23140
Merged
Merged
Conversation
Test Results 23 files 23 suites 3d 20h 29m 35s ⏱️ For more details on these failures, see this check. Results for commit 5d48957. ♻️ This comment has been updated with latest results. |
guitargeek
force-pushed
the
issue-17344
branch
2 times, most recently
from
August 31, 2026 11:49
d1b4e7c to
c82e441
Compare
guitargeek
force-pushed
the
issue-17344
branch
2 times, most recently
from
September 7, 2026 19:33
1c84f5a to
cda4cd5
Compare
Implement event-level multithreading in the RooBatchCompute CPU library and wire it to the existing NumCPU() command argument, which was so far silently ignored by the new CPU evaluation backend (see issue root-project#17344). With NumCPU(n), n > 1, the compute functions and both reductions (reduceSum and reduceNLL) process large batches in parallel tasks, implemented with ROOT::TThreadExecutor. The design keeps the parallel region strictly inside the RooBatchCompute library, so no RooAbsArg state is ever touched concurrently: * Events are split into fixed-size chunks of 16384 events. The chunk boundaries do not depend on the number of threads, and the partial Kahan sums of the reductions are combined in fixed chunk order, so the NLL value is bitwise identical for any NumCPU(n) with n > 1, independent of the requested thread count. The single-threaded code paths are unchanged, so results without NumCPU() are bitwise identical to before. Because the sequential reduction sums in a different order than the chunked parallel one, NumCPU(1) and NumCPU(n > 1) can differ by 1 ULP in the NLL value (observed in about 2 % of sampled parameter points on a 1M-event fit), which can shift the fitted parameters at the level of numerical noise (~1e-13, far below the Minuit tolerance). * Batches with fewer than 32768 events are always evaluated single-threaded. Small fits therefore never pay any scheduling overhead, addressing the main historic concern with parallelizing likelihood evaluations. * Some compute functions use the extra arguments as scratch space or output parameters (e.g. evaluation error counts), so each parallel task works on a private copy and the deltas are merged afterwards. * The parallel tasks run in cached ROOT::TThreadExecutor instances, which all share ROOT's global thread pool. Concurrently running evaluations, or parallel fits on top of user-level parallelism, therefore don't oversubscribe the machine. The first executor created in the process determines the pool size; if the pool was already initialized with a higher concurrency (e.g. by implicit multi-threading), the requested number of threads is still honored by splitting the work into at most as many parallel tasks (with a warning from RTaskArenaWrapper that the pool size is kept). This task-split path has somewhat higher overhead than a pool of exactly the requested size: on 1M-event benchmark fits it was measured 15-30 % slower for small NumCPU values on an oversized pool. The executor cache is deliberately leaked, because this library is loaded with dlopen() and destroying the thread pool after the TBB runtime tore down its scheduler can crash at exit. The number of threads is routed from the NumCPU() argument of both createNLL() and createChi2() via a new RooBatchCompute::Config::setNThreads() through RooFit::Evaluator and RooEvaluatorWrapper. The interleaving strategy argument of NumCPU() has no meaning for event-level splitting and is ignored. The CUDA backend and EvalBackend("codegen_no_grad") warn that the option does not apply (the latter evaluates single-threaded generated code), and with EvalBackend("codegen") only the test statistic values are affected, not the generated gradient code. This also removes the dead code behind ROOBATCHCOMPUTE_USE_IMT, which was an earlier attempt at the same feature. It only parallelized the compute functions but not the reductions (reduceNLL is the single hottest function in unbinned fits), which capped its possible speedup. On the reproducer of issue root-project#17344 (two unbinned channels with 1.05 million events in total and 14 free parameters, 12-core machine), MIGRAD + HESSE takes: ```txt cpu NumCPU(1): 17.2 s (previous behavior for any NumCPU) cpu NumCPU(4): 5.4 s cpu NumCPU(12): 3.6 s legacy NumCPU(12): 15.1 s (multi-process RooRealMPFE) ``` All configurations converge to the same minimum, with bitwise identical fit results among the parallel configurations. The new testRooBatchCompute unit test covers bitwise equality of parallel kernel outputs, the extra-argument merging, and the thread-count independence of the parallel reductions. Closes root-project#17344 🤖 Done with the help of AI
guitargeek
force-pushed
the
issue-17344
branch
from
September 7, 2026 21:09
cda4cd5 to
5d48957
Compare
dpiparo
approved these changes
Sep 8, 2026
dpiparo
left a comment
Member
There was a problem hiding this comment.
Thanks for this PR. Before merging maybe could you check that the suppression file for Valgrind still works?
Contributor
Author
|
Yes it does! If I run some RooFit macro that uses |
Closed
1 task
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Implement event-level multithreading in the RooBatchCompute CPU library and wire it to the existing NumCPU() command argument, which was so far silently ignored by the new CPU evaluation backend (see issue #17344).
With NumCPU(n), n > 1, the compute functions and both reductions (reduceSum and reduceNLL) process large batches in parallel tasks, implemented with ROOT::TThreadExecutor. The design keeps the parallel region strictly inside the RooBatchCompute library, so no RooAbsArg state is ever touched concurrently:
Events are split into fixed-size chunks of 16384 events. The chunk boundaries do not depend on the number of threads, and the partial Kahan sums of the reductions are combined in fixed chunk order, so the NLL value is bitwise identical for any NumCPU(n) with n > 1, independent of the requested thread count. The single-threaded code paths are unchanged, so results without NumCPU() are bitwise identical to before. Because the sequential reduction sums in a different order than the chunked parallel one, NumCPU(1) and NumCPU(n > 1) can differ by 1 ULP in the NLL value (observed in about 2 % of sampled parameter points on a 1M-event fit), which can shift the fitted parameters at the level of numerical noise (~1e-13, far below the Minuit tolerance).
Batches with fewer than 32768 events are always evaluated single-threaded. Small fits therefore never pay any scheduling overhead, addressing the main historic concern with parallelizing likelihood evaluations.
Some compute functions use the extra arguments as scratch space or output parameters (e.g. evaluation error counts), so each parallel task works on a private copy and the deltas are merged afterwards.
The parallel tasks run in cached ROOT::TThreadExecutor instances, which all share ROOT's global thread pool. Concurrently running evaluations, or parallel fits on top of user-level parallelism, therefore don't oversubscribe the machine. The first executor created in the process determines the pool size; if the pool was already initialized with a higher concurrency (e.g. by implicit multi-threading), the requested number of threads is still honored by splitting the work into at most as many parallel tasks (with a warning from RTaskArenaWrapper that the pool size is kept). This task-split path has somewhat higher overhead than a pool of exactly the requested size: on 1M-event benchmark fits it was measured 15-30 % slower for small NumCPU values on an oversized pool. The executor cache is deliberately leaked, because this library is loaded with dlopen() and destroying the thread pool after the TBB runtime tore down its scheduler can crash at exit.
The number of threads is routed from the NumCPU() argument of both createNLL() and createChi2() via a new RooBatchCompute::Config::setNThreads() through RooFit::Evaluator and RooEvaluatorWrapper. The interleaving strategy argument of NumCPU() has no meaning for event-level splitting and is ignored. The CUDA backend and EvalBackend("codegen_no_grad") warn that the option does not apply (the latter evaluates single-threaded generated code), and with EvalBackend("codegen") only the test statistic values are affected, not the generated gradient code.
This also removes the dead code behind ROOBATCHCOMPUTE_USE_IMT, which was an earlier attempt at the same feature. It only parallelized the compute functions but not the reductions (reduceNLL is the single hottest function in unbinned fits), which capped its possible speedup.
On the reproducer of issue #17344 (two unbinned channels with 1.05 million events in total and 14 free parameters, 12-core machine), MIGRAD + HESSE takes:
All configurations converge to the same minimum, with bitwise identical fit results among the parallel configurations. The new testRooBatchCompute unit test covers bitwise equality of parallel kernel outputs, the extra-argument merging, and the thread-count independence of the parallel reductions.
Closes #17344
🤖 Done with the help of AI