From 46f4f979f3cb060c7236bf8011462286af0aaf51 Mon Sep 17 00:00:00 2001
From: Jonas Rembser <jonas.rembser@cern.ch>
Date: Wed, 21 Sep 2022 13:05:25 +0200
Subject: [PATCH] [RF] Consider the `SplitRange` in overlap checks for
 simultaneous PDFs

When the `SplitRange` command arguemnt is used in
`RooAbsPdf::createNLL`, the actual range names used for the fit depend
on the channel, with the range names suffixed by the category name. This
should be considered correctly in the overlap checks.

Closes #11396.
---
 roofit/roofitcore/inc/RooHelpers.h   |  5 ++-
 roofit/roofitcore/src/RooAbsPdf.cxx  |  2 +-
 roofit/roofitcore/src/RooHelpers.cxx | 54 +++++++++++++++++++++-------
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/roofit/roofitcore/inc/RooHelpers.h b/roofit/roofitcore/inc/RooHelpers.h
index ebafdf67b7aaf..30d0430e22ef4 100644
--- a/roofit/roofitcore/inc/RooHelpers.h
+++ b/roofit/roofitcore/inc/RooHelpers.h
@@ -132,7 +132,10 @@ class ChangeOperModeRAII {
 
 std::pair<double, double> getRangeOrBinningInterval(RooAbsArg const* arg, const char* rangeName);
 
-bool checkIfRangesOverlap(RooAbsPdf const& pdf, RooAbsData const& data, std::vector<std::string> const& rangeNames);
+bool checkIfRangesOverlap(RooAbsPdf const& pdf,
+                          RooAbsData const& data,
+                          std::vector<std::string> const& rangeNames,
+                          bool splitRange);
 
 std::string getColonSeparatedNameString(RooArgSet const& argSet);
 RooArgSet selectFromArgSet(RooArgSet const&, std::string const& names);
diff --git a/roofit/roofitcore/src/RooAbsPdf.cxx b/roofit/roofitcore/src/RooAbsPdf.cxx
index e59fd2ebc206a..6398c92605b50 100644
--- a/roofit/roofitcore/src/RooAbsPdf.cxx
+++ b/roofit/roofitcore/src/RooAbsPdf.cxx
@@ -1143,7 +1143,7 @@ RooAbsReal* RooAbsPdf::createNLL(RooAbsData& data, const RooLinkedList& cmdList)
     // Composite case: multiple ranges
     RooArgList nllList ;
     auto tokens = ROOT::Split(rangeName, ",");
-    if (RooHelpers::checkIfRangesOverlap(*this, data, tokens)) {
+    if (RooHelpers::checkIfRangesOverlap(*this, data, tokens, cfg.splitCutRange)) {
       throw std::runtime_error(
               std::string("Error in RooAbsPdf::createNLL! The ranges ") + rangeName + " are overlapping!");
     }
diff --git a/roofit/roofitcore/src/RooHelpers.cxx b/roofit/roofitcore/src/RooHelpers.cxx
index d6e8784cd50df..e0ec13a386d1c 100644
--- a/roofit/roofitcore/src/RooHelpers.cxx
+++ b/roofit/roofitcore/src/RooHelpers.cxx
@@ -14,17 +14,19 @@
  * listed in LICENSE (http://roofit.sourceforge.net/license.txt)             *
  *****************************************************************************/
 
-#include "RooHelpers.h"
-#include "RooAbsPdf.h"
-#include "RooAbsData.h"
-#include "RooDataHist.h"
-#include "RooDataSet.h"
-#include "RooAbsRealLValue.h"
-#include "RooArgList.h"
-#include "RooAbsCategory.h"
-
-#include "ROOT/StringUtils.hxx"
-#include "TClass.h"
+#include <RooHelpers.h>
+
+#include <RooAbsCategory.h>
+#include <RooAbsData.h>
+#include <RooAbsPdf.h>
+#include <RooAbsRealLValue.h>
+#include <RooArgList.h>
+#include <RooDataHist.h>
+#include <RooDataSet.h>
+#include <RooSimultaneous.h>
+
+#include <ROOT/StringUtils.hxx>
+#include <TClass.h>
 
 namespace RooHelpers {
 
@@ -182,7 +184,35 @@ std::pair<double, double> getRangeOrBinningInterval(RooAbsArg const* arg, const
 /// \param[in] pdf the PDF
 /// \param[in] data RooAbsCollection with the observables to check for overlap.
 /// \param[in] rangeNames The names of the ranges.
-bool checkIfRangesOverlap(RooAbsPdf const& pdf, RooAbsData const& data, std::vector<std::string> const& rangeNames) {
+/// \param[in] splitRange If `true`, each component of a RooSimultaneous will
+///                       be checked individually for overlaps, with the range
+///                       names in that component suffixed by `_<cat_label>`.
+///                       See the `SplitRange()` command argument of
+///                       RooAbsPdf::fitTo()` to understand where this is used.
+bool checkIfRangesOverlap(RooAbsPdf const& pdf,
+                          RooAbsData const& data,
+                          std::vector<std::string> const& rangeNames,
+                          bool splitRange)
+{
+  // If the PDF is a RooSimultaneous and the `splitRange` option is set, we
+  // have to check each component PDF with a different set of rangeNames, each
+  // suffixed by the category name.
+  if(splitRange && dynamic_cast<RooSimultaneous const*>(&pdf)) {
+    auto const& simPdf = static_cast<RooSimultaneous const&>(pdf);
+    bool hasOverlap = false;
+    std::vector<std::string> rangeNamesSplit;
+    for (const auto& catState : simPdf.indexCat()) {
+      const std::string& catName = catState.first;
+      for(std::string const& rangeName : rangeNames) {
+        rangeNamesSplit.emplace_back(rangeName + "_" + catName);
+      }
+      hasOverlap |= checkIfRangesOverlap(*simPdf.getPdf(catName.c_str()), data, rangeNamesSplit, false);
+
+      rangeNamesSplit.clear();
+    }
+
+    return hasOverlap;
+  }
 
   auto observables = *pdf.getObservables(data);