From afdc0000b68737106c1893c781b116477c37c22d Mon Sep 17 00:00:00 2001 From: Sandro Wenzel Date: Thu, 12 Nov 2026 18:32:46 +0100 Subject: [PATCH 1/4] Fix dangling branch address in getOrMakeBranch This fixes a dangling address in getOrMakeBranch for an existing branch and adds a unit test. - For an existing branch, the helper set the address of its own by-value parameter, which is gone when the caller fills the branch. - An object is now set with SetObject, which keeps the address in the branch; a pointer to the object is set with SetAddress as before. - The TPC digit writers rely on this when a sector is flushed more than once; the other callers set the address again themselves. Co-Authored-By: Claude Opus 5.5 --- Detectors/Base/CMakeLists.txt | 7 ++ .../Base/include/DetectorsBase/Detector.h | 8 +- Detectors/Base/test/testGetOrMakeBranch.cxx | 73 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 Detectors/Base/test/testGetOrMakeBranch.cxx diff --git a/Detectors/Base/CMakeLists.txt b/Detectors/Base/CMakeLists.txt index e225a2d975ef3..cfcaa8890f875 100644 --- a/Detectors/Base/CMakeLists.txt +++ b/Detectors/Base/CMakeLists.txt @@ -105,6 +105,13 @@ o2_add_test( PUBLIC_LINK_LIBRARIES O2::DetectorsBase LABELS detectorsbase) +o2_add_test( + GetOrMakeBranch + SOURCES test/testGetOrMakeBranch.cxx + COMPONENT_NAME DetectorsBase + PUBLIC_LINK_LIBRARIES O2::DetectorsBase + LABELS detectorsbase) + o2_add_test( O2Tessellated SOURCES test/testO2Tessellated.cxx diff --git a/Detectors/Base/include/DetectorsBase/Detector.h b/Detectors/Base/include/DetectorsBase/Detector.h index 9ed345e6827c5..6e67b59c84e26 100644 --- a/Detectors/Base/include/DetectorsBase/Detector.h +++ b/Detectors/Base/include/DetectorsBase/Detector.h @@ -313,7 +313,13 @@ template TBranch* getOrMakeBranch(TTree& tree, const char* brname, T* ptr) { if (auto br = tree.GetBranch(brname)) { - br->SetAddress(static_cast(&ptr)); + if constexpr (std::is_pointer_v) { + // ptr is the address of the caller's pointer to the object + br->SetAddress(ptr); + } else { + // the branch keeps the object address itself, as for a branch made with Branch() below + br->SetObject(ptr); + } return br; } // otherwise make it diff --git a/Detectors/Base/test/testGetOrMakeBranch.cxx b/Detectors/Base/test/testGetOrMakeBranch.cxx new file mode 100644 index 0000000000000..e9803c2f1956a --- /dev/null +++ b/Detectors/Base/test/testGetOrMakeBranch.cxx @@ -0,0 +1,73 @@ +// Copyright 2019-2026 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 testGetOrMakeBranch.cxx +/// \brief Checks that getOrMakeBranch leaves a valid address on an existing branch + +#define BOOST_TEST_MODULE Test GetOrMakeBranch +#define BOOST_TEST_MAIN +#define BOOST_TEST_DYN_LINK +#include + +#include "DetectorsBase/Detector.h" +#include +#include +#include + +namespace +{ +// attach the data in their own stack frame, which is gone when the branch is filled +__attribute__((noinline)) TBranch* attach(TTree& tree, std::vector* data) +{ + return o2::base::getOrMakeBranch(tree, "data", data); +} + +__attribute__((noinline)) TBranch* attachPointer(TTree& tree, std::vector** data) +{ + return o2::base::getOrMakeBranch(tree, "pointer", data); +} + +// overwrites the stack below the caller +__attribute__((noinline)) void clobberStack() +{ + volatile char buffer[4096]; + std::memset(const_cast(buffer), 0xab, sizeof(buffer)); +} +} // namespace + +BOOST_AUTO_TEST_CASE(FillExistingBranch) +{ + TTree tree("t", "t"); + tree.SetDirectory(nullptr); + std::vector> entries{{1, 2, 3}, {4, 5}, {6}}; + for (auto& entry : entries) { + auto pointer = &entry; + auto br = attach(tree, &entry); + auto brPointer = attachPointer(tree, &pointer); + clobberStack(); + br->Fill(); + brPointer->Fill(); + br->ResetAddress(); + brPointer->ResetAddress(); + } + tree.SetEntries(entries.size()); + + std::vector* read = nullptr; + std::vector* readPointer = nullptr; + tree.SetBranchAddress("data", &read); + tree.SetBranchAddress("pointer", &readPointer); + BOOST_REQUIRE_EQUAL(tree.GetEntries(), entries.size()); + for (size_t i = 0; i < entries.size(); ++i) { + tree.GetEntry(i); + BOOST_CHECK(*read == entries[i]); + BOOST_CHECK(*readPointer == entries[i]); + } +} From 337c9bcaef2742740c068318e7dc5dead22890a1 Mon Sep 17 00:00:00 2001 From: Sandro Wenzel Date: Thu, 12 Nov 2026 18:32:46 +0100 Subject: [PATCH 2/4] Remove stale TPC lane digit files and accept sector seeding in MT mode This fixes stale chunked digit files when the number of TPC lanes changes and lets the multithreaded device accept --tpc-sector-seeding. - The chunked digit merger reads every lane file in the directory, but each lane removed only its own file. After a run with more lanes or threads, the extra lane files stayed and their sectors were published twice. - The first lane now also removes the files of lanes the run does not have; the multithreaded device removes all of them, as before. - The file name and the cleanup are shared by both devices. - --tpc-sector-seeding is now a common option; the multithreaded device always seeds per sector, so it accepts the option without effect. Co-Authored-By: Claude Opus 5.5 --- .../src/TPCDigitizerMTSpec.cxx | 8 ++---- .../src/TPCDigitizerSpec.cxx | 26 +++++-------------- .../src/TPCSectorDigitization.cxx | 19 ++++++++++++++ .../src/TPCSectorDigitization.h | 7 +++++ 4 files changed, 34 insertions(+), 26 deletions(-) diff --git a/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx b/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx index 99075e4e1669b..bbea69f4ccccc 100644 --- a/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx +++ b/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx @@ -29,7 +29,6 @@ #include "SimulationDataFormat/DigitizationContext.h" #include "TPCSimulation/Digitizer.h" #include "CommonUtils/DebugStreamer.h" -#include "CommonUtils/FileSystemUtils.h" #include #include #include @@ -39,7 +38,6 @@ #include #include #include -#include #include #include #include @@ -71,7 +69,7 @@ struct DigitizerWorker { } } - std::string fileName() const { return "tpc_driftime_digits_lane" + std::to_string(id) + ".root"; } + std::string fileName() const { return laneDigitFileName(id); } /// Digitize one sector and write it to the worker's digit file, in the format of the chunked writer of the lane spec void processSector(const steer::DigitizationContext& context, int sector, ULong64_t timeframe, bool withMCTruth) @@ -136,9 +134,7 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer } // the merger picks up every lane file in the directory, so files of an earlier run must not survive - for (const auto& name : o2::utils::listFiles("tpc_driftime_digits_lane.*.root$")) { - std::filesystem::remove(name); - } + removeLaneDigitFiles([](int) { return true; }); ROOT::EnableThreadSafety(); } diff --git a/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx b/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx index 9bd499a5955c1..264ea1efefa84 100644 --- a/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx +++ b/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx @@ -45,7 +45,6 @@ #include "CommonDataFormat/RangeReference.h" #include "SimConfig/DigiParams.h" #include -#include #include "Framework/CCDBParamSpec.h" using namespace o2::framework; @@ -127,29 +126,18 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer // and set the flag to false after mWriteGRP = true; - // clean up (possibly) existing digit files + // the digit files are updated during processing and the merger reads every lane file in the directory, + // so each lane removes its own file of an earlier run and the first lane also those of lanes this run does not have if (mInternalWriter) { - cleanDigitFile(); - } - } - - void cleanDigitFile() - { - // since we update digit files during ordinary processing - // it is better to remove possibly existing files in the same dir - std::stringstream tmp; - tmp << "tpc_driftime_digits_lane" << mLaneId << ".root"; - if (std::filesystem::exists(tmp.str())) { - std::filesystem::remove(tmp.str()); + const int nLanes = ic.services().get().nSlots; + removeLaneDigitFiles([this, nLanes](int lane) { return lane == mLaneId || (mLaneId == 0 && lane >= nLanes); }); } } void writeToROOTFile() { if (!mInternalROOTFlushFile) { - std::stringstream tmp; - tmp << "tpc_driftime_digits_lane" << mLaneId << ".root"; - mInternalROOTFlushFile = new TFile(tmp.str().c_str(), "UPDATE"); + mInternalROOTFlushFile = new TFile(laneDigitFileName(mLaneId).c_str(), "UPDATE"); std::stringstream trname; trname << mSector; mInternalROOTFlushTTree = new TTree(trname.str().c_str(), "o2sim"); @@ -387,14 +375,12 @@ o2::framework::DataProcessorSpec getTPCDigitizerSpec(int channel, bool writeGRP, } std::vector inputs{InputSpec{"collisioncontext", "SIM", "COLLISIONCONTEXT", static_cast(channel), Lifetime::Timeframe}}; - auto options = TPCDigitizerConditions::options(); - options.push_back({"tpc-sector-seeding", VariantType::Bool, false, {"Seed the random numbers of each sector from DigiParams.seed and the sector, so that the digits do not depend on the lane assignment"}}); return DataProcessorSpec{ id.str().c_str(), inputs, outputs, AlgorithmSpec{adaptFromTask(internalwriter, distortionType)}, - options}; + TPCDigitizerConditions::options()}; } o2::framework::WorkflowSpec getTPCDigitizerSpec(int nLanes, std::vector const& sectors, bool mctruth, bool internalwriter, int distortionType) diff --git a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx index cd859672f9038..784ec9e9832ab 100644 --- a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx +++ b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx @@ -30,6 +30,7 @@ #include "Framework/InputRecord.h" #include "Framework/ProcessingContext.h" #include +#include namespace o2::tpc { @@ -128,10 +129,28 @@ std::vector> digitizeSector(Digitizer& dig return events; } +std::string laneDigitFileName(int lane) +{ + return "tpc_driftime_digits_lane" + std::to_string(lane) + ".root"; +} + +void removeLaneDigitFiles(const std::function& select) +{ + const std::regex pattern("tpc_driftime_digits_lane([0-9]+)\\.root"); + for (const auto& entry : std::filesystem::directory_iterator(".")) { + const auto name = entry.path().filename().string(); + std::smatch match; + if (std::regex_match(name, match, pattern) && select(std::stoi(match[1]))) { + std::filesystem::remove(entry.path()); + } + } +} + std::vector TPCDigitizerConditions::options() { using framework::VariantType; return { + {"tpc-sector-seeding", VariantType::Bool, false, {"Seed the random numbers of each sector from DigiParams.seed and the sector, so that the digits do not depend on the lane assignment (always on with --tpc-digi-threads)"}}, {"TPCtriggered", VariantType::Bool, false, {"Impose triggered RO mode (default: continuous)"}}, {"TPCuseCCDB", VariantType::Bool, false, {"true: load calibrations from CCDB; false: use random calibratoins"}}, {"meanLumiDistortions", VariantType::Float, -1.f, {"override lumi of distortion object if >=0"}}, diff --git a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h index daf417e21ae73..16b69c9736bf5 100644 --- a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h +++ b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h @@ -13,6 +13,7 @@ #define STEER_DIGITIZERWORKFLOW_TPCSECTORDIGITIZATION_H_ #include +#include #include #include #include "CommonDataFormat/RangeReference.h" @@ -55,6 +56,12 @@ std::vector> digitizeSector(Digitizer& dig const std::vector& simChains, int sector, ULong64_t timeframe, SectorFlushBuffers& buffers, const std::function& onFlush); +/// Name of the chunked digit file of a lane, which the chunked digit merger reads +std::string laneDigitFileName(int lane); + +/// Remove the chunked digit files in the working directory whose lane passes the selection +void removeLaneDigitFiles(const std::function& select); + /// CCDB conditions and calibration files of the TPC digitizer devices class TPCDigitizerConditions { From dc18766b354552c542095b36c2db717fc783576f Mon Sep 17 00:00:00 2001 From: Sandro Wenzel Date: Thu, 12 Nov 2026 18:55:34 +0100 Subject: [PATCH 3/4] Share the chunked writer and seed selection of the TPC digitizer devices This moves code that the lane and the multithreaded TPC digitizer devices duplicated into TPCSectorDigitization. - LaneDigitFileWriter writes the flushes of a sector to the chunked digit file of a lane, for both devices. - sectorSeedingBaseSeed() selects the base seed of the per-sector seeding, on which equal digits of both devices rely. - TPCDigitizerConditions::update() returns the drift velocity change instead of calling back; finaliseCCDB() returns nothing. - The multithreaded device takes its sectors from the spec instead of the input headers. - Dead helpers and unused includes are removed from the lane device. Co-Authored-By: Claude Opus 5.5 --- .../src/TPCDigitizerMTSpec.cxx | 133 ++++++------------ .../src/TPCDigitizerSpec.cxx | 124 +++------------- .../src/TPCSectorDigitization.cxx | 76 ++++++++-- .../src/TPCSectorDigitization.h | 43 +++++- 4 files changed, 166 insertions(+), 210 deletions(-) diff --git a/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx b/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx index bbea69f4ccccc..53637d545bd7e 100644 --- a/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx +++ b/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx @@ -24,22 +24,17 @@ #include "DataFormatsParameters/GRPObject.h" #include "DataFormatsTPC/TPCSectorHeader.h" #include "DetectorsBase/BaseDPLDigitizer.h" -#include "DetectorsBase/Detector.h" #include "SimConfig/DigiParams.h" #include "SimulationDataFormat/DigitizationContext.h" #include "TPCSimulation/Digitizer.h" #include "CommonUtils/DebugStreamer.h" #include -#include #include -#include #include -#include #include #include #include #include -#include #include #include #include @@ -56,12 +51,7 @@ namespace { /// The state of one worker thread: a digitizer, its own hit chains and its own digit file struct DigitizerWorker { - int id = 0; - Digitizer digitizer; - std::vector chains; - SectorFlushBuffers buffers; - std::exception_ptr error; - + DigitizerWorker(int lane, bool withMCTruth) : id(lane), writer(lane, withMCTruth) {} ~DigitizerWorker() { for (auto chain : chains) { @@ -69,63 +59,47 @@ struct DigitizerWorker { } } - std::string fileName() const { return laneDigitFileName(id); } - - /// Digitize one sector and write it to the worker's digit file, in the format of the chunked writer of the lane spec - void processSector(const steer::DigitizationContext& context, int sector, ULong64_t timeframe, bool withMCTruth) + /// Digitize one sector and write it to the digit file of the worker + void processSector(const steer::DigitizationContext& context, int sector, ULong64_t timeframe) { - std::unique_ptr file(TFile::Open(fileName().c_str(), "UPDATE")); - if (!file || file->IsZombie()) { - throw std::runtime_error("TPC: cannot open " + fileName()); - } - auto tree = new TTree(std::to_string(sector).c_str(), "o2sim"); - tree->SetDirectory(file.get()); // owned by the file - size_t flushCounter = 0; - auto onFlush = [&]() { - ++flushCounter; - auto fill = [&tree](const std::string& name, auto* data) { - auto br = o2::base::getOrMakeBranch(*tree, name.c_str(), data); - br->Fill(); - br->ResetAddress(); - }; - fill("TPCDigit_" + std::to_string(sector), &buffers.digits); - if (withMCTruth) { - fill("TPCDigitMCTruth_" + std::to_string(sector), &buffers.labels); - } - fill("TPCCommonMode_" + std::to_string(sector), &buffers.commonMode); - }; - digitizeSector(digitizer, context, chains, sector, timeframe, buffers, onFlush); - tree->SetEntries(flushCounter); - file->Write("", TObject::kOverwrite); - file->Close(); + writer.beginSector(sector); + digitizeSector(digitizer, context, chains, sector, timeframe, buffers, [this]() { writer.writeFlush(buffers); }); + writer.endSector(); } + + int id = 0; + Digitizer digitizer; + std::vector chains; + SectorFlushBuffers buffers; + LaneDigitFileWriter writer; + std::exception_ptr error; }; + +std::string contextBinding(int sector) +{ + return "collisioncontext" + std::to_string(sector); +} } // namespace class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer { public: - TPCDPLDigitizerMTTask(int nThreads, int distortionType) : BaseDPLDigitizer(o2::base::InitServices::FIELD | o2::base::InitServices::GEOM), mNThreads(nThreads), mDistortionType(distortionType) {} + TPCDPLDigitizerMTTask(int nThreads, std::vector sectors, int distortionType) : BaseDPLDigitizer(o2::base::InitServices::FIELD | o2::base::InitServices::GEOM), mSectors(std::move(sectors)), mNThreads(nThreads), mDistortionType(distortionType) {} void initDigitizerTask(framework::InitContext& ic) override { LOG(info) << "Initializing multithreaded TPC digitization with " << mNThreads << " threads"; - mWithMCTruth = o2::conf::DigiParams::Instance().mctruth; + const bool withMCTruth = o2::conf::DigiParams::Instance().mctruth; const auto triggeredMode = ic.options().get("TPCtriggered"); mConditions.init(ic, mDistortionType); - // the workers share no random numbers: each digitizer seeds every sector from DigiParams.seed - const auto seed = o2::conf::DigiParams::Instance().seed; - if (seed == 0) { - LOG(warning) << "TPC sector seeding with DigiParams.seed=0 is not reproducible"; - } - // with seed 0, gRandom was seeded randomly in the base class - const ULong64_t sectorSeed = seed != 0 ? seed : gRandom->Integer(kMaxUInt); + // the workers share no random numbers: each digitizer seeds every sector from the same base seed, + // so the option --tpc-sector-seeding is always on here + const ULong64_t sectorSeed = sectorSeedingBaseSeed(); for (int i = 0; i < mNThreads; ++i) { - auto& worker = mWorkers.emplace_back(std::make_unique()); - worker->id = i; + auto& worker = mWorkers.emplace_back(std::make_unique(i, withMCTruth)); worker->digitizer.setContinuousReadout(!triggeredMode); worker->digitizer.setDistortionScaleType(mDistortionType); worker->digitizer.setUseSectorSeeding(sectorSeed); @@ -150,12 +124,12 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer LOG(info) << "Processing multithreaded TPC digitization"; auto& primary = mWorkers[0]->digitizer; - mConditions.update(pc, primary, [this](float vdrift, float tdriftOffset) { + if (auto vdrift = mConditions.update(pc, primary)) { for (auto& worker : mWorkers) { - worker->digitizer.setVDrift(vdrift); - worker->digitizer.setTDriftOffset(tdriftOffset); + worker->digitizer.setVDrift(vdrift->vdrift); + worker->digitizer.setTDriftOffset(vdrift->tdriftOffset); } - }); + } if (mDistortionType) { for (size_t i = 1; i < mWorkers.size(); ++i) { mWorkers[i]->digitizer.setSCDistortionsFrom(primary); @@ -169,37 +143,13 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer mSeedingShared = true; } - // the sectors of this timeframe; all inputs carry the same collision context - std::vector sectors; - std::optional contextRef; - for (auto it = pc.inputs().begin(), end = pc.inputs().end(); it != end; ++it) { - for (auto const& inputref : it.parts()) { - if (inputref.spec->lifetime == o2::framework::Lifetime::Condition) { - continue; - } - auto const* sectorHeader = DataRefUtils::getHeader(inputref); - if (sectorHeader == nullptr) { - LOG(error) << "TPC: Sector header missing, skipping input"; - continue; - } - const int sector = sectorHeader->sector(); - if (sector < 0 || sector >= TPCSectorHeader::NSectors) { - throw std::runtime_error("Digitizer can only work on single sectors"); - } - sectors.push_back(sector); - if (!contextRef) { - contextRef = inputref; - } - } - } - if (mWriteGRP) { auto roMode = primary.isContinuousReadout() ? o2::parameters::GRPObject::CONTINUOUS : o2::parameters::GRPObject::PRESENT; LOG(info) << "TPC: Sending ROMode= " << (primary.isContinuousReadout() ? "Continuous" : "Triggered") << " to GRPUpdater"; pc.outputs().snapshot(Output{"TPC", "ROMode", 0}, roMode); mWriteGRP = false; } - if (!contextRef) { + if (mSectors.empty()) { return; } // the chunked digit files and the dynamic sector scheduling hold one timeframe per process @@ -207,14 +157,16 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer LOG(fatal) << "TPC: the multithreaded digitizer supports only one timeframe per process"; } mProcessedTimeframe = true; - auto context = pc.inputs().get(*contextRef); - const ULong64_t timeframe = DataRefUtils::getHeader(*contextRef)->tfCounter; - LOG(info) << "TPC: Processing " << sectors.size() << " sectors with " << context->getEventRecords().size() << " collisions"; + // every sector input carries the same collision context + const auto contextRef = pc.inputs().get(contextBinding(mSectors[0])); + auto context = pc.inputs().get(contextRef); + const ULong64_t timeframe = DataRefUtils::getHeader(contextRef)->tfCounter; + LOG(info) << "TPC: Processing " << mSectors.size() << " sectors with " << context->getEventRecords().size() << " collisions"; TStopwatch timer; timer.Start(); - const size_t nWorkers = std::min(mWorkers.size(), sectors.size()); + const size_t nWorkers = std::min(mWorkers.size(), mSectors.size()); for (size_t i = 0; i < nWorkers; ++i) { context->initSimChains(o2::detectors::DetID::TPC, mWorkers[i]->chains); } @@ -225,13 +177,13 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer for (size_t i = 0; i < nWorkers; ++i) { threads.emplace_back([&, worker = mWorkers[i].get()]() { try { - for (size_t is = nextSector++; is < sectors.size(); is = nextSector++) { - LOG(info) << "TPC: Worker " << worker->id << " processing sector " << sectors[is]; - worker->processSector(*context, sectors[is], timeframe, mWithMCTruth); + for (size_t is = nextSector++; is < mSectors.size(); is = nextSector++) { + LOG(info) << "TPC: Worker " << worker->id << " processing sector " << mSectors[is]; + worker->processSector(*context, mSectors[is], timeframe); } } catch (...) { worker->error = std::current_exception(); - nextSector = sectors.size(); // let the other workers stop early + nextSector = mSectors.size(); // let the other workers stop early } }); } @@ -253,20 +205,21 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer private: std::vector> mWorkers; + std::vector mSectors; ///< the sectors to digitize, one collision context input each TPCDigitizerConditions mConditions; bool mSeedingShared = false; ///< whether the workers got the random rings of the first one bool mProcessedTimeframe = false; ///< whether a timeframe was digitized int mNThreads = 1; int mDistortionType = 0; bool mWriteGRP = true; - bool mWithMCTruth = true; }; o2::framework::DataProcessorSpec getTPCDigitizerMTSpec(int nThreads, std::vector const& sectors, int distortionType) { + // the reader sends the collision context once per sector, so the device subscribes to all of them std::vector inputs; for (auto sector : sectors) { - inputs.emplace_back("collisioncontext" + std::to_string(sector), "SIM", "COLLISIONCONTEXT", static_cast(sector), Lifetime::Timeframe); + inputs.emplace_back(contextBinding(sector), "SIM", "COLLISIONCONTEXT", static_cast(sector), Lifetime::Timeframe); } TPCDigitizerConditions::addInputs(inputs, distortionType); std::vector outputs{{"TPC", "ROMode", 0, Lifetime::Timeframe}}; @@ -276,7 +229,7 @@ o2::framework::DataProcessorSpec getTPCDigitizerMTSpec(int nThreads, std::vector "tpc-digitizer-mt", inputs, outputs, - AlgorithmSpec{adaptFromTask(nThreads, distortionType)}, + AlgorithmSpec{adaptFromTask(nThreads, sectors, distortionType)}, TPCDigitizerConditions::options()}; } diff --git a/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx b/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx index 264ea1efefa84..0e8871c7d86e2 100644 --- a/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx +++ b/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx @@ -29,68 +29,26 @@ #include "TChain.h" #include #include -#include #include "Framework/Task.h" #include "DataFormatsParameters/GRPObject.h" #include "DataFormatsTPC/TPCSectorHeader.h" -#include "TPCBaseRecSim/CDBInterface.h" -#include "TPCBase/ParameterGEM.h" #include "DataFormatsTPC/Digit.h" #include "TPCSimulation/Digitizer.h" #include "TPCSimulation/Detector.h" -#include "TPCSpaceCharge/SpaceCharge.h" #include "DetectorsBase/BaseDPLDigitizer.h" #include "DetectorsBase/Detector.h" -#include "TPCCalibration/VDriftHelper.h" #include "CommonDataFormat/RangeReference.h" #include "SimConfig/DigiParams.h" -#include -#include "Framework/CCDBParamSpec.h" using namespace o2::framework; using SubSpecificationType = o2::framework::DataAllocator::SubSpecificationType; using DigiGroupRef = o2::dataformats::RangeReference; -using SC = o2::tpc::SpaceCharge; namespace o2 { namespace tpc { -template -void copyHelper(T const& origin, T& target) -{ - std::copy(origin.begin(), origin.end(), std::back_inserter(target)); -} -template <> -void copyHelper>(o2::dataformats::MCTruthContainer const& origin, o2::dataformats::MCTruthContainer& target) -{ - target.mergeAtBack(origin); -} - -template -void writeToBranchHelper(TTree& tree, const char* name, T* accum) -{ - auto targetbr = o2::base::getOrMakeBranch(tree, name, accum); - targetbr->Fill(); - targetbr->ResetAddress(); - targetbr->DropBaskets("all"); -} -template <> -void writeToBranchHelper>(TTree& tree, - const char* name, o2::dataformats::MCTruthContainer* accum) -{ - // we convert first of all to IOMCTruthContainer - std::vector buffer; - accum->flatten_to(buffer); - accum->clear_andfreememory(); - o2::dataformats::IOMCTruthContainerView view(buffer); - auto targetbr = o2::base::getOrMakeBranch(tree, name, &view); - targetbr->Fill(); - targetbr->ResetAddress(); - targetbr->DropBaskets("all"); -} - using namespace o2::base; class TPCDPLDigitizerTask : public BaseDPLDigitizer { @@ -113,12 +71,7 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer mDigitizer.setDistortionScaleType(mDistortionType); if (ic.options().get("tpc-sector-seeding")) { - const auto seed = o2::conf::DigiParams::Instance().seed; - if (seed == 0) { - LOG(warning) << "TPC sector seeding with DigiParams.seed=0 is not reproducible"; - } - // with seed 0, gRandom was seeded randomly in the base class - mDigitizer.setUseSectorSeeding(seed != 0 ? seed : gRandom->Integer(kMaxUInt)); + mDigitizer.setUseSectorSeeding(sectorSeedingBaseSeed()); LOG(info) << "TPC: using sector seeding"; } @@ -131,39 +84,7 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer if (mInternalWriter) { const int nLanes = ic.services().get().nSlots; removeLaneDigitFiles([this, nLanes](int lane) { return lane == mLaneId || (mLaneId == 0 && lane >= nLanes); }); - } - } - - void writeToROOTFile() - { - if (!mInternalROOTFlushFile) { - mInternalROOTFlushFile = new TFile(laneDigitFileName(mLaneId).c_str(), "UPDATE"); - std::stringstream trname; - trname << mSector; - mInternalROOTFlushTTree = new TTree(trname.str().c_str(), "o2sim"); - } - { - std::stringstream brname; - brname << "TPCDigit_" << mSector; - auto br = o2::base::getOrMakeBranch(*mInternalROOTFlushTTree, brname.str().c_str(), &mFlushBuffers.digits); - br->Fill(); - br->ResetAddress(); - } - if (mWithMCTruth) { - // labels - std::stringstream brname; - brname << "TPCDigitMCTruth_" << mSector; - auto br = o2::base::getOrMakeBranch(*mInternalROOTFlushTTree, brname.str().c_str(), &mFlushBuffers.labels); - br->Fill(); - br->ResetAddress(); - } - { - // common - std::stringstream brname; - brname << "TPCCommonMode_" << mSector; - auto br = o2::base::getOrMakeBranch(*mInternalROOTFlushTTree, brname.str().c_str(), &mFlushBuffers.commonMode); - br->Fill(); - br->ResetAddress(); + mWriter.emplace(mLaneId, mWithMCTruth); } } @@ -176,10 +97,10 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer { LOG(info) << "Processing TPC digitization"; - mConditions.update(pc, mDigitizer, [this](float vdrift, float tdriftOffset) { - mDigitizer.setVDrift(vdrift); - mDigitizer.setTDriftOffset(tdriftOffset); - }); + if (auto vdrift = mConditions.update(pc, mDigitizer)) { + mDigitizer.setVDrift(vdrift->vdrift); + mDigitizer.setTDriftOffset(vdrift->tdriftOffset); + } for (auto it = pc.inputs().begin(), end = pc.inputs().end(); it != end; ++it) { for (auto const& inputref : it.parts()) { @@ -187,16 +108,6 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer continue; } process(pc, inputref); - if (mInternalWriter) { - mInternalROOTFlushTTree->SetEntries(mFlushCounter); - mInternalROOTFlushFile->Write("", TObject::kOverwrite); - mInternalROOTFlushFile->Close(); - // delete mInternalROOTFlushTTree; --> automatically done by ->Close() - delete mInternalROOTFlushFile; - mInternalROOTFlushFile = nullptr; - } - // TODO: make generic reset method? - mFlushCounter = 0; } } } @@ -232,8 +143,6 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer return; } auto sector = sectorHeader->sector(); - mSector = sector; - mListOfSectors.push_back(sector); LOG(info) << "TPC: Processing sector " << sector; // the active sectors need to be propagated uint64_t activeSectors = 0; @@ -304,11 +213,13 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer TStopwatch timer; timer.Start(); + if (mWriter) { + mWriter->beginSector(sector); + } auto onFlush = [this, digitsAccum, &labelAccum, &commonModeAccum]() { - mFlushCounter++; - if (mInternalWriter) { + if (mWriter) { // the natural place to write out this independent datachunk immediately ... - writeToROOTFile(); + mWriter->writeFlush(mFlushBuffers); } else { // ... or to accumulate and later forward to next DPL proc std::copy(mFlushBuffers.digits.begin(), mFlushBuffers.digits.end(), std::back_inserter(*digitsAccum)); @@ -319,6 +230,9 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer } }; eventAccum = digitizeSector(mDigitizer, *context, mSimChains, sector, dh->tfCounter, mFlushBuffers, onFlush); + if (mWriter) { + mWriter->endSector(); + } if (!mInternalWriter) { // send out to next stage @@ -336,13 +250,9 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer o2::tpc::Digitizer mDigitizer; TPCDigitizerConditions mConditions; std::vector mSimChains; - SectorFlushBuffers mFlushBuffers; // output of the latest flush - std::vector mListOfSectors; // a list of sectors treated by this task - TFile* mInternalROOTFlushFile = nullptr; - TTree* mInternalROOTFlushTTree = nullptr; - size_t mFlushCounter = 0; - int mLaneId = 0; // the id of the current process within the parallel pipeline - int mSector = 0; + SectorFlushBuffers mFlushBuffers; // output of the latest flush + std::optional mWriter; // the chunked digit writer of this lane + int mLaneId = 0; // the id of the current process within the parallel pipeline bool mWriteGRP = false; bool mWithMCTruth = true; bool mInternalWriter = false; diff --git a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx index 784ec9e9832ab..125392ab521c9 100644 --- a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx +++ b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx @@ -29,8 +29,14 @@ #include "Framework/InitContext.h" #include "Framework/InputRecord.h" #include "Framework/ProcessingContext.h" +#include "DetectorsBase/Detector.h" +#include "SimConfig/DigiParams.h" +#include +#include +#include #include #include +#include namespace o2::tpc { @@ -129,6 +135,16 @@ std::vector> digitizeSector(Digitizer& dig return events; } +ULong64_t sectorSeedingBaseSeed() +{ + const auto seed = o2::conf::DigiParams::Instance().seed; + if (seed == 0) { + LOG(warning) << "TPC sector seeding with DigiParams.seed=0 is not reproducible"; + } + // with seed 0, gRandom was seeded randomly in the base class + return seed != 0 ? seed : gRandom->Integer(kMaxUInt); +} + std::string laneDigitFileName(int lane) { return "tpc_driftime_digits_lane" + std::to_string(lane) + ".root"; @@ -146,6 +162,48 @@ void removeLaneDigitFiles(const std::function& select) } } +LaneDigitFileWriter::LaneDigitFileWriter(int lane, bool withMCTruth) : mLane(lane), mWithMCTruth(withMCTruth) {} + +LaneDigitFileWriter::~LaneDigitFileWriter() = default; + +void LaneDigitFileWriter::beginSector(int sector) +{ + const auto name = laneDigitFileName(mLane); + mFile.reset(TFile::Open(name.c_str(), "UPDATE")); + if (!mFile || mFile->IsZombie()) { + throw std::runtime_error("TPC: cannot open " + name); + } + mSector = sector; + mFlushes = 0; + mTree = new TTree(std::to_string(sector).c_str(), "o2sim"); + mTree->SetDirectory(mFile.get()); +} + +void LaneDigitFileWriter::writeFlush(SectorFlushBuffers& buffers) +{ + ++mFlushes; + auto fill = [this](const std::string& name, auto* data) { + auto br = o2::base::getOrMakeBranch(*mTree, name.c_str(), data); + br->Fill(); + br->ResetAddress(); + }; + const auto sector = std::to_string(mSector); + fill("TPCDigit_" + sector, &buffers.digits); + if (mWithMCTruth) { + fill("TPCDigitMCTruth_" + sector, &buffers.labels); + } + fill("TPCCommonMode_" + sector, &buffers.commonMode); +} + +void LaneDigitFileWriter::endSector() +{ + mTree->SetEntries(mFlushes); + mFile->Write("", TObject::kOverwrite); + mFile->Close(); + mFile.reset(); + mTree = nullptr; +} + std::vector TPCDigitizerConditions::options() { using framework::VariantType; @@ -186,12 +244,12 @@ void TPCDigitizerConditions::init(framework::InitContext& ic, int distortionType LOG(info) << "TPC calibrations from CCDB: " << mUseCalibrationsFromCCDB; } -bool TPCDigitizerConditions::finaliseCCDB(framework::ConcreteDataMatcher& matcher, void* obj, Digitizer& digitizer) +void TPCDigitizerConditions::finaliseCCDB(framework::ConcreteDataMatcher& matcher, void* obj, Digitizer& digitizer) { using framework::ConcreteDataMatcher; using SC = SpaceCharge; if (mVDriftHelper.accountCCDBInputs(matcher, obj)) { - return true; + return; } if (matcher == ConcreteDataMatcher(o2::header::gDataOriginTPC, "TPCDIST", 0)) { LOGP(info, "Updating distortion map"); @@ -199,7 +257,7 @@ bool TPCDigitizerConditions::finaliseCCDB(framework::ConcreteDataMatcher& matche if (mMeanLumiDistortions >= 0) { digitizer.setMeanLumiDistortions(mMeanLumiDistortions); } - return true; + return; } if (matcher == ConcreteDataMatcher(o2::header::gDataOriginTPC, "TPCDISTDERIV", 0)) { LOGP(info, "Updating reference distortion map"); @@ -207,22 +265,20 @@ bool TPCDigitizerConditions::finaliseCCDB(framework::ConcreteDataMatcher& matche if (mMeanLumiDistortionsDerivative >= 0) { digitizer.setMeanLumiDistortionsDerivative(mMeanLumiDistortionsDerivative); } - return true; + return; } if (matcher == ConcreteDataMatcher(o2::header::gDataOriginTPC, "TPCGASPARAM", 0)) { LOGP(info, "TPC gas param updated"); ParameterGas::Instance().printKeyValues(true, true); - return true; + return; } if (matcher == ConcreteDataMatcher(o2::header::gDataOriginTPC, "TPCGEMPARAM", 0)) { LOGP(info, "TPC GEM param updated"); ParameterGEM::Instance().printKeyValues(true, true); - return true; } - return false; } -void TPCDigitizerConditions::update(framework::ProcessingContext& pc, Digitizer& digitizer, const std::function& setVDrift) +std::optional TPCDigitizerConditions::update(framework::ProcessingContext& pc, Digitizer& digitizer) { using SC = SpaceCharge; auto& cdb = CDBInterface::instance(); @@ -242,11 +298,12 @@ void TPCDigitizerConditions::update(framework::ProcessingContext& pc, Digitizer& } } + std::optional vdrift; if (mVDriftHelper.isUpdated()) { const auto& vd = mVDriftHelper.getVDriftObject(); LOGP(info, "Updating TPC fast transform map with new VDrift factor of {} wrt reference {} and DriftTimeOffset correction {} wrt {} from source {}", vd.corrFact, vd.refVDrift, vd.timeOffsetCorr, vd.refTimeOffset, mVDriftHelper.getSourceName()); - setVDrift(vd.getVDrift(), vd.getTimeOffset()); + vdrift = VDrift{vd.getVDrift(), vd.getTimeOffset()}; mVDriftHelper.acknowledgeUpdate(); } @@ -259,6 +316,7 @@ void TPCDigitizerConditions::update(framework::ProcessingContext& pc, Digitizer& LOG(info) << "TPC: Using gain map from 'GainMap.root'"; cdb.setGainMapFromFile("GainMap.root"); } + return vdrift; } } // namespace o2::tpc diff --git a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h index 16b69c9736bf5..9fbf646cc58f4 100644 --- a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h +++ b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h @@ -13,6 +13,8 @@ #define STEER_DIGITIZERWORKFLOW_TPCSECTORDIGITIZATION_H_ #include +#include +#include #include #include #include @@ -27,6 +29,8 @@ #include "Framework/InputSpec.h" class TChain; +class TFile; +class TTree; namespace o2 { @@ -56,12 +60,37 @@ std::vector> digitizeSector(Digitizer& dig const std::vector& simChains, int sector, ULong64_t timeframe, SectorFlushBuffers& buffers, const std::function& onFlush); +/// The base seed of the per-sector seeding: DigiParams.seed, or a random seed from gRandom if that is 0 +ULong64_t sectorSeedingBaseSeed(); + /// Name of the chunked digit file of a lane, which the chunked digit merger reads std::string laneDigitFileName(int lane); /// Remove the chunked digit files in the working directory whose lane passes the selection void removeLaneDigitFiles(const std::function& select); +/// Writes the flushes of the sectors of one lane to the chunked digit file of the lane +class LaneDigitFileWriter +{ + public: + LaneDigitFileWriter(int lane, bool withMCTruth); + ~LaneDigitFileWriter(); + /// open the lane file and start the tree of a sector + void beginSector(int sector); + /// write the latest flush of the current sector + void writeFlush(SectorFlushBuffers& buffers); + /// write the tree of the current sector and close the file + void endSector(); + + private: + std::unique_ptr mFile; + TTree* mTree = nullptr; ///< owned by the file + size_t mFlushes = 0; + int mLane = 0; + int mSector = 0; + bool mWithMCTruth = true; +}; + /// CCDB conditions and calibration files of the TPC digitizer devices class TPCDigitizerConditions { @@ -71,12 +100,18 @@ class TPCDigitizerConditions /// CCDB inputs common to the TPC digitizer devices static void addInputs(std::vector& inputs, int distortionType); + /// drift velocity and time offset to apply to the digitizers + struct VDrift { + float vdrift; + float tdriftOffset; + }; + void init(framework::InitContext& ic, int distortionType); - /// pass a CCDB object to the digitizer; true if it was a TPC condition - bool finaliseCCDB(framework::ConcreteDataMatcher& matcher, void* obj, Digitizer& digitizer); + /// pass a CCDB object to the digitizer + void finaliseCCDB(framework::ConcreteDataMatcher& matcher, void* obj, Digitizer& digitizer); /// fetch the conditions of a timeframe and prepare the digitizer, which then holds the distortions - /// \param setVDrift called with the drift velocity and time offset when they changed - void update(framework::ProcessingContext& pc, Digitizer& digitizer, const std::function& setVDrift); + /// \return the drift velocity and time offset if they changed + std::optional update(framework::ProcessingContext& pc, Digitizer& digitizer); private: VDriftHelper mVDriftHelper{}; From 5d6cf70b7ab9bed1e84968d2b6c565717e075847 Mon Sep 17 00:00:00 2001 From: Sandro Wenzel Date: Thu, 12 Nov 2026 21:31:33 +0100 Subject: [PATCH 4/4] Share one TPC calibration snapshot between digitizers instead of a CDB lock This replaces the process-wide lock around the CDBInterface reads of the TPC digitizer by a calibration snapshot that one thread reads per timeframe. - DigitizerCalibrations holds the gain, pedestal, noise, threshold, per-pad ion tail and common mode maps, a copy of the dead channel map and the CMC flag. - TPCDigitizerConditions::update() reads it once per timeframe, after the other conditions; both devices pass it to their digitizers, which only read it. - Digitizer::init() no longer reads the CDBInterface or takes a lock; a digitizer used on its own reads a snapshot itself. - The dead channel map is copied once per timeframe instead of once per sector. - The one-time settings report moved into the snapshot read, so no worker thread reads the CDBInterface. Co-Authored-By: Claude Opus 5.5 --- Detectors/TPC/simulation/CMakeLists.txt | 1 + .../include/TPCSimulation/DigitContainer.h | 10 +-- .../include/TPCSimulation/Digitizer.h | 10 ++- .../TPCSimulation/DigitizerCalibrations.h | 41 ++++++++++++ .../include/TPCSimulation/GEMAmplification.h | 4 ++ .../include/TPCSimulation/SAMPAProcessing.h | 4 ++ .../TPC/simulation/src/DigitContainer.cxx | 54 +++------------ Detectors/TPC/simulation/src/Digitizer.cxx | 37 +++++----- .../simulation/src/DigitizerCalibrations.cxx | 67 +++++++++++++++++++ .../TPC/simulation/src/GEMAmplification.cxx | 7 ++ .../TPC/simulation/src/SAMPAProcessing.cxx | 12 ++++ .../src/TPCDigitizerMTSpec.cxx | 7 +- .../src/TPCDigitizerSpec.cxx | 1 + .../src/TPCSectorDigitization.cxx | 2 + .../src/TPCSectorDigitization.h | 4 ++ 15 files changed, 186 insertions(+), 75 deletions(-) create mode 100644 Detectors/TPC/simulation/include/TPCSimulation/DigitizerCalibrations.h create mode 100644 Detectors/TPC/simulation/src/DigitizerCalibrations.cxx diff --git a/Detectors/TPC/simulation/CMakeLists.txt b/Detectors/TPC/simulation/CMakeLists.txt index 7b89784e6799f..e42afc0ae4b54 100644 --- a/Detectors/TPC/simulation/CMakeLists.txt +++ b/Detectors/TPC/simulation/CMakeLists.txt @@ -15,6 +15,7 @@ o2_add_library(TPCSimulation src/DigitContainer.cxx src/DigitGlobalPad.cxx src/Digitizer.cxx + src/DigitizerCalibrations.cxx src/DigitTime.cxx src/ElectronTransport.cxx src/GEMAmplification.cxx diff --git a/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h b/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h index ed47188602ac1..45ee692a52a9b 100644 --- a/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h +++ b/Detectors/TPC/simulation/include/TPCSimulation/DigitContainer.h @@ -22,6 +22,7 @@ #include "DataFormatsTPC/Defs.h" #include "TPCSimulation/DigitTime.h" #include "TPCSimulation/SAMPAProcessing.h" +#include "TPCSimulation/DigitizerCalibrations.h" #include "TPCBase/CalDet.h" #include "CommonUtils/DebugStreamer.h" #include "TPCBase/ParameterDetector.h" @@ -95,6 +96,9 @@ class DigitContainer /// Must be called after the CDBInterface is configured; fillOutputContainer calls it lazily otherwise void updateCalibrations(); + /// Use the calibration objects of a snapshot, which the container keeps alive + void updateCalibrations(std::shared_ptr calib); + private: TimeBin mFirstTimeBin = 0; ///< First time bin to consider TimeBin mEffectiveTimeBin = 0; ///< Effective time bin of that digit @@ -104,14 +108,12 @@ class DigitContainer std::unique_ptr mPrevDigArr; ///< Keep track of ToT and ion tail cumul from last time bin o2::utils::DebugStreamer mStreamer; ///< Debug streamer const CalPad* mPadParams[3] = {nullptr, nullptr, nullptr}; //!< Ion tail fraction, ion tail slope and common mode k-values per pad - const CalDet* mDeadMap = nullptr; //!< Dead channel map, points to mDeadMapCopy if set - std::unique_ptr> mDeadMapCopy; //!< Own copy of the dead channel map, which the CDBInterface rebuilds on each access + const CalDet* mDeadMap = nullptr; //!< Dead channel map of the calibration snapshot, if used + std::shared_ptr mCalibrations; //!< Calibration snapshot in use bool mCMCEnabled = false; //!< Common mode correction enabled in the FEE config (DigiMode Auto) bool mCalibrationsUpdated = false; //!< Whether updateCalibrations was called SAMPAProcessing* mSAMPAProcessing = nullptr; //!< SAMPAProcessing used for the output, the singleton if not set std::vector> mLabelCollector; //!< Workspace to sort the MC labels of a digit - - void reportSettings(); }; inline DigitContainer::DigitContainer() diff --git a/Detectors/TPC/simulation/include/TPCSimulation/Digitizer.h b/Detectors/TPC/simulation/include/TPCSimulation/Digitizer.h index 63f7de331cc26..e94bfab97faf1 100644 --- a/Detectors/TPC/simulation/include/TPCSimulation/Digitizer.h +++ b/Detectors/TPC/simulation/include/TPCSimulation/Digitizer.h @@ -17,6 +17,7 @@ #define ALICEO2_TPC_Digitizer_H_ #include "TPCSimulation/DigitContainer.h" +#include "TPCSimulation/DigitizerCalibrations.h" #include "TPCSimulation/Point.h" #include "TPCBase/Mapper.h" @@ -66,9 +67,12 @@ class Digitizer Digitizer(const Digitizer&) = delete; Digitizer& operator=(const Digitizer&) = delete; - /// Initializer + /// Initializer, with the calibrations set by setCalibrations or else read from the CDBInterface void init(); + /// Use this calibration snapshot from the next init() on; digitizers in several threads share one snapshot + void setCalibrations(std::shared_ptr calib) { mCalibrations = std::move(calib); } + /// Process a single hit group /// \param hits Container with TPC hit groups /// \param eventID ID of the event to be processed @@ -164,6 +168,8 @@ class Digitizer private: /// Fill the random rings of the sector seeding, if not done yet void initSectorSeeding(); + /// the calibration snapshot, read from the CDBInterface if none was set + const DigitizerCalibrations& calibrations(); ElectronTransport& electronTransport(); GEMAmplification& gemAmplification(); @@ -177,6 +183,8 @@ class Digitizer bool mUseSectorSeeding = false; //!< Whether sector seeding is used bool mFlushDebugStreamer = true; //!< Whether flush() flushes the debug streamer + std::shared_ptr mCalibrations; //!< Calibration snapshot used by init() + DigitContainer mDigitContainer; ///< Container for the Digits std::vector mSignalArray; //!< Workspace for the shaped signal of one electron std::unique_ptr mSpaceChargeOwned; //!< Distortions created by the digitizer itself diff --git a/Detectors/TPC/simulation/include/TPCSimulation/DigitizerCalibrations.h b/Detectors/TPC/simulation/include/TPCSimulation/DigitizerCalibrations.h new file mode 100644 index 0000000000000..8ea6defac1af5 --- /dev/null +++ b/Detectors/TPC/simulation/include/TPCSimulation/DigitizerCalibrations.h @@ -0,0 +1,41 @@ +// 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 DigitizerCalibrations.h +/// \brief Calibration objects of the TPC digitizer, shared read-only between digitizers + +#ifndef ALICEO2_TPC_DIGITIZERCALIBRATIONS_H_ +#define ALICEO2_TPC_DIGITIZERCALIBRATIONS_H_ + +#include "TPCBase/CalDet.h" +#include + +namespace o2::tpc +{ + +/// Calibration objects of the TPC digitizer, read from the CDBInterface by one thread and then only read +struct DigitizerCalibrations { + const CalPad* gainMap = nullptr; + const CalPad* pedestals = nullptr; + const CalPad* pedestalsCRU = nullptr; + const CalPad* noise = nullptr; + const CalPad* zeroSuppression = nullptr; + const CalPad* padParams[3] = {nullptr, nullptr, nullptr}; ///< ion tail fraction, ion tail slope and common mode k-values per pad, if used + std::unique_ptr> deadMap; ///< own copy of the dead channel map, if used, since the CDBInterface rebuilds it on each access + bool cmcEnabled = false; ///< common mode correction enabled in the FEE config (DigiMode Auto) + + /// Read the calibration objects from the CDBInterface, which is not thread safe + static std::shared_ptr fetch(); +}; + +} // namespace o2::tpc + +#endif diff --git a/Detectors/TPC/simulation/include/TPCSimulation/GEMAmplification.h b/Detectors/TPC/simulation/include/TPCSimulation/GEMAmplification.h index c30302644a07e..a68f34a3d4682 100644 --- a/Detectors/TPC/simulation/include/TPCSimulation/GEMAmplification.h +++ b/Detectors/TPC/simulation/include/TPCSimulation/GEMAmplification.h @@ -22,6 +22,7 @@ #include "TPCBase/CRU.h" #include "TPCBase/PadPos.h" #include "TPCBase/CalDet.h" +#include "TPCSimulation/DigitizerCalibrations.h" namespace o2 { @@ -56,6 +57,9 @@ class GEMAmplification /// Update the OCDB parameters cached in the class. To be called once per event void updateParameters(); + /// Update the parameters cached in the class, with the gain map from the calibration snapshot + void updateParameters(const DigitizerCalibrations& calib); + /// Compute the number of electrons after amplification in a full stack of four GEM foils /// \param nElectrons Number of electrons arriving at the first amplification stage (GEM1) /// \return Number of electrons after amplification in a full stack of four GEM foils diff --git a/Detectors/TPC/simulation/include/TPCSimulation/SAMPAProcessing.h b/Detectors/TPC/simulation/include/TPCSimulation/SAMPAProcessing.h index 8d6e7acf9d0fa..b7665e61f0eae 100644 --- a/Detectors/TPC/simulation/include/TPCSimulation/SAMPAProcessing.h +++ b/Detectors/TPC/simulation/include/TPCSimulation/SAMPAProcessing.h @@ -20,6 +20,7 @@ #include "TPCBase/PadPos.h" #include "TPCBase/CalDet.h" +#include "TPCSimulation/DigitizerCalibrations.h" #include "TPCBase/CRU.h" #include "MathUtils/RandomRing.h" #include "TPCBase/ParameterDetector.h" @@ -58,6 +59,9 @@ class SAMPAProcessing /// Update the OCDB parameters cached in the class. To be called once per event void updateParameters(float vdrift = 0); + /// Update the parameters cached in the class, with the maps from the calibration snapshot + void updateParameters(float vdrift, const DigitizerCalibrations& calib); + /// Conversion from a given number of electrons into ADC value without taking into account saturation (vectorized) /// \param nElectrons Number of electrons in time bin /// \return ADC value diff --git a/Detectors/TPC/simulation/src/DigitContainer.cxx b/Detectors/TPC/simulation/src/DigitContainer.cxx index aeb6da355759f..85d04854b5540 100644 --- a/Detectors/TPC/simulation/src/DigitContainer.cxx +++ b/Detectors/TPC/simulation/src/DigitContainer.cxx @@ -14,14 +14,11 @@ /// \author Andi Mathis, TU München, andreas.mathis@ph.tum.de #include "TPCSimulation/DigitContainer.h" -#include #include #include #include #include "TPCBase/Mapper.h" -#include "TPCBaseRecSim/CDBInterface.h" #include "TPCBase/ParameterElectronics.h" -#include "TPCBase/IonTailSettings.h" #include "SimConfig/DigiParams.h" using namespace o2::tpc; @@ -135,51 +132,16 @@ void DigitContainer::fillOutputContainer(std::vector& output, void DigitContainer::updateCalibrations() { - const auto& eleParam = ParameterElectronics::Instance(); - auto& cdb = CDBInterface::instance(); - - // ion tail per pad parameters - mPadParams[0] = mPadParams[1] = mPadParams[2] = nullptr; - if (eleParam.doIonTailPerPad) { - const auto& itSettings = IonTailSettings::Instance(); - if (itSettings.padITCorrFile.size()) { - cdb.setFEEParamsFromFile(itSettings.padITCorrFile); - } - mPadParams[0] = &cdb.getITFraction(); - mPadParams[1] = &cdb.getITExpLambda(); - } - if (eleParam.doCommonModePerPad) { - mPadParams[2] = &cdb.getCMkValues(); - } - - // dead channel map - mDeadMap = nullptr; - mDeadMapCopy.reset(); - if (eleParam.applyDeadMap) { - mDeadMapCopy = std::make_unique>(cdb.getDeadChannelMap()); - mDeadMap = mDeadMapCopy.get(); - } + updateCalibrations(DigitizerCalibrations::fetch()); +} +void DigitContainer::updateCalibrations(std::shared_ptr calib) +{ + mCalibrations = std::move(calib); + std::copy(std::begin(mCalibrations->padParams), std::end(mCalibrations->padParams), std::begin(mPadParams)); + mDeadMap = mCalibrations->deadMap.get(); // common mode correction in the FEE, needed to resolve DigiMode Auto - mCMCEnabled = (eleParam.DigiMode == DigitzationMode::Auto) && cdb.getFEEConfig().isCMCEnabled(); + mCMCEnabled = mCalibrations->cmcEnabled; mCalibrationsUpdated = true; - - static std::atomic reportedSettings{false}; - if (!reportedSettings.exchange(true)) { - reportSettings(); - if (mDeadMap) { - LOGP(info, "Using dead map with {} masked pads", mDeadMap->getSum()); - } - } -} - -void DigitContainer::reportSettings() -{ - auto& cdb = CDBInterface::instance(); - const auto& eleParam = ParameterElectronics::Instance(); - const auto& feeConfig = cdb.getFEEConfig(); - LOGP(info, "ParameterElectronics: doIonTail={}, doIonTailPerPad={}, doCommonModePerPad={}, doSaturationTail={}, doNoiseEmptyPads={}, applyDeadMap={}, commonModeCoupling={}, DigiMode={}", - eleParam.doIonTail, eleParam.doIonTailPerPad, eleParam.doCommonModePerPad, eleParam.doSaturationTail, eleParam.doNoiseEmptyPads, eleParam.applyDeadMap, eleParam.commonModeCoupling, (int)eleParam.DigiMode); - feeConfig.printShort(); } diff --git a/Detectors/TPC/simulation/src/Digitizer.cxx b/Detectors/TPC/simulation/src/Digitizer.cxx index 9c7cd1b3649cb..c071f013e7268 100644 --- a/Detectors/TPC/simulation/src/Digitizer.cxx +++ b/Detectors/TPC/simulation/src/Digitizer.cxx @@ -24,14 +24,12 @@ #include "TPCSimulation/GEMAmplification.h" #include "TPCSimulation/Point.h" #include "TPCSimulation/SAMPAProcessing.h" -#include "TPCBaseRecSim/CDBInterface.h" #include "TPCSpaceCharge/SpaceCharge.h" #include "TPCBase/Mapper.h" #include "TPCCalibration/CorrMapParam.h" #include #include -#include ClassImp(o2::tpc::Digitizer); @@ -43,13 +41,6 @@ Digitizer::Digitizer() = default; namespace { -/// serialises the calls that read calibrations through the CDBInterface, which is not thread safe -std::mutex& calibrationMutex() -{ - static std::mutex mutex; - return mutex; -} - // splitmix64 finaliser, to derive well separated seeds from the base seed ULong64_t mixSeed(ULong64_t x) { @@ -115,7 +106,7 @@ void Digitizer::setSectorSeedingFrom(Digitizer& other) void Digitizer::init() { - std::lock_guard lock(calibrationMutex()); + const auto& calib = calibrations(); if (mUseSectorSeeding) { initSectorSeeding(); const ULong64_t sectorSeed = mixSeed(mSeed ^ mixSeed(static_cast(mSector) + 1)); @@ -124,14 +115,19 @@ void Digitizer::init() mElectronTransport->setRandomRingPositions(rng); mSAMPAProcessing->setRandomRingPositions(rng); } - auto& gemAmplification = this->gemAmplification(); - gemAmplification.updateParameters(); - auto& electronTransport = this->electronTransport(); - electronTransport.updateParameters(mVDrift); - auto& sampaProcessing = this->sampaProcessing(); - // the calibration update may reload FEE parameters from file, so SAMPAProcessing picks up its pointers afterwards - mDigitContainer.updateCalibrations(); - sampaProcessing.updateParameters(mVDrift); + gemAmplification().updateParameters(calib); + electronTransport().updateParameters(mVDrift); + mDigitContainer.updateCalibrations(mCalibrations); + sampaProcessing().updateParameters(mVDrift, calib); +} + +const DigitizerCalibrations& Digitizer::calibrations() +{ + if (!mCalibrations) { + // a digitizer used on its own; with several threads the caller sets one snapshot for all + mCalibrations = DigitizerCalibrations::fetch(); + } + return *mCalibrations; } void Digitizer::process(const std::vector& hits, @@ -316,10 +312,7 @@ void Digitizer::setStartTime(double time) // note that negative times w.r.t start of timeframe/data-taking == mOutputDigitTimeOffset // will yield the 0-th bin (due to casting logic in sampaProcessing) SAMPAProcessing& sampaProcessing = this->sampaProcessing(); - { - std::lock_guard lock(calibrationMutex()); - sampaProcessing.updateParameters(mVDrift); - } + sampaProcessing.updateParameters(mVDrift, calibrations()); const auto timediff = time - mOutputDigitTimeOffset; const auto starttimebin = sampaProcessing.getTimeBinFromTime(timediff); mDigitContainer.setStartTime(starttimebin); diff --git a/Detectors/TPC/simulation/src/DigitizerCalibrations.cxx b/Detectors/TPC/simulation/src/DigitizerCalibrations.cxx new file mode 100644 index 0000000000000..dae6098ce550e --- /dev/null +++ b/Detectors/TPC/simulation/src/DigitizerCalibrations.cxx @@ -0,0 +1,67 @@ +// 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 DigitizerCalibrations.cxx +/// \brief Calibration objects of the TPC digitizer, shared read-only between digitizers + +#include "TPCSimulation/DigitizerCalibrations.h" +#include "TPCBaseRecSim/CDBInterface.h" +#include "TPCBase/ParameterElectronics.h" +#include "TPCBase/IonTailSettings.h" +#include "Framework/Logger.h" + +namespace o2::tpc +{ + +std::shared_ptr DigitizerCalibrations::fetch() +{ + const auto& eleParam = ParameterElectronics::Instance(); + auto& cdb = CDBInterface::instance(); + auto calib = std::make_shared(); + + // the same order as the digitizer used: the FEE parameter file may replace the pedestals, noise and thresholds + calib->gainMap = &cdb.getGainMap(); + + if (eleParam.doIonTailPerPad) { + const auto& itSettings = IonTailSettings::Instance(); + if (itSettings.padITCorrFile.size()) { + cdb.setFEEParamsFromFile(itSettings.padITCorrFile); + } + calib->padParams[0] = &cdb.getITFraction(); + calib->padParams[1] = &cdb.getITExpLambda(); + } + if (eleParam.doCommonModePerPad) { + calib->padParams[2] = &cdb.getCMkValues(); + } + if (eleParam.applyDeadMap) { + calib->deadMap = std::make_unique>(cdb.getDeadChannelMap()); + } + calib->cmcEnabled = (eleParam.DigiMode == DigitzationMode::Auto) && cdb.getFEEConfig().isCMCEnabled(); + + calib->pedestals = &cdb.getPedestals(); + calib->pedestalsCRU = &cdb.getPedestalsCRU(); + calib->noise = &cdb.getNoise(); + calib->zeroSuppression = &cdb.getZeroSuppressionThreshold(); + + static bool reportedSettings = false; + if (!reportedSettings) { + reportedSettings = true; + LOGP(info, "ParameterElectronics: doIonTail={}, doIonTailPerPad={}, doCommonModePerPad={}, doSaturationTail={}, doNoiseEmptyPads={}, applyDeadMap={}, commonModeCoupling={}, DigiMode={}", + eleParam.doIonTail, eleParam.doIonTailPerPad, eleParam.doCommonModePerPad, eleParam.doSaturationTail, eleParam.doNoiseEmptyPads, eleParam.applyDeadMap, eleParam.commonModeCoupling, (int)eleParam.DigiMode); + cdb.getFEEConfig().printShort(); + if (calib->deadMap) { + LOGP(info, "Using dead map with {} masked pads", calib->deadMap->getSum()); + } + } + return calib; +} + +} // namespace o2::tpc diff --git a/Detectors/TPC/simulation/src/GEMAmplification.cxx b/Detectors/TPC/simulation/src/GEMAmplification.cxx index 0dadc00ee1f95..5b7fb3c30ca21 100644 --- a/Detectors/TPC/simulation/src/GEMAmplification.cxx +++ b/Detectors/TPC/simulation/src/GEMAmplification.cxx @@ -125,6 +125,13 @@ void GEMAmplification::updateParameters() mGainMap = &(cdb.getGainMap()); } +void GEMAmplification::updateParameters(const DigitizerCalibrations& calib) +{ + mGEMParam = &(ParameterGEM::Instance()); + mGasParam = &(ParameterGas::Instance()); + mGainMap = calib.gainMap; +} + int GEMAmplification::getStackAmplification(int nElectrons) { /// We start with an arbitrary number of electrons given to the first amplification stage diff --git a/Detectors/TPC/simulation/src/SAMPAProcessing.cxx b/Detectors/TPC/simulation/src/SAMPAProcessing.cxx index bee8b02f06e8f..2100ce781130e 100644 --- a/Detectors/TPC/simulation/src/SAMPAProcessing.cxx +++ b/Detectors/TPC/simulation/src/SAMPAProcessing.cxx @@ -42,6 +42,18 @@ void SAMPAProcessing::updateParameters(float vdrift) mVDrift = vdrift > 0 ? vdrift : mGasParam->DriftV; } +void SAMPAProcessing::updateParameters(float vdrift, const DigitizerCalibrations& calib) +{ + mGasParam = &(ParameterGas::Instance()); + mDetParam = &(ParameterDetector::Instance()); + mEleParam = &(ParameterElectronics::Instance()); + mPedestalMap = calib.pedestals; + mPedestalMapCRU = calib.pedestalsCRU; + mNoiseMap = calib.noise; + mZeroSuppression = calib.zeroSuppression; + mVDrift = vdrift > 0 ? vdrift : mGasParam->DriftV; +} + void SAMPAProcessing::getShapedSignal(float ADCsignal, float driftTime, std::vector& signalArray) const { const float timeBinTime = getTimeBinTime(driftTime); diff --git a/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx b/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx index 53637d545bd7e..9347857584294 100644 --- a/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx +++ b/Steer/DigitizerWorkflow/src/TPCDigitizerMTSpec.cxx @@ -124,11 +124,14 @@ class TPCDPLDigitizerMTTask : public o2::base::BaseDPLDigitizer LOG(info) << "Processing multithreaded TPC digitization"; auto& primary = mWorkers[0]->digitizer; - if (auto vdrift = mConditions.update(pc, primary)) { - for (auto& worker : mWorkers) { + const auto vdrift = mConditions.update(pc, primary); + for (auto& worker : mWorkers) { + if (vdrift) { worker->digitizer.setVDrift(vdrift->vdrift); worker->digitizer.setTDriftOffset(vdrift->tdriftOffset); } + // the workers only read the calibrations, which update() fetched in this thread + worker->digitizer.setCalibrations(mConditions.calibrations()); } if (mDistortionType) { for (size_t i = 1; i < mWorkers.size(); ++i) { diff --git a/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx b/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx index 0e8871c7d86e2..e837b7e504b43 100644 --- a/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx +++ b/Steer/DigitizerWorkflow/src/TPCDigitizerSpec.cxx @@ -101,6 +101,7 @@ class TPCDPLDigitizerTask : public BaseDPLDigitizer mDigitizer.setVDrift(vdrift->vdrift); mDigitizer.setTDriftOffset(vdrift->tdriftOffset); } + mDigitizer.setCalibrations(mConditions.calibrations()); for (auto it = pc.inputs().begin(), end = pc.inputs().end(); it != end; ++it) { for (auto const& inputref : it.parts()) { diff --git a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx index 125392ab521c9..8bc3654eaac61 100644 --- a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx +++ b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.cxx @@ -316,6 +316,8 @@ std::optional TPCDigitizerConditions::update(fra LOG(info) << "TPC: Using gain map from 'GainMap.root'"; cdb.setGainMapFromFile("GainMap.root"); } + // all reads of the CDBInterface for the digitizers happen here, in one thread + mCalibrations = DigitizerCalibrations::fetch(); return vdrift; } diff --git a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h index 9fbf646cc58f4..e72c45c98b9a2 100644 --- a/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h +++ b/Steer/DigitizerWorkflow/src/TPCSectorDigitization.h @@ -23,6 +23,7 @@ #include "SimulationDataFormat/MCCompLabel.h" #include "SimulationDataFormat/MCTruthContainer.h" #include "TPCSimulation/CommonMode.h" +#include "TPCSimulation/DigitizerCalibrations.h" #include "TPCCalibration/VDriftHelper.h" #include "Framework/ConcreteDataMatcher.h" #include "Framework/ConfigParamSpec.h" @@ -112,9 +113,12 @@ class TPCDigitizerConditions /// fetch the conditions of a timeframe and prepare the digitizer, which then holds the distortions /// \return the drift velocity and time offset if they changed std::optional update(framework::ProcessingContext& pc, Digitizer& digitizer); + /// the calibration snapshot of the timeframe, read by update() and shared read-only by the digitizers + const std::shared_ptr& calibrations() const { return mCalibrations; } private: VDriftHelper mVDriftHelper{}; + std::shared_ptr mCalibrations; int mDistortionType = 0; bool mUseCalibrationsFromCCDB = false; bool mRecalcDistortions = false;