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

Improve semantics of data member #17927

Merged
merged 1 commit into from
Mar 11, 2025
Merged
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
4 changes: 2 additions & 2 deletions tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public:
{
fLoopManager->Register(this);
// We suppress errors that TTreeReader prints regarding the missing branch
fLoopManager->GetSuppressErrorsForMissingBranches().push_back(fColumnNames[0]);
fLoopManager->InsertSuppressErrorsForMissingBranch(fColumnNames[0]);
}

RDefaultValueFor(const RDefaultValueFor &) = delete;
Expand All @@ -86,7 +86,7 @@ public:
~RDefaultValueFor()
{
fLoopManager->Deregister(this);
ROOT::Internal::RDF::Erase(fColumnNames[0], fLoopManager->GetSuppressErrorsForMissingBranches());
fLoopManager->EraseSuppressErrorsForMissingBranch(fColumnNames[0]);
}

void InitSlot(TTreeReader *r, unsigned int slot) final
Expand Down
4 changes: 2 additions & 2 deletions tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public:
{
fLoopManager->Register(this);
// We suppress errors that TTreeReader prints regarding the missing branch
fLoopManager->GetSuppressErrorsForMissingBranches().push_back(fColumnNames[0]);
fLoopManager->InsertSuppressErrorsForMissingBranch(fColumnNames[0]);
}

RFilterWithMissingValues(const RFilterWithMissingValues &) = delete;
Expand All @@ -93,7 +93,7 @@ public:
// must Deregister objects from the RLoopManager here, before the fPrevNodePtr data member is destroyed:
// otherwise if fPrevNodePtr is the RLoopManager, it will be destroyed before the calls to Deregister happen.
fLoopManager->Deregister(this);
ROOT::Internal::RDF::Erase(fColumnNames[0], fLoopManager->GetSuppressErrorsForMissingBranches());
fLoopManager->EraseSuppressErrorsForMissingBranch(fColumnNames[0]);
}

bool CheckFilters(unsigned int slot, Long64_t entry) final
Expand Down
21 changes: 14 additions & 7 deletions tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class RLoopManager : public RNodeBase {
// List of branches for which we want to suppress the printed error about
// missing branch when switching to a new tree. This is modified by readers,
// so must be declared before them in this class.
std::vector<std::string> fSuppressErrorsForMissingBranches{};
std::set<std::string> fSuppressErrorsForMissingBranches{};
ROOT::Internal::RDF::RStringCache fCachedColNames;
std::set<std::pair<std::string_view, std::unique_ptr<ROOT::Internal::RDF::RDefinesWithReaders>>>
fUniqueDefinesWithReaders;
Expand Down Expand Up @@ -293,15 +293,22 @@ public:
return fUniqueVariationsWithReaders;
}

std::vector<std::string> &GetSuppressErrorsForMissingBranches() { return fSuppressErrorsForMissingBranches; }
const std::vector<std::string> &GetSuppressErrorsForMissingBranches() const
{
return fSuppressErrorsForMissingBranches;
}

void SetTTreeLifeline(std::any lifeline);

void SetDataSource(std::unique_ptr<ROOT::RDF::RDataSource> dataSource);

void InsertSuppressErrorsForMissingBranch(const std::string &branchName)
{
fSuppressErrorsForMissingBranches.insert(branchName);
}
void EraseSuppressErrorsForMissingBranch(const std::string &branchName)
{
fSuppressErrorsForMissingBranches.erase(branchName);
}
const std::set<std::string> &GetSuppressErrorsForMissingBranches() const
{
return fSuppressErrorsForMissingBranches;
}
};

