Skip to content

Commit 3af7f8f

Browse files
minjungkim12claude
andcommitted
Address code review comments: rename thresholds and remove unnecessary copy
Changes based on code review feedback from vkucera: 1. Rename threshold constants to follow "quantity + object" naming pattern: - FT0AThreshold → AmplitudeThresholdFT0A - FT0CThreshold → AmplitudeThresholdFT0C - ZDCThreshold → EnergyThresholdZDC Updated function parameters and all usages in utilsUpcHf.h. 2. Remove unnecessary centrality copy in UPC process function: - Removed `float cent{centrality};` declaration - Use `centrality` variable directly throughout the function This eliminates redundant variable and improves code clarity. 3. Use literal values for Configurable defaults: - Configurables require literal values, not constexpr references - Changed from using defaults namespace to 100.0f, 50.0f, 1.0f 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a51719e commit 3af7f8f

File tree

3 files changed

+21
-22
lines changed

3 files changed

+21
-22
lines changed

PWGHF/D2H/Tasks/taskD0.cxx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ struct HfTaskD0 {
9999
Configurable<std::string> irSource{"irSource", "ZNC hadronic", "Estimator of the interaction rate (Recommended: pp --> T0VTX, Pb-Pb --> ZNC hadronic)"};
100100

101101
// UPC gap determination thresholds
102-
Configurable<float> upcFT0AThreshold{"upcFT0AThreshold", hf_upc::defaults::FT0AThreshold, "FT0-A amplitude threshold for UPC gap determination (a.u.)"};
103-
Configurable<float> upcFT0CThreshold{"upcFT0CThreshold", hf_upc::defaults::FT0CThreshold, "FT0-C amplitude threshold for UPC gap determination (a.u.)"};
104-
Configurable<float> upcZDCThreshold{"upcZDCThreshold", hf_upc::defaults::ZDCThreshold, "ZDC energy threshold for UPC gap determination (a.u.)"};
102+
Configurable<float> upcFT0AThreshold{"upcFT0AThreshold", 100.0f, "FT0-A amplitude threshold for UPC gap determination (a.u.)"};
103+
Configurable<float> upcFT0CThreshold{"upcFT0CThreshold", 50.0f, "FT0-C amplitude threshold for UPC gap determination (a.u.)"};
104+
Configurable<float> upcZDCThreshold{"upcZDCThreshold", 1.0f, "ZDC energy threshold for UPC gap determination (a.u.)"};
105105

106106
HfEventSelection hfEvSel; // event selection and monitoring
107107

@@ -598,7 +598,6 @@ struct HfTaskD0 {
598598
int const gapTypeInt = hf_upc::gapTypeToInt(gap);
599599
const auto thisCollId = collision.globalIndex();
600600
const auto& groupedD0Candidates = candidates.sliceBy(candD0PerCollision, thisCollId);
601-
float cent{centrality};
602601
float occ{-1.f};
603602

604603
for (const auto& candidate : groupedD0Candidates) {
@@ -631,7 +630,7 @@ struct HfTaskD0 {
631630
candidate.mlProbD0()[0], candidate.mlProbD0()[1], candidate.mlProbD0()[2],
632631
static_cast<double>(hfHelper.yD0(candidate)), static_cast<double>(d0Type)};
633632
if (storeCentrality) {
634-
valuesToFill.push_back(cent);
633+
valuesToFill.push_back(centrality);
635634
}
636635
if (storeOccupancyAndIR) {
637636
valuesToFill.push_back(occ);
@@ -655,7 +654,7 @@ struct HfTaskD0 {
655654
std::vector<double> valuesToFill{static_cast<double>(mass), static_cast<double>(ptCandidate),
656655
static_cast<double>(hfHelper.yD0(candidate)), static_cast<double>(d0Type)};
657656
if (storeCentrality) {
658-
valuesToFill.push_back(cent);
657+
valuesToFill.push_back(centrality);
659658
}
660659
if (storeOccupancyAndIR) {
661660
valuesToFill.push_back(occ);

PWGHF/D2H/Tasks/taskDplus.cxx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ struct HfTaskDplus {
8585
Configurable<std::string> ccdbPathGrpMag{"ccdbPathGrpMag", "GLO/Config/GRPMagField", "CCDB path of the GRPMagField object (Run 3)"};
8686

8787
// UPC gap determination thresholds
88-
Configurable<float> upcFT0AThreshold{"upcFT0AThreshold", hf_upc::defaults::FT0AThreshold, "FT0-A amplitude threshold for UPC gap determination (a.u.)"};
89-
Configurable<float> upcFT0CThreshold{"upcFT0CThreshold", hf_upc::defaults::FT0CThreshold, "FT0-C amplitude threshold for UPC gap determination (a.u.)"};
90-
Configurable<float> upcZDCThreshold{"upcZDCThreshold", hf_upc::defaults::ZDCThreshold, "ZDC energy threshold for UPC gap determination (a.u.)"};
88+
Configurable<float> upcFT0AThreshold{"upcFT0AThreshold", 100.0f, "FT0-A amplitude threshold for UPC gap determination (a.u.)"};
89+
Configurable<float> upcFT0CThreshold{"upcFT0CThreshold", 50.0f, "FT0-C amplitude threshold for UPC gap determination (a.u.)"};
90+
Configurable<float> upcZDCThreshold{"upcZDCThreshold", 1.0f, "ZDC energy threshold for UPC gap determination (a.u.)"};
9191

9292
HfEventSelection hfEvSel; // event selection and monitoring
9393

PWGHF/Utils/utilsUpcHf.h

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,34 +32,34 @@ enum class GapType : uint8_t {
3232
/// \brief Default thresholds for gap determination
3333
namespace defaults
3434
{
35-
constexpr float FT0AThreshold = 100.0f; ///< FT0-A amplitude threshold (a.u.)
36-
constexpr float FT0CThreshold = 50.0f; ///< FT0-C amplitude threshold (a.u.)
37-
constexpr float ZDCThreshold = 1.0f; ///< ZDC energy threshold (a.u.)
35+
constexpr float AmplitudeThresholdFT0A = 100.0f; ///< Amplitude threshold for FT0-A (a.u.)
36+
constexpr float AmplitudeThresholdFT0C = 50.0f; ///< Amplitude threshold for FT0-C (a.u.)
37+
constexpr float EnergyThresholdZDC = 1.0f; ///< Energy threshold for ZDC (a.u.)
3838
} // namespace defaults
3939

4040
/// \brief Determine gap type based on FIT and ZDC signals
4141
/// \param ft0A FT0-A amplitude
4242
/// \param ft0C FT0-C amplitude
4343
/// \param zdcA ZDC-A (ZNA) common energy
4444
/// \param zdcC ZDC-C (ZNC) common energy
45-
/// \param ft0AThreshold Threshold for FT0-A (default: 100.0)
46-
/// \param ft0CThreshold Threshold for FT0-C (default: 50.0)
47-
/// \param zdcThreshold Threshold for ZDC (default: 1.0)
45+
/// \param amplitudeThresholdFT0A Threshold for FT0-A (default: 100.0)
46+
/// \param amplitudeThresholdFT0C Threshold for FT0-C (default: 50.0)
47+
/// \param energyThresholdZDC Threshold for ZDC (default: 1.0)
4848
/// \return Gap type classification
4949
inline GapType determineGapType(float ft0A, float ft0C, float zdcA, float zdcC,
50-
float ft0AThreshold = defaults::FT0AThreshold,
51-
float ft0CThreshold = defaults::FT0CThreshold,
52-
float zdcThreshold = defaults::ZDCThreshold)
50+
float amplitudeThresholdFT0A = defaults::AmplitudeThresholdFT0A,
51+
float amplitudeThresholdFT0C = defaults::AmplitudeThresholdFT0C,
52+
float energyThresholdZDC = defaults::EnergyThresholdZDC)
5353
{
5454
// Gap on A-side: FT0-A empty, FT0-C active, ZNA empty, ZNC active
55-
if (ft0A < ft0AThreshold && ft0C > ft0CThreshold &&
56-
zdcA < zdcThreshold && zdcC > zdcThreshold) {
55+
if (ft0A < amplitudeThresholdFT0A && ft0C > amplitudeThresholdFT0C &&
56+
zdcA < energyThresholdZDC && zdcC > energyThresholdZDC) {
5757
return GapType::GapA;
5858
}
5959

6060
// Gap on C-side: FT0-A active, FT0-C empty, ZNA active, ZNC empty
61-
if (ft0A > ft0AThreshold && ft0C < ft0CThreshold &&
62-
zdcA > zdcThreshold && zdcC < zdcThreshold) {
61+
if (ft0A > amplitudeThresholdFT0A && ft0C < amplitudeThresholdFT0C &&
62+
zdcA > energyThresholdZDC && zdcC < energyThresholdZDC) {
6363
return GapType::GapC;
6464
}
6565

0 commit comments

Comments
 (0)