Skip to content

Conversation

@ChSonnabend
Copy link
Collaborator

This PR brings the functionality of adding the deconvolution kernel flags for the NN evaluation without overwriting the charge values in the digit map, as well as a small, but important bug-fix to the flag setting in the fillInputNNSingleElement GPU kernel

@ChSonnabend ChSonnabend requested a review from davidrohr as a code owner June 8, 2025 13:40
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2025

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

@alibuild
Copy link
Collaborator

alibuild commented Jun 8, 2025

Error while checking build/O2/fullCI_slc9 for 57ff901 at 2025-06-08 17:54:

## sw/BUILD/O2-latest/log
/sw/BUILD/e809ec1524cdb86283302c2df876f7d09709a7ae/O2/GPU/GPUTracking/include_gpu_onthefly/GPUReconstructionKernelList.h:100:51: error: '__private bool' cannot be used as the type of a kernel parameter
/sw/BUILD/e809ec1524cdb86283302c2df876f7d09709a7ae/O2/GPU/include_gpu_onthefly/krnl_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags.cu(31): error: identifier "GPUCA_LB_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags" is undefined
ninja: build stopped: subcommand failed.

Full log here.

@alibuild
Copy link
Collaborator

alibuild commented Jun 9, 2025

Error while checking build/O2/fullCI_slc9 for 464402f at 2025-06-09 23:01:

## sw/BUILD/O2-latest/log
/sw/BUILD/9f5c9f4115ce95d34c2cd251875474573e12e439/O2/GPU/GPUTracking/include_gpu_onthefly/GPUReconstructionKernelList.h:100:51: error: '__private bool' cannot be used as the type of a kernel parameter
/sw/BUILD/9f5c9f4115ce95d34c2cd251875474573e12e439/O2/GPU/include_gpu_onthefly/krnl_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags.cu(31): error: identifier "GPUCA_LB_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags" is undefined
ninja: build stopped: subcommand failed.

Full log here.

@alibuild
Copy link
Collaborator

Error while checking build/O2/fullCI_slc9 for e70e4d9 at 2025-06-10 15:04:

## sw/BUILD/O2-latest/log
/sw/BUILD/8666e43c0e7b80c45658633c41df62856734a78f/O2/GPU/GPUTracking/include_gpu_onthefly/GPUReconstructionKernelList.h:100:51: error: '__private bool' cannot be used as the type of a kernel parameter
/sw/BUILD/8666e43c0e7b80c45658633c41df62856734a78f/O2/GPU/include_gpu_onthefly/krnl_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags.cu(31): error: identifier "GPUCA_LB_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags" is undefined
ninja: build stopped: subcommand failed.

Full log here.

davidrohr
davidrohr previously approved these changes Jun 11, 2025
Copy link
Collaborator

@davidrohr davidrohr left a comment

Choose a reason for hiding this comment

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

looks ok to me, shall I merge it?

@ChSonnabend
Copy link
Collaborator Author

ChSonnabend commented Jun 11, 2025

@davidrohr Jep, good from my side. But only once the CI is green. Last time it failed due to the boolean I added in the deconvolution kernel... Just want to see it go through

@davidrohr
Copy link
Collaborator

Ah, please do not use bool in GPU code, use uint8_t:

/sw/BUILD/8666e43c0e7b80c45658633c41df62856734a78f/O2/GPU/GPUTracking/include_gpu_onthefly/GPUReconstructionKernelList.h:100:51: error: '__private bool' cannot be used as the type of a kernel parameter
  100 | GPUCA_KRNL_LB((GPUTPCCFDeconvolution), (), (,bool overwriteCharge), (,overwriteCharge), (,bool), 81)

@alibuild
Copy link
Collaborator

Error while checking build/O2/fullCI_slc9 for f6a3d5a at 2025-06-12 13:16:

## sw/BUILD/o2codechecker-latest/log
100% tests passed, 0 tests failed out of 1


## sw/BUILD/O2-latest/log
/sw/BUILD/03df136b29fbe15dcc92d7ecf91d41364365c856/O2/GPU/include_gpu_onthefly/krnl_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags.cu(31): error: identifier "GPUCA_LB_GPUTPCNNClusterizerKernels_publishDeconvolutionFlags" is undefined
ninja: build stopped: subcommand failed.

Full log here.

@alibuild
Copy link
Collaborator

alibuild commented Jun 12, 2025

Error while checking build/O2/fullCI_slc9 for ff62032 at 2025-06-14 13:52:

input queue has size 81
output queue has size 1026
input queue has size 71
output queue has size 1026
input queue has size 65
output queue has size 1028
input queue has size 52
output queue has size 1028
input queue has size 46
output queue has size 1028
input queue has size 35
output queue has size 1030
input queue has size 23
output queue has size 1030
input queue has size 17
output queue has size 1030
input queue has size 6
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
input queue has size 0
output queue has size 1030
++ 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.

@ChSonnabend
Copy link
Collaborator Author

Ready to merge. Build failure seems unrelated.

@davidrohr davidrohr merged commit 86b1969 into AliceO2Group:dev Jun 16, 2025
10 of 11 checks passed
@ChSonnabend ChSonnabend deleted the nn_clusterizer_bugfixes branch June 18, 2025 18:30
mhemmer-cern pushed a commit to mhemmer-cern/AliceO2 that referenced this pull request Sep 9, 2025
…O2Group#14378)

* First bug-fixes and optimizations for deconvolution flags

* Adding publishing logic for deconvolution flags

* Adjusting kernels.cmake

* Please consider the following formatting changes

* Bug-fix for time-position and boundary check in fillInputSingleElement

* Fix for kernels.cmake and naming

* Changing to uint8_t

* Adding kernel definition

---------

Co-authored-by: ALICE Action Bot <alibuild@cern.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants