Skip to content

Commit

Permalink
[tree] Fix tests with new logic to detect mismatched friend entries
Browse files Browse the repository at this point in the history
  • Loading branch information
vepadulano committed Sep 4, 2024
1 parent 26af523 commit 846aebf
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 48 deletions.
107 changes: 71 additions & 36 deletions tree/dataframe/test/dataframe_datasetspec.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -334,46 +334,83 @@ TEST_P(RDatasetSpecTest, Ranges)
std::logic_error);
}

// reuse the possible ranges from above
TEST_P(RDatasetSpecTest, Friends)
TEST_P(RDatasetSpecTest, FriendsEqualSize)
{

// Test the canonical case: main tree and friend trees have all equal size.
RDatasetSpec spec;
// pick the second sample as the main chain, so that can test shorter, equal-sized, longer friends
spec.AddSample({data[1].name, data[1].trees[0][0].tree, data[1].fileGlobs});
for (const auto &d : data) {
std::vector<std::string> treeNames{};
std::vector<std::string> fileGlobs{};
std::vector<std::string> treeNamesExpanded{};
std::vector<std::string> fileGlobsExpanded{};
for (auto i = 0u; i < d.fileGlobs.size(); ++i) {
for (const auto &t : d.trees[i]) {
treeNamesExpanded.emplace_back(t.tree);
fileGlobsExpanded.emplace_back(t.file);
}
treeNames.emplace_back(d.trees[i][0].tree);
fileGlobs.emplace_back(d.fileGlobs[i]);
const auto &d = data[1];
std::vector<std::string> treeNames{};
std::vector<std::string> fileGlobs{};
std::vector<std::string> treeNamesExpanded{};
std::vector<std::string> fileGlobsExpanded{};
for (auto i = 0u; i < d.fileGlobs.size(); ++i) {
for (const auto &t : d.trees[i]) {
treeNamesExpanded.emplace_back(t.tree);
fileGlobsExpanded.emplace_back(t.file);
}
spec.WithGlobalFriends(treeNames, fileGlobs, "friend_glob_" + d.name);
spec.WithGlobalFriends(treeNamesExpanded, fileGlobsExpanded, "friend_expanded_" + d.name);
treeNames.emplace_back(d.trees[i][0].tree);
fileGlobs.emplace_back(d.fileGlobs[i]);
}
spec.WithGlobalFriends(treeNames, fileGlobs, "friend_glob_" + d.name);
spec.WithGlobalFriends(treeNamesExpanded, fileGlobsExpanded, "friend_expanded_" + d.name);

auto df = RDataFrame(spec);
std::unordered_map<std::string, ROOT::RDF::RResultPtr<std::vector<ULong64_t>>> res;
for (const auto &d : data) {
res["friend_glob_" + d.name + "x"] = df.Take<ULong64_t>("friend_glob_" + d.name + ".x");
res["friend_expanded_" + d.name + "x"] = df.Take<ULong64_t>("friend_expanded_" + d.name + ".x");
auto take_glob = df.Take<ULong64_t>("friend_glob_" + d.name + ".x");
auto take_expanded = df.Take<ULong64_t>("friend_expanded_" + d.name + ".x");
std::sort(take_glob->begin(), take_glob->end());
std::sort(take_expanded->begin(), take_expanded->end());
EXPECT_VEC_SEQ_EQ(*take_glob, ROOT::TSeq<ULong64_t>(d.sampleStart, d.sampleStart + 24));
EXPECT_VEC_SEQ_EQ(*take_expanded, ROOT::TSeq<ULong64_t>(d.sampleStart, d.sampleStart + 24));
}

TEST_P(RDatasetSpecTest, FriendsLonger)
{
// Test the case where there are still entries in the friend trees after
// processing of the main tree finishes. This should issue a warning
RDatasetSpec spec;
spec.AddSample({data[0].name, data[0].trees[0][0].tree, data[0].fileGlobs});
const auto &d = data[1];
std::vector<std::string> treeNames{};
std::vector<std::string> fileGlobs{};
for (auto i = 0u; i < d.fileGlobs.size(); ++i) {
treeNames.emplace_back(d.trees[i][0].tree);
fileGlobs.emplace_back(d.fileGlobs[i]);
}
spec.WithGlobalFriends(treeNames, fileGlobs, "friend_glob_" + d.name);

for (auto i = 0u; i < data.size(); ++i) {
std::sort(res["friend_glob_" + data[i].name + "x"]->begin(), res["friend_glob_" + data[i].name + "x"]->end());
std::sort(res["friend_expanded_" + data[i].name + "x"]->begin(),
res["friend_expanded_" + data[i].name + "x"]->end());
// case i = 0 is shorter friend, which currently leads to undesired behaviour
// see: https://github.com/root-project/root/issues/9137
if (i > 0) {
EXPECT_VEC_SEQ_EQ(*res["friend_glob_" + data[i].name + "x"],
ROOT::TSeq<ULong64_t>(data[i].sampleStart, data[i].sampleStart + 24));
}
auto df = RDataFrame(spec);
auto take_glob = df.Take<ULong64_t>("friend_glob_" + d.name + ".x");
// The warning about longer friend only makes sense for the single-threaded case
if (!GetParam())
ROOT_EXPECT_WARNING(*take_glob, "SetEntryBase",
"Last entry available from main tree '' was 14 but friend tree 'subTree' has more entries "
"beyond the end of the main tree.");
}

TEST_P(RDatasetSpecTest, FriendsShorter)
{
// Test the case where the friend trees are shorter than the main one.
// This should throw an exception.
RDatasetSpec spec;
spec.AddSample({data[1].name, data[1].trees[0][0].tree, data[1].fileGlobs});
const auto &d = data[0];
std::vector<std::string> treeNames{};
std::vector<std::string> fileGlobs{};
for (auto i = 0u; i < d.fileGlobs.size(); ++i) {
treeNames.emplace_back(d.trees[i][0].tree);
fileGlobs.emplace_back(d.fileGlobs[i]);
}
spec.WithGlobalFriends(treeNames, fileGlobs, "friend_glob_" + d.name);

auto df = RDataFrame(spec);
auto take_glob = df.Take<ULong64_t>("friend_glob_" + d.name + ".x");
try {
*take_glob;
} catch (const std::runtime_error &err) {
const std::string msg = "Cannot read entry 15 from friend tree 'tree'. The friend tree has less entries than the "
"main tree. Make sure all trees of the dataset have the same number of entries.";
EXPECT_STREQ(err.what(), msg.c_str());
}
}

Expand All @@ -382,8 +419,7 @@ TEST_P(RDatasetSpecTest, Histo1D)
RDatasetSpec spec;
spec.AddSample({"real0", "tree"s, {"specTestFile0.root"s}});
spec.AddSample({"real1", {{"tree"s, "specTestFile00*.root"s}}});
// 1 friend with entries from 15 up to 39 -> shortened to have the size of the main chain
spec.WithGlobalFriends({{"subTree"s, "specTestFile1*.root"s}}, "friend"s);
spec.WithGlobalFriends("subTree", std::vector<std::string>{"specTestFile1.root", "specTestFile11.root"}, "friend");
ROOT::RDataFrame d(spec);

auto h1 = d.Histo1D(::TH1D("h1", "h1", 10, 0, 10), "x");
Expand Down Expand Up @@ -429,8 +465,7 @@ TEST_P(RDatasetSpecTest, FilterDependingOnVariation)
RDatasetSpec spec;
spec.AddSample({"real0", "tree"s, {"specTestFile0.root"s}});
spec.AddSample({"real1", {{"tree"s, "specTestFile00*.root"s}}});
// 1 friend with entries from 15 up to 39 -> shortened to have the size of the main chain
spec.WithGlobalFriends({{"subTree"s, "specTestFile1*.root"s}}, "friend"s);
spec.WithGlobalFriends("subTree", std::vector<std::string>{"specTestFile1.root", "specTestFile11.root"}, "friend");
ROOT::RDataFrame df(spec);

auto sum = df.Vary(
Expand Down
12 changes: 6 additions & 6 deletions tree/dataframe/test/dataframe_friends.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ class RDFAndFriends : public ::testing::Test {
TTree t("t3", "t3");
float arr[4];
t.Branch("arr", arr, "arr[4]/F");
for (auto i : ROOT::TSeqU(4)) {
for (auto j : ROOT::TSeqU(4)) {
for (auto i : ROOT::TSeqU(kSizeSmall)) {
for (auto j : ROOT::TSeqU(kSizeSmall)) {
arr[j] = i + j;
}
t.Fill();
Expand Down Expand Up @@ -221,15 +221,15 @@ TEST_F(RDFAndFriends, FriendAliasMT)
ROOT::EnableImplicitMT(4u);
TFile f1(kFile1);
auto t1 = f1.Get<TTree>("t");
TFile f2(kFile4);
auto t2 = f2.Get<TTree>("t");
TFile f2(kFile2);
auto t2 = f2.Get<TTree>("t2");
t1->AddFriend(t2, "myfriend");
ROOT::RDataFrame d(*t1);
auto x = d.Min<int>("x");
auto t = d.Take<int>("myfriend.x");
auto t = d.Take<int>("myfriend.y");
EXPECT_EQ(*x, 1);
for (auto v : t)
EXPECT_EQ(v, 4);
EXPECT_EQ(v, 2);
ROOT::DisableImplicitMT();
}

Expand Down
18 changes: 12 additions & 6 deletions tree/treeplayer/test/ttreereader_friends.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ TEST_F(TTreeReaderFriends, MainTreeShorterTTree)
TTreeReaderValue<int> y(r, "y");

ROOT_EXPECT_WARNING(r.SetEntry(2), "SetEntryBase",
"Friend tree '" + std::string(fMainTreeName) +
"Last entry available from main tree '" + std::string(fFriendTreeName) +
"' was 1 but friend tree '" + std::string(fMainTreeName) +
"' has more entries beyond the end of the main tree.");
}

Expand All @@ -153,7 +154,8 @@ TEST_F(TTreeReaderFriends, MainTreeShorterTChain)
TTreeReaderValue<int> x(r, "x");
TTreeReaderValue<int> y(r, "y");
ROOT_EXPECT_WARNING(r.SetEntry(2), "SetEntryBase",
"Friend tree '" + std::string(fMainTreeName) +
"Last entry available from main tree '" + std::string(fFriendTreeName) +
"' was 1 but friend tree '" + std::string(fMainTreeName) +
"' has more entries beyond the end of the main tree.");
}

Expand All @@ -175,11 +177,13 @@ TEST_F(TTreeReaderFriends, MainTreeShorterTTreeExtraFriend)

ROOT::TestSupport::CheckDiagsRAII diags;
diags.requiredDiag(kWarning, "SetEntryBase",
"Friend tree '" + std::string(fMainTreeName) +
"Last entry available from main tree '" + std::string(fFriendTreeName) +
"' was 1 but friend tree '" + std::string(fMainTreeName) +
"' has more entries beyond the end of the main tree.",
/*matchFullMessage*/ true);
diags.requiredDiag(kWarning, "SetEntryBase",
"Friend tree '" + std::string(fExtraTreeName) +
"Last entry available from main tree '" + std::string(fFriendTreeName) +
"' was 1 but friend tree '" + std::string(fExtraTreeName) +
"' has more entries beyond the end of the main tree.",
/*matchFullMessage*/ true);
r.SetEntry(2);
Expand All @@ -199,11 +203,13 @@ TEST_F(TTreeReaderFriends, MainTreeShorterTChainExtraFriend)

ROOT::TestSupport::CheckDiagsRAII diags;
diags.requiredDiag(kWarning, "SetEntryBase",
"Friend tree '" + std::string(fMainTreeName) +
"Last entry available from main tree '" + std::string(fFriendTreeName) +
"' was 1 but friend tree '" + std::string(fMainTreeName) +
"' has more entries beyond the end of the main tree.",
/*matchFullMessage*/ true);
diags.requiredDiag(kWarning, "SetEntryBase",
"Friend tree '" + std::string(fExtraTreeName) +
"Last entry available from main tree '" + std::string(fFriendTreeName) +
"' was 1 but friend tree '" + std::string(fExtraTreeName) +
"' has more entries beyond the end of the main tree.",
/*matchFullMessage*/ true);
r.SetEntry(2);
Expand Down

0 comments on commit 846aebf

Please sign in to comment.