From da4bd6a9c80eafd600d284bdd512692dcc2fd06e Mon Sep 17 00:00:00 2001 From: Sandro Wenzel Date: Sun, 8 Nov 2026 22:30:26 +0100 Subject: [PATCH 1/2] Fix MC labels carried over between sectors in the TPC digitizer This fixes a problem in the TPC DigitContainer reset and adds a unit test. - With DigiParams.maxOrbitsToDigitize set, time bins beyond the timeframe end stay in the container after the final flush. - reset() cleared only their charges, so their MC labels were attached to digits of the next sector processed by the same lane. - reset() now deletes all remaining time bins, and the destructor frees them. - testTPCDigitContainerReset checks that no label of one sector reaches the next. https://its.cern.ch/jira/browse/O2-7310 Co-Authored-By: Claude Opus 5.5 --- .../include/TPCSimulation/DigitContainer.h | 15 ++-- Detectors/TPC/simulation/test/CMakeLists.txt | 7 ++ .../test/testTPCDigitContainerReset.cxx | 70 +++++++++++++++++++ 3 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 Detectors/TPC/simulation/test/testTPCDigitContainerReset.cxx diff --git a/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h b/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h index 2a4e9e5b923ce..bc580612b2da4 100644 --- a/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h +++ b/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h @@ -44,7 +44,12 @@ class DigitContainer DigitContainer(); /// Destructor - ~DigitContainer() = default; + ~DigitContainer() + { + for (auto time : mTimeBins) { + delete time; + } + } /// Reset the container void reset(); @@ -106,11 +111,11 @@ inline void DigitContainer::reset() { mFirstTimeBin = 0; mEffectiveTimeBin = 0; - for (auto& time : mTimeBins) { - if (time) { - time->reset(); - } + // drop all time bins, including those beyond the timeframe limit, so that no MC labels survive + for (auto time : mTimeBins) { + delete time; } + mTimeBins.assign(mOffset, nullptr); if (mPrevDigArr) { std::fill(mPrevDigArr->begin(), mPrevDigArr->end(), PrevDigitInfo{}); } diff --git a/Detectors/TPC/simulation/test/CMakeLists.txt b/Detectors/TPC/simulation/test/CMakeLists.txt index cac0c1ef5b839..b416285ff7aad 100644 --- a/Detectors/TPC/simulation/test/CMakeLists.txt +++ b/Detectors/TPC/simulation/test/CMakeLists.txt @@ -16,6 +16,13 @@ o2_add_test(DigitContainer SOURCES testTPCDigitContainer.cxx ENVIRONMENT O2_ROOT=${CMAKE_BINARY_DIR}/stage) +o2_add_test(DigitContainerReset + LABELS tpc + PUBLIC_LINK_LIBRARIES O2::TPCSimulation + COMPONENT_NAME tpc + SOURCES testTPCDigitContainerReset.cxx + ENVIRONMENT O2_ROOT=${CMAKE_BINARY_DIR}/stage) + o2_add_test(ElectronTransport LABELS tpc PUBLIC_LINK_LIBRARIES O2::TPCSimulation diff --git a/Detectors/TPC/simulation/test/testTPCDigitContainerReset.cxx b/Detectors/TPC/simulation/test/testTPCDigitContainerReset.cxx new file mode 100644 index 0000000000000..78a66c6fe01b1 --- /dev/null +++ b/Detectors/TPC/simulation/test/testTPCDigitContainerReset.cxx @@ -0,0 +1,70 @@ +// Copyright 2019-2020 CERN and copyright holders of ALICE O2. +// See https://alice-o2.web.cern.ch/copyright for details of the copyright holders. +// All rights not expressly granted are reserved. +// +// This software is distributed under the terms of the GNU General Public +// License v3 (GPL Version 3), copied verbatim in the file "COPYING". +// +// In applying this license CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +/// \file testTPCDigitContainerReset.cxx +/// \brief Test that DigitContainer::reset() leaves no MC labels behind for the next sector + +#define BOOST_TEST_MODULE Test TPC DigitContainer reset +#define BOOST_TEST_MAIN +#define BOOST_TEST_DYN_LINK +#include +#include +#include +#include "DataFormatsTPC/Digit.h" +#include "TPCSimulation/DigitContainer.h" +#include "TPCSimulation/SAMPAProcessing.h" +#include "TPCBaseRecSim/CDBInterface.h" + +namespace o2::tpc +{ + +// A sector whose last time bins lie beyond the timeframe limit must not pass their labels to the next sector +BOOST_AUTO_TEST_CASE(DigitContainer_noLabelCarryOver) +{ + auto& cdb = CDBInterface::instance(); + cdb.setUseDefaults(); + // limit the flush to one orbit (time bin 447) and propagate the ADC values unchanged + o2::conf::ConfigurableParam::updateFromString(fmt::format("DigiParams.maxOrbitsToDigitize=1;TPCEleParam.DigiMode={}", (int)DigitzationMode::PropagateADC)); + const Mapper& mapper = Mapper::instance(); + SAMPAProcessing::instance(); + + const GlobalPadNumber globalPad = mapper.getPadNumberInROC(PadROCPos(CRU(0).roc(), PadPos(10, 10))); + const MCCompLabel labelFirst(1, 1, 0, false); + const MCCompLabel labelSecond(2, 2, 0, false); + + DigitContainer digitContainer; + digitContainer.reset(); + + // first sector: a signal beyond the timeframe limit, which the final flush does not reach + digitContainer.addDigit(labelFirst, CRU(0), 600, globalPad, 100); + std::vector digits; + dataformats::MCTruthContainer labels; + std::vector commonMode; + digitContainer.fillOutputContainer(digits, labels, commonMode, Sector(0), 0, true, true); + BOOST_CHECK(digits.empty()); + + // second sector, as after Digitizer::setSector(): same pad, early time bin + digitContainer.reset(); + digits.clear(); + labels.clear(); + commonMode.clear(); + digitContainer.addDigit(labelSecond, CRU(10), 153, globalPad, 100); + digitContainer.fillOutputContainer(digits, labels, commonMode, Sector(1), 0, true, true); + + BOOST_REQUIRE(digits.size() == 1); + const auto digitLabels = labels.getLabels(0); + BOOST_CHECK_EQUAL(digitLabels.size(), 1); + for (const auto& label : digitLabels) { + BOOST_CHECK(label == labelSecond); + } +} + +} // namespace o2::tpc From a357bbabf3622b7d6ecaf8154ea27c708d46a4fe Mon Sep 17 00:00:00 2001 From: Sandro Wenzel Date: Sun, 8 Nov 2026 22:55:00 +0100 Subject: [PATCH 2/2] Make the TPC DigitContainer non-copyable DigitContainer owns the time bins it deletes, so a copy would free them twice. --- .../TPC/simulation/include/TPCSimulation/DigitContainer.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h b/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h index bc580612b2da4..8d19aec4d2acc 100644 --- a/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h +++ b/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h @@ -51,6 +51,9 @@ class DigitContainer } } + DigitContainer(const DigitContainer&) = delete; + DigitContainer& operator=(const DigitContainer&) = delete; + /// Reset the container void reset();