diff --git a/tree/tree/inc/TChain.h b/tree/tree/inc/TChain.h index d90971763f836..6773a3732e746 100644 --- a/tree/tree/inc/TChain.h +++ b/tree/tree/inc/TChain.h @@ -115,8 +115,6 @@ class TChain : public TTree { TObjArray *GetListOfFiles() const {return fFiles;} TObjArray *GetListOfLeaves() override; const char *GetAlias(const char *aliasName) const override; - Double_t GetMaximum(const char *columname) override; - Double_t GetMinimum(const char *columname) override; Int_t GetNbranches() override; Long64_t GetReadEntry() const override; TList *GetStatus() const { return fStatus; } diff --git a/tree/tree/inc/TTree.h b/tree/tree/inc/TTree.h index 006b7b9fc7f3b..d6fce55392fc7 100644 --- a/tree/tree/inc/TTree.h +++ b/tree/tree/inc/TTree.h @@ -185,6 +185,9 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker Int_t SetBranchAddressImp(const char *bname, void *add, TBranch **ptr, TClass *realClass, EDataType datatype, bool isptr); + // Helper method to factorize the processing logic for GetMinium, GetMaximum + double ComputeExtremum(const char *columname, double errVal, bool (*cmp)(double, double)); + protected: friend TBranch *ROOT::Internal::TreeUtils::CallBranchImpRef(TTree &tree, const char *branchname, TClass *ptrClass, EDataType datatype, void *addobj, Int_t bufsize, @@ -200,8 +203,7 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker virtual Int_t CheckBranchAddressType(TBranch* branch, TClass* ptrClass, EDataType datatype, bool ptr); virtual TBranch *BronchExec(const char* name, const char* classname, void* addobj, bool isptrptr, Int_t bufsize, Int_t splitlevel); friend TBranch *TTreeBranchImpRef(TTree *tree, const char* branchname, TClass* ptrClass, EDataType datatype, void* addobj, Int_t bufsize, Int_t splitlevel); - Int_t SetBranchAddressImp(TBranch *branch, void* addr, TBranch** ptr); - virtual TLeaf *GetLeafImpl(const char* branchname, const char* leafname); + Int_t SetBranchAddressImp(TBranch *branch, void *addr, TBranch **ptr); Long64_t GetCacheAutoSize(bool withDefault = false); char GetNewlineValue(std::istream &inputStream); @@ -218,6 +220,12 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker virtual Int_t SetBranchAddress(const char *bname, void *add, TBranch **ptr, TClass *realClass, EDataType datatype, bool isptr, bool suppressMissingBranchError); + // Group of methods to help better separating logic used in GetLeaf + TBranch *FindBranchFromSelf(const char *branchName); + TBranch *FindBranchFromFriends(const char *branchName); + TLeaf *SearchLeafInListOfLeaves(const char *branchName, const char *leafName); + TLeaf *SearchLeafInListOfFriends(const char *branchName, const char *leafName); + class TFriendLock { // Helper class to prevent infinite recursion in the // usage of TTree Friends. Implemented in TTree.cxx. diff --git a/tree/tree/src/TChain.cxx b/tree/tree/src/TChain.cxx index 88da9fcc421c6..134a468afba82 100644 --- a/tree/tree/src/TChain.cxx +++ b/tree/tree/src/TChain.cxx @@ -1134,76 +1134,6 @@ TObjArray* TChain::GetListOfLeaves() return nullptr; } -//////////////////////////////////////////////////////////////////////////////// -/// Return maximum of column with name columname. - -Double_t TChain::GetMaximum(const char* columname) -{ - Double_t cmax = -DBL_MAX; - TLeaf *leaf = nullptr; - TBranch *branch = nullptr; - Int_t treenumber = -1; - for (Long64_t i = 0; i < fEntries; ++i) { - Long64_t entryNumber = this->GetEntryNumber(i); - if (entryNumber < 0) - break; - Long64_t localEntryNumber = this->LoadTree(entryNumber); - if (localEntryNumber < 0) - break; - if (treenumber != this->GetTreeNumber()) { - leaf = this->GetLeaf(columname); - if (leaf) - branch = leaf->GetBranch(); - } - treenumber = this->GetTreeNumber(); - if (!branch) - continue; - branch->GetEntry(localEntryNumber); - for (Int_t j = 0; j < leaf->GetLen(); ++j) { - Double_t val = leaf->GetValue(j); - if (val > cmax) { - cmax = val; - } - } - } - return cmax; -} - -//////////////////////////////////////////////////////////////////////////////// -/// Return minimum of column with name columname. - -Double_t TChain::GetMinimum(const char* columname) -{ - Double_t cmin = DBL_MAX; - TLeaf *leaf = nullptr; - TBranch *branch = nullptr; - Int_t treenumber = -1; - for (Long64_t i = 0; i < fEntries; ++i) { - Long64_t entryNumber = this->GetEntryNumber(i); - if (entryNumber < 0) - break; - Long64_t localEntryNumber = this->LoadTree(entryNumber); - if (localEntryNumber < 0) - break; - if (treenumber != this->GetTreeNumber()) { - leaf = this->GetLeaf(columname); - if (leaf) - branch = leaf->GetBranch(); - } - treenumber = this->GetTreeNumber(); - if (!branch) - continue; - branch->GetEntry(localEntryNumber); - for (Int_t j = 0; j < leaf->GetLen(); ++j) { - Double_t val = leaf->GetValue(j); - if (val < cmin) { - cmin = val; - } - } - } - return cmin; -} - //////////////////////////////////////////////////////////////////////////////// /// Return the number of branches of the current tree. /// diff --git a/tree/tree/src/TTree.cxx b/tree/tree/src/TTree.cxx index 26ab8bdb4f33d..a9778d501ed3b 100644 --- a/tree/tree/src/TTree.cxx +++ b/tree/tree/src/TTree.cxx @@ -4882,64 +4882,45 @@ static TBranch *R__FindBranchHelper(TObjArray *list, const char *branchname) { return nullptr; } -//////////////////////////////////////////////////////////////////////////////// -/// Return the branch that correspond to the path 'branchname', which can -/// include the name of the tree or the omitted name of the parent branches. -/// In case of ambiguity, returns the first match. -/// \sa TTree::GetBranch - -TBranch* TTree::FindBranch(const char* branchname) +TBranch *TTree::FindBranchFromSelf(const char *branchName) { - // We already have been visited while recursively looking - // through the friends tree, let return - if (kFindBranch & fFriendLockStatus) { - return nullptr; - } - - if (!branchname) - return nullptr; - - TBranch* branch = nullptr; // If the first part of the name match the TTree name, look for the right part in the - // list of branches. - // This will allow the branchname to be preceded by - // the name of this tree. - if (strncmp(fName.Data(),branchname,fName.Length())==0 && branchname[fName.Length()]=='.') { - branch = R__FindBranchHelper( GetListOfBranches(), branchname + fName.Length() + 1); - if (branch) return branch; - } + // list of branches. This will allow the branchName to be preceded by the name of this tree. + if (strncmp(fName.Data(), branchName, fName.Length()) == 0 && branchName[fName.Length()] == '.') + if (auto *br = R__FindBranchHelper(GetListOfBranches(), branchName + fName.Length() + 1)) + return br; + // If we did not find it, let's try to find the full name in the list of branches. - branch = R__FindBranchHelper(GetListOfBranches(), branchname); - if (branch) return branch; + if (auto *br = R__FindBranchHelper(GetListOfBranches(), branchName)) + return br; - // If we still did not find, let's try to find it within each branch assuming it does not the branch name. - TIter next(GetListOfBranches()); - while ((branch = (TBranch*) next())) { - TBranch* nestedbranch = branch->FindBranch(branchname); - if (nestedbranch) { + // If we still did not find, let's try to find it within each branch assuming it does not contain the branch name. + for (auto *branch : ROOT::Detail::TRangeStaticCast(*GetListOfBranches())) + if (auto *nestedbranch = branch->FindBranch(branchName)) return nestedbranch; - } - } - // Search in list of friends. + return nullptr; +} + +TBranch *TTree::FindBranchFromFriends(const char *branchName) +{ if (!fFriends) { return nullptr; } + TFriendLock lock(this, kFindBranch); - TIter nextf(fFriends); - TFriendElement* fe = nullptr; - while ((fe = (TFriendElement*) nextf())) { - TTree* t = fe->GetTree(); + for (auto *frEl : ROOT::Detail::TRangeStaticCast(*fFriends)) { + TTree *t = frEl->GetTree(); if (!t) { continue; } // If the alias is present replace it with the real name. - const char *subbranch = strstr(branchname, fe->GetName()); - if (subbranch != branchname) { + const char *subbranch = strstr(branchName, frEl->GetName()); + if (subbranch != branchName) { subbranch = nullptr; } if (subbranch) { - subbranch += strlen(fe->GetName()); + subbranch += strlen(frEl->GetName()); if (*subbranch != '.') { subbranch = nullptr; } else { @@ -4950,13 +4931,38 @@ TBranch* TTree::FindBranch(const char* branchname) if (subbranch) { name << t->GetName() << "." << subbranch; } else { - name << branchname; - } - branch = t->FindBranch(name.str().c_str()); - if (branch) { - return branch; + name << branchName; } + if (auto *br = t->FindBranch(name.str().c_str())) + return br; + } + + return nullptr; +} + +//////////////////////////////////////////////////////////////////////////////// +/// Return the branch that correspond to the path 'branchname', which can +/// include the name of the tree or the omitted name of the parent branches. +/// In case of ambiguity, returns the first match. +/// \sa TTree::GetBranch + +TBranch *TTree::FindBranch(const char *branchname) +{ + // We already have been visited while recursively looking + // through the friends tree, let return + if (kFindBranch & fFriendLockStatus) { + return nullptr; } + + if (!branchname) + return nullptr; + + if (auto *br = FindBranchFromSelf(branchname)) + return br; + + if (auto *br = FindBranchFromFriends(branchname)) + return br; + return nullptr; } @@ -6196,51 +6202,30 @@ TIterator* TTree::GetIteratorOnAllLeaves(bool dir) return new TTreeFriendLeafIter(this, dir); } -//////////////////////////////////////////////////////////////////////////////// -/// Return pointer to the 1st Leaf named name in any Branch of this -/// Tree or any branch in the list of friend trees. -/// -/// The leaf name can contain the name of a friend tree with the -/// syntax: friend_dir_and_tree.full_leaf_name -/// the friend_dir_and_tree can be of the form: -/// ~~~ {.cpp} -/// TDirectoryName/TreeName -/// ~~~ - -TLeaf* TTree::GetLeafImpl(const char* branchname, const char *leafname) +TLeaf *TTree::SearchLeafInListOfLeaves(const char *branchName, const char *leafName) { - TLeaf *leaf = nullptr; - if (branchname) { - TBranch *branch = FindBranch(branchname); - if (branch) { - leaf = branch->GetLeaf(leafname); - if (leaf) { - return leaf; - } - } - } - TIter nextl(GetListOfLeaves()); - while ((leaf = (TLeaf*)nextl())) { - if (strcmp(leaf->GetFullName(), leafname) != 0 && strcmp(leaf->GetName(), leafname) != 0) - continue; // leafname does not match GetName() nor GetFullName(), this is not the right leaf - if (branchname) { - // check the branchname is also a match + for (auto *leaf : ROOT::Detail::TRangeStaticCast(*GetListOfLeaves())) { + if (strcmp(leaf->GetFullName(), leafName) != 0 && strcmp(leaf->GetName(), leafName) != 0) + continue; // leafName does not match GetName() nor GetFullName(), this is not the right leaf + if (branchName) { + // check the branchName is also a match TBranch *br = leaf->GetBranch(); // if a quick comparison with the branch full name is a match, we are done - if (!strcmp(br->GetFullName(), branchname)) + if (!strcmp(br->GetFullName(), branchName)) return leaf; - UInt_t nbch = strlen(branchname); + UInt_t nbch = strlen(branchName); const char* brname = br->GetName(); TBranch *mother = br->GetMother(); - if (strncmp(brname,branchname,nbch)) { + if (strncmp(brname, branchName, nbch)) { if (mother != br) { const char *mothername = mother->GetName(); UInt_t motherlen = strlen(mothername); - if (!strcmp(mothername, branchname)) { + if (!strcmp(mothername, branchName)) { return leaf; - } else if (nbch > motherlen && strncmp(mothername,branchname,motherlen)==0 && (mothername[motherlen-1]=='.' || branchname[motherlen]=='.')) { + } else if (nbch > motherlen && strncmp(mothername, branchName, motherlen) == 0 && + (mothername[motherlen - 1] == '.' || branchName[motherlen] == '.')) { // The left part of the requested name match the name of the mother, let's see if the right part match the name of the branch. - if (strncmp(brname,branchname+motherlen+1,nbch-motherlen-1)) { + if (strncmp(brname, branchName + motherlen + 1, nbch - motherlen - 1)) { // No it does not continue; } // else we have match so we can proceed. @@ -6262,48 +6247,61 @@ TLeaf* TTree::GetLeafImpl(const char* branchname, const char *leafname) } return leaf; } + + return nullptr; +} + +TLeaf *TTree::SearchLeafInListOfFriends(const char *branchName, const char *leafName) +{ if (!fFriends) return nullptr; - TFriendLock lock(this,kGetLeaf); - TIter next(fFriends); - TFriendElement *fe; - while ((fe = (TFriendElement*)next())) { - TTree *t = fe->GetTree(); - if (t) { - leaf = t->GetLeaf(branchname, leafname); - if (leaf) return leaf; - } - } + // The corresponding check is in GetLeaf + TFriendLock lock(this, kGetLeaf); - //second pass in the list of friends when the leaf name - //is prefixed by the tree name + for (auto *frEl : ROOT::Detail::TRangeStaticCast(*fFriends)) + if (auto *t = frEl->GetTree()) + if (auto *leaf = t->GetLeaf(branchName, leafName)) + return leaf; + + // Second pass in the list of friends when the leaf name is prefixed by the tree name TString strippedArg; - next.Reset(); - while ((fe = (TFriendElement*)next())) { - TTree *t = fe->GetTree(); + for (auto *frEl : ROOT::Detail::TRangeStaticCast(*fFriends)) { + TTree *t = frEl->GetTree(); if (!t) continue; - const char *subname = strstr(leafname,fe->GetName()); - if (subname != leafname) continue; - Int_t l = strlen(fe->GetName()); - subname += l; - if (*subname != '.') continue; - subname++; - strippedArg += subname; - leaf = t->GetLeaf(branchname,subname); - if (leaf) return leaf; + const char *subLeafName = strstr(leafName, frEl->GetName()); + if (subLeafName != leafName) + continue; + Int_t l = strlen(frEl->GetName()); + subLeafName += l; + if (*subLeafName != '.') + continue; + subLeafName++; + strippedArg += subLeafName; + if (auto *leaf = t->GetLeaf(branchName, subLeafName)) + return leaf; } + return nullptr; } //////////////////////////////////////////////////////////////////////////////// -/// Return pointer to the 1st Leaf named name in any Branch of this -/// Tree or any branch in the list of friend trees. +/// Searches in this tree and any of its friends for a leaf named \p leafname in branch \p branchname , returns first +/// match or nullptr if no match. /// -/// The leaf name can contain the name of a friend tree with the -/// syntax: friend_dir_and_tree.full_leaf_name -/// the friend_dir_and_tree can be of the form: +/// Search order: /// -/// TDirectoryName/TreeName - +/// 1. Look for a \p branchname match (via FindBranch(branchname)): +/// a. In the list of branches of this tree +/// b. Recursively in nested branches of each branch of this tree +/// c. In the friends of this tree +/// 2. Look for matching \p branchname and \p leafname in list of leaves of this tree +/// 3. Look for matching \p branchname and \p leafname in friends of this tree (eventually calling GetLeaf on each +/// friend) +/// +/// \note \p branchname can be an empty string, in which case the function will return the first leaf with matching +/// \p leafname in any branch of this tree or any of its friends following the search order above. +/// +/// \note \p leafname can contain the name of a friend tree with the syntax: `friend_dir_and_tree.full_leaf_name`. In +/// particular, `friend_dir_and_tree` can be of the form `TDirectoryName/TreeName`. TLeaf* TTree::GetLeaf(const char* branchname, const char *leafname) { if (leafname == nullptr) return nullptr; @@ -6314,16 +6312,27 @@ TLeaf* TTree::GetLeaf(const char* branchname, const char *leafname) return nullptr; } - return GetLeafImpl(branchname,leafname); + if (auto *br = FindBranch(branchname)) + if (auto leaf = br->GetLeaf(leafname)) + return leaf; + + if (auto *leaf = SearchLeafInListOfLeaves(branchname, leafname)) + return leaf; + + if (auto *leaf = SearchLeafInListOfFriends(branchname, leafname)) + return leaf; + + return nullptr; } //////////////////////////////////////////////////////////////////////////////// -/// Return pointer to first leaf named "name" in any branch of this -/// tree or its friend trees. +/// Searches in this tree and any of its friends for a leaf named \p leafname , returns first leaf matching in any +/// branch. /// -/// \param[in] name may be in the form 'branch/leaf' +/// See TTree::GetLeaf(const char* branchname, const char *leafname) for a description of the search order. +/// +/// \note \p name may be in the form `branch/leaf` /// - TLeaf* TTree::GetLeaf(const char *name) { // Return nullptr if name is invalid or if we have @@ -6334,41 +6343,176 @@ TLeaf* TTree::GetLeaf(const char *name) std::string path(name); const auto sep = path.find_last_of('/'); if (sep != std::string::npos) - return GetLeafImpl(path.substr(0, sep).c_str(), name+sep+1); + return GetLeaf(path.substr(0, sep).c_str(), name + sep + 1); - return GetLeafImpl(nullptr, name); + return GetLeaf(nullptr, name); } +namespace { + //////////////////////////////////////////////////////////////////////////////// -/// Return maximum of column with name columname. -/// if the Tree has an associated TEventList or TEntryList, the maximum -/// is computed for the entries in this list. +/// \brief Helper detecting *any* file transition of a tree dataset +/// +/// This is a generic helper, works if the dataset is a TTree or a TChain, and +/// transitively detects transitions in friends. +/// +/// Comparing `TChain::GetTreeNumber()` before and after a call to +/// `TChain::LoadTree` only detects that the chain itself switched to another of +/// its own sub-trees. It does *not* detect that one of the (possibly indirect) +/// friends of the chain switched to a new file: in that case the cached +/// TLeaf/TBranch pointers become dangling even though the tree number of the +/// chain is unchanged. +/// +/// `TChain::LoadTree` (both when the chain itself moves to a new tree and, via +/// `TChain::RefreshFriendAddresses`, when only a friend was updated) calls +/// `fNotify->Notify()`. Subscribing to that notification is therefore the +/// reliable way to know that anything in the friend graph moved. +/// +/// This derives directly from TNotifyLinkBase rather than using TNotifyLink +/// because the latter would require a dictionary for the instantiation. +/// +/// We could also use +/// ``` +/// struct TLeafRefresher { +/// bool fDirty = true; +/// bool Notify() { fDirty = true; return true; } +/// }; +/// ``` +/// declared in TChain.h or InternalTreeUtils.hxx and genereate a dictionary for +/// TNotifyLink. +class FileTransitionDetector final : public TNotifyLinkBase { + /// Set to true initially so that the very first iteration performs the lookup. + bool fChanged = true; + TTree &fChain; + +public: + FileTransitionDetector(TTree &chain) : fChain(chain) { PrependLink(fChain); } + + ~FileTransitionDetector() override { RemoveLink(fChain); } + FileTransitionDetector(const FileTransitionDetector &) = delete; + FileTransitionDetector &operator=(const FileTransitionDetector &) = delete; + FileTransitionDetector(FileTransitionDetector &&) = delete; + FileTransitionDetector &operator=(FileTransitionDetector &&) = delete; + + /// Must return true: returning false would make TChain::LoadTree fail with -6. + Bool_t Notify() override + { + fChanged = true; + // Propagate to the rest of the list of subscribers, as TNotifyLink does. + if (fNext) + return fNext->Notify(); + return true; + } + + /// Returns true (once) if the chain or any of its direct or indirect friends + /// switched to a new tree since the last call. + bool CheckAndReset() + { + bool changed = fChanged; + fChanged = false; + return changed; + } +}; +} // anonymous namespace -Double_t TTree::GetMaximum(const char* columname) +//////////////////////////////////////////////////////////////////////////////// +/// Computes the extremum (minimum or maximum) for the input column name +/// +/// It takes into account the following situations: +/// +/// * The dataset is a TTree and contains the input column +/// * The dataset is a TChain and contains the input column, in which case the methods detect file switching and update +/// the leaf pointer correctly. +/// * The dataset is a TChain, contains the input column, but some files miss it, in which case the methods skip the +/// entries from those files. +/// * The dataset has a friend TTree which contains the input column +/// * The dataset is a TChain and has a friend TChain which contains the input column, in which case the methods detect +/// file switching on the friend and update the leaf pointer correctly. +/// * The dataset is a TChain and has a friend TChain. The input column is partially available in either the main or the +/// friend chain. This can happen for example if the main chain has some files missing the input column and the user +/// knowingly injects the input column in the files of the friend chain. In this case, the methods detect file switching +/// at the boundary between files of the main chain, but also detect if there are file switches in the friend chain. +/// Notably, the entries must still be overall aligned between the main chain and the friend one. +double TTree::ComputeExtremum(const char *columname, double errVal, bool (*cmp)(double, double)) { - TLeaf* leaf = this->GetLeaf(columname); + // Ensure the TTree cursor is brought back to the current entry after computing the value + struct CurrentEntryRAII { + + Long64_t fCurrentEntry; + TTree &fTree; + + CurrentEntryRAII(TTree &tree) : fCurrentEntry(tree.GetReadEntry()), fTree(tree) {} + + ~CurrentEntryRAII() { fTree.LoadTree(fCurrentEntry); } + } raii{*this}; + + // Initial lookup of the leaf name, this will find it whether it's in the + // current tree or in any of its friends + TLeaf *leaf = GetLeaf(columname); if (!leaf) { return 0; } + TBranch *branch = leaf->GetBranch(); + assert(branch); // leaf without a branch is not allowed by construction // create cache if wanted if (fCacheDoAutoInit) SetCacheSizeAux(); - TBranch* branch = leaf->GetBranch(); - Double_t cmax = -DBL_MAX; + FileTransitionDetector fileTransition{*this}; + double extremum{errVal}; for (Long64_t i = 0; i < fEntries; ++i) { - Long64_t entryNumber = this->GetEntryNumber(i); + const auto entryNumber = GetEntryNumber(i); if (entryNumber < 0) break; - branch->GetEntry(entryNumber); - for (Int_t j = 0; j < leaf->GetLen(); ++j) { - Double_t val = leaf->GetValue(j); - if (val > cmax) { - cmax = val; + const auto localEntryNumber = LoadTree(entryNumber); + if (localEntryNumber < 0) + break; + + // At every entry, we check if the processing has triggered a switch to + // a new file. We detect both a switch of the current tree in the chain + // (if this tree is a TChain) as well as a switch in any of its direct + // and indirect friends (if they are also a TChain) + if (fileTransition.CheckAndReset()) { + branch = nullptr; + leaf = GetLeaf(columname); + if (leaf) { + branch = leaf->GetBranch(); + assert(branch); // leaf without a branch is not allowed by construction + } + } + + // We accept that the leaf may not be present in one or more files in case + // it was found in a chain, we just continue processing the next entry + if (!leaf) + continue; + + // If the branch belongs to a friend, the local entry number of the friend + // may differ from the one of the chain (e.g. when the friend is indexed). + // The owning TTree has already been positioned by TChain::LoadTree, so + // its read entry is the correct one to use. + auto *owningTree = branch->GetTree(); + branch->GetEntry(owningTree->GetReadEntry()); + + auto leafLen{leaf->GetLen()}; + for (decltype(leafLen) j = 0; j < leafLen; ++j) { + auto val = leaf->GetValue(j); + if (cmp(val, extremum)) { + extremum = val; } } } - return cmax; + + return extremum; +} + +//////////////////////////////////////////////////////////////////////////////// +/// Return maximum of column with name columname. +/// if the Tree has an associated TEventList or TEntryList, the maximum +/// is computed for the entries in this list. + +Double_t TTree::GetMaximum(const char *columname) +{ + return ComputeExtremum(columname, std::numeric_limits::lowest(), [](double a, double b) { return a > b; }); } //////////////////////////////////////////////////////////////////////////////// @@ -6386,29 +6530,7 @@ Long64_t TTree::GetMaxTreeSize() Double_t TTree::GetMinimum(const char* columname) { - TLeaf* leaf = this->GetLeaf(columname); - if (!leaf) { - return 0; - } - - // create cache if wanted - if (fCacheDoAutoInit) - SetCacheSizeAux(); - - TBranch* branch = leaf->GetBranch(); - Double_t cmin = DBL_MAX; - for (Long64_t i = 0; i < fEntries; ++i) { - Long64_t entryNumber = this->GetEntryNumber(i); - if (entryNumber < 0) break; - branch->GetEntry(entryNumber); - for (Int_t j = 0;j < leaf->GetLen(); ++j) { - Double_t val = leaf->GetValue(j); - if (val < cmin) { - cmin = val; - } - } - } - return cmin; + return ComputeExtremum(columname, std::numeric_limits::max(), [](double a, double b) { return a < b; }); } //////////////////////////////////////////////////////////////////////////////// diff --git a/tree/tree/test/TTreeRegressions.cxx b/tree/tree/test/TTreeRegressions.cxx index 5aba6fe4cddf8..77e52aae230af 100644 --- a/tree/tree/test/TTreeRegressions.cxx +++ b/tree/tree/test/TTreeRegressions.cxx @@ -1,5 +1,6 @@ #include "TMemFile.h" #include "TLeaf.h" +#include "TChain.h" #include "TTree.h" #include "TInterpreter.h" #include "TSystem.h" @@ -9,7 +10,9 @@ #include "gtest/gtest.h" +#include #include +#include // ROOT-10702 TEST(TTreeRegressions, CompositeTypeWithNameClash) @@ -322,3 +325,357 @@ TEST(TTreeRegressions, DrawAutoBinning) delete h; delete gROOT->FindObject("c1"); } + +// Regressions for https://github.com/root-project/root/issues/22652 +struct RegressionGH22652 : public ::testing::Test { + + constexpr static auto fMainTreeName{"main"}; + constexpr static auto fFriendTreeName{"friend"}; + constexpr static auto fMainTreeFileName{"main_global.root"}; + constexpr static auto fNFiles{3}; + constexpr static auto fNEntriesPerFile{100}; + // file 0: w in [4,5) file 1: w in [0,1) file 2: w in [9,10) + constexpr static std::array fLowerBounds{4.0, 0.0, 9.0}; + constexpr static std::array fMainChainFileNames{"main_1.root", "main_2.root", "main_3.root"}; + constexpr static std::array fFriendChainFileNames{"friend_1.root", "friend_2.root", + "friend_3.root"}; + + constexpr static auto fNShortFiles{6}; + constexpr static auto fNEntriesPerShortFile{50}; + constexpr static std::array fShortFriendChainFileNames{ + "short_friend_1.root", "short_friend_2.root", "short_friend_3.root", + "short_friend_4.root", "short_friend_5.root", "short_friend_6.root"}; + // We set the minimum value in the second file to check that the branch address is updated by TChain::GetMinimum + constexpr static std::array fShortFriendChainValues{10, 0, 30, 40, 50, 60}; + + constexpr static std::array fMainAlternatingChainFileNames{ + "main_alternating_1.root", "main_alternating_2.root", "main_alternating_3.root"}; + constexpr static std::array fFriendAlternatingChainFileNames{ + "friend_alternating_1.root", "friend_alternating_2.root", "friend_alternating_3.root"}; + + constexpr static std::array fMainChainDifferentEntryLayout{"main_different_entry_layout_1.root", + "main_different_entry_layout_2.root"}; + + constexpr static std::array fFriendChainDifferentEntryLayout{ + "friend_different_entry_layout_01.root", "friend_different_entry_layout_02.root", + "friend_different_entry_layout_03.root", "friend_different_entry_layout_04.root", + "friend_different_entry_layout_05.root", "friend_different_entry_layout_06.root", + "friend_different_entry_layout_07.root", "friend_different_entry_layout_08.root", + "friend_different_entry_layout_09.root", "friend_different_entry_layout_10.root"}; + + static void SetUpTestSuite() + { + { + // Main TTree with cumulated number of entries + auto fd = std::make_unique(fMainTreeFileName, "RECREATE"); + auto td = std::make_unique(fMainTreeName, fMainTreeName); + double x{}; + td->Branch("x", &x); + for ([[maybe_unused]] const auto &_ : fMainChainFileNames) + for (int i = 0; i < fNEntriesPerFile; ++i) + td->Fill(); + fd->Write(); + } + + for (int i = 0; i < fNFiles; i++) { + // Trees for the main chain + { + auto fd = std::make_unique(fMainChainFileNames[i], "RECREATE"); + auto td = std::make_unique(fMainTreeName, fMainTreeName); + double x{}; + td->Branch("x", &x); + for (int j = 0; j < fNEntriesPerFile; j++) { + // x in [-0.5, 0.495] + x = j * 0.01 - 0.5; + td->Fill(); + } + fd->Write(); + } + + // Trees for the friend chain + { + auto ff = std::make_unique(fFriendChainFileNames[i], "RECREATE"); + auto tf = std::make_unique(fFriendTreeName, fFriendTreeName); + double w{}; + tf->Branch("w", &w); + for (int j = 0; j < fNEntriesPerFile; j++) { + // w in [fLowerBounds[i], fLowerBounds[i] + 1) + w = fLowerBounds[i] + j * (1.0 / fNEntriesPerFile); + tf->Fill(); + } + ff->Write(); + } + } + + // A second friend chain with the total number of entries but twice as many files (half of the entries per file) + for (auto i = 0; i < fNShortFiles; i++) { + auto fd = std::make_unique(fShortFriendChainFileNames[i], "RECREATE"); + auto td = std::make_unique(fFriendTreeName, fFriendTreeName); + double w{}; + td->Branch("w", &w); + for (int j = 0; j < fNEntriesPerShortFile; j++) { + w = fShortFriendChainValues[i]; + td->Fill(); + } + fd->Write(); + } + + // Alternating branch in main and friend trees + // Main chain + { + auto f = std::make_unique(fMainAlternatingChainFileNames[0], "RECREATE"); + auto t = std::make_unique(fMainTreeName, fMainTreeName); + double w{}; + t->Branch("w", &w); + for (int j = 0; j < fNEntriesPerFile; j++) { + // w in [fLowerBounds[i], fLowerBounds[i] + 1) + w = fLowerBounds[0] + j * (1.0 / fNEntriesPerFile); + t->Fill(); + } + f->Write(); + } + { + auto f = std::make_unique(fMainAlternatingChainFileNames[1], "RECREATE"); + auto t = std::make_unique(fMainTreeName, fMainTreeName); + for (int j = 0; j < fNEntriesPerFile; j++) { + t->Fill(); + } + f->Write(); + } + { + auto f = std::make_unique(fMainAlternatingChainFileNames[2], "RECREATE"); + auto t = std::make_unique(fMainTreeName, fMainTreeName); + double w{}; + t->Branch("w", &w); + for (int j = 0; j < fNEntriesPerFile; j++) { + // w in [fLowerBounds[i], fLowerBounds[i] + 1) + w = fLowerBounds[2] + j * (1.0 / fNEntriesPerFile); + t->Fill(); + } + f->Write(); + } + + // Friend chain + { + auto f = std::make_unique(fFriendAlternatingChainFileNames[0], "RECREATE"); + auto t = std::make_unique(fFriendTreeName, fFriendTreeName); + for (int j = 0; j < fNEntriesPerFile; j++) { + t->Fill(); + } + f->Write(); + } + { + auto f = std::make_unique(fFriendAlternatingChainFileNames[1], "RECREATE"); + auto t = std::make_unique(fFriendTreeName, fFriendTreeName); + double w{}; + t->Branch("w", &w); + for (int j = 0; j < fNEntriesPerFile; j++) { + // w in [fLowerBounds[i], fLowerBounds[i] + 1) + w = fLowerBounds[1] + j * (1.0 / fNEntriesPerFile); + // Inject max value in this file to exercise the logic with GetMaximum + if (j == (fNEntriesPerFile - 1)) + w = 150; + t->Fill(); + } + f->Write(); + } + { + auto f = std::make_unique(fFriendAlternatingChainFileNames[2], "RECREATE"); + auto t = std::make_unique(fFriendTreeName, fFriendTreeName); + for (int j = 0; j < fNEntriesPerFile; j++) { + t->Fill(); + } + f->Write(); + } + + // Alternating branch to process between main and friend chain, but the + // friend chain has a different entries layout than the main one + constexpr auto nEntriesMainDifferentLayout{10}; + constexpr auto nEntriesFriendDifferentLayout{2}; + { + auto f = std::make_unique(fMainChainDifferentEntryLayout[0], "RECREATE"); + auto t = std::make_unique(fMainTreeName, fMainTreeName); + double w{}; + t->Branch("w", &w); + for (int j = 0; j < nEntriesMainDifferentLayout; j++) { + w = -1 * j; + t->Fill(); + } + f->Write(); + } + { + auto f = std::make_unique(fMainChainDifferentEntryLayout[1], "RECREATE"); + auto t = std::make_unique(fMainTreeName, fMainTreeName); + for (int j = 0; j < nEntriesMainDifferentLayout; j++) { + t->Fill(); + } + f->Write(); + } + + { + for (auto i = 0; i < 5; i++) { + auto f = std::make_unique(fFriendChainDifferentEntryLayout[i], "RECREATE"); + auto t = std::make_unique(fFriendTreeName, fFriendTreeName); + for (int j = 0; j < nEntriesFriendDifferentLayout; j++) { + t->Fill(); + } + f->Write(); + } + + for (auto i = 5; i < 10; i++) { + auto f = std::make_unique(fFriendChainDifferentEntryLayout[i], "RECREATE"); + auto t = std::make_unique(fFriendTreeName, fFriendTreeName); + double w{}; + t->Branch("w", &w); + for (int j = 0; j < nEntriesFriendDifferentLayout; j++) { + w = i * j; + t->Fill(); + } + f->Write(); + } + } + } + + static void TearDownTestSuite() + { + std::remove(fMainTreeFileName); + + for (const auto &f : fMainChainFileNames) + std::remove(f); + + for (const auto &f : fFriendChainFileNames) + std::remove(f); + + for (const auto &f : fMainAlternatingChainFileNames) + std::remove(f); + + for (const auto &f : fFriendAlternatingChainFileNames) + std::remove(f); + + for (const auto &f : fMainChainDifferentEntryLayout) + std::remove(f); + + for (const auto &f : fFriendChainDifferentEntryLayout) + std::remove(f); + } +}; + +TEST_F(RegressionGH22652, RunMainTChain) +{ + // Main is a TChain, friend is a TChain, entries are aligned + auto m = std::make_unique(fMainTreeName); + for (const auto &fn : fMainChainFileNames) + m->Add(fn); + + auto fc = std::make_unique(fFriendTreeName); + for (const auto &fn : fFriendChainFileNames) + fc->Add(fn); + + m->AddFriend(fc.get()); + + EXPECT_DOUBLE_EQ(m->GetMinimum("w"), 0.0); + EXPECT_DOUBLE_EQ(m->GetMaximum("w"), 9.99); +} + +TEST_F(RegressionGH22652, RunMainTTree) +{ + // Main is a TTree, friend is a TChain, entries are aligned + + auto fm = std::make_unique(fMainTreeFileName); + std::unique_ptr m{fm->Get(fMainTreeName)}; + + auto fc = std::make_unique(fFriendTreeName); + for (const auto &fn : fFriendChainFileNames) + fc->Add(fn); + + m->AddFriend(fc.get()); + + EXPECT_DOUBLE_EQ(m->GetMinimum("w"), 0.0); + EXPECT_DOUBLE_EQ(m->GetMaximum("w"), 9.99); +} + +TEST_F(RegressionGH22652, TChainFriendWithShorterFiles) +{ + // Main is a TChain, friend is a TChain, total number of entries is the same + // but the friend TChain has double the number of files and half the entries + // per file. This exercises in particular the correct updating of the branch + // addresses of the friend TChain when it switches to another file even though + // the main TChain is still traversing the same file. + auto m = std::make_unique(fMainTreeName); + for (const auto &fn : fMainChainFileNames) + m->Add(fn); + + auto fc = std::make_unique(fFriendTreeName); + for (const auto &fn : fShortFriendChainFileNames) + fc->Add(fn); + + m->AddFriend(fc.get()); + + EXPECT_DOUBLE_EQ(m->GetMinimum("w"), 0.0); + EXPECT_DOUBLE_EQ(m->GetMaximum("w"), 60.0); +} + +TEST_F(RegressionGH22652, BranchAlternatingInMainAndFriendTChain) +{ + // Main TChain and friend TChain, same number of files, same number of entries per file, aligned. + // The branch to be processed, "w", is found alternating in trees of the main and the friend one: + // main file 0 has w, friend file 1 does not have w + // main file 1 does not have w, friend file 1 has w + // main file 2 has w, friend file 2 does not have w + // GetMinimum, GetMaximum should be able to detect the presence of branch "w" + // in the second file and connect to it to get the missing values from the + // tree of the main chain + auto m = std::make_unique(fMainTreeName); + for (const auto &fn : fMainAlternatingChainFileNames) + m->Add(fn); + + auto fc = std::make_unique(fFriendTreeName); + for (const auto &fn : fFriendAlternatingChainFileNames) + fc->Add(fn); + + m->AddFriend(fc.get()); + + EXPECT_DOUBLE_EQ(m->GetMinimum("w"), 0.0); + EXPECT_DOUBLE_EQ(m->GetMaximum("w"), 150.0); +} + +TEST_F(RegressionGH22652, BranchMissingInSecondFileOfChain) +{ + // Main TChain with three files: + // main file 0 has w + // main file 1 does not have w + // main file 2 has w + // GetMinimum, GetMaximum should be able to compute their values skipping + // the trees where the branch is not present + + auto m = std::make_unique(fMainTreeName); + for (const auto &fn : fMainAlternatingChainFileNames) + m->Add(fn); + + EXPECT_DOUBLE_EQ(m->GetMinimum("w"), 4.0); + EXPECT_DOUBLE_EQ(m->GetMaximum("w"), 9.99); +} + +TEST_F(RegressionGH22652, AlternatingBranchWithDifferentEntryLayoutInMainAndFriend) +{ + + // Main TChain has two files. Each file has 10 entries. File 1 has branch "w", File 2 does not have it + // Friend TChain has 10 files. Each files has 2 entries. + // Main and friend are "aligned" such that for each file of main there are 5 corresponding files of friend + // The first five files of friend do not have branch "w", so that GetMinimum/Maximum should get the values of "w" + // from the main chain The second five files of friend have branch "w", so that GetMinimum/GetMaximum should get the + // values of "w" from the friend chain GetMinimum/Maximum should detect that while we are in the entries of the + // second file of the main chain, we may be jumping to another file in the friend chain + + auto m = std::make_unique(fMainTreeName); + for (const auto &fn : fMainChainDifferentEntryLayout) + m->Add(fn); + auto fc = std::make_unique(fFriendTreeName); + for (const auto &fn : fFriendChainDifferentEntryLayout) + fc->Add(fn); + + m->AddFriend(fc.get()); + + EXPECT_DOUBLE_EQ(m->GetMinimum("w"), -9.); + EXPECT_DOUBLE_EQ(m->GetMaximum("w"), 9.); +}