Skip to content

Commit

Permalink
[RF] Fix ranged integrals with factorized variables
Browse files Browse the repository at this point in the history
The `RooRealIntegral` class is smart enough to figure out which
variables the function the integrated function doesn't depend on and
trivially integrates them itself by multiplying with the variable
definition range.

However, if the integration range is a subrange of the variable range,
this was not considered. This resulted in wrong results. for integrals
like `pdf.createIntegral(x, "subrange")`, where the pdf doesn't depend
on x. These kind of integrals can occur in the projections that the
RooAddPdf does, so it's important that they work, and fixing this
partially addresses root-project#11486.

This change also fixes a so-far unknown bug in the `RooXYChi2Var`, which
also used these kind of integrals. Without this fix, the `Integrate()`
feature for `chi2FitTo()` was completely broken, which can be seen in
the output of the `rf609` tutorial with any previous ROOT version. The
tutorial looks okay by chance, because the function is dominted by the
quadratic term in `x` that is constant in the fit. But use can see in
the fit result that something is off. Unfortunately, this tutorial was
also turned into a unit test in stressRooFit, so the reference results
need to be updated.

To demonstrate that things work correctly now, a new unit test was
implemented that does the closure check of the `integrate()` feature of
the `RooXYChi2Var` with a linear fit function.
  • Loading branch information
guitargeek committed Jan 11, 2023
1 parent 67f70e8 commit 7911d90
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 2 deletions.
2 changes: 1 addition & 1 deletion roofit/roofitcore/inc/RooRealIntegral.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class RooRealIntegral : public RooAbsReal {
const RooArgSet& anaIntVars() const { return _anaList ; }

RooArgSet intVars() const { RooArgSet tmp(_sumList) ; tmp.add(_intList) ; tmp.add(_anaList) ; tmp.add(_facList) ; return tmp ; }
const char* intRange() { return _rangeName ? _rangeName->GetName() : nullptr ; }
const char* intRange() const { return _rangeName ? _rangeName->GetName() : nullptr ; }
const RooAbsReal& integrand() const { return *_function; }

void setCacheNumeric(bool flag) {
Expand Down
2 changes: 1 addition & 1 deletion roofit/roofitcore/src/RooRealIntegral.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ double RooRealIntegral::evaluate() const
// Multiply by fit range for 'real' dependents
if (arg->IsA()->InheritsFrom(RooAbsRealLValue::Class())) {
RooAbsRealLValue* argLV = (RooAbsRealLValue*)arg ;
retVal *= (argLV->getMax() - argLV->getMin()) ;
retVal *= (argLV->getMax(intRange()) - argLV->getMin(intRange())) ;
}
// Multiply by number of states for category dependents
if (arg->IsA()->InheritsFrom(RooAbsCategoryLValue::Class())) {
Expand Down
60 changes: 60 additions & 0 deletions roofit/roofitcore/test/testTestStatistics.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <RooNLLVar.h>
#include <RooRandom.h>
#include <RooPlot.h>
#include <RooPolyVar.h>
#include <RooRealVar.h>
#include <RooWorkspace.h>

Expand Down Expand Up @@ -345,6 +346,65 @@ TEST(RooNLLVar, CopyRangedNLL)
EXPECT_FLOAT_EQ(nllrange->getVal(), nllrangeClone->getVal());
}

/// When using the Integrate() command argument in chi2FitTo, the result should
/// be identical to a fit without bin integration if the fit function is
/// linear. This is a good cross check to see if the integration works.
/// Inspired by the rf609_xychi2fit tutorial.
TEST(RooXYChi2Var, IntegrateLinearFunction)
{
using namespace RooFit;

// Make weighted XY dataset with asymmetric errors stored The StoreError()
// argument is essential as it makes the dataset store the error in addition
// to the values of the observables. If errors on one or more observables
// are asymmetric, one can store the asymmetric error using the
// StoreAsymError() argument
RooRealVar x("x", "x", -11, 11);
RooRealVar y("y", "y", -10, 200);
RooDataSet dxy("dxy", "dxy", {x, y}, StoreError({x, y}));

const double aTrue = 0.1;
const double bTrue = 10.0;

// Fill an example dataset with X,err(X),Y,err(Y) values
for (int i = 0; i <= 10; i++) {

// Set X value and error
x = -10 + 2 * i;
x.setError(i < 5 ? 0.5 / 1. : 1.0 / 1.);

// Set Y value and error
y = aTrue * x.getVal() + bTrue;
y.setError(std::sqrt(y.getVal()));

dxy.add({x, y});
}

// Make linear fit function
RooRealVar a("a", "a", 0.0, -10, 10);
RooRealVar b("b", "b", 0.0, -100, 100);
RooArgList coefs{b, a};
RooPolyVar f("f", "f", x, coefs);

RooArgSet savedValues;
coefs.snapshot(savedValues);

// Fit chi^2 using X and Y errors
std::unique_ptr<RooFitResult> fit1{f.chi2FitTo(dxy, YVar(y), Save(), PrintLevel(-1), Optimize(0))};

coefs.assign(savedValues);
// Alternative: fit chi^2 integrating f(x) over ranges defined by X errors,
// rather than taking point at center of bin
std::unique_ptr<RooFitResult> fit2{f.chi2FitTo(dxy, YVar(y), Integrate(true), Save(), PrintLevel(-1), Optimize(0))};

// Verify that the fit result is compatible with true values within the error
EXPECT_NEAR(getVal("a", fit1->floatParsFinal()), .1, getErr("a", fit1->floatParsFinal()));
EXPECT_NEAR(getVal("b", fit1->floatParsFinal()), 10.0, getErr("b", fit1->floatParsFinal()));

EXPECT_NEAR(getVal("a", fit2->floatParsFinal()), .1, getErr("a", fit2->floatParsFinal()));
EXPECT_NEAR(getVal("b", fit2->floatParsFinal()), 10.0, getErr("b", fit2->floatParsFinal()));
}

class OffsetBinTest : public testing::TestWithParam<std::tuple<std::string, bool, bool, bool>> {
void SetUp() override
{
Expand Down
Binary file modified test/stressRooFit_ref.root
Binary file not shown.

0 comments on commit 7911d90

Please sign in to comment.