-
Notifications
You must be signed in to change notification settings - Fork 483
DPL Analysis: use offset cache for sorted grouping #14571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
REQUEST FOR PRODUCTION RELEASES: This will add The following labels are available |
0d219a5 to
0315fa1
Compare
|
I have tested this with Correlations on hyperloop (which I understand uses grouping) and it seems to. I do not see any particular change in performance though. @ddobrigk Could you comment on wether your tests improved? I will then merge. |
|
@ddobrigk you can use my tag on hyperloop (eulisse-local) to have a build with the PR included. |
|
Hi @ktf yes good point. Let me test right away, will write once I have results ! |
|
To clarify: the performance in the usual cases - grouping of tracks over collisions in normal AODs - should be unchanged. The performance impact will be quite noticeable in derived data that packs a lot of entries in the tables used for grouping. |
Yes, sure. I will run the same synthetic tests in which I vary the number of elements to group from an average of 1e-2 to 100 elements per "collision", so that we can directly gauge. The main problem is I screwed something up and am now stuck recompiling (sorry). More soon ... |
|
@aalkin @ktf I finished running a first set of benchmarks with a 'synthetic' AO2D scenario in which I have multiple test sizes for elements being grouped to collisions, from an average of 1e-2 elements per collision to 10^4 elements per collision. This PR helps a lot already: see the benchmark result below, with the improved version gaining, say, up to an order of magnitude in the (very common!) range around 1-10 elements (e.g. tracks, V0s, etc) grouped per grouping entity (e.g. collision): it is much improved! :-) For reference, the 'custom' grouping option corresponds to this code (very simple, very naive, and capable of dealing with the unsorted case). If you like, I can also explore other scenarios (fixed DF size at X megabytes, etc) for testing: just let me know :-)
|

Original algorithm was performing extremely poorly for grouping by very large (in terms of number of rows) tables, typical for the derived data, due to being O(N^2). Precalculating the offset cache at the start solves this issue, dramatically improving the performance for the extreme cases.
@ddobrigk