diff --git a/tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx b/tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx index 536e8b0292b75..45df52d56ed20 100644 --- a/tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx +++ b/tree/dataframe/inc/ROOT/RDF/ActionHelpers.hxx @@ -541,6 +541,13 @@ public: } } + RHistFillHelper MakeNew(void *newResult, std::string_view /*variation*/ = "nominal") + { + auto &result = *static_cast *>(newResult); + result->Clear(); + return RHistFillHelper(result, fContexts.size()); + } + std::string GetActionName() { return "Hist"; } }; @@ -588,6 +595,13 @@ public: void Finalize() {} + RHistEngineFillHelper MakeNew(void *newResult, std::string_view /*variation*/ = "nominal") + { + auto &result = *static_cast *>(newResult); + result->Clear(); + return RHistEngineFillHelper(result); + } + std::string GetActionName() { return "Hist"; } }; #endif diff --git a/tree/dataframe/inc/ROOT/RDFHelpers.hxx b/tree/dataframe/inc/ROOT/RDFHelpers.hxx index 2b3115a39289e..d39ea1379f0fe 100644 --- a/tree/dataframe/inc/ROOT/RDFHelpers.hxx +++ b/tree/dataframe/inc/ROOT/RDFHelpers.hxx @@ -219,6 +219,36 @@ namespace Experimental { // N*#input_cols column readers // // ...and each RFilter and RDefine knows for what universe it needs to construct column readers ("nominal" by default). + +namespace Internal { + /// @brief Helper function to add a copy of an object to a vector of shared_ptrs, used for variations. + /// Default implementation uses copy constructor, which should work for most objects types since they are copied for each slot. + /// @tparam T An object that is used as result of a RDataFrame action, e.g. a histogram + /// @param vec The vector of shared_ptrs to which a copy of obj will be added + /// @param obj The object to be copied and added to vec + template + void addCopyForVariations(std::vector> &vec, const T & obj) + { + vec.emplace_back(std::make_shared(obj)); + } + + /// @brief Specialization of addCopyForVariations for ROOT::Experimental::RHist objects, which are not copyable but clonable + template + void addCopyForVariations(std::vector>> &vec, const ROOT::Experimental::RHist & obj) + { + // Note that Clone() returns a value and not a pointer, so we call 'new' and pass that value to the RHist move constructor + vec.emplace_back(std::make_shared>(obj.Clone())); + } + + /// @brief Specialization of addCopyForVariations for ROOT::Experimental::RHistEngine objects, which are not copyable but clonable + template + void addCopyForVariations(std::vector>> &vec, const ROOT::Experimental::RHistEngine & obj) + { + // Note that Clone() returns a value and not a pointer, so we call 'new' and pass that value to the RHist move constructor + vec.emplace_back(std::make_shared>(obj.Clone())); + } +} + template RResultMap VariationsFor(RResultPtr resPtr) { @@ -243,9 +273,9 @@ RResultMap VariationsFor(RResultPtr resPtr) // clone the result once for each variation variedResults.reserve(nVariations); for (auto i = 0u; i < nVariations; ++i){ - // implicitly assuming that T is copiable: this should be the case - // for all result types in use, as they are copied for each slot - variedResults.emplace_back(new T{*resPtr.fObjPtr}); + + // make a copy of the result object for this variation + Internal::addCopyForVariations(variedResults, *resPtr.fObjPtr); // Check if the result's type T inherits from TNamed if constexpr (std::is_base_of::value) { diff --git a/tree/dataframe/test/dataframe_hist.cxx b/tree/dataframe/test/dataframe_hist.cxx index f5fe396b50de3..01cbd53825e06 100644 --- a/tree/dataframe/test/dataframe_hist.cxx +++ b/tree/dataframe/test/dataframe_hist.cxx @@ -455,6 +455,78 @@ TEST_P(RDFHist, WeightInvalidNumberOfArgumentsJit) EXPECT_THROW(dfXW.Hist(engine, {"x", "x"}, "w"), std::invalid_argument); } +TEST_P(RDFHist, Variations) +{ + RDataFrame df(10); + auto dfX = df.Define("x", [](ULong64_t e) -> Float_t { return e + 5.5f; }, {"rdfentry_"}) + .Vary("x", [](Float_t x) { return ROOT::RVecF{x - 1.0f, x + 1.0f}; }, {"x"}, 2); + + const RRegularAxis axis(10, {5.0, 15.0}); + auto hist = dfX.Hist({axis}, {"x"}); + auto vars = ROOT::RDF::Experimental::VariationsFor(hist); + + // Check nominal + EXPECT_EQ(hist->GetNEntries(), 10); + for (auto index : axis.GetNormalRange()) { + EXPECT_EQ(hist->GetBinContent(index), 1.0); + } + + // Check variations + EXPECT_EQ(vars.GetKeys().size(), 3); // nominal + 2 variations + for (const auto &key : vars.GetKeys()) { + auto &varHist = vars[key]; + EXPECT_EQ(varHist.GetNEntries(), 10); + for (auto index : axis.GetNormalRange()) { + if (key == "nominal") { + EXPECT_EQ(varHist.GetBinContent(index), 1.0); + } else if (key == "x:0") { + EXPECT_EQ(varHist.GetBinContent(index), (index.GetIndex() == 9 ? 0.0 : 1.0)); + } else if (key == "x:1") { + EXPECT_EQ(varHist.GetBinContent(index), (index.GetIndex() == 0 ? 0.0 : 1.0)); + } else { + FAIL() << "Unexpected variation key: " << key; + } + } + } +} + +TEST_P(RDFHist, VariationsEngine) +{ + RDataFrame df(10); + auto dfX = df.Define("x", [](ULong64_t e) -> Float_t { return e + 5.5f; }, {"rdfentry_"}) + .Vary("x", [](Float_t x) { return ROOT::RVecF{x - 1.0f, x + 1.0f}; }, {"x"}, 2); + + const RRegularAxis axis(10, {5.0, 15.0}); + auto hist = std::make_shared>(axis); + auto resPtr = dfX.Hist(hist, {"x"}); + auto vars = ROOT::RDF::Experimental::VariationsFor(resPtr); + + // Trigger the run + resPtr.GetValue(); + + // Check nominal + for (auto index : axis.GetNormalRange()) { + EXPECT_EQ(hist->GetBinContent(index), 1.0); + } + + // Check variations + EXPECT_EQ(vars.GetKeys().size(), 3); // nominal + 2 variations + for (const auto &key : vars.GetKeys()) { + auto &varHist = vars[key]; + for (auto index : axis.GetNormalRange()) { + if (key == "nominal") { + EXPECT_EQ(varHist.GetBinContent(index), 1.0); + } else if (key == "x:0") { + EXPECT_EQ(varHist.GetBinContent(index), (index.GetIndex() == 9 ? 0.0 : 1.0)); + } else if (key == "x:1") { + EXPECT_EQ(varHist.GetBinContent(index), (index.GetIndex() == 0 ? 0.0 : 1.0)); + } else { + FAIL() << "Unexpected variation key: " << key; + } + } + } +} + INSTANTIATE_TEST_SUITE_P(Seq, RDFHist, ::testing::Values(false)); #ifdef R__USE_IMT