/// \brief Create an RLoopManager that reads a TChain.
Expand Down
5 changes: 2 additions & 3 deletions tree/dataframe/src/RDFInterfaceUtils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -931,10 +931,9 @@ ColumnNames_t GetValidatedColumnNames(RLoopManager &lm, const unsigned int nColu
// Look for a possible overlap between the unknown columns and the
// columns we should ignore for the purpose of the following exception
std::set<std::string> intersection;
auto colsToIgnore = lm.GetSuppressErrorsForMissingBranches();
const auto &colsToIgnore = lm.GetSuppressErrorsForMissingBranches();
std::sort(unknownColumns.begin(), unknownColumns.end());
std::sort(colsToIgnore.begin(), colsToIgnore.end());
std::set_intersection(unknownColumns.begin(), unknownColumns.end(), colsToIgnore.begin(), colsToIgnore.end(),
std::set_intersection(unknownColumns.cbegin(), unknownColumns.cend(), colsToIgnore.cbegin(), colsToIgnore.cend(),
std::inserter(intersection, intersection.begin()));
if (intersection.empty()) {
std::string errMsg = std::string("Unknown column") + (unknownColumns.size() > 1 ? "s: " : ": ");
Expand Down
8 changes: 4 additions & 4 deletions tree/treeplayer/inc/ROOT/TTreeProcessorMT.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public:
const std::vector<std::string> &fileNames,
const ROOT::TreeUtils::RFriendInfo &friendInfo,
const TEntryList &entryList, const std::vector<Long64_t> &nEntries,
const std::vector<std::string> &suppressErrorsForMissingBranches);
const std::set<std::string> &suppressErrorsForMissingBranches);
void Reset();
};
} // End of namespace Internal
Expand All @@ -97,7 +97,7 @@ private:

// List of branches for which we want to suppress the printed error about
// missing branch when switching to a new tree
std::vector<std::string> fSuppressErrorsForMissingBranches{};
std::set<std::string> fSuppressErrorsForMissingBranches{};

public:
TTreeProcessorMT(std::string_view filename, std::string_view treename = "", UInt_t nThreads = 0u,
Expand All @@ -109,10 +109,10 @@ public:
const std::pair<Long64_t, Long64_t> &globalRange = {0, std::numeric_limits<Long64_t>::max()}):
TTreeProcessorMT(std::vector<std::string_view>(filenames), treename, nThreads, globalRange) {}
TTreeProcessorMT(TTree &tree, const TEntryList &entries, UInt_t nThreads = 0u,
const std::vector<std::string> &suppressErrorsForMissingBranches = {});
const std::set<std::string> &suppressErrorsForMissingBranches = {});
TTreeProcessorMT(TTree &tree, UInt_t nThreads = 0u,
const std::pair<Long64_t, Long64_t> &globalRange = {0, std::numeric_limits<Long64_t>::max()},
const std::vector<std::string> &suppressErrorsForMissingBranches = {});
const std::set<std::string> &suppressErrorsForMissingBranches = {});

void Process(std::function<void(TTreeReader &)> func);

Expand Down
5 changes: 3 additions & 2 deletions tree/treeplayer/inc/TTreeReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <deque>
#include <iterator>
#include <unordered_map>
#include <set>
#include <string>

class TDictionary;
Expand Down Expand Up @@ -184,7 +185,7 @@ class TTreeReader : public TObject {
TTreeReader();

TTreeReader(TTree *tree, TEntryList *entryList = nullptr, bool warnAboutLongerFriends = true,
const std::vector<std::string> &suppressErrorsForMissingBranches = {});
const std::set<std::string> &suppressErrorsForMissingBranches = {});
TTreeReader(const char *keyname, TDirectory *dir, TEntryList *entryList = nullptr);
TTreeReader(const char *keyname, TEntryList *entryList = nullptr) : TTreeReader(keyname, nullptr, entryList) {}

