Skip to content

Commit 416ac68

Browse files
wdconincCopilotpre-commit-ci[bot]Copilot
authored
feat: use free-threading IRT1 when detected as thread-safe (#2370)
### Briefly, what does this PR introduce? This PR makes avoids locking IRT1 when a thread-safe IRT1 is detected (e.g. the version with the changes in eic/irt#49). If a thread-unsafe IRT1 is detected, we continue locking. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue: avoid locking IRT in EICrecon code with thread-safe design) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
1 parent fd16dea commit 416ac68

File tree

2 files changed

+74
-9
lines changed

2 files changed

+74
-9
lines changed

src/algorithms/pid/IrtCherenkovParticleID.cc

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <podio/RelationRange.h>
2525
#include <algorithm>
2626
#include <cmath>
27+
#include <concepts>
2728
#include <cstddef>
2829
#include <functional>
2930
#include <iterator>
@@ -36,9 +37,66 @@
3637
#include "algorithms/pid/IrtCherenkovParticleIDConfig.h"
3738
#include "algorithms/pid/Tools.h"
3839

40+
// Feature detection: Check if RadiatorHistory has GetLocations() method
41+
namespace {
42+
template <typename T>
43+
concept HasGetLocations = requires(const T& t) {
44+
{ t.GetLocations() } -> std::same_as<const std::vector<std::pair<TVector3, TVector3>>&>;
45+
};
46+
47+
constexpr bool IRT_HAS_RADIATOR_HISTORY_GET_LOCATIONS = HasGetLocations<RadiatorHistory>;
48+
49+
// Helper classes using template specialization to call appropriate methods
50+
// We use templated helper methods and delayed name lookup to avoid
51+
// compilation errors when methods don't exist in the unused specialization
52+
template <bool UseRadiatorHistory> struct IrtHelpers;
53+
54+
// Specialization for new IRT (RadiatorHistory has the methods)
55+
template <> struct IrtHelpers<true> {
56+
template <typename RH, typename CR>
57+
static void SetTrajectoryBinCount(RH* rad_history, CR* /*rad*/, unsigned bins) {
58+
rad_history->SetTrajectoryBinCount(bins);
59+
}
60+
61+
template <typename RH, typename CR> static void ResetLocations(RH* rad_history, CR* /*rad*/) {
62+
rad_history->ResetLocations();
63+
}
64+
65+
template <typename RH, typename CR>
66+
static void AddLocation(RH* rad_history, CR* /*rad*/, const TVector3& x, const TVector3& n) {
67+
rad_history->AddLocation(x, n);
68+
}
69+
};
70+
71+
// Specialization for old IRT (CherenkovRadiator has the methods)
72+
template <> struct IrtHelpers<false> {
73+
template <typename RH, typename CR>
74+
static void SetTrajectoryBinCount(RH* /*rad_history*/, CR* rad, unsigned bins) {
75+
rad->SetTrajectoryBinCount(bins);
76+
}
77+
78+
template <typename RH, typename CR> static void ResetLocations(RH* /*rad_history*/, CR* rad) {
79+
rad->ResetLocations();
80+
}
81+
82+
template <typename RH, typename CR>
83+
static void AddLocation(RH* /*rad_history*/, CR* rad, const TVector3& x, const TVector3& n) {
84+
rad->AddLocation(x, n);
85+
}
86+
};
87+
88+
using IrtHelper = IrtHelpers<IRT_HAS_RADIATOR_HISTORY_GET_LOCATIONS>;
89+
} // namespace
90+
3991
namespace eicrecon {
4092

4193
void IrtCherenkovParticleID::init(CherenkovDetectorCollection* irt_det_coll) {
94+
// lock for old IRT versions that use shared radiator state
95+
[[maybe_unused]] std::unique_lock<std::mutex> lock;
96+
if constexpr (!IRT_HAS_RADIATOR_HISTORY_GET_LOCATIONS) {
97+
lock = std::unique_lock<std::mutex>(m_irt_det_mutex);
98+
}
99+
42100
// members
43101
m_irt_det_coll = irt_det_coll;
44102

@@ -77,7 +135,6 @@ void IrtCherenkovParticleID::init(CherenkovDetectorCollection* irt_det_coll) {
77135
trace("Rebinning refractive index tables to have {} bins", m_cfg.numRIndexBins);
78136
for (auto [rad_name, irt_rad] : m_irt_det->Radiators()) {
79137
// FIXME: m_cfg.numRIndexBins should be a service configurable
80-
std::lock_guard<std::mutex> lock(m_irt_det_mutex);
81138
auto ri_lookup_table_orig = irt_rad->m_ri_lookup_table;
82139
if (ri_lookup_table_orig.size() != m_cfg.numRIndexBins) {
83140
irt_rad->m_ri_lookup_table.clear();
@@ -97,7 +154,6 @@ void IrtCherenkovParticleID::init(CherenkovDetectorCollection* irt_det_coll) {
97154

98155
// check radiators' configuration, and pass it to `m_irt_det`'s radiators
99156
for (auto [rad_name, irt_rad] : m_pid_radiators) {
100-
std::lock_guard<std::mutex> lock(m_irt_det_mutex);
101157
// find `cfg_rad`, the associated `IrtCherenkovParticleIDConfig` radiator
102158
auto cfg_rad_it = m_cfg.radiators.find(rad_name);
103159
if (cfg_rad_it != m_cfg.radiators.end()) {
@@ -179,9 +235,13 @@ void IrtCherenkovParticleID::process(const IrtCherenkovParticleID::Input& input,
179235
auto irt_particle = std::make_unique<ChargedParticle>();
180236

181237
// loop over radiators
182-
// note: this must run exclusively since irt_rad points to shared IRT objects that are
183-
// owned by the RichGeo_service; it holds state (e.g. irt_rad->ResetLocation())
184-
std::lock_guard<std::mutex> lock(m_irt_det_mutex);
238+
// note: for old IRT versions, this must run exclusively since irt_rad points to shared IRT
239+
// objects that are owned by the RichGeo_service; it holds state (e.g. IrtHelpers::ResetLocations())
240+
[[maybe_unused]] std::unique_lock<std::mutex> lock;
241+
if constexpr (!IRT_HAS_RADIATOR_HISTORY_GET_LOCATIONS) {
242+
lock = std::unique_lock<std::mutex>(m_irt_det_mutex);
243+
}
244+
185245
for (auto [rad_name, irt_rad] : m_pid_radiators) {
186246

187247
// get the `charged_particle` for this radiator
@@ -198,21 +258,23 @@ void IrtCherenkovParticleID::process(const IrtCherenkovParticleID::Input& input,
198258
trace("No propagated track points in radiator '{}'", rad_name);
199259
continue;
200260
}
201-
irt_rad->SetTrajectoryBinCount(charged_particle.points_size() - 1);
202261

203262
// start a new IRT `RadiatorHistory`
204263
// - must be a raw pointer for `irt` compatibility
205264
// - it will be destroyed when `irt_particle` is destroyed
206265
auto* irt_rad_history = new RadiatorHistory();
266+
267+
IrtHelper::SetTrajectoryBinCount(irt_rad_history, irt_rad,
268+
charged_particle.points_size() - 1);
207269
irt_particle->StartRadiatorHistory({irt_rad, irt_rad_history});
208270

209271
// loop over `TrackPoint`s of this `charged_particle`, adding each to the IRT radiator
210-
irt_rad->ResetLocations();
272+
IrtHelper::ResetLocations(irt_rad_history, irt_rad);
211273
trace("TrackPoints in '{}' radiator:", rad_name);
212274
for (const auto& point : charged_particle.getPoints()) {
213275
TVector3 position = Tools::PodioVector3_to_TVector3(point.position);
214276
TVector3 momentum = Tools::PodioVector3_to_TVector3(point.momentum);
215-
irt_rad->AddLocation(position, momentum);
277+
IrtHelper::AddLocation(irt_rad_history, irt_rad, position, momentum);
216278
trace(Tools::PrintTVector3(" point: x", position));
217279
trace(Tools::PrintTVector3(" p", momentum));
218280
}

src/algorithms/pid/IrtCherenkovParticleID.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ class IrtCherenkovParticleID : public IrtCherenkovParticleIDAlgorithm,
5959
void process(const Input&, const Output&) const;
6060

6161
private:
62-
// any access (R or W) to m_irt_det_coll, m_irt_det, m_pid_radiators must be locked
62+
// Locking for m_irt_det_coll, m_irt_det, m_pid_radiators:
63+
// - Required for old/non-thread-safe IRT versions (detected by !IRT_HAS_RADIATOR_HISTORY_GET_LOCATIONS)
64+
// that use shared radiator state
65+
// - Not required for newer thread-safe IRT versions (IRT_HAS_RADIATOR_HISTORY_GET_LOCATIONS)
6366
inline static std::mutex m_irt_det_mutex;
6467
CherenkovDetectorCollection* m_irt_det_coll;
6568
CherenkovDetector* m_irt_det;

0 commit comments

Comments
 (0)