Skip to content

Commit 6c167ed

Browse files
committed
Request slice infos for Preslice declarations
Non-optional Preslice declarations no longer fail when the column is missing A declaration that cannot be used is skipped with a warning: - the table is not an input and does not have the column - the table is not an input (non-optional only) - the table does not have the column (non-optional only)
1 parent 3e68711 commit 6c167ed

4 files changed

Lines changed: 101 additions & 7 deletions

File tree

‎Framework/Core/include/Framework/ASoA.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
#if defined(__CLING__)
1616
#error "Please do not include this file in ROOT dictionary generation"
1717
#endif
18+
#include "Framework/InputSpec.h"
1819
#include "Framework/Concepts.h"
19-
#include "Framework/ConcreteDataMatcher.h"
2020
#include "Framework/Pack.h" // IWYU pragma: export
2121
#include "Framework/FunctionalHelpers.h" // IWYU pragma: export
2222
#include "Headers/DataHeader.h" // IWYU pragma: export
@@ -1450,8 +1450,8 @@ struct PresliceBase : public Policy {
14501450

14511451
PresliceBase(expressions::BindingNode index_)
14521452
: Policy{Entry(
1453-
o2::soa::getLabelFromTypeForKey<T, OPT>(std::string{index_.name}),
1454-
o2::soa::getMatcherFromTypeForKey<T, OPT>(std::string{index_.name}),
1453+
o2::soa::getLabelFromTypeForKey<T, true>(std::string{index_.name}),
1454+
o2::soa::getMatcherFromTypeForKey<T, true>(std::string{index_.name}),
14551455
std::string{index_.name}
14561456
)}
14571457
{

‎Framework/Core/include/Framework/AnalysisManagers.h‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,71 @@ bool replaceOrigin(T& presliceGroup, header::DataOrigin const& newOrigin)
627627
return true;
628628
}
629629

630+
template <typename T>
631+
requires(!is_preslice<T> && !is_preslice_group<T>)
632+
bool addSlicingInputs(T&, std::vector<InputSpec>&, header::DataOrigin const&)
633+
{
634+
return false;
635+
}
636+
637+
/// check if any of the tables the sliced type is based on is an input of the task
638+
template <soa::is_table T>
639+
bool isSlicedTableInput(std::vector<InputSpec> const& inputs, header::DataOrigin const& newOrigin)
640+
{
641+
auto isInput = [&inputs, &newOrigin](ConcreteDataMatcher matcher) {
642+
if ((matcher.origin == header::DataOrigin{"AOD"}) && (newOrigin != header::DataOrigin{"AOD"})) {
643+
matcher = replaceOrigin(matcher, newOrigin);
644+
}
645+
return std::ranges::any_of(inputs, [&matcher](InputSpec const& input) { return DataSpecUtils::match(input, matcher); });
646+
};
647+
return [&isInput]<size_t... Is>(std::index_sequence<Is...>) {
648+
return (isInput(o2::aod::matcher<T::originals[Is]>()) || ...);
649+
}(std::make_index_sequence<T::originals.size()>{});
650+
}
651+
652+
/// all the process function inputs are already added at this point, so a Preslice can only
653+
/// amend them. Depending on whether the sliced table is an input of the task and whether it has
654+
/// the index column, there are 4 cases:
655+
/// 1. no table, no column - likely an incorrect declaration, warning for both Preslice and PresliceOptional
656+
/// 2. no table, column - Preslice that never works, or a common declaration in a templated task that is
657+
/// not effective in this specialization, warning for Preslice only
658+
/// 3. table, no column - the intended case for PresliceOptional, a mistake for Preslice, warning for Preslice only
659+
/// 4. table, column - slicing input is added
660+
template<is_preslice T>
661+
bool addSlicingInputs(T& preslice, std::vector<InputSpec>& inputs, header::DataOrigin const& newOrigin)
662+
{
663+
using target_t = typename T::target_t;
664+
auto const& [binding, matcher, key, enabled] = preslice.bindingKey;
665+
if (preslice.isMissing()) {
666+
if (!isSlicedTableInput<target_t>(inputs, newOrigin)) {
667+
LOGP(warn, "Preslice declared on {} is skipped: {} is not an input of any process function and does not have column {}, the declaration is likely incorrect",
668+
o2::soa::getLabelFromType<target_t>(), o2::soa::getLabelFromType<target_t>(), key);
669+
} else if constexpr (!T::optional) {
670+
LOGP(warn, "Preslice declared on {} is skipped: it does not have column {}, use PresliceOptional if the column is not always expected",
671+
o2::soa::getLabelFromType<target_t>(), key);
672+
}
673+
return true;
674+
}
675+
if (std::ranges::none_of(inputs, [&matcher](InputSpec const& input) { return DataSpecUtils::match(input, matcher); })) {
676+
if constexpr (!T::optional) {
677+
LOGP(warn, "Preslice declared on {}/{} ({}) is skipped: {} is not an input of any process function, use PresliceOptional if the declaration is not effective in every specialization of a templated task",
678+
binding, key, DataSpecUtils::describe(matcher), binding);
679+
}
680+
return true;
681+
}
682+
DataSpecUtils::updateInputList(inputs, inputForEntry(preslice.bindingKey, std::same_as<typename T::policy_t, framework::PreslicePolicySorted>));
683+
return true;
684+
}
685+
686+
template <is_preslice_group T>
687+
bool addSlicingInputs(T&& presliceGroup, std::vector<InputSpec>& inputs, header::DataOrigin const& newOrigin)
688+
{
689+
homogeneous_apply_refs<true>([&inputs, &newOrigin](auto& preslice){
690+
return addSlicingInputs(preslice, inputs, newOrigin);
691+
}, presliceGroup);
692+
return true;
693+
}
694+
630695
template <typename T>
631696
requires(!is_preslice<T> && !is_preslice_group<T>)
632697
bool registerCache(T&, Cache&, Cache&)

‎Framework/Core/include/Framework/AnalysisTask.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,11 @@ DataProcessorSpec adaptAnalysisTask(ConfigContext const& ctx, Args&&... args)
635635
// replace origins in Preslice declarations
636636
homogeneous_apply_refs_sized<numElements>([&newOrigin](auto& element) { return analysis_task_parsers::replaceOrigin(element, newOrigin); }, *task.get());
637637

638+
// add slicing inputs from Preslice declarations
639+
homogeneous_apply_refs_sized<numElements>([&inputs, &newOrigin](auto& element){
640+
return analysis_task_parsers::addSlicingInputs(element, inputs, newOrigin);
641+
}, *task.get());
642+
638643
auto algo = AlgorithmSpec::InitCallback
639644
{
640645
[task = task, expressionInfos, inputInfos, newOrigin, newOriginStr](InitContext& ic) mutable {

‎Framework/Core/test/test_AnalysisTask.cxx‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,11 @@ struct KTask {
177177
std::shared_ptr<int> someSharedInt;
178178
};
179179

180+
181+
// PresliceOptional ignores situations where the table is present, but does
182+
// not have the requested column
180183
struct LTask {
181184
SliceCache cache;
182-
Preslice<aod::Tracks> perCol = aod::track::collisionId;
183-
PresliceOptional<aod::Tracks> perPart = aod::mctracklabel::mcParticleId;
184185
PresliceUnsorted<aod::McCollisionLabels> perMcCol = aod::mccollisionlabel::mcCollisionId;
185186
PresliceUnsortedOptional<aod::Collisions> perMcColopt = aod::mccollisionlabel::mcCollisionId;
186187
void process(aod::McCollision const&, soa::SmallGroups<soa::Join<aod::Collisions, aod::McCollisionLabels>> const&) {}
@@ -189,14 +190,31 @@ struct LTask {
189190
struct MTask {
190191
SliceCache cache;
191192
struct : public PresliceGroup {
192-
Preslice<aod::Tracks> perCol = aod::track::collisionId;
193-
PresliceOptional<aod::Tracks> perPart = aod::mctracklabel::mcParticleId;
194193
PresliceUnsorted<aod::McCollisionLabels> perMcCol = aod::mccollisionlabel::mcCollisionId;
195194
PresliceUnsortedOptional<aod::Collisions> perMcColopt = aod::mccollisionlabel::mcCollisionId;
196195
} foo;
197196
void process(aod::McCollision const&, soa::SmallGroups<soa::Join<aod::Collisions, aod::McCollisionLabels>> const&) {}
198197
};
199198

199+
// Preslices that cannot be used are skipped with a warning:
200+
// - table is not an input and does not have the column
201+
// - table is not an input
202+
// - table does not have the column
203+
struct NTask {
204+
SliceCache cache;
205+
Preslice<aod::Tracks> perPart = aod::mctracklabel::mcParticleId;
206+
Preslice<aod::Tracks> perCol = aod::track::collisionId;
207+
Preslice<aod::Collisions> perMcCol = aod::mccollisionlabel::mcCollisionId;
208+
void process(aod::McCollision const&, soa::SmallGroups<soa::Join<aod::Collisions, aod::McCollisionLabels>> const&) {}
209+
};
210+
211+
// optional Preslice on a table that is not an input of any process function is skipped silently
212+
struct OTask {
213+
SliceCache cache;
214+
PresliceOptional<aod::Tracks> perCol = aod::track::collisionId;
215+
void process(aod::McCollision const&, soa::SmallGroups<soa::Join<aod::Collisions, aod::McCollisionLabels>> const&) {}
216+
};
217+
200218
TEST_CASE("AdaptorCompilation")
201219
{
202220
auto cfgc = makeEmptyConfigContext();
@@ -276,6 +294,12 @@ TEST_CASE("AdaptorCompilation")
276294

277295
auto task13 = adaptAnalysisTask<MTask>(*cfgc, TaskName{"test13"});
278296
REQUIRE(task13.inputs.size() == 4); // 3 base + 1 actual slice
297+
298+
auto task14 = adaptAnalysisTask<NTask>(*cfgc, TaskName{"test14"});
299+
REQUIRE(task14.inputs.size() == 4); // 3 base + 1 actual slice, all Preslices skipped
300+
301+
auto task15 = adaptAnalysisTask<OTask>(*cfgc, TaskName{"test15"});
302+
REQUIRE(task15.inputs.size() == 4); // 3 base + 1 actual slice, optional Preslice skipped
279303
}
280304

281305
TEST_CASE("TestPartitionIteration")

0 commit comments

Comments
 (0)