TPC QC: Add GPUErrorQA task #14397

ariedel-cern · 2025-06-12T11:14:22Z

Following up on #13964

github-actions · 2025-06-12T11:15:20Z

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0
async-2024-PbPb-apass1
async-2024-ppRef-apass1
async-2024-PbPb-apass2
async-2023-PbPb-apass5

wiechula

Thanks @ariedel-cern . Please check for compiler warnings and fix them.

wiechula · 2025-06-12T11:49:13Z

Detectors/TPC/qc/src/GPUErrorQA.cxx

+  auto f = std::unique_ptr<TFile>(TFile::Open(filename.data(), "recreate"));
+  TObjArray arr;
+  arr.SetName("GPUErrorQA_Hists");
+  for (const auto& [name, hist] : mMapHist) {


Avoid warning.

Suggested change

for (const auto& [name, hist] : mMapHist) {

for ([[maybe_unused]] const auto& [name, hist] : mMapHist) {

Added. Looking at other PRs, the full CI seems to be broken at the moment. Testing locally, I see no compiler warnings.

alibuild · 2025-06-12T12:55:28Z

Error while checking build/O2/fullCI_slc9 for 41445c2 at 2025-06-12 14:55:

input queue has size 75
output queue has size 1
input queue has size 67
output queue has size 1
input queue has size 54
output queue has size 1
input queue has size 47
output queue has size 1
input queue has size 39
output queue has size 1
input queue has size 24
output queue has size 1
input queue has size 18
output queue has size 1
input queue has size 7
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
++ cp thinned_compile_commands.json compile_commands.json
++ CHECKS='-*,modernize-avoid-bind,modernize-deprecated-headers,modernize-make-shared,modernize-raw-string-literal,modernize-redundant-void-arg,modernize-replace-auto-ptr,modernize-replace-random-shuffle,modernize-shrink-to-fit,modernize-unary-static-assert,modernize-use-equals-default,modernize-use-noexcept,modernize-use-nullptr,modernize-use-override,modernize-use-transparent-functors,modernize-use-uncaught-exceptions,readability-braces-around-statements'
++ tee error-log.txt
+++ which O2codecheck
+++ find /sw/slc9_x86-64/GCC-Toolchain/v14.2.0-alice2-1/lib -name crtbegin.o -exec dirname '{}' ';'
++ run_O2CodeChecker.py -j 32 -clang-tidy-binary /sw/slc9_x86-64/o2codechecker/v18.1.2.1-local1/bin/O2codecheck -clang-apply-replacements-binary /sw/slc9_x86-64/Clang/v18.1.8-21/bin-safe/clang-apply-replacements -extra-args=--extra-arg=--gcc-install-dir=/sw/slc9_x86-64/GCC-Toolchain/v14.2.0-alice2-1/lib/gcc/x86_64-unknown-linux-gnu/14.2.0 '-header-filter=.*SOURCES(?!.*/3rdparty/).*' '-checks=-*,modernize-avoid-bind,modernize-deprecated-headers,modernize-make-shared,modernize-raw-string-literal,modernize-redundant-void-arg,modernize-replace-auto-ptr,modernize-replace-random-shuffle,modernize-shrink-to-fit,modernize-unary-static-assert,modernize-use-equals-default,modernize-use-noexcept,modernize-use-nullptr,modernize-use-override,modernize-use-transparent-functors,modernize-use-uncaught-exceptions,readability-braces-around-statements'
No checks enabled.
Unable to run clang-tidy.

Full log here.

alibuild · 2025-06-12T16:52:28Z

Error while checking build/O2/fullCI_slc9 for 46787b5 at 2025-06-15 21:24:

input queue has size 76
output queue has size 1
input queue has size 68
output queue has size 1
input queue has size 56
output queue has size 1
input queue has size 48
output queue has size 1
input queue has size 40
output queue has size 1
input queue has size 25
output queue has size 1
input queue has size 21
output queue has size 1
input queue has size 7
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
input queue has size 0
output queue has size 1
++ cp thinned_compile_commands.json compile_commands.json
++ CHECKS='-*,modernize-avoid-bind,modernize-deprecated-headers,modernize-make-shared,modernize-raw-string-literal,modernize-redundant-void-arg,modernize-replace-auto-ptr,modernize-replace-random-shuffle,modernize-shrink-to-fit,modernize-unary-static-assert,modernize-use-equals-default,modernize-use-noexcept,modernize-use-nullptr,modernize-use-override,modernize-use-transparent-functors,modernize-use-uncaught-exceptions,readability-braces-around-statements'
++ tee error-log.txt
+++ which O2codecheck
+++ find /sw/slc9_x86-64/GCC-Toolchain/v14.2.0-alice2-1/lib -name crtbegin.o -exec dirname '{}' ';'
++ run_O2CodeChecker.py -j 32 -clang-tidy-binary /sw/slc9_x86-64/o2codechecker/v18.1.2.1-local1/bin/O2codecheck -clang-apply-replacements-binary /sw/slc9_x86-64/Clang/v18.1.8-21/bin-safe/clang-apply-replacements -extra-args=--extra-arg=--gcc-install-dir=/sw/slc9_x86-64/GCC-Toolchain/v14.2.0-alice2-1/lib/gcc/x86_64-unknown-linux-gnu/14.2.0 '-header-filter=.*SOURCES(?!.*/3rdparty/).*' '-checks=-*,modernize-avoid-bind,modernize-deprecated-headers,modernize-make-shared,modernize-raw-string-literal,modernize-redundant-void-arg,modernize-replace-auto-ptr,modernize-replace-random-shuffle,modernize-shrink-to-fit,modernize-unary-static-assert,modernize-use-equals-default,modernize-use-noexcept,modernize-use-nullptr,modernize-use-override,modernize-use-transparent-functors,modernize-use-uncaught-exceptions,readability-braces-around-statements'
No checks enabled.
Unable to run clang-tidy.

Full log here.

davidrohr · 2025-06-15T13:04:51Z

Detectors/TPC/qc/src/GPUErrorQA.cxx

+  // get gpu error names
+  // copied from GPUErrors.h
+  static std::unordered_map<uint32_t, const char*> errorNames = {
+#define GPUCA_ERROR_CODE(num, name, ...) {num, GPUCA_M_STR(name)},


Can't you just include GPUErrors.h instead of copy & pasting the code?

Hi David, the errorNames were defined in GPUError.cxx, not in the header. I was not sure if this was done on purpose, so that's why I copied the code.
I moved the definition now to the header, so I can include it in the QA task.

Ah, I was confused due to your comment // copied from GPUErrors.h. Indeed, the definition was in the CXX, and I'd prefer it to stay in the CXX. No need to full the header file with the strings.
Could you put a

#ifndef GPUCA_GPUCODE_DEVICE static std::unordered_map<uint32_t, const char*> errorNames #endif

in GPUErrors.h, but leave the actual definition in the cxx.
And please also protect the <unordered_map> include in the header with the same ifdef.

davidrohr · 2025-06-17T07:46:07Z

Detectors/TPC/qc/src/GPUErrorQA.cxx

+  // for convienence, label each bin with the error name
+  for (size_t bin = 1; bin < mMapHist["ErrorCounter"]->GetNbinsX(); bin++) {
+    auto const& it = o2::gpu::errorNames.find(bin);
+    mMapHist["ErrorCounter"]->GetXaxis()->SetBinLabel(bin, it->second);


Just in case we will have a gap in the error numbers in the future, I would propose to check if it is a valid error Name.

alibuild · 2025-06-17T08:44:28Z

Error while checking build/O2/fullCI_slc9 for 7b5264a at 2025-06-17 10:44:

## sw/BUILD/O2-latest/log
/sw/SOURCES/O2/14397-slc9_x86-64/0/GPU/GPUTracking/Global/GPUErrors.h:20:10: fatal error: 'unordered_map' file not found
/sw/SOURCES/O2/14397-slc9_x86-64/0/GPU/GPUTracking/Global/GPUErrors.h:20:10: fatal error: unordered_map: No such file or directory
ninja: build stopped: subcommand failed.

Full log here.

davidrohr

The new approach looks good in general, but see the 2 comments below:

davidrohr · 2025-06-17T15:21:17Z

GPU/GPUTracking/Global/GPUErrors.cxx

  memset(mErrors, 0, GPUCA_MAX_ERRORS * sizeof(*mErrors));
 }

+static std::unordered_map<uint32_t, const char*> errorNames = {


Can you move this as static variable inside getErrorNames, so that it is not a global symbol?
Alternatively, please put it in o2::gpu::internal namespace.

davidrohr · 2025-06-17T15:22:03Z

GPU/GPUTracking/Global/GPUErrors.h

 #include "GPUCommonDef.h"
+#include "GPUDefMacros.h"
+#include <unordered_map>



Still, you need to protect unordered_map with #ifndeg GPUCA_GPUCODE_DEVICE.
We do not include any std headers in GPU kernel code.
And I think you don't need GPUDefMacros.h, do you?

Done. I also fixed up the includes.

alibuild · 2025-06-17T16:10:04Z

Error while checking build/O2/fullCI_slc9 for 8f676bc at 2025-06-17 18:10:

## sw/BUILD/O2-latest/log
/sw/SOURCES/O2/14397-slc9_x86-64/0/GPU/GPUTracking/Global/GPUErrors.h:20:10: fatal error: unordered_map: No such file or directory
/sw/SOURCES/O2/14397-slc9_x86-64/0/GPU/GPUTracking/Global/GPUErrors.h:20:10: fatal error: 'unordered_map' file not found
ninja: build stopped: subcommand failed.

Full log here.

alibuild · 2025-06-18T10:22:49Z

Error while checking build/O2/fullCI_slc9 for 7fd0535 at 2025-07-01 01:46:

## sw/BUILD/O2-latest/log
/sw/SOURCES/O2/14397-slc9_x86-64/0/GPU/GPUTracking/Global/GPUErrors.h:41:21: error: no template named 'unordered_map' in namespace 'std'
ninja: build stopped: subcommand failed.

Full log here.

ariedel-cern · 2025-06-24T14:17:37Z

Hi @davidrohr , thanks a lot for all the feedback. Is this PR now OK to be merged? I think the 2 failed checks are due to some issue with O2Physics.

davidrohr · 2025-06-26T08:42:07Z

Hi @ariedel-cern : The FullCI error is genuine, you also have to #ifdef-protect
static const std::unordered_map<uint32_t, const char*>& getErrorNames();

Feat: add skeleton for GPUErrorQA task

ab5cad3

ariedel-cern requested review from davidrohr, shahor02 and wiechula as code owners June 12, 2025 11:14

ariedel-cern mentioned this pull request Jun 12, 2025

TPC QC: Add GPUErrorQA task AliceO2Group/QualityControl#2564

Merged

wiechula reviewed Jun 12, 2025

View reviewed changes

davidrohr reviewed Jun 15, 2025

View reviewed changes

davidrohr reviewed Jun 17, 2025

View reviewed changes

ariedel-cern force-pushed the gpuerror branch from 0048689 to 8f676bc Compare June 17, 2025 15:18

davidrohr reviewed Jun 17, 2025

View reviewed changes

ariedel-cern added 4 commits June 18, 2025 09:54

Feat: add GPUErrorQA class

3873f12

Feat: movde error names to GPUErrors.h

78c3f10

Fix: expose gpu error names with static function

0772de2

Feat: account for missing error codes

7fd0535

ariedel-cern force-pushed the gpuerror branch from 8f676bc to 7fd0535 Compare June 18, 2025 07:56

Fix: add header guard

1b72526

davidrohr merged commit c2f066a into AliceO2Group:dev Jul 3, 2025
11 checks passed

	for (const auto& [name, hist] : mMapHist) {
	for ([[maybe_unused]] const auto& [name, hist] : mMapHist) {

TPC QC: Add GPUErrorQA task #14397

TPC QC: Add GPUErrorQA task #14397

Uh oh!

Conversation

ariedel-cern commented Jun 12, 2025

Uh oh!

github-actions bot commented Jun 12, 2025

Uh oh!

wiechula left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ariedel-cern Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alibuild commented Jun 12, 2025

Uh oh!

alibuild commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alibuild commented Jun 17, 2025

Uh oh!

davidrohr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alibuild commented Jun 17, 2025

Uh oh!

alibuild commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ariedel-cern commented Jun 24, 2025

Uh oh!

davidrohr commented Jun 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

ariedel-cern Jun 17, 2025 •

edited

Loading

alibuild commented Jun 12, 2025 •

edited

Loading

alibuild commented Jun 18, 2025 •

edited

Loading