From a904970e984bc097adc444be0180e4480b10807b Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Thu, 20 Feb 2025 18:09:05 +0100 Subject: [PATCH 1/4] [tree] Respect TChain's entryList when GetMinimum/Maximum Fixes https://its.cern.ch/jira/browse/ROOT-7067 Fixes https://its.cern.ch/jira/browse/ROOT-8505 --- tree/tree/src/TChain.cxx | 83 ++++++++++++++++++++++++---- tree/tree/test/TChainRegressions.cxx | 49 +++++++++++++++- 2 files changed, 119 insertions(+), 13 deletions(-) diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 3cdae7fd8ceeb..2f0f632a00ff8 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -30,6 +30,7 @@ the trees in the chain. #include #include #include +#include #include "TBranch.h" #include "TBrowser.h" @@ -66,6 +67,26 @@ the trees in the chain. ClassImp(TChain); +/** + * @brief Helper function getting a map between treeNumber (key) defined in an entryList, and the position of the latter in the collection (value). + * Only those entry lists that are active (treeNumber != -1) are used to fill the map. + * @param elist the TChain's TEntryList + * @return map of int to int + */ + std::map GetEntryListMap(const TEntryList* elist) + { + const auto *elists = elist->GetLists(); + const auto ne = elists->GetEntries(); + std::map map_elists; + for (Int_t e = 0; e < ne; ++e) { + auto el = static_cast(elists->At(e)); + if (el && el->GetTreeNumber()>=0) { + map_elists[el->GetTreeNumber()] = e; + } + } + return map_elists; + } + //////////////////////////////////////////////////////////////////////////////// /// Default constructor. @@ -1169,12 +1190,31 @@ TObjArray* TChain::GetListOfLeaves() Double_t TChain::GetMaximum(const char* columname) { Double_t theMax = -DBL_MAX; - for (Int_t file = 0; file < fNtrees; file++) { - Long64_t first = fTreeOffset[file]; - LoadTree(first); - Double_t curmax = fTree->GetMaximum(columname); - if (curmax > theMax) { - theMax = curmax; + auto elist = GetEntryList(); + if (!elist || !elist->GetLists()) { + for (Int_t file = 0; file < fNtrees; file++) { + Long64_t first = fTreeOffset[file]; + LoadTree(first); + Double_t curmax = fTree->GetMaximum(columname); + if (curmax > theMax) { + theMax = curmax; + } + } + } + else { + auto map_elists = GetEntryListMap(elist); + for (Int_t file = 0; file < fNtrees; file++) { + Long64_t first = fTreeOffset[file]; + LoadTree(first); + const auto prev = fTree->GetEntryList(); + if (map_elists.find(file) != map_elists.end()) { + fTree->SetEntryList(static_cast(elist->GetLists()->At(map_elists[file]))); + } + Double_t curmax = fTree->GetMaximum(columname); + fTree->SetEntryList(prev); + if (curmax > theMax) { + theMax = curmax; + } } } return theMax; @@ -1186,12 +1226,31 @@ Double_t TChain::GetMaximum(const char* columname) Double_t TChain::GetMinimum(const char* columname) { Double_t theMin = DBL_MAX; - for (Int_t file = 0; file < fNtrees; file++) { - Long64_t first = fTreeOffset[file]; - LoadTree(first); - Double_t curmin = fTree->GetMinimum(columname); - if (curmin < theMin) { - theMin = curmin; + auto elist = GetEntryList(); + if (!elist || !elist->GetLists()) { + for (Int_t file = 0; file < fNtrees; file++) { + Long64_t first = fTreeOffset[file]; + LoadTree(first); + Double_t curmin = fTree->GetMinimum(columname); + if (curmin < theMin) { + theMin = curmin; + } + } + } + else { + auto map_elists = GetEntryListMap(elist); + for (Int_t file = 0; file < fNtrees; file++) { + Long64_t first = fTreeOffset[file]; + LoadTree(first); + const auto prev = fTree->GetEntryList(); + if (map_elists.find(file) != map_elists.end()) { + fTree->SetEntryList(static_cast(elist->GetLists()->At(map_elists[file]))); + } + Double_t curmin = fTree->GetMinimum(columname); + fTree->SetEntryList(prev); + if (curmin < theMin) { + theMin = curmin; + } } } return theMin; diff --git a/tree/tree/test/TChainRegressions.cxx b/tree/tree/test/TChainRegressions.cxx index 7af610f6ad722..fa0660ccac780 100644 --- a/tree/tree/test/TChainRegressions.cxx +++ b/tree/tree/test/TChainRegressions.cxx @@ -2,7 +2,9 @@ #include #include #include - +#include +#include + #include "gtest/gtest.h" class TTreeCache; @@ -30,3 +32,48 @@ TEST(TChain, GetReadCacheBug) gSystem->Unlink(filename); } + +// ROOT-7097, ROOT-8505 +TEST(TChain, GetMinMaxEntryList) +{ + std::unique_ptr file1(TFile::Open("t1_7067.root", "RECREATE")); + TTree t1("t", ""); + int value; + t1.Branch("value", &value); + value = 0; + t1.Fill(); + value = 1; + t1.Fill(); + value = 2; + t1.Fill(); + value = 3; + t1.Fill(); + file1->Write(); + file1->Close(); + + std::unique_ptr file2(TFile::Open("t2_7067.root", "RECREATE")); + TTree t2("t", ""); + //int value; + t2.Branch("value", &value); + value = 10; + t2.Fill(); + value = 11; + t2.Fill(); + value = 12; + t2.Fill(); + value = 13; + t2.Fill(); + file2->Write(); + file2->Close(); + + TChain ch("t"); + ch.AddFile("t1_7067.root"); + ch.AddFile("t2_7067.root"); + EXPECT_FLOAT_EQ(ch.GetMinimum("value"), 0.); + EXPECT_FLOAT_EQ(ch.GetMaximum("value"), 13.); + ch.Draw(">>myList", "value<11 && value >1", "entrylist"); + TEntryList* myList = static_cast(gDirectory->Get("myList")); + ch.SetEntryList(myList); + EXPECT_FLOAT_EQ(ch.GetMinimum("value"), 2.); + EXPECT_FLOAT_EQ(ch.GetMaximum("value"), 10.); +} \ No newline at end of file From 742a64c72cb9193402448a06744c8745eae92fb5 Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Thu, 20 Feb 2025 18:20:13 +0100 Subject: [PATCH 2/4] [nfc] clang-format --- tree/tree/src/TChain.cxx | 20 +++++++++----------- tree/tree/test/TChainRegressions.cxx | 4 ++-- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 2f0f632a00ff8..40b8fcddde219 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -68,19 +68,19 @@ the trees in the chain. ClassImp(TChain); /** - * @brief Helper function getting a map between treeNumber (key) defined in an entryList, and the position of the latter in the collection (value). - * Only those entry lists that are active (treeNumber != -1) are used to fill the map. + * @brief Helper function getting a map between treeNumber (key) defined in an entryList, and the position of the latter + * in the collection (value). Only those entry lists that are active (treeNumber != -1) are used to fill the map. * @param elist the TChain's TEntryList * @return map of int to int */ - std::map GetEntryListMap(const TEntryList* elist) + std::map GetEntryListMap(const TEntryList *elist) { const auto *elists = elist->GetLists(); const auto ne = elists->GetEntries(); std::map map_elists; for (Int_t e = 0; e < ne; ++e) { - auto el = static_cast(elists->At(e)); - if (el && el->GetTreeNumber()>=0) { + auto el = static_cast(elists->At(e)); + if (el && el->GetTreeNumber() >= 0) { map_elists[el->GetTreeNumber()] = e; } } @@ -1200,15 +1200,14 @@ Double_t TChain::GetMaximum(const char* columname) theMax = curmax; } } - } - else { + } else { auto map_elists = GetEntryListMap(elist); for (Int_t file = 0; file < fNtrees; file++) { Long64_t first = fTreeOffset[file]; LoadTree(first); const auto prev = fTree->GetEntryList(); if (map_elists.find(file) != map_elists.end()) { - fTree->SetEntryList(static_cast(elist->GetLists()->At(map_elists[file]))); + fTree->SetEntryList(static_cast(elist->GetLists()->At(map_elists[file]))); } Double_t curmax = fTree->GetMaximum(columname); fTree->SetEntryList(prev); @@ -1236,15 +1235,14 @@ Double_t TChain::GetMinimum(const char* columname) theMin = curmin; } } - } - else { + } else { auto map_elists = GetEntryListMap(elist); for (Int_t file = 0; file < fNtrees; file++) { Long64_t first = fTreeOffset[file]; LoadTree(first); const auto prev = fTree->GetEntryList(); if (map_elists.find(file) != map_elists.end()) { - fTree->SetEntryList(static_cast(elist->GetLists()->At(map_elists[file]))); + fTree->SetEntryList(static_cast(elist->GetLists()->At(map_elists[file]))); } Double_t curmin = fTree->GetMinimum(columname); fTree->SetEntryList(prev); diff --git a/tree/tree/test/TChainRegressions.cxx b/tree/tree/test/TChainRegressions.cxx index fa0660ccac780..562378deebf75 100644 --- a/tree/tree/test/TChainRegressions.cxx +++ b/tree/tree/test/TChainRegressions.cxx @@ -53,7 +53,7 @@ TEST(TChain, GetMinMaxEntryList) std::unique_ptr file2(TFile::Open("t2_7067.root", "RECREATE")); TTree t2("t", ""); - //int value; + // int value; t2.Branch("value", &value); value = 10; t2.Fill(); @@ -72,7 +72,7 @@ TEST(TChain, GetMinMaxEntryList) EXPECT_FLOAT_EQ(ch.GetMinimum("value"), 0.); EXPECT_FLOAT_EQ(ch.GetMaximum("value"), 13.); ch.Draw(">>myList", "value<11 && value >1", "entrylist"); - TEntryList* myList = static_cast(gDirectory->Get("myList")); + TEntryList* myList = static_cast(gDirectory->Get("myList")); ch.SetEntryList(myList); EXPECT_FLOAT_EQ(ch.GetMinimum("value"), 2.); EXPECT_FLOAT_EQ(ch.GetMaximum("value"), 10.); From 7dff410de758b056e42440e6291377dc4744f5fa Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Thu, 20 Feb 2025 18:21:15 +0100 Subject: [PATCH 3/4] [nfc] leading whitespace clangformat --- tree/tree/src/TChain.cxx | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 40b8fcddde219..bad3c3f229e78 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -73,19 +73,19 @@ ClassImp(TChain); * @param elist the TChain's TEntryList * @return map of int to int */ - std::map GetEntryListMap(const TEntryList *elist) - { - const auto *elists = elist->GetLists(); - const auto ne = elists->GetEntries(); - std::map map_elists; - for (Int_t e = 0; e < ne; ++e) { - auto el = static_cast(elists->At(e)); - if (el && el->GetTreeNumber() >= 0) { - map_elists[el->GetTreeNumber()] = e; - } - } - return map_elists; - } +std::map GetEntryListMap(const TEntryList *elist) +{ + const auto *elists = elist->GetLists(); + const auto ne = elists->GetEntries(); + std::map map_elists; + for (Int_t e = 0; e < ne; ++e) { + auto el = static_cast(elists->At(e)); + if (el && el->GetTreeNumber() >= 0) { + map_elists[el->GetTreeNumber()] = e; + } + } + return map_elists; +} //////////////////////////////////////////////////////////////////////////////// /// Default constructor. From ea5938c541cb1812f27d0236588ec1f9036b90f4 Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Thu, 20 Feb 2025 18:23:01 +0100 Subject: [PATCH 4/4] [nfc] clangf --- tree/tree/test/TChainRegressions.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/tree/test/TChainRegressions.cxx b/tree/tree/test/TChainRegressions.cxx index 562378deebf75..9c051183d05e2 100644 --- a/tree/tree/test/TChainRegressions.cxx +++ b/tree/tree/test/TChainRegressions.cxx @@ -72,7 +72,7 @@ TEST(TChain, GetMinMaxEntryList) EXPECT_FLOAT_EQ(ch.GetMinimum("value"), 0.); EXPECT_FLOAT_EQ(ch.GetMaximum("value"), 13.); ch.Draw(">>myList", "value<11 && value >1", "entrylist"); - TEntryList* myList = static_cast(gDirectory->Get("myList")); + TEntryList *myList = static_cast(gDirectory->Get("myList")); ch.SetEntryList(myList); EXPECT_FLOAT_EQ(ch.GetMinimum("value"), 2.); EXPECT_FLOAT_EQ(ch.GetMaximum("value"), 10.);