[RF] `createChi2` ignores ranges provided by the Range() option

[tofitsch]

Check duplicate issues.

• Checked for duplicates

Description

this commit
removed the line

if (!hdata->valid()) continue;

in roofit/roofitcore/src/RooChi2Var.cxx which is supposed to skip bins outside the specified range when calling e.g.

pdf->createChi2(data_hist, Range("range_name"));
pdf->getVal()

As a result, the reported chi2 value is the same, no matter what range is provided.
Adding a printout like

std::cout << i << " " << nPdf << " " << nData << std::endl;

a few lines below where the valid line should be, indicated that all bins were considered for the chi2 calculation and thus helped to find the cause of the issue.

Since the valid() function was removed from RooDataHist in the same commit one cannot simply add the line back in. Instead createChi2(Range()) should probably make use of he RooAbsData::reduce() implementation as mentioned in the message of that commit:
"In a subrange fit, the RooAbsOptTestStatistic is creating a clone of the dataset with the subrange only using RooAbsData::reduce(). So all entries are valid by definition."

Apparently this was implemented for the other test statistics (e.g. createNLL) but not createChi2.

Using createNLL(data_hist, Range("range_name") appears to take the ranges into account correctly and there is an overload of createNLL taking RooAbsData (allowing for the intended RooAbsData::reduce() method) rather than taking RooDataHist. No such overload exists for createChi2().

tested on main (@4f17792277dd51dcf238c8a05f8d3f093de4ddcd) tag:v6-36-00

Reproducer

Simply run this macro. It does not require any input.

root -l -q test.cpp

test.cpp:

{

  using namespace RooFit;

  RooRealVar x("x", "x", 0., 1.);
  x.setRange("lo", 0., .5);
  x.setRange("hi", .5, 1.);
  x.setRange("full", 0., 1.);

  RooUniform uniform("uniform", "", x);
  RooRealVar nbkg("nbkg", "", 11.);
  RooExtendPdf pdf("pdf", "", uniform, nbkg);

  TH1D h("h", "h", 10, 0., 1.);

  for (int i = 1; i <= 10; ++i) 
    h.SetBinContent(i, 1.);

  // histogram integral: 10. (slightly off from nbkg = 11. so that chi2 is not 0.)

  RooDataHist dh("dh", "dh", x, &h);

  for (string range : {"", "full", "lo", "hi", "lo,hi"}) {

    auto chi2 = unique_ptr<RooAbsReal> (
      range == ""
      ? pdf.createChi2 (
         dh,
         Extended(true),
         DataError(RooAbsData::Poisson)
       )
      : pdf.createChi2 (
         dh,
         Extended(true),
         DataError(RooAbsData::Poisson),
         Range(range.c_str())
       )
    );

    cout << "chi2(" << range << ")  = " << chi2->getVal() << endl;

  }

}

A uniform pdf on the range 0. to 1. with integral 11. is created and compared with a uniform histogram with 10 bins, also on the range 0. to 1. but with integral 11.

chi2 is calculated using Poisson errors first without specifying a range
and then with the ranges
full (0. to 1.)
lo (0. to .5)
hi (.5 to 1.)
and the union lo,hi (0. to 1.)

First, here is the result with an older root version where the valid() line is still there and where the chi2 calculation appears to be working correctly:
output with root-v6-26-08:

> chi2()      = 0.0189114
> chi2(full)  = 0.0189114
> chi2(lo)    = 1.36162
> chi2(hi)    = 1.36162
> chi2(lo,hi) = 2.72324

Don't mind the normalisation difference between chi2(lo,hi) and chi2(full) which is not the focus of this issue.
It is understood to be related to the ndof (factor 144 = 12^2; 12 bins including under/overflow).

Now with the current main:
output with root-v6-36-04:

chi2()        = 0.0189114
chi2(full)    = 0.0189114
chi2(lo)      = 0.0189114
chi2(hi)      = 0.0189114
chi2(lo,hi)   = 0.0189114

all results are as if no range is provided at all.
When exchanging createChi2 with e.g. createNLL in the example above, one gets different values for the different ranges, as intended.

ROOT version

ROOT v6.36.04
Built for linuxx8664gcc on Aug 25 2025, 09:43:06
From tags/v6-36-04@v6-36-04
With c++ (Ubuntu 11.4.0-1ubuntu1~22.04.2) 11.4.0
Binary directory: ~/install/root/bin

latest commit: 4f17792

Installation method

build from source

Operating system

Linux

Additional context

No response

[guitargeek]

Thank you very much for the report! I'm working on it. But I have one doubt:

Don't mind the normalisation difference between chi2(lo,hi) and chi2(full) which is not the focus of this issue.
It is understood to be related to the ndof (factor 144 = 12^2; 12 bins including under/overflow).

Well, we should not ignore these unexpectedly different numbers either 🙂 especially since lo,hi==full, I'm surprised that the results for the two fits are different. How much did you understand that these differences are related to the ndof? I mean if the ranges are equivalent, should the chi2 not be identical, because the number of bins is the same?

[tofitsch]

Thanks! Actually you're right. I just noticed that in that particular example the ratio of chi2(full) to chi2(lo,hi) happened to be exactly 144 and didn't think much more about it. But now, trying different values for the number of bins etc doesn't generally give a ratio that is exactly an integer. Not quite sure how this factor actually comes about.
I agree that chi2(full) and chi2(lo,hi) should be the same value.

[tofitsch]

Ok, looking a bit more into the normalisation:
In the example above, let's not use the poisson errors to make calculations by hand easier. I.e. we remove the DataError(RooAbsData::Poisson) from the reproducer macro.
Each bin should then have nPdf = 1.1, nData = 1., and sigma = sqrt(nPdf) = 1.049 and therefore chi2_bin = (nPDF - nData)^2 / sigma^2 = 0.00909091.
The total chi2 value for the full range (10 bins) should therefore be chi2_bin * 10. = 0.0909091 and accordingly for lo and hi (5 bins each) it should be chi2_bin * 5. = 0.04545455 and for lo,hi combined it should be 0.0909091 again.
Printing some of the terms used in the loop over all bins (this line) when using the full range we get this (Here eExt = nPdf - nData, eInt = sigma, term = chi2_bin to use the same naming as in the code):
full:

bin  nData  nPdf  _funcClone->getVal(_normSet)  normFactor  hdata->binVolume() eExt     eInt       term
  0      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  1      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  2      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  3      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  4      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  5      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  6      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  7      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  8      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
  9      1   1.1                             1          11                 0.1  0.1  1.04881 0.00909091
result: 0.0909091

All good, as we expected from the by-hand calculation above.

now with the lo range:

bin nData  nPdf _funcClone->getVal(_normSet) normFactor hdata->binVolume() eExt     eInt      term
  0     1   2.2                            2         11                0.1  1.2  1.48324  0.654545
  1     1   2.2                            2         11                0.1  1.2  1.48324  0.654545
  2     1   2.2                            2         11                0.1  1.2  1.48324  0.654545
  3     1   2.2                            2         11                0.1  1.2  1.48324  0.654545
  4     1   2.2                            2         11                0.1  1.2  1.48324  0.654545
result: 3.27273

The original difference is in the _funcClone->getVal(_normSet) in this line. nPdf, eExt, eInt, and term change accordingly.
The getVal tries to accommodate for the fact that the range is not the full range by a factor full_width / range_width. But all subsequent calculations to get chi2 don't expect this to be taken into account.
That's where the difference between chi2(full) and chi2(lo,hi) comes from. In chi2(lo,hi) nPdf is too large by a factor of 2 for each bin. It has nothing to do with the ndof.

[guitargeek]

Seems like the old ROOT versions have some normalization problems going on. I have opened a PR now, implementing the ranged chi-square fit as I think it's correct:

• [RF] Support ranged chi-square fits #21552

I have extended your test script also for the case of functions (non-pdfs) and non-extended pdfs:

void repro()
{
   auto changeMsgLvl = std::make_unique<RooHelpers::LocalChangeMsgLevel>(RooFit::WARNING);

   using namespace RooFit;

   RooRealVar x("x", "x", 0., 1.);
   x.setRange("lo", 0., .5);
   x.setRange("hi", .5, 1.);
   x.setRange("full", 0., 1.);

   RooRealVar nbkg("nbkg", "", 11.);

   // Flat function that integrates to the expected number of events
   RooFormulaVar flat("flat", "nbkg + x - x", {nbkg, x});

   RooUniform uniform("uniform", "", x);
   RooExtendPdf pdf("pdf", "", uniform, nbkg);

   int nBins = 10;

   TH1D h("h", "h", nBins, 0., 1.);

   for (int i = 1; i <= nBins; ++i)
      h.SetBinContent(i, 1.);

   // histogram integral: 10. (slightly off from nbkg = 11. so that chi2 is not 0.)

   RooDataHist dh("dh", "dh", x, &h);

   // Extended pdf
   std::cout << "Extended pdf" << std::endl;
   for (string range : {"", "full", "lo", "hi", "lo,hi"}) {

      auto chi2 = unique_ptr<RooAbsReal>(
         range == "" ? pdf.createChi2(dh, Extended(true), DataError(RooAbsData::Poisson))
                     : pdf.createChi2(dh, Extended(true), DataError(RooAbsData::Poisson), Range(range.c_str())));

      cout << "chi2(" << range << ")  = " << chi2->getVal() << endl;
   }
   std::cout << std::endl;

   // Non-extended pdf
   std::cout << "Non-extended pdf" << std::endl;
   for (string range : {"", "full", "lo", "hi", "lo,hi"}) {

      auto chi2 = unique_ptr<RooAbsReal>(
         range == "" ? uniform.createChi2(dh, DataError(RooAbsData::Poisson))
                     : uniform.createChi2(dh, DataError(RooAbsData::Poisson), Range(range.c_str())));

      cout << "chi2(" << range << ")  = " << chi2->getVal() << endl;
   }
   std::cout << std::endl;

   // Function
   std::cout << "Function" << std::endl;
   for (string range : {"", "full", "lo", "hi", "lo,hi"}) {

      auto chi2 = unique_ptr<RooAbsReal>(
         range == "" ? flat.createChi2(dh, DataError(RooAbsData::Poisson))
                     : flat.createChi2(dh, DataError(RooAbsData::Poisson), Range(range.c_str())));

      cout << "chi2(" << range << ")  = " << chi2->getVal() << endl;
   }
   std::cout << std::endl;
}

Here is the output:

Extended pdf
chi2()  = 0.0189114
chi2(full)  = 0.0189114
chi2(lo)  = 0.00945569
chi2(hi)  = 0.00945569
chi2(lo,hi)  = 0.0189114

Non-extended pdf
chi2()  = 0
chi2(full)  = 0
chi2(lo)  = 0
chi2(hi)  = 0
chi2(lo,hi)  = 0

Function
chi2()  = 0.0189114
chi2(full)  = 0.0189114
chi2(lo)  = 0.00945569
chi2(hi)  = 0.00945569
chi2(lo,hi)  = 0.0189114

Does that make sense to you?

[tofitsch]

Looks good to me. Many thanks!

[guitargeek]

Nice! Would a backport of this to the 6.38 for the next patch release be sufficient for you, or do you need also backports to older ROOT branches? I think we have a 6.36 patch release lined up too, so we can include the fix also there.

[tofitsch]

6.38 would be sufficient for us