From c4611e8d6315abb17399d55ddf79b32f67fc2889 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Thu, 25 Jun 2026 17:33:37 +0200 Subject: [PATCH 1/2] deprecate a problematic feature in HistogramRegistry; avoid including ASoA.h --- .../include/Framework/HistogramRegistry.h | 30 +------- Framework/Core/src/HistogramRegistry.cxx | 1 + .../Core/test/benchmark_HistogramRegistry.cxx | 2 - .../Core/test/test_HistogramRegistry.cxx | 71 ++++++++++--------- 4 files changed, 39 insertions(+), 65 deletions(-) diff --git a/Framework/Core/include/Framework/HistogramRegistry.h b/Framework/Core/include/Framework/HistogramRegistry.h index 49ef006f84a79..5b254a5ba8181 100644 --- a/Framework/Core/include/Framework/HistogramRegistry.h +++ b/Framework/Core/include/Framework/HistogramRegistry.h @@ -13,15 +13,11 @@ #define FRAMEWORK_HISTOGRAMREGISTRY_H_ #include "Framework/HistogramSpec.h" -#include "Framework/ASoA.h" -#include "Framework/FunctionalHelpers.h" -#include "Framework/Logger.h" #include "Framework/OutputRef.h" #include "Framework/OutputObjHeader.h" #include "Framework/OutputSpec.h" -#include "Framework/SerializationMethods.h" -#include "Framework/TableBuilder.h" #include "Framework/RuntimeError.h" +#include "Framework/Expressions.h" #include "StepTHn.h" #include @@ -293,23 +289,6 @@ void HistFiller::fillHistAny(std::shared_ptr hist, Ts... positionAndWeight) HistFiller::badHistogramFill(hist->GetName()); } -template -void HistFiller::fillHistAny(std::shared_ptr hist, const T& table, const o2::framework::expressions::Filter& filter) - requires(!ValidComplexFillStep) && requires(T t) { t.asArrowTable(); } -{ - auto s = o2::framework::expressions::createSelection(table.asArrowTable(), filter); - auto filtered = o2::soa::Filtered{{table.asArrowTable()}, s}; - for (auto& t : filtered) { - fillHistAny(hist, (*(static_cast(t).getIterator()))...); - } -} - -template -void HistFiller::fillHistAny(std::shared_ptr hist, const T& table, const o2::framework::expressions::Filter& filter) -{ - HistFiller::badHistogramFill(hist->GetName()); -} - template double HistFiller::getSize(std::shared_ptr hist, double fillFraction) { @@ -479,12 +458,5 @@ extern template std::shared_ptr HistogramRegistry::add(cha extern template std::shared_ptr HistogramRegistry::add(char const* const name, char const* const title, HistType histType, const std::vector& axes, bool callSumw2); extern template std::shared_ptr HistogramRegistry::add(char const* const name, char const* const title, const HistogramConfigSpec& histConfigSpec, bool callSumw2); extern template std::shared_ptr HistogramRegistry::add(char const* const name, char const* const title, HistType histType, const std::vector& axes, bool callSumw2); - -template -void HistogramRegistry::fill(const HistName& histName, const T& table, const o2::framework::expressions::Filter& filter) -{ - std::visit([&table, &filter](auto&& hist) { HistFiller::fillHistAny(hist, table, filter); }, mRegistryValue[getHistIndex(histName)]); -} - } // namespace o2::framework #endif // FRAMEWORK_HISTOGRAMREGISTRY_H_ diff --git a/Framework/Core/src/HistogramRegistry.cxx b/Framework/Core/src/HistogramRegistry.cxx index 9caa7cbd1f48e..87b92f058cb5d 100644 --- a/Framework/Core/src/HistogramRegistry.cxx +++ b/Framework/Core/src/HistogramRegistry.cxx @@ -10,6 +10,7 @@ // or submit itself to any jurisdiction. #include "Framework/HistogramRegistry.h" +#include "Framework/ASoA.h" #include #include #include diff --git a/Framework/Core/test/benchmark_HistogramRegistry.cxx b/Framework/Core/test/benchmark_HistogramRegistry.cxx index aec1cfa9c8aaf..d8b7f3438913b 100644 --- a/Framework/Core/test/benchmark_HistogramRegistry.cxx +++ b/Framework/Core/test/benchmark_HistogramRegistry.cxx @@ -10,7 +10,6 @@ // or submit itself to any jurisdiction. #include "Framework/HistogramRegistry.h" -#include "Framework/Logger.h" #include "TList.h" @@ -19,7 +18,6 @@ using namespace o2::framework; using namespace arrow; -using namespace o2::soa; /// Number of lookups to perform const int nLookups = 100000; diff --git a/Framework/Core/test/test_HistogramRegistry.cxx b/Framework/Core/test/test_HistogramRegistry.cxx index fe470683a1614..0abad41e87124 100644 --- a/Framework/Core/test/test_HistogramRegistry.cxx +++ b/Framework/Core/test/test_HistogramRegistry.cxx @@ -10,6 +10,8 @@ // or submit itself to any jurisdiction. #include "Framework/HistogramRegistry.h" +#include "Framework/ASoA.h" +#include "Framework/TableBuilder.h" #include using namespace o2; @@ -70,40 +72,41 @@ TEST_CASE("HistogramRegistryLookup") */ } -TEST_CASE("HistogramRegistryExpressionFill") -{ - TableBuilder builderA; - auto rowWriterA = builderA.persist({"x", "y"}); - rowWriterA(0, 0.0f, -2.0f); - rowWriterA(0, 1.0f, -4.0f); - rowWriterA(0, 2.0f, -1.0f); - rowWriterA(0, 3.0f, -5.0f); - rowWriterA(0, 4.0f, 0.0f); - rowWriterA(0, 5.0f, -9.0f); - rowWriterA(0, 6.0f, -7.0f); - rowWriterA(0, 7.0f, -4.0f); - auto tableA = builderA.finalize(); - REQUIRE(tableA->num_rows() == 8); - using TestA = o2::soa::InPlaceTable<"A/1"_h, o2::soa::Index<>, test::X, test::Y>; - TestA tests{tableA}; - REQUIRE(8 == tests.size()); - - /// Construct a registry object with direct declaration - HistogramRegistry registry{ - "registry", { - {"x", "test x", {HistType::kTH1F, {{100, 0.0f, 10.0f}}}}, // - {"xy", "test xy", {HistType::kTH2F, {{100, -10.0f, 10.01f}, {100, -10.0f, 10.01f}}}} // - } // - }; - - /// Fill histogram with expression and table - registry.fill(HIST("x"), tests, test::x > 3.0f); - REQUIRE(registry.get(HIST("x"))->GetEntries() == 4); - - /// Fill histogram with expression and table - registry.fill(HIST("xy"), tests, test::x > 3.0f && test::y > -5.0f); - REQUIRE(registry.get(HIST("xy"))->GetEntries() == 2); -} +// FIXME: feature not used in its current state, requires rework +// TEST_CASE("HistogramRegistryExpressionFill") +// { +// TableBuilder builderA; +// auto rowWriterA = builderA.persist({"x", "y"}); +// rowWriterA(0, 0.0f, -2.0f); +// rowWriterA(0, 1.0f, -4.0f); +// rowWriterA(0, 2.0f, -1.0f); +// rowWriterA(0, 3.0f, -5.0f); +// rowWriterA(0, 4.0f, 0.0f); +// rowWriterA(0, 5.0f, -9.0f); +// rowWriterA(0, 6.0f, -7.0f); +// rowWriterA(0, 7.0f, -4.0f); +// auto tableA = builderA.finalize(); +// REQUIRE(tableA->num_rows() == 8); +// using TestA = o2::soa::InPlaceTable<"A/1"_h, o2::soa::Index<>, test::X, test::Y>; +// TestA tests{tableA}; +// REQUIRE(8 == tests.size()); + +// /// Construct a registry object with direct declaration +// HistogramRegistry registry{ +// "registry", { +// {"x", "test x", {HistType::kTH1F, {{100, 0.0f, 10.0f}}}}, // +// {"xy", "test xy", {HistType::kTH2F, {{100, -10.0f, 10.01f}, {100, -10.0f, 10.01f}}}} // +// } // +// }; + +// /// Fill histogram with expression and table +// registry.fill(HIST("x"), tests, test::x > 3.0f); +// REQUIRE(registry.get(HIST("x"))->GetEntries() == 4); + +// /// Fill histogram with expression and table +// registry.fill(HIST("xy"), tests, test::x > 3.0f && test::y > -5.0f); +// REQUIRE(registry.get(HIST("xy"))->GetEntries() == 2); +// } TEST_CASE("HistogramRegistryStepTHn") { From 07c3332c7b7ccfcc33bdc2eb74717826ff775c59 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Fri, 26 Jun 2026 11:11:46 +0200 Subject: [PATCH 2/2] restore placeholders --- .../Core/include/Framework/HistogramRegistry.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Framework/Core/include/Framework/HistogramRegistry.h b/Framework/Core/include/Framework/HistogramRegistry.h index 5b254a5ba8181..a09a9988fb576 100644 --- a/Framework/Core/include/Framework/HistogramRegistry.h +++ b/Framework/Core/include/Framework/HistogramRegistry.h @@ -289,6 +289,17 @@ void HistFiller::fillHistAny(std::shared_ptr hist, Ts... positionAndWeight) HistFiller::badHistogramFill(hist->GetName()); } +template +void HistFiller::fillHistAny(std::shared_ptr, const T&, const o2::framework::expressions::Filter&) + requires(!ValidComplexFillStep) && requires(T t) { t.asArrowTable(); } +{ +} + +template +void HistFiller::fillHistAny(std::shared_ptr, const T&, const o2::framework::expressions::Filter&) +{ +} + template double HistFiller::getSize(std::shared_ptr hist, double fillFraction) { @@ -458,5 +469,11 @@ extern template std::shared_ptr HistogramRegistry::add(cha extern template std::shared_ptr HistogramRegistry::add(char const* const name, char const* const title, HistType histType, const std::vector& axes, bool callSumw2); extern template std::shared_ptr HistogramRegistry::add(char const* const name, char const* const title, const HistogramConfigSpec& histConfigSpec, bool callSumw2); extern template std::shared_ptr HistogramRegistry::add(char const* const name, char const* const title, HistType histType, const std::vector& axes, bool callSumw2); + +template +void HistogramRegistry::fill(const HistName& histName, const T& table, const o2::framework::expressions::Filter& filter) +{ + std::visit([&table, &filter](auto&& hist) { HistFiller::fillHistAny(hist, table, filter); }, mRegistryValue[getHistIndex(histName)]); +} } // namespace o2::framework #endif // FRAMEWORK_HISTOGRAMREGISTRY_H_