Expand Down Expand Up @@ -338,7 +339,7 @@ class TTreeReader : public TObject {

// List of branches for which we want to suppress the printed error about
// missing branch when switching to a new tree
std::vector<std::string> fSuppressErrorsForMissingBranches{};
std::set<std::string> fSuppressErrorsForMissingBranches{};
std::vector<std::string> fMissingProxies{};

friend class ROOT::Internal::TTreeReaderValueBase;
Expand Down
6 changes: 3 additions & 3 deletions tree/treeplayer/src/TTreeProcessorMT.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ std::unique_ptr<TTreeReader>
TTreeView::GetTreeReader(Long64_t start, Long64_t end, const std::vector<std::string> &treeNames,
const std::vector<std::string> &fileNames, const ROOT::TreeUtils::RFriendInfo &friendInfo,
const TEntryList &entryList, const std::vector<Long64_t> &nEntries,
const std::vector<std::string> &suppressErrorsForMissingBranches)
const std::set<std::string> &suppressErrorsForMissingBranches)
{
const bool hasEntryList = entryList.GetN() > 0;
const bool usingLocalEntries = friendInfo.fFriendNames.empty() && !hasEntryList;
Expand Down Expand Up @@ -412,7 +412,7 @@ TTreeProcessorMT::TTreeProcessorMT(const std::vector<std::string_view> &filename
/// \param[in] nThreads Number of threads to create in the underlying thread-pool. The semantics of this argument are
/// the same as for TThreadExecutor.
TTreeProcessorMT::TTreeProcessorMT(TTree &tree, const TEntryList &entries, UInt_t nThreads,
const std::vector<std::string> &suppressErrorsForMissingBranches)
const std::set<std::string> &suppressErrorsForMissingBranches)
: fFileNames(Internal::TreeUtils::GetFileNamesFromTree(tree)),
fTreeNames(Internal::TreeUtils::GetTreeFullPaths(tree)),
fEntryList(entries),
Expand All @@ -430,7 +430,7 @@ TTreeProcessorMT::TTreeProcessorMT(TTree &tree, const TEntryList &entries, UInt_
/// the same as for TThreadExecutor.
/// \param[in] globalRange Global entry range to process, {begin (inclusive), end (exclusive)}.
TTreeProcessorMT::TTreeProcessorMT(TTree &tree, UInt_t nThreads, const EntryRange &globalRange,
const std::vector<std::string> &suppressErrorsForMissingBranches)
const std::set<std::string> &suppressErrorsForMissingBranches)
: fFileNames(Internal::TreeUtils::GetFileNamesFromTree(tree)),
fTreeNames(Internal::TreeUtils::GetTreeFullPaths(tree)),
fFriendInfo(Internal::TreeUtils::GetFriendInfo(tree, /*retrieveEntries*/ true)),
Expand Down
7 changes: 3 additions & 4 deletions tree/treeplayer/src/TTreeReader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ TTreeReader::TTreeReader() : fNotify(this), fFriendProxies() {}
/// per chain.SetEntryList(&entryList).

TTreeReader::TTreeReader(TTree *tree, TEntryList *entryList /*= nullptr*/, bool warnAboutLongerFriends,
const std::vector<std::string> &suppressErrorsForMissingBranches)
const std::set<std::string> &suppressErrorsForMissingBranches)
: fTree(tree),
fEntryList(entryList),
fNotify(this),
Expand Down Expand Up @@ -384,9 +384,8 @@ bool TTreeReader::SetProxies()
// where the first tree of the chain does not contain that branch. In such
// case, we need to postpone the creation of the corresponding proxy until
// we find the branch in a following tree of the chain.
const bool suppressErrorsForThisBranch =
(std::find(fSuppressErrorsForMissingBranches.cbegin(), fSuppressErrorsForMissingBranches.cend(),
reader->fBranchName.View()) != fSuppressErrorsForMissingBranches.cend());
const bool suppressErrorsForThisBranch = (fSuppressErrorsForMissingBranches.find(reader->fBranchName.Data()) !=
fSuppressErrorsForMissingBranches.cend());
// Because of the situation described above, we may have some proxies
// already created and some not, if their branch was not available so far.
// Make sure we do not recreate the proxy unnecessarily, unless the
Expand Down
6 changes: 2 additions & 4 deletions tree/treeplayer/src/TTreeReaderArray.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,8 @@ void ROOT::Internal::TTreeReaderArrayBase::CreateProxy()

// Tell the branch proxy to suppress the errors for missing branch if this
// branch name is found in the list of suppressions
const bool suppressErrorsForThisBranch =
(std::find(fTreeReader->fSuppressErrorsForMissingBranches.cbegin(),
fTreeReader->fSuppressErrorsForMissingBranches.cend(),
fBranchName.Data()) != fTreeReader->fSuppressErrorsForMissingBranches.cend());
const bool suppressErrorsForThisBranch = (fTreeReader->fSuppressErrorsForMissingBranches.find(fBranchName.Data()) !=
fTreeReader->fSuppressErrorsForMissingBranches.cend());

TDictionary *branchActualType = nullptr;
TBranch *branch = nullptr;
Expand Down
3 changes: 1 addition & 2 deletions tree/treeplayer/src/TTreeReaderValue.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,7 @@ void ROOT::Internal::TTreeReaderValueBase::CreateProxy()
// one tree in the chain has that branch
const auto &suppressErrorsForMissingBranches = fTreeReader->fSuppressErrorsForMissingBranches;
const bool suppressErrorsForThisBranch =
(std::find(suppressErrorsForMissingBranches.cbegin(), suppressErrorsForMissingBranches.cend(),
originalBranchName) != suppressErrorsForMissingBranches.cend());
(suppressErrorsForMissingBranches.find(originalBranchName) != suppressErrorsForMissingBranches.cend());

if (!branch) {
// We had an error, the branch name had no "." or we simply did not find anything.
Expand Down
Loading