Skip to content

[Common] alternative matching as default for OO, NeNe, occupancy update #13537

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.

Sign up for GitHub

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

Jump to bottom
Merged
altsybee merged 3 commits into from
Oct 27, 2025
Merged

[Common] alternative matching as default for OO, NeNe, occupancy update #13537

altsybee merged 3 commits into from
Oct 27, 2025

Conversation

@altsybee
Copy link
Collaborator

@altsybee altsybee commented Oct 24, 2025 •
edited
Loading

  • "Alternative" bc-collision matching for OO and Ne-Ne (based on the filling scheme pattern) is now the default
  • Modification of the occupancy calculation: info at the ROF borders is "replaced" with the scaled FT0 ampl.
  • The same is applied at TF borders --> extended range with available info for occupancy calculation --> recovery of a few % of Pb-Pb statistics in analyses which use occupancy cuts
  • Refinement of the logic of the kNoCollInTimeRangeNarrow occupancy ev sel bit
  • Several configurables for tests are removed
  • Temporary workaround for MC prod LHC25f3 is removed

@github-actions
Copy link

github-actions bot commented Oct 24, 2025 •
edited
Loading

O2 linter results: ❌ 27 errors, ⚠️ 9 warnings, 🔕 47 disabled

@github-actions github-actions bot changed the title alternative matching as default for OO, NeNe, occupancy update [Common] alternative matching as default for OO, NeNe, occupancy update Oct 24, 2025
@alibuild
Copy link
Collaborator

alibuild commented Oct 24, 2025 •
edited
Loading

Error while checking build/O2Physics/o2 for ff8aebf at 2025-10-25 01:31:

## sw/BUILD/O2Physics-latest/log
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
/sw/SOURCES/O2Physics/13537-slc9_x86-64/0/Common/Tools/EventSelectionModule.h:1381:13: error: unused variable 'bcInTF' [-Werror=unused-variable]
/sw/SOURCES/O2Physics/13537-slc9_x86-64/0/Common/Tools/EventSelectionModule.h:1381:13: error: unused variable 'bcInTF' [-Werror=unused-variable]
ninja: build stopped: subcommand failed.

Full log here.

@altsybee altsybee enabled auto-merge (squash) October 26, 2025 08:48
@altsybee altsybee merged commit 22c28ea into AliceO2Group:master Oct 27, 2025
11 of 13 checks passed
vkucera
vkucera reviewed Oct 27, 2025
View reviewed changes
Common/Tools/EventSelectionModule.h
Comment on lines +693 to +698
if (dt >= -40 && dt < -5) // collisions in the past // o2-linter: disable=magic-number
wOccup = 1. / 1225 * (dt + 40) * (dt + 40); // o2-linter: disable=magic-number
else if (dt >= -5 && dt < 15) // collisions near a given one // o2-linter: disable=magic-number
wOccup = 1;
else if (dt >= 15 && dt < 40) // collisions from the future // o2-linter: disable=magic-number
wOccup = -0.4 / 25 * dt + 1.24; // o2-linter: disable=magic-number
Copy link
Collaborator

@vkucera vkucera Oct 27, 2025

Choose a reason for hiding this comment

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

@altsybee @ddobrigk The magic numbers make the code completely incomprehensible to anyone who doesn't already know what they mean, making this class a black box to the rest of the Collaboration. Please write code that your colleagues can understand instead of silencing linter errors.

ThePhDane pushed a commit to ThePhDane/O2Physics that referenced this pull request Nov 3, 2025
@altsybee @ThePhDane
[Common] alternative matching as default for OO, NeNe, occupancy upda…
0e1c3a3 
…te (AliceO2Group#13537)
lmattei01 pushed a commit to lmattei01/O2Physics that referenced this pull request Dec 5, 2025
@altsybee
[Common] alternative matching as default for OO, NeNe, occupancy upda…
76b5dba 
…te (AliceO2Group#13537)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkucera vkucera vkucera left review comments

@ddobrigk ddobrigk ddobrigk approved these changes

@alibuild alibuild Awaiting requested review from alibuild alibuild is a code owner

@jgrosseo jgrosseo Awaiting requested review from jgrosseo jgrosseo is a code owner

@iarsene iarsene Awaiting requested review from iarsene iarsene is a code owner

@ktf ktf Awaiting requested review from ktf ktf is a code owner

Assignees

No one assigned

Labels

common

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@altsybee @alibuild @vkucera @ddobrigk