Skip to content

[Common] Add zdc-extra-table-reader task#16028

Open
udmitrie wants to merge 14 commits into
AliceO2Group:masterfrom
udmitrie:zdcextrareader
Open

[Common] Add zdc-extra-table-reader task#16028
udmitrie wants to merge 14 commits into
AliceO2Group:masterfrom
udmitrie:zdcextrareader

Conversation

@udmitrie
Copy link
Copy Markdown
Contributor

@udmitrie udmitrie commented Apr 29, 2026

zdc-extra-table-reader is needed to read AOD/ZDCEXTRA derived data and proceed with multi-step calibration (Q-vectors recentering) of ZDC data

udmitrie added 2 commits April 29, 2026 18:47
@udmitrie
Add zdc-extra-table-reader task
e4cb846 
zdc-extra-table-reader is needed to read AOD/ZDCEXTRA derived data and proceed with multistep calibration (Q-vectors recentering) of ZDC data
@udmitrie
Add zdc-extra-table-reader workflow to CMakeLists
2faab54
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026 •
edited
Loading

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

@udmitrie
Fix o2 linter errors
7aed372 
Refactor logging and constants in zdcExtraTableReader.
Replaced std::cerr logging with LOGF for error handling and updated constant names for consistency.
@udmitrie udmitrie marked this pull request as ready for review April 29, 2026 17:05
@udmitrie udmitrie marked this pull request as draft April 29, 2026 18:21
@udmitrie
Update zdc-extra-table-reader
a36b0b5 
Add check number of fired towers with isZN*SpDeterminable (to reconstruct Q-vector at least 2 fired towers needed);
Replace FindBin with FindFixBin; 
update variables.
@alibuild alibuild mentioned this pull request Apr 29, 2026
Merged
@udmitrie udmitrie marked this pull request as ready for review April 29, 2026 19:49
@udmitrie
Copy link
Copy Markdown
Contributor Author

udmitrie commented Apr 29, 2026

Dear code owners,
Please take a look at this pull request.

There are no O2 linter errors connected to my changes.
The job is failing due to workflow naming convention violations in old tasks in Common/Tasks/CMakeLists.txt.

vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx
vkucera
vkucera reviewed May 6, 2026
View reviewed changes
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
@alibuild
Copy link
Copy Markdown
Collaborator

alibuild commented May 6, 2026 •
edited
Loading

Error while checking build/O2Physics/staging for 5397abb at 2026-05-07 21:51:

## sw/BUILD/O2Physics-latest/log
CMake Error at /sw/slc9_x86-64/CMake/v4.1.4-2/share/cmake-4.1/Modules/CMakeTestCCompiler.cmake:67 (message):
    2026-05-07T21:51:31.735+0200 [70:140192746028672] [buildboxcommon_grpcretrier.cpp:177] [ERROR] Retry limit (0) exceeded for "ActionCache.GetActionResult()", last gRPC error was [14: failed to connect to all addresses; last error: UNKNOWN: ipv4:127.0.0.1:8085: Failed to connect to remote host: Connection refused]
    2026-05-07T21:51:31.735+0200 [70:140192746028672] [executioncontext.cpp:544] [ERROR] Error while querying action cache at "http://localhost:8085": 14: failed to connect to all addresses; last error: UNKNOWN: ipv4:127.0.0.1:8085: Failed to connect to remote host: Connection refused
    2026-05-07T21:51:31.736+0200 [70:140192746028672] [buildboxcommon_grpcretrier.cpp:177] [ERROR] Retry limit (0) exceeded for "FindMissingBlobs()", last gRPC error was [14: failed to connect to all addresses; last error: UNKNOWN: ipv4:127.0.0.1:8085: Failed to connect to remote host: Connection refused]
    2026-05-07T21:51:31.736+0200 [70:140192746028672] [executioncontext.cpp:908] [ERROR] Error while uploading resources to CAS at "http://localhost:8085": 14: failed to connect to all addresses; last error: UNKNOWN: ipv4:127.0.0.1:8085: Failed to connect to remote host: Connection refused
    ninja: build stopped: subcommand failed.

Full log here.

@alibuild
Copy link
Copy Markdown
Collaborator

