Skip to content

Commit

Permalink
[RF] Fix corruption of input pdf after using RooStats::SPlots
Browse files Browse the repository at this point in the history
In SPlots, `RooAbsArg::attachDataSet()` is called on the input pdf,
redirecting the parameters of the pdf to the RooRealVars in the dataset.
This is not reversed, leaving the pdf in a corrupt state when the
dataset gets deleted. This can happen in particular if the SPlots object
created an owning clone of the dataset, attaches the pdf, and then goes
out of scope.

This commit suggests to not attach the pdf to the dataset, as it is not
necessary. When looping over the dataset and evaluating the pdf later,
the pdf variables get synced with the dataset variables anyway. This was
done via `RooStats::SetParameters`, which is just a wrapper around
`RooAbsCollection::assign()`. I suggest to use `assign()` directly to make
more explicit to the reader what happens.

Closes Jira issue [ROOT-8018](https://sft.its.cern.ch/jira/browse/ROOT-8018).

A simpler reproducer of the problem based on the notebook in the Jira
issue is this one:

```Python
import ROOT

bdt = ROOT.RooRealVar("BDT", "some awesome BDT", 0.0, 1.0)
mass = ROOT.RooRealVar("mass", "invariant mass", 5100.0, 5300, "MeV/c^{2}")

sigyield = ROOT.RooRealVar("sigyield", "signal yield", 100, 0, 100000)
bkgyield = ROOT.RooRealVar("bkgyield", "background yield", 900, 0, 100000)

bmassPDF = ROOT.RooGaussian(
    "bmass",
    "B mass shape",
    mass,
    ROOT.RooFit.RooConst(5200.0),
    ROOT.RooFit.RooConst(20.0),
)
bkgmPDF = ROOT.RooExponential(
    "bkgmass", "bkg mass shape", mass, ROOT.RooFit.RooConst(-1.0 / 200.0)
)
combmPDF = ROOT.RooAddPdf(
    "fullmasspdf",
    "full mass pdf",
    ROOT.RooArgList(bmassPDF, bkgmPDF),
    ROOT.RooArgList(sigyield, bkgyield),
)

toydata = combmPDF.generate(ROOT.RooArgSet(bdt, mass), 10000)
ROOT.SetOwnership(toydata, True)

set1 = ROOT.RooArgList(sigyield, bkgyield)
set2 = ROOT.RooArgList()

def make_splot(toydata):
    smalldata = toydata.reduce(ROOT.RooFit.Cut("BDT>0.0"))
    ROOT.SetOwnership(smalldata, True)

    splot = ROOT.RooStats.SPlot(
        "splot", "splot", smalldata, combmPDF, set1, set2, True, True
    )

make_splot(toydata)
make_splot(toydata)
```
  • Loading branch information
guitargeek committed Oct 11, 2022
1 parent 5568f61 commit dfdbe42
Showing 1 changed file with 1 addition and 6 deletions.
7 changes: 1 addition & 6 deletions roofit/roostats/src/SPlot.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -471,11 +471,6 @@ void SPlot::AddSWeight( RooAbsPdf* pdf, const RooArgList &yieldsTmp,
RooArgSet vars(*fSData->get() );
vars.remove(projDeps, kTRUE, kTRUE);

// Attach data set

// const_cast<RooAbsPdf*>(pdf)->attachDataSet(*fSData);

pdf->attachDataSet(*fSData);

// first calculate the pdf values for all species and all events
std::vector<RooAbsRealLValue*> yieldvars ;
Expand Down Expand Up @@ -544,7 +539,7 @@ void SPlot::AddSWeight( RooAbsPdf* pdf, const RooArgList &yieldsTmp,
{
//WVE: FIX THIS PART, EVALUATION PROGRESS!!

RooStats::SetParameters(fSData->get(ievt), pdfvars);
pdfvars->assign(*fSData->get(ievt));

for(Int_t k = 0; k < nspec; ++k) {
auto theVar = static_cast<RooAbsRealLValue*>(yieldvars[k]);
Expand Down

0 comments on commit dfdbe42

Please sign in to comment.