AliceO2 icon indicating copy to clipboard operation
AliceO2 copied to clipboard

TPC: move PadFlags and related classes to TPCCore

Open ktf opened this issue 1 month ago • 20 comments

o2::tpc::PadFlags and in particular vectors of thereof are affected by an old ROOT bug in serializing std::vector<enum struct : short>.

Because of this we have a custom streamer which needs to be initialised very early in order to function correctly.

This is fine, however, due to the fact we invoke TClass::GetClass() too early, ROOT is forced to load a bunch of extra stuff, unneededly, resulting in much increased memory footprint in analysis, which happens to use DataFormatsTPC.

This makes sure the custom streamer is not initialised statically by DataFormatsTPC and prevents ROOT from loading the kitchen sink when the TClass::GetClass is invoked too early.

ktf avatar Dec 11 '25 16:12 ktf

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

github-actions[bot] avatar Dec 11 '25 16:12 github-actions[bot]

Error while checking build/O2/fullCI_slc9 for 35c1e8826ff5bf4671970d56ba0d2e587cdd6b08 at 2025-12-11 23:11:

## sw/BUILD/QualityControl-latest/log
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/CalDetPublisher.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/Clusters.cxx:21:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/LaserTracks.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/ClusterVisualizer.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/RawDigits.cxx:20:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/Utility.cxx:23:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/IDCs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/IDCsVsSACs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/SACs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
ninja: build stopped: cannot make progress due to previous errors.


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
grep: error-log.txt: binary file matches
++ GRERR=1
++ [[ 1 == 0 ]]
++ mkdir -p /sw/INSTALLROOT/590369407955fae4cc5f88307f03cce665cad258/slc9_x86-64/o2checkcode/1.0-local150/etc/modulefiles
++ cat
--

Full log here.

alibuild avatar Dec 11 '25 17:12 alibuild

For my understanding, in which situation will this help and what do you mean by kitchen sink? Isn't the static ROOT variable loaded in TPCBase (versus DataFormatsTPC)?

sawenzel avatar Dec 12 '25 08:12 sawenzel

Macros fixed to use the new TPCCore package when appropriate.

To be clear, the code is just moved to avoid having it explode memory in analysis. For reco / sim / calibration the change should be simply a mechanical one.

ktf avatar Dec 12 '25 08:12 ktf

If this is still causing trouble with the custom streamer, we could reconsider switching to a v2 object, and get rid of the streamer?

davidrohr avatar Dec 12 '25 08:12 davidrohr

v2 is fine for me as long as no one asks to build dozens of old releases to read new data with old release series.

ktf avatar Dec 12 '25 11:12 ktf

actually #14918 might be all we need to drop the dependency on DataFormatsTPC in O2Physics.

ktf avatar Dec 12 '25 12:12 ktf

#14918 looks indeed much easier more convenient

sawenzel avatar Dec 12 '25 12:12 sawenzel

Indeed. However we might still have a dependency on DataFormatsTPC via one of the other DataFormats... I am trying to do a build.

ktf avatar Dec 12 '25 12:12 ktf

#14918 is actually a good cleanup, but the dependency comes in from a different place.. So we do need this.

ktf avatar Dec 12 '25 12:12 ktf

Error while checking build/O2/fullCI_slc9 for 697219108611ec1230e979d0aa3ac0e2f121e294 at 2025-12-17 12:15:

## sw/BUILD/QualityControl-latest/log
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/CalDetPublisher.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/Clusters.cxx:21:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/LaserTracks.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/ClusterVisualizer.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/RawDigits.cxx:20:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/Utility.cxx:23:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/IDCs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/IDCsVsSACs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/SACs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
ninja: build stopped: cannot make progress due to previous errors.

Full log here.

alibuild avatar Dec 12 '25 15:12 alibuild

Error while checking build/O2/fullCI_slc9 for 92ddb2bc67dfadbe09707f25dc98fb99cf2c8479 at 2025-12-12 18:16:

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


