Skip to content

[Common,PWGCF] EventPlaneHelper: invoke geometry invariant helper once#16444

Open
ktf wants to merge 2 commits into
AliceO2Group:masterfrom
ktf:pr16444
Open

[Common,PWGCF] EventPlaneHelper: invoke geometry invariant helper once#16444
ktf wants to merge 2 commits into
AliceO2Group:masterfrom
ktf:pr16444

Conversation

@ktf

@ktf ktf commented May 29, 2026

Copy link
Copy Markdown
Member

calculateChannelCenter() was called inside
GetPhiFT0 on every channel, recomputing all 208 channel
positions from scratch each time despite the geometry being
constant.

@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown

O2 linter results: ❌ 13 errors, ⚠️ 46 warnings, 🔕 0 disabled

@github-actions github-actions Bot changed the title EventPlaneHelper: avoid copying FT0 Geometry on every call [Common,PWGCF] EventPlaneHelper: avoid copying FT0 Geometry on every call May 29, 2026
@ktf

ktf commented Jun 1, 2026

Copy link
Copy Markdown
Member Author

@dsekihat @ddobrigk can you please find out who should approve this? It's a rather significant hot spot in our profiles for no particular reason.

ktf added 2 commits June 8, 2026 13:42
GetPhiFT0 and SumQvectors took o2::ft0::Geometry by value,
causing a full copy (TObject heap allocation + destruction) on
every invocation. SumQvectors is called ~208 times per event,
so this was ~14% of CPU in Q-vector trains.
calculateChannelCenter() was called inside
GetPhiFT0 on every channel, recomputing all 208 channel
positions from scratch each time despite the geometry being
constant.
@ktf ktf changed the title [Common,PWGCF] EventPlaneHelper: avoid copying FT0 Geometry on every call EventPlaneHelper: avoid copying FT0 Geometry on every call Jun 8, 2026
@github-actions github-actions Bot removed the pwgcf label Jun 8, 2026
@github-actions github-actions Bot changed the title EventPlaneHelper: avoid copying FT0 Geometry on every call [Common] EventPlaneHelper: avoid copying FT0 Geometry on every call Jun 8, 2026
@ktf

ktf commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

Giving there was no follow up, I have split some purely technical improvement (using a const &) from the minor change in the algorithm. I will merge this one if it passes tests.

@alibuild

alibuild commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/o2 for a97b2a1 at 2026-06-08 13:46:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/slc9_x86-64-slc9_x86-64/0/Common/Core/EventPlaneHelper.cxx:77:33: error: passing 'const o2::ft0::Geometry' as 'this' argument discards qualifiers [-fpermissive]
ninja: build stopped: subcommand failed.

Full log here.

@alibuild

alibuild commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/staging for a97b2a1 at 2026-06-08 13:50:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/slc9_x86-64-slc9_x86-64/0/Common/Core/EventPlaneHelper.cxx:77:33: error: passing 'const o2::ft0::Geometry' as 'this' argument discards qualifiers [-fpermissive]
ninja: build stopped: subcommand failed.

Full log here.

@ktf ktf changed the title [Common] EventPlaneHelper: avoid copying FT0 Geometry on every call EventPlaneHelper: invoke geometry invariant helper once Jun 8, 2026
@ktf ktf requested a review from lauraser as a code owner June 8, 2026 20:26
@github-actions github-actions Bot added the pwgcf label Jun 8, 2026
@github-actions github-actions Bot changed the title EventPlaneHelper: invoke geometry invariant helper once [Common,PWGCF] EventPlaneHelper: invoke geometry invariant helper once Jun 8, 2026
@ktf

ktf commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

Actually the trivial change does not work without the reshuffling of the method. @dsekihat can you follow up on this please? It should be fairly trivial to review.

@dsekihat

dsekihat commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Dear @ktf , I apologize for my delay. I overlooked automatic notifications. The PR is good for me, too.
For qVectorTable, @jikim1290 should follow up. Please make sure that the calibration parameters do not change.
For CF part, CF code owners should follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants