Skip to content

[PWGEM,Photon] Clean up code base - #17485

Merged
mhemmer-cern merged 4 commits into
AliceO2Group:masterfrom
mhemmer-cern:em-emcal-flow
Aug 18, 2026
Merged

[PWGEM,Photon] Clean up code base#17485
mhemmer-cern merged 4 commits into
AliceO2Group:masterfrom
mhemmer-cern:em-emcal-flow

Conversation

@mhemmer-cern

@mhemmer-cern mhemmer-cern commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator
  • Move old unused table producer tasks to Legacy folder to reduce compile time but keeping the code for completeness.
  • Try and overhaul #include <Framework/ASoA.h>. If .begin(), .end() or .globalIndex() is used it should be included. If none of the functions coming from there are used it should be removed.
  • Add CCDB Columns and SOA Timestamped tables for o2::emcal::BadChannelMap and o2::parameters::GRPMagField. This should help speed up CCDB access by putting the CCDB objects into shared memory. Especially useful for Dalitz + PCM skimmer and every task that runs with multiple sub-wagons.
  • Remove configurable that are no longer needed due to the above addition.
  • Reduce PresliceOptional calls in Pi0EtaToGammaGamma header files to have the bare minimum argument.
  • Add Concepts where possible to improve compile time.
  • Add Index column to the MinMTracks and MinMSTracks table.
  • Fix linter errors and warnings
  • Fix wrong position of header guard in PWGEM/PhotonMeson/DataModel/EventTables.h (was after all includes, is now before all includes)
  • Fix the partition used in the Pi0Flow task by using INT16_MAX, since there currently is a bug in arrow when using values above INT16_MAX when cutting on uint16_t values. Bugfix in arrow should come soon tho, see alisw/arrow@0f5153d

- Move old unsused table producer tasks to `Legacy` folder to reduce compile time but keeping the code for completeness
- Remove `#include <ASoA.h>` where ever possible to reduce compile time
- Add CCDB Columns and SOA Timestamped tables for `o2::emcal::BadChannelMap` and `o2::parameters::GRPMagField`. This should help speed up CCDB access by putting the CCDB objects into shared memory. Especially useful for Dalitz + PCM skimmer and every task that runs with multiple sub-wagons.
- Remove configurable that are no longer needed due to the above addition
- Reduce `PresliceOptional` calls in Pi0EtaToGammaGamma to have the bare minimum argument.
- Add Concepts where possible to improve compile time
- Fix linter errors and warnings
- Fix wrong position of header guard in `PWGEM/PhotonMeson/DataModel/EventTables.h` (was after all includes, is now before all includes)
- Fix the partition used in the Pi0Flow task by using `INT16_MAX`, since there currently is a bug in arrow when using values above `INT16_MAX` when cutting on uint16_t values. Bugfix in arrow should come soon tho.
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

O2 linter results: ❌ 906 errors, ⚠️ 1426 warnings, 🔕 120 disabled

@alibuild

This comment was marked as outdated.

@alibuild

This comment was marked as outdated.

@mhemmer-cern
mhemmer-cern marked this pull request as draft August 18, 2026 05:52
- Add `#include ASoA.h` back whenever `.begin()` or `.end()` was used in the code.
- Move more tasks to `Legacy` folder
- Add CCDB column usage to more `emcalPhotonMcTask`
@mhemmer-cern

Copy link
Copy Markdown
Collaborator Author

code-check will fail due to some Legacy code, but I do not want to spend time fixing code that is specifically marked as old and unused.

- Move `skimmerDalitzEE.cxx` to `Legacy`, `skimmerPrimaryElectronFromDalitzEE.cxx` should be used instead
- Add `#include <Framework/ASoA.h>` everywhere with `globalIndex()` in use
- Fix different linter warnings and errors and some of the touched files
@mhemmer-cern
mhemmer-cern marked this pull request as ready for review August 18, 2026 10:59
@alibuild

This comment was marked as outdated.

@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/code-check for 91ea153 at 2026-08-18 19:09:

## sw/BUILD/O2Physics-code-check-latest/log
--
========== List of issues found ==========
++ echo 'Found 24 errors and 1 warnings.'
Found 24 errors and 1 warnings.
++ [[ 24 -gt 0 ]]
++ cat /sw/BUILD/73f16dd90e3b7dd44cfcec7b774e6655e759030a/O2Physics-code-check/errors.txt
PWGEM/PhotonMeson/Legacy/gammaConversionsTruthOnlyMc.cxx:94:78: error: the const qualified parameter 'theFirstDaughter' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversionsTruthOnlyMc.cxx:94:105: error: the parameter 'theSecondDaughter' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:37:24: error: parameter 'config_' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:39:23: error: parameter 'config_' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:40:33: error: initializer for member 'm' is redundant [readability-redundant-member-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:139:20: error: initializer for member 'mPath' is redundant [readability-redundant-member-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:140:30: error: initializer for member 'mContainer' is redundant [readability-redundant-member-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:145:15: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:146:3: error: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:151:15: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:152:3: error: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:159:15: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:161:3: error: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:164:3: error: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/gammaConversions.h:174:15: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:62:8: error: constructor does not initialize these fields: ccdb, mRunNumber, d_bz [cppcoreguidelines-pro-type-member-init,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:158:39: error: use nullptr [modernize-use-nullptr,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:159:43: error: use nullptr [modernize-use-nullptr,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:244:7: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:347:7: error: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays,-warnings-as-errors]
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:394:7: error: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays,-warnings-as-errors]
PWGEM/PhotonMeson/Tasks/MaterialBudgetMC.cxx:134:23: error: the variable 'pair_cut_name' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
PWGEM/PhotonMeson/Tasks/SinglePhotonMC.cxx:22:1: error: included header PHOSPhotonCut.h is not used directly [misc-include-cleaner,-warnings-as-errors]
PWGEM/PhotonMeson/Tasks/TagAndProbe.cxx:143:21: error: the variable 'pair_cut_name' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
++ [[ 1 -gt 0 ]]
++ cat /sw/BUILD/73f16dd90e3b7dd44cfcec7b774e6655e759030a/O2Physics-code-check/warnings.txt
PWGEM/PhotonMeson/Legacy/skimmerDalitzEE.cxx:160:24: warning: statement should be inside braces [readability-braces-around-statements]
++ [[ 24 -gt 0 ]]
++ exit 1
--

Full log here.

@mhemmer-cern

mhemmer-cern commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

I will merge this now and take care of these:

PWGEM/PhotonMeson/Tasks/MaterialBudgetMC.cxx:134:23: error: the variable 'pair_cut_name' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
PWGEM/PhotonMeson/Tasks/SinglePhotonMC.cxx:22:1: error: included header PHOSPhotonCut.h is not used directly [misc-include-cleaner,-warnings-as-errors]
PWGEM/PhotonMeson/Tasks/TagAndProbe.cxx:143:21: error: the variable 'pair_cut_name' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]

In another PR later.

@mhemmer-cern
mhemmer-cern merged commit 79b47fa into AliceO2Group:master Aug 18, 2026
14 of 16 checks passed
@mhemmer-cern
mhemmer-cern deleted the em-emcal-flow branch August 18, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datamodel photon Photon and neutral meson PAG label pwgem

Development

Successfully merging this pull request may close these issues.

3 participants