Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hist] Chi-square computation in Binned likelihood fits #11143

Closed
lmoneta opened this issue Aug 8, 2022 · 2 comments · Fixed by #14763
Closed

[hist] Chi-square computation in Binned likelihood fits #11143

lmoneta opened this issue Aug 8, 2022 · 2 comments · Fixed by #14763
Assignees
Labels
bug fixathon This issue can be tackled at a ROOT fixathon

Comments

@lmoneta
Copy link
Member

lmoneta commented Aug 8, 2022

As reported in https://root-forum.cern.ch/t/chi-square-definition-in-root/51083/3,
the reported chi-square from TF1::GetChisquare and from FitResult is not correct.
A Neyman chi-square is computed but using also the empty bins.
See https://github.com/root-project/root/blob/master/math/mathcore/src/FitResult.cxx#L183

We should return in this case as chi-square the value suggested by Baker-Cousins, i.e. 2 * MinNLL.

In addition we have also :

  • Documentation for TF1::GetChisquare and ROOT::Fit::FitResult::Chi2 is missing.
  • TH1::Chisquare and ROOT::Fit::Chisquare should compute also the Pearson chi-square
@lmoneta lmoneta added the bug label Aug 8, 2022
@lmoneta lmoneta self-assigned this Aug 8, 2022
@dpiparo dpiparo added the fixathon This issue can be tackled at a ROOT fixathon label Feb 5, 2024
@atolosadelgado
Copy link
Contributor

atolosadelgado commented Feb 13, 2024

Hi,

I have tested the method TH1::Chisquare and seems to calculate the Pearson Chi2 properly

// File: test.cxx
// test TH1::Chisquare function. Pearson Chi2 is calculated as expected
// To run the script: root test.cxx
void test()
{

    int nbins = 100;
    double xmin =-5;
    double xmax = 5;

    TF1 * f = new TF1("f","gausn");
    double nentries = 1000;
    double binwidth = (xmax-xmin)/nbins;
    f->SetParameters(nentries*binwidth,0.,1.);
    f->SetRange(-10,10);

    TH1D *h = new TH1D("h","", nbins, xmin,xmax);
    h->FillRandom("f2",nentries);
    h->Draw();
    f->Draw("same");

    //-- Chi2 Pearson
    double chi2_th1 = h->Chisquare( f , "P");

    // note: below it is assumed that "bin error" = sqrt( f("bin center") )
    double chi2_P = 0.;
    for (int i = 1; i <= h->GetNbinsX(); i++) {
        double v = f->Eval(h->GetBinCenter(i));
        if (v) chi2_P += TMath::Sq(v - h->GetBinContent(i)) / TMath::Abs(v);
    }
    std::cout << "hand calculated Pearson chi2 = " << chi2_P << "\n";
    std::cout << "ROOT TH1::Chisquare     chi2 = " << chi2_th1 << "\n";

}

Question: Should ROOT::Fit::FitResult::Chi2 call TH1::Chisquare to avoid writing the same piece of code in two different places?

@lmoneta
Copy link
Member Author

lmoneta commented Feb 19, 2024

ROOT::Fit::FitResult::Chi2() returns the chi2 after fitting, TH1::Chisquare calls the function ROOT::Fit::Chisquare which compute the chi2 with an arbitrary function and histogram. The implementation is the same using the ROOT::Fit::Chi2FCN class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fixathon This issue can be tackled at a ROOT fixathon
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants