[RF] change default LikelihoodJob task splitting mode

Set the default LikelihoodJob tasks to be component-wise splits rather than event-wise splits
root-project · Sep 23, 2024 · d581c61 · d581c61
1 parent 9b5309f
commit d581c61
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/roofit/roofitcore/src/TestStatistics/LikelihoodJob.cxx b/roofit/roofitcore/src/TestStatistics/LikelihoodJob.cxx
@@ -112,24 +112,26 @@ void LikelihoodJob::update_state()
    }
 }
 
-/// \warning In automatic mode, this function can start MultiProcess (forks, starts workers, etc)!
 std::size_t LikelihoodJob::getNEventTasks()
 {
    std::size_t val = n_event_tasks_;
    if (val == MultiProcess::Config::LikelihoodJob::automaticNEventTasks) {
-      val = get_manager()->process_manager().N_workers();
+      val = 1;
    }
    if (val > likelihood_->getNEvents()) {
       val = likelihood_->getNEvents();
    }
    return val;
 }
 
+/// \warning In automatic mode, this function can start MultiProcess (forks, starts workers, etc)!
 std::size_t LikelihoodJob::getNComponentTasks()
 {
    std::size_t val = n_component_tasks_;
    if (val == MultiProcess::Config::LikelihoodJob::automaticNComponentTasks) {
-      val = 1;
+      val = get_manager()
+               ->process_manager()
+               .N_workers(); // get_manager() is the call that can start MultiProcess, mentioned above
    }
    if (val > likelihood_->getNComponents()) {
       val = likelihood_->getNComponents();

diff --git a/roofit/roofitcore/test/TestStatistics/testLikelihoodJob.cxx b/roofit/roofitcore/test/TestStatistics/testLikelihoodJob.cxx
@@ -129,6 +129,10 @@ TEST_F(LikelihoodJobTest, UnbinnedGaussian1DSelectedParameterValues)
    // Bisecting the number of events to find the number at which the deviation starts occurring in the
    // likelihood result showed that event 9860 was the main culprit (which has index 9859)
 
+   // We also need to split over events precisely with 2 workers to trigger the deviation:
+   RooFit::MultiProcess::Config::LikelihoodJob::defaultNEventTasks = RooFit::MultiProcess::Config::getDefaultNWorkers();
+   RooFit::MultiProcess::Config::LikelihoodJob::defaultNComponentTasks = 1;
+
    RooRealVar *mu = w.var("mu");
    mu->setVal(-2.8991551193432676392);
 
@@ -146,6 +150,12 @@ TEST_F(LikelihoodJobTest, UnbinnedGaussian1DSelectedParameterValues)
    EXPECT_NE(nll0, nll1.Sum());
    // they differ only a bit:
    EXPECT_DOUBLE_EQ(nll0, nll1.Sum());
+
+   // reset static variables to automatic
+   RooFit::MultiProcess::Config::LikelihoodJob::defaultNEventTasks =
+      RooFit::MultiProcess::Config::LikelihoodJob::automaticNEventTasks;
+   RooFit::MultiProcess::Config::LikelihoodJob::defaultNComponentTasks =
+      RooFit::MultiProcess::Config::LikelihoodJob::automaticNComponentTasks;
 }
 
 TEST_F(LikelihoodJobTest, UnbinnedGaussian1DTwice)