Re: TH1::Rebin fix for HEAD

From: Rene Brun <Rene.Brun_at_cern.ch>
Date: Tue, 06 Feb 2007 08:29:01 +0100

Hi Constantin,

Thanks for this remark. The fix is now in CVS. Note that I did not use your patch

  -no need for an extra variable isCloned
  -Float --> Double for binmax
  -For portability reasons, you cannot make the change suggested for the 

loop index

Rene Brun

Constantin Loizides wrote:
> Hi Rene,
>
> the attached patch fixes a problem introduced a few days
> ago in TH1::Rebin in Rev 1.325:
> http://root.cern.ch/viewcvs/hist/src/TH1.cxx.diff?r1=1.325&r2=1.326&sortby=date
>
>
> Basically, one has to check if hNew is really a different
> histogram as otherwise the axes are the same. See
> my proposed to solution to the bug.
>
> Constantin
>
> ------------------------------------------------------------------------
>
> Index: TH1.cxx
> ===================================================================
> RCS file: /user/cvs/root/hist/src/TH1.cxx,v
> retrieving revision 1.330
> diff -u -w -r1.330 TH1.cxx
> --- TH1.cxx 3 Feb 2007 06:40:25 -0000 1.330
> +++ TH1.cxx 4 Feb 2007 20:39:49 -0000
> @@ -5057,18 +5057,20 @@
> // Save old bin contents into a new array
> Double_t entries = fEntries;
> Double_t *oldBins = new Double_t[nbins+2];
> - Int_t bin, i;
> - for (bin=0;bin<nbins+2;bin++) oldBins[bin] = GetBinContent(bin);
> +
> + for (Int_t bin=0;bin<nbins+2;bin++) oldBins[bin] = GetBinContent(bin);
> Double_t *oldErrors = 0;
> if (fSumw2.fN != 0) {
> oldErrors = new Double_t[nbins+2];
> - for (bin=0;bin<nbins+2;bin++) oldErrors[bin] = GetBinError(bin);
> + for (Int_t bin=0;bin<nbins+2;bin++) oldErrors[bin] = GetBinError(bin);
> }
>
> // create a clone of the old histogram if newname is specified
> TH1 *hnew = this;
> + Bool_t isCloned=kFALSE;
> if ((newname && strlen(newname) > 0) || xbins) {
> hnew = (TH1*)Clone(newname);
> + isCloned=kTRUE;
> }
>
> // change axis specs and rebuild bin contents array::RebinAx
> @@ -5117,12 +5119,14 @@
> // copy merged bin contents (ignore under/overflows)
> Int_t oldbin = 1;
> Double_t binContent, binError;
> - for (bin = 1;bin<=newbins;bin++) {
> + for (Int_t bin = 1;bin<=newbins;bin++) {
> binContent = 0;
> binError = 0;
> Int_t imax = ngroup;
> - for (i=0;i<ngroup;i++) {
> - if (fXaxis.GetBinCenter(oldbin+i) > hnew->GetXaxis()->GetBinUpEdge(bin)) {
> + Float_t binmax = hnew->GetXaxis()->GetBinUpEdge(bin);
> + for (Int_t i=0;i<ngroup;i++) {
> + if( (!isCloned && (oldbin+i > nbins)) ||
> + ( isCloned && (fXaxis.GetBinCenter(oldbin+i) > binmax)) ){
> imax = i;
> break;
> }
> @@ -5131,6 +5135,7 @@
> }
> hnew->SetBinContent(bin,binContent);
> if (oldErrors) hnew->SetBinError(bin,TMath::Sqrt(binError));
> +
> oldbin += imax;
> }
> hnew->SetBinContent(0,oldBins[0]);
>
Received on Tue Feb 06 2007 - 08:29:09 CET