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] fix regression in AddBinContentND and add test #17573

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions hist/hist/inc/TH2.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ class TH2 : public TH1 {
using TH1::AddBinContent;
/// Increment 2D bin content by 1.
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t binx, Int_t biny) { AddBinContent(GetBin(binx, biny)); }
virtual void AddBinContent(Int_t binx, Int_t biny) = 0;
/// Increment 2D bin content by a weight w.
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t binx, Int_t biny, Double_t w) { AddBinContent(GetBin(binx, biny), w); }
virtual void AddBinContent(Int_t binx, Int_t biny, Double_t w) = 0;
Int_t BufferEmpty(Int_t action=0) override;
void Copy(TObject &hnew) const override;
Int_t Fill(Double_t x, Double_t y) override;
Expand Down Expand Up @@ -159,6 +159,8 @@ class TH2C : public TH2, public TArrayC {

void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny) override { AddBinContent(GetBin(binx, biny)); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Double_t w) override { AddBinContent(GetBin(binx, biny), w); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -200,6 +202,8 @@ class TH2S : public TH2, public TArrayS {

void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny) override { AddBinContent(GetBin(binx, biny)); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Double_t w) override { AddBinContent(GetBin(binx, biny), w); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -241,6 +245,8 @@ class TH2I : public TH2, public TArrayI {

void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny) override { AddBinContent(GetBin(binx, biny)); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Double_t w) override { AddBinContent(GetBin(binx, biny), w); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -280,6 +286,8 @@ class TH2L : public TH2, public TArrayL64 {
~TH2L() override;
void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny) override { AddBinContent(GetBin(binx, biny)); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Double_t w) override { AddBinContent(GetBin(binx, biny), w); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -326,6 +334,8 @@ class TH2F : public TH2, public TArrayF {
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t bin, Double_t w) override
{fArray[bin] += Float_t (w);}
void AddBinContent(Int_t binx, Int_t biny) override { AddBinContent(GetBin(binx, biny)); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Double_t w) override { AddBinContent(GetBin(binx, biny), w); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -373,6 +383,8 @@ class TH2D : public TH2, public TArrayD {
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t bin, Double_t w) override
{fArray[bin] += Double_t (w);}
void AddBinContent(Int_t binx, Int_t biny) override { AddBinContent(GetBin(binx, biny)); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Double_t w) override { AddBinContent(GetBin(binx, biny), w); } ///< \copydoc TH2::AddBinContent(Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down
6 changes: 4 additions & 2 deletions hist/hist/inc/TH2Poly.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ class TH2Poly : public TH2 {

//functions not to be used for TH2Poly

void AddBinContent(Int_t) override; ///< NOT IMPLEMENTED for TH2Poly
void AddBinContent(Int_t, Double_t) override; ///< NOT IMPLEMENTED for TH2Poly
void AddBinContent(Int_t) override; ///< NOT IMPLEMENTED for TH2Poly
void AddBinContent(Int_t, Double_t) override; ///< NOT IMPLEMENTED for TH2Poly
void AddBinContent(Int_t, Int_t) override; ///< NOT IMPLEMENTED for TH2Poly
void AddBinContent(Int_t, Int_t, Double_t) override; ///< NOT IMPLEMENTED for TH2Poly

Int_t Fill(Double_t) override{return -1;} ///< NOT IMPLEMENTED for TH2Poly
Int_t Fill(Double_t , const char *, Double_t) override{return -1;} ///< NOT IMPLEMENTED for TH2Poly
Expand Down
16 changes: 14 additions & 2 deletions hist/hist/inc/TH3.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ class TH3 : public TH1, public TAtt3D {
using TH1::AddBinContent;
/// Increment 3D bin content by 1.
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) { AddBinContent(GetBin(binx, biny, binz)); }
virtual void AddBinContent(Int_t binx, Int_t biny, Int_t binz) = 0;
/// Increment 3D bin content by a weight w.
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) { AddBinContent(GetBin(binx, biny, binz), w); }
virtual void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) = 0;
Int_t BufferEmpty(Int_t action = 0) override;
void Copy(TObject &hnew) const override;
virtual Int_t Fill(Double_t x, Double_t y, Double_t z);
Expand Down Expand Up @@ -175,6 +175,8 @@ class TH3C : public TH3, public TArrayC {

void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) override { AddBinContent(GetBin(binx, biny, binz)); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) override { AddBinContent(GetBin(binx, biny, binz), w); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -213,6 +215,8 @@ class TH3S : public TH3, public TArrayS {

void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) override { AddBinContent(GetBin(binx, biny, binz)); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) override { AddBinContent(GetBin(binx, biny, binz), w); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -251,6 +255,8 @@ class TH3I : public TH3, public TArrayI {

void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) override { AddBinContent(GetBin(binx, biny, binz)); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) override { AddBinContent(GetBin(binx, biny, binz), w); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -289,6 +295,8 @@ class TH3L : public TH3, public TArrayL64 {
~TH3L() override;
void AddBinContent(Int_t bin) override;
void AddBinContent(Int_t bin, Double_t w) override;
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) override { AddBinContent(GetBin(binx, biny, binz)); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) override { AddBinContent(GetBin(binx, biny, binz), w); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -333,6 +341,8 @@ class TH3F : public TH3, public TArrayF {
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t bin, Double_t w) override
{fArray[bin] += Float_t (w);}
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) override { AddBinContent(GetBin(binx, biny, binz)); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) override { AddBinContent(GetBin(binx, biny, binz), w); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down Expand Up @@ -376,6 +386,8 @@ class TH3D : public TH3, public TArrayD {
/// Passing an out-of-range bin leads to undefined behavior
void AddBinContent(Int_t bin, Double_t w) override
{fArray[bin] += Double_t (w);}
void AddBinContent(Int_t binx, Int_t biny, Int_t binz) override { AddBinContent(GetBin(binx, biny, binz)); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t)
void AddBinContent(Int_t binx, Int_t biny, Int_t binz, Double_t w) override { AddBinContent(GetBin(binx, biny, binz), w); } ///< \copydoc TH3::AddBinContent(Int_t,Int_t,Int_t,Double_t)
void Copy(TObject &hnew) const override;
void Reset(Option_t *option="") override;
void SetBinsLength(Int_t n=-1) override;
Expand Down
12 changes: 12 additions & 0 deletions hist/hist/src/TH2Poly.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1750,3 +1750,15 @@ void TH2Poly::AddBinContent(Int_t, Double_t)
{
Error("AddBinContent", "Not implemented for TH2Poly");
}
////////////////////////////////////////////////////////////////////////////////
/// NOT IMPLEMENTED for TH2Poly
void TH2Poly::AddBinContent(Int_t, Int_t)
{
Error("AddBinContent", "Not implemented for TH2Poly");
}
////////////////////////////////////////////////////////////////////////////////
/// NOT IMPLEMENTED for TH2Poly
void TH2Poly::AddBinContent(Int_t, Int_t, Double_t)
{
Error("AddBinContent", "Not implemented for TH2Poly");
}
16 changes: 16 additions & 0 deletions hist/hist/test/test_TH1.cxx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "gtest/gtest.h"

#include "TH1.h"
#include "TH2.h"
#include "TH3.h"
#include "TH1F.h"
#include "THLimitsFinder.h"

Expand Down Expand Up @@ -75,3 +77,17 @@ TEST(TH1, DumpOutput)
const std::string output = testing::internal::GetCapturedStdout();
EXPECT_TRUE(output.find(line_fArray) != std::string::npos) << "Could not find '" << line_fArray << "' in the multiline output '" << output;
}

// https://github.com/root-project/root/issues/17552
TEST(TH1, AddBinContent)
{
TH1F h1("h1", "h1", 10, 0, 1);
h1.AddBinContent(1,1.);
EXPECT_FLOAT_EQ(h1.GetBinContent(1),1.);
TH2F h2("h2", "h2", 10, 0, 1, 2, 0, 3);
h2.AddBinContent(1,1,1.);
EXPECT_FLOAT_EQ(h2.GetBinContent(1,1),1.);
TH3F h3("h3", "h3", 5, 0, 1, 2, 0, 2, 2, 0, 3);;
h3.AddBinContent(1,1,1,1.);
EXPECT_FLOAT_EQ(h3.GetBinContent(1,1,1),1.);
}
Loading