[RF] Consolidate the unbinned fit backend benchmarks

root/roofit/atlas-benchmarks/README.md

@@ -33,10 +33,8 @@ Notes:
 - The `time` builtin has to stay wrapped around `root` itself: its `user` line is
   the total bar height, and the measured steps are subtracted from it to get the
   "Other" slice.
-- Out of the box this gives two bars, `cpu` and `codegen`. The `legacy` backend
-  no longer exists in recent ROOT; on an older ROOT, add it to the loop in
-  `run_atlasHiggsBackendComparison.sh` to get a third bar. The plot script drops
-  any backend whose log is absent, so no other change is needed.
+- Out of the box this gives two bars, `cpu` and `codegen`. The plot script
+  drops any backend whose log is absent.
 - The plot script takes the log directory as an argument, so several runs can be
   compared: `python3 atlasHiggsBackendComparison_make_plot.py atlas some-other-run`
   writes one PDF per directory, named after it.

root/roofit/atlas-benchmarks/atlasHiggsBackendComparison.C

@@ -65,7 +65,7 @@ void atlasHiggsBackendComparison(const char *evalBackend = "cpu", bool doHessian
          *data, GlobalObservables(*globObs), Offset(true), Optimize(2), EvalBackend(evalBackend))};
 
    // Only the codegen NLL is a RooEvaluatorWrapper, so this cast is not valid
-   // for the legacy and cpu backends.
+   // for the cpu backend.
    if (isCodegen && doHessian) {
       static_cast<RooFit::Experimental::RooEvaluatorWrapper &>(*nll).generateHessian();
    }

root/roofit/atlas-benchmarks/atlasHiggsBackendComparison_make_plot.py

@@ -18,7 +18,7 @@
     nargs="*",
     default=["atlas"],
     metavar="DIR",