alibuild commented May 8, 2026 •
edited
Loading

Error while checking build/O2Physics/o2 for 5397abb at 2026-06-01 11:58:

## sw/BUILD/O2Physics-latest/log
c++: fatal error: Killed signal terminated program cc1plus
ninja: build stopped: subcommand failed.

Full log here.

@vkucera
Copy link
Copy Markdown
Collaborator

vkucera commented Jun 1, 2026

@udmitrie @dsekihat Based on the commit history this PR would have been already labelled as stale. What is the status?

@udmitrie
Copy link
Copy Markdown
Contributor Author

udmitrie commented Jun 1, 2026

Dear @vkucera, I am planning to update this PR this week, addressing your comments.
Sorry for the delay!

udmitrie and others added 2 commits June 3, 2026 15:10
@udmitrie
Refactor event selection and memory management
8811f8c 
Updated event selection configuration and histogram handling in zdcExtraTableReader.cxx. Refactored selection bits and improved memory management in the clearCache method.
@alibuild
Please consider the following formatting changes
49629c5
@alibuild alibuild mentioned this pull request Jun 3, 2026
Merged
vkucera
vkucera reviewed Jun 4, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@vkucera vkucera left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Since you are adding a new task, please make sure it passes all checks without warnings.

Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment on lines +219 to +221
Configurable<int> qxyNbins{"qxyNbins", 100, "Number of bins in QxQy histograms"};
Configurable<float> qxyMin{"qxyMin", -2.0f, "Lower edge for QxQy histograms"};
Configurable<float> qxyMax{"qxyMax", 2.0f, "Upper edge for QxQy histograms"};
Copy link
Copy Markdown
Collaborator

@vkucera vkucera Jun 4, 2026

Choose a reason for hiding this comment

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

Use ConfigurableAxis.

Copy link
Copy Markdown
Contributor Author

@udmitrie udmitrie Jun 5, 2026

Choose a reason for hiding this comment

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

Hi @vkucera,

The 5D histograms require a different number of bins (qNbins5D) but share the same min/max limits as the standard 1D/2D histograms. Since I can't reuse the limits for the 5D AxisSpec, switching to ConfigurableAxis would force me to either hardcode the limits for the 5D axes or duplicate the configurable variables, which actually makes no sense

Copy link
Copy Markdown
Collaborator

@vkucera vkucera Jun 5, 2026

Choose a reason for hiding this comment

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

It is not the case here. These configurables are used always together as {qxyNbins, qxyMin, qxyMax, ...}.

Copy link
Copy Markdown
Contributor Author

@udmitrie udmitrie Jun 5, 2026

Choose a reason for hiding this comment

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

Fixed

Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
@udmitrie
address PR review comments
3ff6bc0 
- Replace 0.0 returns with fatal logs for missing CCDB objects 
- Rename Configurable flags
- Rename 'nShift' to 'nHarmonics' to clarify its physical meaning
- Remove 'm' prefixes from variable names
- Reduce nesting in `loadCalibrations`
@alibuild alibuild mentioned this pull request Jun 5, 2026
Closed
@udmitrie
Fix formatting
368c4aa 
Fix code formatting
@udmitrie
Copy link
Copy Markdown
Contributor Author

udmitrie commented Jun 5, 2026

Dear code owners,

Please note that the failed CI checks are not related to my zdc-extra-table-reader task:

  • o2linter is failing due to naming errors in CMakeLists.txt for unrelated tasks (e.g., propagatorQa, cpvQa);
  • MegaLinter is failing due to the osv-scanner error (according to the recent announcement in the O2 Analysis Announcements channel, this is a known side effect of the linter upgrade)

@vkucera
Copy link
Copy Markdown
Collaborator

vkucera commented Jun 5, 2026

@udmitrie Since you are adding a new file, please fix the warnings as well.

udmitrie added 2 commits June 5, 2026 18:43
@udmitrie
Replace TMath functions with std namespace equivalents
d83ac12
@udmitrie
Add QxQy ConfigurableAxis
bf83055 
Replaced individual Qx and Qy axis configurables with a shared ConfigurableAxis
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

@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

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

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

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

common

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@udmitrie @alibuild @vkucera