From a1c1b835c1500e3066a87c268038fdfd2cfef9db Mon Sep 17 00:00:00 2001 From: Matthias Kleiner Date: Wed, 9 Sep 2026 17:05:09 +0200 Subject: [PATCH 1/6] [Common] Fix TPC side determination in VDrift correction --- Common/Core/TPCVDriftManager.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Common/Core/TPCVDriftManager.h b/Common/Core/TPCVDriftManager.h index d7c5b25be19..180ae7887bb 100644 --- a/Common/Core/TPCVDriftManager.h +++ b/Common/Core/TPCVDriftManager.h @@ -131,7 +131,8 @@ class TPCVDriftManager } // impose new Z coordinate - track.setZ(track.getZ() + ((track.getTgl() < 0.) ? -dDrift : dDrift)); + const auto sides = (trackExtra.flags() & (o2::aod::track::TrackFlags::TPCSideA | o2::aod::track::TrackFlags::TPCSideC)); + track.setZ(track.getZ() + (sides == o2::aod::track::TrackFlags::TPCSideC ? -dDrift : (sides == o2::aod::track::TrackFlags::TPCSideA ? dDrift : 0))); if constexpr (std::is_base_of_v) { track.setCov(track.getSigmaZ2() + dDriftErr * dDriftErr, o2::track::kSigZ2); } From ea25e4392ce0fb90d9040365712a392b1343159e Mon Sep 17 00:00:00 2001 From: Matthias Kleiner Date: Wed, 9 Sep 2026 17:07:51 +0200 Subject: [PATCH 2/6] [Common] Fix magic number lint error in TPC drift volume check Co-Authored-By: Claude Sonnet 5 --- Common/Core/TPCVDriftManager.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Common/Core/TPCVDriftManager.h b/Common/Core/TPCVDriftManager.h index 180ae7887bb..0dd634071ca 100644 --- a/Common/Core/TPCVDriftManager.h +++ b/Common/Core/TPCVDriftManager.h @@ -115,7 +115,7 @@ class TPCVDriftManager float dTime = tTB - trackExtra.trackTime(); float dDrift = dTime * mTPCVDriftNS; float dDriftErr = tTBErr * mTPCVDriftNS; - if (dDriftErr < 0.f || dDrift > 250.f) { // we cannot move a track outside the drift volume + if (dDriftErr < 0.f || dDrift > mMaxDriftCm) { // we cannot move a track outside the drift volume if (mOutside < mWarningLimit) { LOGP(warn, "Skipping correction outside of tpc volume with dDrift={} +- {}", dDrift, dDriftErr); const auto trackBC = trackExtra.template collision_as().template foundBC_as().globalBC(); @@ -157,6 +157,7 @@ class TPCVDriftManager o2::ccdb::BasicCCDBManager* mCCDB{}; // reference to initialized ccdb manager static constexpr unsigned int mWarningLimit{10}; + static constexpr float mMaxDriftCm{250.f}; // TPC drift volume half-length in cm // Counters unsigned int mCalls{0}; // total number of calls From be7148bacb931e1fdc47ed46430eea4e448b4588 Mon Sep 17 00:00:00 2001 From: Matthias Kleiner Date: Fri, 11 Sep 2026 18:47:52 +0200 Subject: [PATCH 3/6] [Common] Add legacy fallback for TPC side detection without side flags The TPCSideA/TPCSideC flags were only added to the AOD format in February 2026, so datasets produced before that always have neither bit set. Detect CE-crossing tracks from the track's asymmetric time margins and otherwise infer the side from a cross-check of Z and tgl sign, per TPC domain expert guidance, instead of silently skipping the correction for all tracks in older datasets. Co-Authored-By: Claude Sonnet 5 --- Common/Core/TPCVDriftManager.h | 46 ++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/Common/Core/TPCVDriftManager.h b/Common/Core/TPCVDriftManager.h index 0dd634071ca..eddb9208311 100644 --- a/Common/Core/TPCVDriftManager.h +++ b/Common/Core/TPCVDriftManager.h @@ -132,7 +132,48 @@ class TPCVDriftManager // impose new Z coordinate const auto sides = (trackExtra.flags() & (o2::aod::track::TrackFlags::TPCSideA | o2::aod::track::TrackFlags::TPCSideC)); - track.setZ(track.getZ() + (sides == o2::aod::track::TrackFlags::TPCSideC ? -dDrift : (sides == o2::aod::track::TrackFlags::TPCSideA ? dDrift : 0))); + float zShift = 0.f; + if (sides == o2::aod::track::TrackFlags::TPCSideA) { + zShift = dDrift; + } else if (sides == o2::aod::track::TrackFlags::TPCSideC) { + zShift = -dDrift; + } else if (sides == 0) { + // Fallback for datasets produced before the TPC side flags were introduced (Feb. 2026). + o2::aod::track::extensions::TPCTimeErrEncoding tEnc; + tEnc.encoding.timeErr = trackExtra.trackTimeRes(); + const float dFwd = tEnc.getDeltaTFwd(); + const float dBwd = tEnc.getDeltaTBwd(); + // Equal, small forward/backward margins mean the track is bounded on both ends, + // i.e. it crosses the CE: it cannot be moved and is already corrected elsewhere. + const bool crossesCE = (dFwd == dBwd) && (dFwd < mMaxCECrossingDeltaTNS); + if (!crossesCE) { + const bool zPositive = track.getZ() > 0.f; + const bool tglPositive = track.getTgl() > 0.f; + int side = 0; // +1 = A, -1 = C, 0 = undetermined -> leave uncorrected + if (zPositive == tglPositive) { + // Consistent sign: the track converges to Z=0 at the beamline by construction. + side = tglPositive ? 1 : -1; + } else if (dBwd == 0.f && dFwd > 0.f) { + // Bounded backward at the CE with room forward: no clusters on the opposite + // side, so trust the measured Z rather than the (here inverted) tgl. + side = zPositive ? 1 : -1; + } else if (dBwd > 0.f) { + // Large tgl track bounded at the readout side instead: trust tgl. + side = tglPositive ? 1 : -1; + } + // else: degenerate case, track touches both CE and readout -> cannot be deduced/moved. + // (in practice unreachable here: dFwd==dBwd==0 would already satisfy crossesCE above, + // since dFwd/dBwd are always >= 0; kept explicit to mirror the reference logic 1:1.) + + if (side > 0) { + zShift = dDrift; + } else if (side < 0) { + zShift = -dDrift; + } + } + } + // else: track has clusters on both sides (crossed the CE) and is already corrected elsewhere + track.setZ(track.getZ() + zShift); if constexpr (std::is_base_of_v) { track.setCov(track.getSigmaZ2() + dDriftErr * dDriftErr, o2::track::kSigZ2); } @@ -157,7 +198,8 @@ class TPCVDriftManager o2::ccdb::BasicCCDBManager* mCCDB{}; // reference to initialized ccdb manager static constexpr unsigned int mWarningLimit{10}; - static constexpr float mMaxDriftCm{250.f}; // TPC drift volume half-length in cm + static constexpr float mMaxDriftCm{250.f}; // TPC drift volume half-length in cm + static constexpr float mMaxCECrossingDeltaTNS{1000.f}; // ~1 us, ballpark forward/backward time margin of a CE-crossing track // Counters unsigned int mCalls{0}; // total number of calls From ec64032604683def3f4076401d237a62c11c27f3 Mon Sep 17 00:00:00 2001 From: Matthias Kleiner Date: Mon, 14 Sep 2026 13:06:05 +0200 Subject: [PATCH 4/6] [Common] Add global switch for TPC side-based VDrift correction Introduce TPCVDriftManagerParam::useSideBasedCorrection (default off) to gate the new TPC-side-flag-based correction behind a ConfigurableParam, settable from any workflow via --configKeyValues without touching each task that owns a TPCVDriftManager instance. Keeps existing analyses on the legacy tgl-sign behaviour until explicitly opted in for testing. Co-Authored-By: Claude Sonnet 5 --- Common/Core/TPCVDriftManager.h | 92 ++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 38 deletions(-) diff --git a/Common/Core/TPCVDriftManager.h b/Common/Core/TPCVDriftManager.h index eddb9208311..578f45522e6 100644 --- a/Common/Core/TPCVDriftManager.h +++ b/Common/Core/TPCVDriftManager.h @@ -14,6 +14,8 @@ #include #include +#include +#include #include #include #include @@ -26,6 +28,15 @@ namespace o2::aod::common { +struct TPCVDriftManagerParam : public o2::conf::ConfigurableParamHelper { + // Use the TPC side flags (with legacy-data fallback) instead of the tgl-sign-based correction. + // Off by default so that existing analyses see no change in results until this is explicitly + // enabled for testing. + bool useSideBasedCorrection = false; + + O2ParamDef(TPCVDriftManagerParam, "TPCVDriftManager"); +}; + // Thin wrapper for vdrift ccdb queries should partially mirror VDriftHelper class. // Allows to move TPC standalone tracks under the assumption of a different // collision than the track is associated to. @@ -131,48 +142,53 @@ class TPCVDriftManager } // impose new Z coordinate - const auto sides = (trackExtra.flags() & (o2::aod::track::TrackFlags::TPCSideA | o2::aod::track::TrackFlags::TPCSideC)); float zShift = 0.f; - if (sides == o2::aod::track::TrackFlags::TPCSideA) { - zShift = dDrift; - } else if (sides == o2::aod::track::TrackFlags::TPCSideC) { - zShift = -dDrift; - } else if (sides == 0) { - // Fallback for datasets produced before the TPC side flags were introduced (Feb. 2026). - o2::aod::track::extensions::TPCTimeErrEncoding tEnc; - tEnc.encoding.timeErr = trackExtra.trackTimeRes(); - const float dFwd = tEnc.getDeltaTFwd(); - const float dBwd = tEnc.getDeltaTBwd(); - // Equal, small forward/backward margins mean the track is bounded on both ends, - // i.e. it crosses the CE: it cannot be moved and is already corrected elsewhere. - const bool crossesCE = (dFwd == dBwd) && (dFwd < mMaxCECrossingDeltaTNS); - if (!crossesCE) { - const bool zPositive = track.getZ() > 0.f; - const bool tglPositive = track.getTgl() > 0.f; - int side = 0; // +1 = A, -1 = C, 0 = undetermined -> leave uncorrected - if (zPositive == tglPositive) { - // Consistent sign: the track converges to Z=0 at the beamline by construction. - side = tglPositive ? 1 : -1; - } else if (dBwd == 0.f && dFwd > 0.f) { - // Bounded backward at the CE with room forward: no clusters on the opposite - // side, so trust the measured Z rather than the (here inverted) tgl. - side = zPositive ? 1 : -1; - } else if (dBwd > 0.f) { - // Large tgl track bounded at the readout side instead: trust tgl. - side = tglPositive ? 1 : -1; - } - // else: degenerate case, track touches both CE and readout -> cannot be deduced/moved. - // (in practice unreachable here: dFwd==dBwd==0 would already satisfy crossesCE above, - // since dFwd/dBwd are always >= 0; kept explicit to mirror the reference logic 1:1.) - - if (side > 0) { - zShift = dDrift; - } else if (side < 0) { - zShift = -dDrift; + if (!TPCVDriftManagerParam::Instance().useSideBasedCorrection) { + // Legacy behaviour (default): infer the side from tgl alone. + zShift = (track.getTgl() < 0.f) ? -dDrift : dDrift; + } else { + const auto sides = (trackExtra.flags() & (o2::aod::track::TrackFlags::TPCSideA | o2::aod::track::TrackFlags::TPCSideC)); + if (sides == o2::aod::track::TrackFlags::TPCSideA) { + zShift = dDrift; + } else if (sides == o2::aod::track::TrackFlags::TPCSideC) { + zShift = -dDrift; + } else if (sides == 0) { + // Fallback for datasets produced before the TPC side flags were introduced (Feb. 2026). + o2::aod::track::extensions::TPCTimeErrEncoding tEnc; + tEnc.encoding.timeErr = trackExtra.trackTimeRes(); + const float dFwd = tEnc.getDeltaTFwd(); + const float dBwd = tEnc.getDeltaTBwd(); + // Equal, small forward/backward margins mean the track is bounded on both ends, + // i.e. it crosses the CE: it cannot be moved and is already corrected elsewhere. + const bool crossesCE = (dFwd == dBwd) && (dFwd < mMaxCECrossingDeltaTNS); + if (!crossesCE) { + const bool zPositive = track.getZ() > 0.f; + const bool tglPositive = track.getTgl() > 0.f; + int side = 0; // +1 = A, -1 = C, 0 = undetermined -> leave uncorrected + if (zPositive == tglPositive) { + // Consistent sign: the track converges to Z=0 at the beamline by construction. + side = tglPositive ? 1 : -1; + } else if (dBwd == 0.f && dFwd > 0.f) { + // Bounded backward at the CE with room forward: no clusters on the opposite + // side, so trust the measured Z rather than the (here inverted) tgl. + side = zPositive ? 1 : -1; + } else if (dBwd > 0.f) { + // Large tgl track bounded at the readout side instead: trust tgl. + side = tglPositive ? 1 : -1; + } + // else: degenerate case, track touches both CE and readout -> cannot be deduced/moved. + // (in practice unreachable here: dFwd==dBwd==0 would already satisfy crossesCE above, + // since dFwd/dBwd are always >= 0; kept explicit to mirror the reference logic 1:1.) + + if (side > 0) { + zShift = dDrift; + } else if (side < 0) { + zShift = -dDrift; + } } } + // else: track has clusters on both sides (crossed the CE) and is already corrected elsewhere } - // else: track has clusters on both sides (crossed the CE) and is already corrected elsewhere track.setZ(track.getZ() + zShift); if constexpr (std::is_base_of_v) { track.setCov(track.getSigmaZ2() + dDriftErr * dDriftErr, o2::track::kSigZ2); From 0757bf54a9931374ebf268730bd796e1a36dfc38 Mon Sep 17 00:00:00 2001 From: Matthias Kleiner Date: Tue, 15 Sep 2026 08:58:39 +0200 Subject: [PATCH 5/6] [Common] Use a member setter instead of ConfigurableParam for the side-based correction switch O2ParamDef only declares the static sInstance member; defining it requires O2ParamImpl in a compiled .cxx, but TPCVDriftManager is a header-only library with none, so Instance() left an unresolved symbol for every consumer. Switch to a plain member + setter, as suggested in review; tasks using TPCVDriftManager can later expose this via their own Configurable. Co-Authored-By: Claude Sonnet 5 --- Common/Core/TPCVDriftManager.h | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/Common/Core/TPCVDriftManager.h b/Common/Core/TPCVDriftManager.h index 578f45522e6..01e86c76419 100644 --- a/Common/Core/TPCVDriftManager.h +++ b/Common/Core/TPCVDriftManager.h @@ -14,8 +14,6 @@ #include #include -#include -#include #include #include #include @@ -28,15 +26,6 @@ namespace o2::aod::common { -struct TPCVDriftManagerParam : public o2::conf::ConfigurableParamHelper { - // Use the TPC side flags (with legacy-data fallback) instead of the tgl-sign-based correction. - // Off by default so that existing analyses see no change in results until this is explicitly - // enabled for testing. - bool useSideBasedCorrection = false; - - O2ParamDef(TPCVDriftManagerParam, "TPCVDriftManager"); -}; - // Thin wrapper for vdrift ccdb queries should partially mirror VDriftHelper class. // Allows to move TPC standalone tracks under the assumption of a different // collision than the track is associated to. @@ -48,6 +37,14 @@ class TPCVDriftManager mCCDB = ccdb; } + // Use the TPC side flags (with legacy-data fallback) instead of the tgl-sign-based correction. + // Off by default so that existing analyses see no change in results until this is explicitly + // enabled for testing. Tasks using TPCVDriftManager can expose this via their own Configurable. + void setUseSideBasedCorrection(bool value) noexcept + { + mUseSideBasedCorrection = value; + } + void update(uint64_t timestamp) noexcept { // Keep the object we already have if it is still valid for this timestamp. @@ -143,7 +140,7 @@ class TPCVDriftManager // impose new Z coordinate float zShift = 0.f; - if (!TPCVDriftManagerParam::Instance().useSideBasedCorrection) { + if (!mUseSideBasedCorrection) { // Legacy behaviour (default): infer the side from tgl alone. zShift = (track.getTgl() < 0.f) ? -dDrift : dDrift; } else { @@ -206,6 +203,7 @@ class TPCVDriftManager private: bool mValid{false}; + bool mUseSideBasedCorrection{false}; // off by default: preserves legacy tgl-sign behaviour // Factors float mTPCVDriftNS{0.f}; // drift velocity in cm/ns From e8496d1b6c028b668305fc21628bd7d744b234c8 Mon Sep 17 00:00:00 2001 From: SCHOTTER Romain <47983209+romainschotter@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:58:20 +0200 Subject: [PATCH 6/6] Dummy commit to restart CI --- Common/Core/TPCVDriftManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common/Core/TPCVDriftManager.h b/Common/Core/TPCVDriftManager.h index 01e86c76419..21f98ebffaf 100644 --- a/Common/Core/TPCVDriftManager.h +++ b/Common/Core/TPCVDriftManager.h @@ -48,7 +48,7 @@ class TPCVDriftManager void update(uint64_t timestamp) noexcept { // Keep the object we already have if it is still valid for this timestamp. - // firstTime/lastTime are the first and last timestamps of the TFs the correction + // firstTime/lastTime are the first and last timestamps of the TF the correction // was derived from, so the validity range is closed on both ends. if (mVD != nullptr && timestamp >= static_cast(mVD->firstTime) && timestamp <= static_cast(mVD->lastTime)) { return;