[RF] ToyMCSampler improvements for binned toys

Adds the ability to control the ToyMCSampler to emit binned toys. Makes
use of this new capability in the HypoTestCalculator in cases where the
observed data is binned.

(cherry picked from commit 95320d1)

Author: cburgard

roofit/roofitcore/inc/RooGlobalFunc.h

@@ -358,7 +358,7 @@ RooCmdArg Conditional(const RooArgSet& pdfSet, const RooArgSet& depSet, bool dep
  * @{
  */
 // RooAbsPdf::generate arguments
-RooCmdArg ProtoData(const RooDataSet& protoData, bool randomizeOrder=false, bool resample=false) ;
+RooCmdArg ProtoData(const RooAbsData& protoData, bool randomizeOrder=false, bool resample=false) ;
 RooCmdArg NumEvents(Int_t numEvents) ;
 RooCmdArg NumEvents(double numEvents) ;
 RooCmdArg AutoBinned(bool flag=true) ;

roofit/roofitcore/src/RooAbsPdf.cxx

@@ -1200,7 +1200,7 @@ RooAbsGenContext* RooAbsPdf::autoGenContext(const RooArgSet &vars, const RooData
 ///       as binned generation is always executed at the top-level node for a regular
 ///       PDF, so for those it only mattes that the top-level node is tagged.
 ///
-/// <tr><td> ProtoData(const RooDataSet& data, bool randOrder)
+/// <tr><td> ProtoData(const RooAbsData& data, bool randOrder)
 ///          <td> Use specified dataset as prototype dataset. If randOrder in ProtoData() is set to true,
 ///               the order of the events in the dataset will be read in a random order if the requested
 ///               number of events to be generated does not match the number of events in the prototype dataset.

roofit/roofitcore/src/RooBinnedGenContext.cxx

@@ -60,7 +60,7 @@ RooBinnedGenContext::RooBinnedGenContext(const RooAbsPdf &model, const RooArgSet
   if (prototype)
     {
       RooArgSet coefNSet(vars) ;
-      coefNSet.add(*prototype->get()) ;
+      coefNSet.add(*prototype->get(),true) ;
       _pdf->fixAddCoefNormalization(coefNSet) ;
     }
 

roofit/roofitcore/src/RooGenContext.cxx

@@ -87,7 +87,7 @@ RooGenContext::RooGenContext(const RooAbsPdf &model, const RooArgSet &vars,
   // Optionally fix RooAddPdf normalizations
   if (prototype&&_pdfClone->dependsOn(*prototype->get())) {
     RooArgSet fullNormSet(vars) ;
-    fullNormSet.add(*prototype->get()) ;
+    fullNormSet.add(*prototype->get(),true) ;
     _pdfClone->fixAddCoefNormalization(fullNormSet) ;
   }
 

roofit/roofitcore/src/RooGlobalFunc.cxx

@@ -807,7 +807,7 @@ RooCmdArg Conditional(const RooArgSet &pdfSet, const RooArgSet &depSet, bool dep
 };
 
 // RooAbsPdf::generate arguments
-RooCmdArg ProtoData(const RooDataSet &protoData, bool randomizeOrder, bool resample)
+RooCmdArg ProtoData(const RooAbsData &protoData, bool randomizeOrder, bool resample)
 {
    return RooCmdArg("PrototypeData", randomizeOrder, resample, 0, 0, nullptr, nullptr, &protoData, nullptr);
 }

roofit/roostats/inc/RooStats/ToyMCSampler.h

@@ -229,14 +229,14 @@ class ToyMCSampler: public TestStatSampler {
          fAdaptiveLowLimit = low_threshold;
       }
 
-      void SetProtoData(const RooDataSet* d) { fProtoData = d; }
+      void SetProtoData(const RooAbsData* d) { fProtoData = d; }
 
    protected:
 
       const RooArgList* EvaluateAllTestStatistics(RooAbsData& data, const RooArgSet& poi, DetailedOutputAggregator& detOutAgg);
 
       /// helper for GenerateToyData
-      std::unique_ptr<RooAbsData> Generate(RooAbsPdf &pdf, RooArgSet &observables, const RooDataSet *protoData=nullptr, int forceEvents=0) const;
+      std::unique_ptr<RooAbsData> Generate(RooAbsPdf &pdf, RooArgSet &observables, const RooAbsData *protoData=nullptr, int forceEvents=0) const;
 
       /// helper method for clearing  the cache
       virtual void ClearCache();
@@ -272,7 +272,7 @@ class ToyMCSampler: public TestStatSampler {
       double fAdaptiveLowLimit;
       double fAdaptiveHighLimit;
 
-      const RooDataSet *fProtoData = nullptr; ///< in dev
+      const RooAbsData *fProtoData = nullptr; ///< in dev
 
       mutable NuisanceParametersSampler *fNuisanceParametersSampler = nullptr; ///<!
 

roofit/roostats/src/HypoTestCalculatorGeneric.cxx

@@ -63,8 +63,20 @@ HypoTestCalculatorGeneric::HypoTestCalculatorGeneric(
                                                   *altModel.GetPdf(),
                                                   altModel.GetSnapshot());
 
-      fDefaultSampler = new ToyMCSampler(*fDefaultTestStat, 1000);
-      fTestStatSampler = fDefaultSampler;
+      auto toymcs = new ToyMCSampler(*fDefaultTestStat, 1000);
+      const bool dataIsBinned = dynamic_cast<const RooDataHist*>(fData);
+      if (dataIsBinned) {
+         // Ensure the ToyMCSampler generates toys with the same structure as the observed data.
+         // TODO: understand why this is needed only in the RooDataHist case,
+         // but results in backwards-incompatibility for RooDataSet, which
+         // manifests for example in the stressRooStats tests.
+         // See also: https://github.com/root-project/root/pull/20171
+         toymcs->SetProtoData(&data);
+      }
+      toymcs->SetGenerateBinned(dataIsBinned);  // if observed is RooDataHist -> generate RooDataHist toys
+
+      fDefaultSampler = toymcs;
+      fTestStatSampler = toymcs;
    }
 
 

roofit/roostats/src/ToyMCSampler.cxx

@@ -476,7 +476,7 @@ RooAbsData* ToyMCSampler::GenerateToyData(RooArgSet& paramPoint, double& weight,
 /// or whether it should use the expected number of events. It also takes
 /// into account the option to generate a binned data set (*i.e.* RooDataHist).
 
-std::unique_ptr<RooAbsData> ToyMCSampler::Generate(RooAbsPdf &pdf, RooArgSet &observables, const RooDataSet* protoData, int forceEvents) const {
+std::unique_ptr<RooAbsData> ToyMCSampler::Generate(RooAbsPdf &pdf, RooArgSet &observables, const RooAbsData* protoData, int forceEvents) const {
 
   if(fProtoData) {
     protoData = fProtoData;