Skip to content

StepTHn: do a pre lookup for the bin using a uniform grid#15444

Merged
ktf merged 2 commits into
AliceO2Group:devfrom
ktf:pr15444
Jun 1, 2026
Merged

StepTHn: do a pre lookup for the bin using a uniform grid#15444
ktf merged 2 commits into
AliceO2Group:devfrom
ktf:pr15444

Conversation

@ktf
Copy link
Copy Markdown
Member

@ktf ktf commented May 27, 2026

Rather than doing a O(log N) binary search for the correct bin, precompute a uniform index for a O(1) lookup and find the actual bin using a short linear search.


Stack created with Sapling. Best reviewed with ReviewStack.

@ktf
Copy link
Copy Markdown
Member Author

ktf commented May 27, 2026

@jgrosseo with these two I gain 10% in performance for the Correlation business. On hyperloop the results seem to be compatible, for what I can tell. Can you validate and eventually merge?

@jgrosseo
Copy link
Copy Markdown
Collaborator

I can look into this next week...

ktf added 2 commits June 1, 2026 10:59
Replaces 4 virtual calls to GetAt / SetAt / AddAt  with just one to updateBin.
Rather than doing a O(log N) binary search for the correct bin, precompute a uniform index for a O(1) lookup and find the actual bin using a short linear search.
@ktf
Copy link
Copy Markdown
Member Author

ktf commented Jun 1, 2026

@jgrosseo: I ran your script and did a comparison for both #15443 and this one.

The simple one (#15443) which simply gets rid of the repeated dynamic dispatch resulted in identical AnalysisRoot.root files (and by consequence, also dphi_corr.root files are identical).

This one was a bit more tricky, because a few bins were differing because of some rounding errors due slightly different bin finding logic. This was fixed and the logic is now exactly the same as TAxis::FindBin. The results are now identical, both for dphi_corr.root and for the actual AnalysisResults.root content.

I attach a PDF with the validation: dphi_comparison.pdf for this PR.

I think both PRs are ready to be merged.

@jgrosseo
Copy link
Copy Markdown
Collaborator

jgrosseo commented Jun 1, 2026

Thanks for the extensive validation. Fingers crossed :)

@ktf ktf requested a review from jgrosseo June 1, 2026 13:49
@ktf
Copy link
Copy Markdown
Member Author

ktf commented Jun 1, 2026

Yes, and it's monday, so we have a full week to fix it... :-D

@ktf ktf merged commit e1b27a6 into AliceO2Group:dev Jun 1, 2026
17 checks passed
@ktf
Copy link
Copy Markdown
Member Author

ktf commented Jun 1, 2026

discussed privately and merged.

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.

2 participants