## sw/BUILD/QualityControl-latest/log
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/Clusters.cxx:21:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/CalDetPublisher.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/RawDigits.cxx:20:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/LaserTracks.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/Utility.cxx:23:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/ClusterVisualizer.cxx:18:10: fatal error: TPCBase/Painter.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/IDCs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/IDCsVsSACs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
/sw/SOURCES/QualityControl/v1.185.0/v1.185.0/Modules/TPC/src/SACs.cxx:18:10: fatal error: TPCBase/CDBInterface.h: No such file or directory
ninja: build stopped: cannot make progress due to previous errors.


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
grep: error-log.txt: binary file matches
++ GRERR=1
++ [[ 1 == 0 ]]
++ mkdir -p /sw/INSTALLROOT/057c1ba8c2345d9ba14f9d0b266dc0d59b7171ba/slc9_x86-64/o2checkcode/1.0-local46/etc/modulefiles
++ cat
--

Full log here.

alibuild avatar Dec 12 '25 17:12 alibuild

This is confirmed to work in reducing memory usage for Analysis. Now trying something less intrusive, however if it does not work I would recommend merging this.

ktf avatar Dec 15 '25 09:12 ktf

AliceO2Group/QualityControl#2630 needed for QualityControl to compile with this.

ktf avatar Dec 15 '25 12:12 ktf

Just for my understanding, what are we trying to fix with this PR? Compatibility with new O2 and old ROOT? We went for this custom streamer since we said it is simpler than uploading many new objects, but meanwhile I am not sure anymore. This is getting more and more complicated, and at some point we should stop creating a mess, and instead just say we do not suppport the old ROOT any more, or is there a reason to support it? And if we drop the support, at some point we can also drop the streamer and just upload new CCDB objects in the correct format. I propose we discuss this again with all involved people, perhaps at the S&C meeting.

davidrohr avatar Dec 15 '25 12:12 davidrohr

No, we are trying to work around the fact that with the custom streamer as it is written, ROOT needs to load a bunch of dictionaries which were previously not loaded by simply linking the DataFormatsTPC library. This results in an exploded memory footprint for any analysis / workflow that depends, directly or indirectly, on DataFormatsTPC.

This patch solves the problem by moving the static in a new library (libTPCCore) which is not linked by any package needed by analysis.

As a side remark on this attempt, I have the impression that we could improve O2 memory footprint by a more careful usage of dependencies, in particular, avoiding having a dependency on TCanvas in TPCBase (and elsewhere).

Let's discuss in the meeting.

ktf avatar Dec 15 '25 12:12 ktf

Ok, but that means it is a fallout of the custom streamer. In general I absolutely fully agree to reduce dependencies, but I would do this independent from the streamer stuff.

So my opi ion is then again, someone needa to write a script to reupload all the objects, and then let's just get rid of all this custon streames mess.

davidrohr avatar Dec 15 '25 13:12 davidrohr

I have no objections if the detector experts want to clean their detector specific objects in CCDB and DPG deprecates old releases accessing them.

That said even without the custom streamer, this refactoring makes sense to me. I can simply change the commit message. None of the classes I moved are needed in analysis and they just bring a lot of baggage with them (including TCanvas...).

ktf avatar Dec 15 '25 14:12 ktf

Even better would be to get rid of any dependency on detector code in analysis, of course, however that's not something I can personally do in the short term.

ktf avatar Dec 15 '25 14:12 ktf

As discussed in the meeting yesterday, waiting for @wiechula to greenlight this (independently of the custom streamer).

ktf avatar Dec 16 '25 08:12 ktf

I am happy to rename when I am back. Merry Christmas!🎄

ktf avatar Dec 25 '25 19:12 ktf

Is this now superseded by https://github.com/AliceO2Group/AliceO2/pull/14918, or is this restructuring still needed?

wiechula avatar Dec 31 '25 11:12 wiechula