Skip to content

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

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

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

Conversation

@udmitrie
Copy link
Copy Markdown
Contributor

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

zdc-extra-table-reader is needed to read AOD/ZDCEXTRA derived data and proceed with multistep calibration (Q-vectors recentering) of ZDC data
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

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

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
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.
@udmitrie udmitrie marked this pull request as ready for review April 29, 2026 19:49
@udmitrie
Copy link
Copy Markdown
Contributor Author

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.

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 thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
Comment on lines +635 to +637
} else {
LOGF(error, " >> CCDB TList is NULL for path: %s. Check object type (TList vs TFile).", folder.c_str());
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reverse the logic to return early if (!lst) and reduce nesting.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not addressed

Comment thread Common/Tasks/zdcExtraTableReader.cxx Outdated
@alibuild
Copy link
Copy Markdown
Collaborator

alibuild commented May 6, 2026

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

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
Updated event selection configuration and histogram handling in zdcExtraTableReader.cxx. Refactored selection bits and improved memory management in the clearCache method.
Copy link
Copy Markdown
Collaborator

@vkucera vkucera left a comment

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.

double getMeanQFromMap(THn* h, double cent, double vx, double vy, double vz)
{
if (!h) {
// LOGF(error, "[MeanQ] Null THn pointer");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you not throw a fatal?

Comment on lines +180 to +181
if (!axCent || !axVx || !axVy || !axVz) {
LOGF(error, "[MeanQ] One of THn axes is null");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you not throw a fatal?

Comment on lines +191 to +193
double meanQ = h->GetBinContent(idx);

return meanQ;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You can just return directly.

Comment on lines +200 to +201
if (!h) {
return 0.0;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you not throw a fatal?

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

Choose a reason for hiding this comment

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

Use ConfigurableAxis.


Configurable<int> phiNbins{"phiNbins", 60, "Bins in phi"};

Configurable<int> nTowersFired{"nTowersFired", 2, "Minimum number of towers fired for Q-vector determination"};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Name the configurable such that it is clear that this is a minimum.

Comment on lines +251 to +256
Configurable<bool> ifSel8{"ifSel8", true, "Event selection: Sel8"};
Configurable<bool> ifZVtxCut{"ifZVtxCut", true, "Event selection: zVtx cut set in producer (tipically < 10 cm)"};
Configurable<bool> ifOccupancyCut{"ifOccupancyCut", false, "Event selection: occupancy cut set in producer"};
Configurable<bool> ifNoSameBunchPileup{"ifNoSameBunchPileup", false, "Event selection: no same bunch pileup"};
Configurable<bool> ifIsGoodZvtxFT0vsPV{"ifIsGoodZvtxFT0vsPV", false, "Event selection: good Zvtx FT0 vs PV"};
Configurable<bool> ifNoCollInTimeRangeStandard{"ifNoCollInTimeRangeStandard", false, "Event selection: no collision in time range standard"};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do these start with if?
If you are you trying to communicate that this enables the application of a cut, use a verb, like enable, apply, use, select, reject,...


Configurable<bool> ifShiftCorrection{"ifShiftCorrection", false, "Apply shift correction (Read from CCDB)"};
Configurable<bool> fillShiftHistos{"fillShiftHistos", true, "Fill shift profiles (Write to output)"};
Configurable<int> nShift{"nShift", 10, "Number of harmonics"};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The name of the variable indicates that this a number of shifts while the documentation says that it's a number of harmonics. So what is it?

NEventSelections
};

int mCurrentRunNumber{-1};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What does m mean?

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

Labels

Development

Successfully merging this pull request may close these issues.

3 participants