-    help="directory holding {legacy,cpu,codegen}.log (default: atlas)",
+    help="directory holding {cpu,codegen}.log (default: atlas)",
 )
 parser.add_argument(
     "--json",
@@ -122,13 +122,11 @@ def parse_output(filename):
 
 
 # Bars appear left to right in this order. A backend whose log is absent is
-# dropped rather than being an error, so a run without the legacy backend simply
-# yields a two-bar plot.
-backend_names = {"RooFit legacy": "legacy", "RooFit": "cpu", "RooFit AD": "codegen"}
-
-# The legacy backend no longer exists in recent ROOT, so out of the box this
-# produces a two-bar plot. To compare ROOT versions instead of backends, relabel:
-# backend_names = {"ROOT 6.32": "legacy", "ROOT 6.40": "cpu", "ROOT 6.40 AD": "codegen"}
+# dropped rather than being an error, so a run without one of the backends
+# simply yields fewer bars. To compare ROOT versions instead of backends,
+# relabel, e.g.:
+# backend_names = {"ROOT 6.40": "cpu", "ROOT 6.40 AD": "codegen"}
+backend_names = {"RooFit": "cpu", "RooFit AD": "codegen"}
 
 # One row per (run, backend, step) for --json: a tidy table stays easy to
 # concatenate across runs and to pivot downstream.

root/roofit/atlas-benchmarks/roofitAtlasHiggsBenchmark.cxx

@@ -39,7 +39,6 @@ struct BenchmarkData {
    RooAbsPdf *pdf = nullptr;
    std::unique_ptr<RooAbsPdf> ownedPdf;
    RooAbsData *data = nullptr;
-   std::unique_ptr<RooAbsReal> nll;
    std::unique_ptr<RooAbsReal> batchedNll;
    std::unique_ptr<RooAbsReal> codegenNll;
 };
@@ -52,9 +51,8 @@ BenchmarkData &bmdata()
 
 static void benchCreateNLL(benchmark::State &state)
 {
-   const std::string evalBackend = state.range(0) == 2 ? "codegen" : (state.range(0) == 1 ? "cpu" : "legacy");
-   auto &nllPtr =
-      state.range(0) == 2 ? bmdata().codegenNll : (state.range(0) == 1 ? bmdata().batchedNll : bmdata().nll);
+   const std::string evalBackend = state.range(0) == 2 ? "codegen" : "cpu";
+   auto &nllPtr = state.range(0) == 2 ? bmdata().codegenNll : bmdata().batchedNll;
 
    auto mc = static_cast<RooStats::ModelConfig *>(bmdata().ws->obj("ModelConfig"));
    RooArgSet const *globObs = mc->GetGlobalObservables();
@@ -74,8 +72,7 @@ static void benchCreateNLL(benchmark::State &state)
 static void benchEvaluateNLL(benchmark::State &state)
 {
    bool kickAllParameters = state.range(0);
-   auto &nllPtr =
-      state.range(1) == 2 ? bmdata().codegenNll : (state.range(1) == 1 ? bmdata().batchedNll : bmdata().nll);
+   auto &nllPtr = state.range(1) == 2 ? bmdata().codegenNll : bmdata().batchedNll;
 
    RooArgSet parameters;
    nllPtr->getParameters(nullptr, parameters);
@@ -96,8 +93,7 @@ static void benchEvaluateNLL(benchmark::State &state)
 
 static void benchMinimizeNLL(benchmark::State &state)
 {
-   auto &nllPtr =
-      state.range(0) == 2 ? bmdata().codegenNll : (state.range(0) == 1 ? bmdata().batchedNll : bmdata().nll);
+   auto &nllPtr = state.range(0) == 2 ? bmdata().codegenNll : bmdata().batchedNll;
 
    RooArgSet parameters;
    nllPtr->getParameters(nullptr, parameters);
@@ -115,28 +111,23 @@ static void benchMinimizeNLL(benchmark::State &state)
    parameters.assign(initialParams);
 }
 
-// BENCHMARK(benchCreateNLL)->Name("createNLL")->Args({0})->Unit(kSecond)->Iterations(1);
 // BENCHMARK(benchCreateNLL)->Name("createNLL_CPU")->Args({1})->Unit(kSecond)->Iterations(1);
 BENCHMARK(benchCreateNLL)->Name("createNLL_CodeGenAD")->Args({2})->Unit(kSecond)->Iterations(1);
-// BENCHMARK(benchEvaluateNLL)->Name("evaluateNLL")->Args({1, 0})->Unit(kMillisecond);
 // BENCHMARK(benchEvaluateNLL)->Name("evaluateNLL_CPU")->Args({1, 1})->Unit(kMillisecond);
 // BENCHMARK(benchEvaluateNLL)->Name("evaluateNLL_CodeGenAD")->Args({1, 2})->Unit(kMillisecond);
-// BENCHMARK(benchEvaluateNLL)->Name("evaluateNLL_SingleKick")->Args({0, 0})->Unit(kMillisecond);
 // BENCHMARK(benchEvaluateNLL)->Name("evaluateNLL_CPU_SingleKick")->Args({0, 1})->Unit(kMillisecond);
 // BENCHMARK(benchEvaluateNLL)->Name("evaluateNLL_CodeGenAD_SingleKick")->Args({0, 2})->Unit(kMillisecond);
-// BENCHMARK(benchMinimizeNLL)->Name("minimizeNLL")->Args({0})->Unit(kSecond)->Iterations(1);
 // BENCHMARK(benchMinimizeNLL)->Name("minimizeNLL_CPU")->Args({1})->Unit(kSecond)->Iterations(1);
 BENCHMARK(benchMinimizeNLL)->Name("minimizeNLL_CodeGenAD")->Args({2})->Unit(kSecond)->Iterations(1);
 
 // The channels 221 to 231 inclusive of the full combination workspace are
 // unfortunately corrupt. They contain RooAddPdfs that are affected by the
 // notorious server-proxy-desyncing that can happen if the RooFit frameworks do
-// the wrong thing. Th new the EvalBackend uses the client-server links to build
-// the computation graph for evaluation, and the old RooFit uses the proxies
-// (the client-server links are only used to the dirty flag propagation in the
-// old RooFit). As the servers and proxies are out of sync in some channels, we
-// have no idea what is actually correct and hence these channels are
-// unsuitable for validation and benchmarking. They are replaced here by
+// the wrong thing. The evaluation backends use the client-server links to
+// build the computation graph for evaluation, while the proxies are what the
+// pdfs were built with. As the servers and proxies are out of sync in some
+// channels, we have no idea what is actually correct and hence these channels
+// are unsuitable for validation and benchmarking. They are replaced here by
 // uniform pdfs.
 void maskBrokenChannels()
 {

root/roofit/atlas-benchmarks/run_atlasHiggsBackendComparison.sh

@@ -8,9 +8,6 @@
 #
 # The `time` builtin has to wrap root itself, because the plot script reads its
 # "user" line as the total bar height.
-#
-# The legacy backend is not run here, as it no longer exists in recent ROOT.
-# Add it to the loop below on an older ROOT to get a third bar.
 
 mkdir -p atlas
 

root/roofit/histfactory/benchHistFactory.cxx

@@ -81,11 +81,8 @@ static void benchHistFactory001(benchmark::State &state)
 
 auto const unit = benchmark::kMillisecond;
 
-auto Legacy = static_cast<int>(RooFit::EvalBackend::Value::Legacy);
 auto Cpu = static_cast<int>(RooFit::EvalBackend::Value::Cpu);
 
-BENCHMARK(benchHistFactory001)->Unit(unit)->Args({1, Legacy})->Name("hf001__BinnedFitOptimization_ON___BatchMode_OFF");
-BENCHMARK(benchHistFactory001)->Unit(unit)->Args({0, Legacy})->Name("hf001__BinnedFitOptimization_OFF__BatchMode_OFF");
 BENCHMARK(benchHistFactory001)->Unit(unit)->Args({1, Cpu})->Name("hf001__BinnedFitOptimization_ON___BatchMode_ON_");
 BENCHMARK(benchHistFactory001)->Unit(unit)->Args({0, Cpu})->Name("hf001__BinnedFitOptimization_OFF__BatchMode_ON_");
 

root/roofit/roofit/CMakeLists.txt

@@ -3,18 +3,12 @@ RB_ADD_GBENCHMARK(benchRooFitBinned
   LABEL long
   LIBRARIES Core Hist MathCore RIO RooFit RooStats RooFitCore HistFactory)
 
-RB_ADD_GBENCHMARK(benchRooFitUnbinned
-  RooFitUnBinnedBenchmarks.cxx
-  LABEL long
-  LIBRARIES Core Hist MathCore RIO RooFit RooStats RooFitCore HistFactory)
-
 RB_ADD_GBENCHMARK(benchRooFitBackends
   benchRooFitBackends.cxx
   LABEL long
   LIBRARIES Core MathCore RooFitCore RooFit)
 
 if(cuda)
-  target_compile_definitions(benchRooFitUnbinned PRIVATE DO_BENCH_ROOFIT_CUDA)
   target_compile_definitions(benchRooFitBackends PRIVATE DO_BENCH_ROOFIT_CUDA)
 endif()