Skip to content

DPL Analysis: preliminary work for multiple process() functions - #6019

Merged
ktf merged 13 commits into
AliceO2Group:devfrom
aalkin:multiple-process-preliminary
May 3, 2021
Merged

DPL Analysis: preliminary work for multiple process() functions#6019
ktf merged 13 commits into
AliceO2Group:devfrom
aalkin:multiple-process-preliminary

Conversation

@aalkin

@aalkin aalkin commented Apr 28, 2021

Copy link
Copy Markdown
Member

Comment thread Framework/Core/include/Framework/AnalysisTask.h Outdated
@aalkin
aalkin force-pushed the multiple-process-preliminary branch from 6de3b63 to 020c84d Compare April 30, 2021 09:00
@aalkin

aalkin commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

@ktf @jgrosseo this is a working example of multiple process functions in an analysis task. Process functions can have different signatures, only the selected functions will affect what the task subscribes to. The only caveat currently is that filters are applied for each of the process functions, so I'll have to add some caching behavior for selections.

Comment thread Analysis/Tutorials/src/histogramsFullTracks.cxx Outdated
Comment thread Analysis/Tutorials/src/histogramsFullTracks.cxx Outdated
ktf
ktf previously approved these changes Apr 30, 2021

@ktf ktf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful. Apart from the capitalisation, I have nothing else to add.

@jgrosseo

Copy link
Copy Markdown
Collaborator

Excellent! This was very fast. I am in favour of merging this.

Next, we can look at how to make this more than trivially configurable.

For instance:
adaptAnalysisTask<MultipleProcessExample>(cfgc, ProcessesOptional{&MultipleProcessExample::processRec, cfgc.options().get<bool>("doRec"), &MultipleProcessExample::processGen, cfgc.options().get<bool>("doGen"), &MultipleProcessExample::processResolution}, cfgc.options().get<bool>("doResolution"))};

@aalkin

aalkin commented Apr 30, 2021

Copy link
Copy Markdown
Member Author

It still needs filter caching, otherwise it would be a performance problem. Related to that is the question whether we also want to have optionally different filters for different processes.

Configuration like that is not trivial, we'll have to discuss what we can do.

@ktf

ktf commented Apr 30, 2021

Copy link
Copy Markdown
Member

why performance problem? doing the filters twice is what would happen right now with two devices, no?

@aalkin

aalkin commented May 2, 2021

Copy link
Copy Markdown
Member Author

It seems adding proper caching also requires also changing how Filtered behaves, currently it always has an internal copy of a selection vector. Since this is already functional I'm marking it ready for review and caching will be the next improvement for this.

@aalkin
aalkin marked this pull request as ready for review May 2, 2021 11:23
@aalkin
aalkin requested review from a team, ginnocen, iarsene and jgrosseo as code owners May 2, 2021 11:23
@ktf
ktf merged commit 5835f06 into AliceO2Group:dev May 3, 2021
@jgrosseo

jgrosseo commented May 3, 2021

Copy link
Copy Markdown
Collaborator

@aalkin I think the filter caching should anyway go beyond the same process. Our idea is to pull this into a common process if there are several tasks (in independent processes) having the same filter, right?

@aalkin

aalkin commented May 3, 2021

Copy link
Copy Markdown
Member Author

@aalkin I think the filter caching should anyway go beyond the same process. Our idea is to pull this into a common process if there are several tasks (in independent processes) having the same filter, right?

At a baseline we need to cache the filter result (row list) in a task, so that it can be supplied to each process() in the task that requires same-ish argument. For example if all process() functions require some sort of join based on aod::Collisions we only need to create the row list once and re-use it each time we create the Filtered argument for a given process(). I already have an idea on how to implement it, but I need to invent a good static way to determine which process() arguments are equivalent in terms of required filters so I can avoid runtime checks.

@aalkin
aalkin deleted the multiple-process-preliminary branch May 30, 2022 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants