Redefined get_primary function to return first stable particle

[AkshayaVijay]

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

What is the urgency of this PR?

• High (please describe reason below)
• Medium
• [x ] Low

What kind of change does this PR introduce?

• Bug fix (issue #__)

• [ x] New feature (issue #__)
  Truth Cluster association now gives the stable particle as the primary particle in case of an unstable particle.
  for example : in case of pi0, currently, the get_primary returns the pi0 and no intermediate photon information is available.
  With this change, get_primary returns photons instead of pi0.

• Optimization (issue #__)

• Updated parameters, constants (issue #__)

• Updated documentation

• other: __

Please check if any of the following apply

• This PR requires changes to geometry (epic PR: __)
• This PR requires changes to EDM4eic (EDM PR: __)
• This PR introduces breaking changes. Please describe changes users need to make below.
• This PR changes default behavior. Please describe changes below.
• AI was used in preparing this PR. Please describe usage below.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:25

• Use the C++ header <cstdlib> instead of the C header <stdlib.h> so std::abs(int) is declared in the correct namespace without relying on implementation-defined behavior.

#include <stdlib.h>
#include <algorithm>

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:301

• This loop over chain is a no-op (it only checks isAvailable() and then discards the result). It should be removed to avoid dead code and confusion about intended primary-selection logic.

  for (std::size_t index = 0; index < chain.size(); ++index) {
    const auto& particle = chain[index];

    if (!particle.isAvailable()) {
      continue;

[github-actions]

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:301

• This loop over chain has an empty body and has no effect. It looks like leftover/debug code and makes the primary-selection logic harder to follow; please remove it (or implement the intended logic).

  for (std::size_t index = 0; index < chain.size(); ++index) {
    const auto& particle = chain[index];

    if (!particle.isAvailable()) {
      continue;
    }
  }

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:313

• There is existing unit test coverage for CalorimeterClusterRecoCoG, but it doesn’t exercise the new prompt-decay PDG filtering behavior (e.g. pi0 -> gamma). Please add a test that verifies a prompt-decay parent in promptDecayPDGs is skipped and a stable daughter is returned/associated.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };
  for (std::size_t index = 0; index < chain.size(); ++index) {
    const auto& particle = chain[index];

    if (!particle.isAvailable()) {
      continue;
    }
  }
  for (auto iterator = chain.rbegin(); iterator != chain.rend(); ++iterator) {
    if (!iterator->isAvailable()) {
      continue;
    }
    const bool isPrompt = is_prompt_decay_particle(*iterator);

    if (isPrompt) {
      continue;
    }

    return *iterator;
  }

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:294

• The PR description says truth cluster association now returns the stable particle for prompt decays, but ImagingClusterReco::get_primary() still uses the old parent-walk logic (no prompt-decay PDG filtering). If the intent is to change truth association globally, this update looks incomplete.

This issue also appears in the following locations of the same file:

• line 291
• line 295

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };

src/algorithms/calorimetry/SimCalorimeterHitProcessorConfig.h:34

• The default promptDecayPDGs list is duplicated in multiple config structs. This makes it easy for the lists to drift over time; consider centralizing the default list (shared constant/helper) or at least documenting the physics rationale for each PDG in one place.

  double timeWindow{100 * edm4eic::unit::ns};
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};
};

src/algorithms/calorimetry/SimCalorimeterHitProcessor.cc:201

• lookup_primary() can return an unavailable edm4hep::MCParticle (when the contribution has no particle). That value is then used as part of the hit_map key and later dereferenced (e.g., getPDG()/getObjectID()), which can lead to collisions or undefined behavior. Consider skipping such contributions early in the loop.

      edm4hep::MCParticle primary = lookup_primary(contrib, m_cfg.promptDecayPDGs);

No Clang-Tidy warnings found so I assume my comments were addressed

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/algorithms/calorimetry/ImagingClusterReco.cc:21

• <stdlib.h> is a C header and is the only usage of this header in this directory. Since the code uses std::abs, prefer the C++ header <cstdlib> for consistent declarations in std:: and consistency with the other calorimetry sources (which include <cstdlib>).

#include <stdlib.h>

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:307

• The new promptDecayPDGs logic changes truth association grouping behavior (e.g. skipping pi0/eta/etc in the ancestry chain). There is already a unit test for this algorithm, but it doesn’t cover the new prompt-decay behavior; please add a case where a contribution comes from a stable daughter (e.g. PDG 22) whose parent is a prompt particle (e.g. PDG 111) and assert that get_primary returns the daughter rather than the prompt parent.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };

  for (auto iterator = chain.rbegin(); iterator != chain.rend(); ++iterator) {
    if (!iterator->isAvailable()) {
      continue;
    }
    const bool isPrompt = is_prompt_decay_particle(*iterator);

    if (isPrompt) {
      continue;
    }

    return *iterator;
  }

src/algorithms/calorimetry/SimCalorimeterHitProcessorConfig.h:34

• The default promptDecayPDGs list is duplicated across multiple calorimetry config structs in this PR (SimCalorimeterHitProcessorConfig, ImagingClusterRecoConfig, CalorimeterClusterRecoCoGConfig). Duplicating the defaults increases the risk of the lists diverging over time; consider centralizing this into a shared constant (e.g. in a small common header) and referencing it from each config.

  // time window for grouping contributions
  double timeWindow{100 * edm4eic::unit::ns};
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};
};

src/algorithms/calorimetry/SimCalorimeterHitProcessor.cc:88

• The “prompt decay” primary-selection logic is now implemented in three places (SimCalorimeterHitProcessor’s lookup_primary, ImagingClusterReco::get_primary, CalorimeterClusterRecoCoG::get_primary) with near-identical code. To avoid subtle behavior drift and make future adjustments safer, consider factoring this into a shared utility function used by all three call sites.

edm4hep::MCParticle lookup_primary(const edm4hep::CaloHitContribution& contrib,
                                   const std::vector<int>& promptDecayPDGs) {
  edm4hep::MCParticle current = contrib.getParticle();

  if (!current.isAvailable()) {
    return current;
  }

  const edm4hep::MCParticle original = current;
  std::vector<edm4hep::MCParticle> chain{current};
  while (current.getGeneratorStatus() == 0 && current.parents_size() > 0) {
    const auto parent = current.getParents(0);
    if (!parent.isAvailable()) {
      break;
    }
    current = parent;
    chain.push_back(current);
  }
  const auto is_prompt = [&promptDecayPDGs](const edm4hep::MCParticle& particle) {
    return std::ranges::find(promptDecayPDGs, std::abs(particle.getPDG())) != promptDecayPDGs.end();
  };
  for (auto iterator = chain.rbegin(); iterator != chain.rend(); ++iterator) {
    if (!iterator->isAvailable()) {
      continue;
    }
    if (is_prompt(*iterator)) {
      continue;
    }
    return *iterator;
  }
  return original;
}

[github-actions]

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/algorithms/calorimetry/SimCalorimeterHitProcessorConfig.h:33

• New config field promptDecayPDGs is added without any explanation, while surrounding fields are documented. This makes it unclear which PDGs should go here and how they affect primary selection.

  double timeWindow{100 * edm4eic::unit::ns};
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};

src/algorithms/calorimetry/CalorimeterClusterRecoCoGConfig.h:30

• New config field promptDecayPDGs is undocumented. Given this struct is user-facing configuration, add a short comment describing how these PDG codes affect get_primary() selection.

  bool enableEtaBounds = false;
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:296

• The updated get_primary() behavior (skipping promptDecayPDGs like pi0) isn’t covered by the existing CalorimeterClusterRecoCoG unit test, which only exercises a non-prompt parent/child chain. Please add a test case with a prompt particle (e.g., pi0 -> gamma) to lock in the intended behavior and prevent regressions.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };

  for (auto iterator = chain.rbegin(); iterator != chain.rend(); ++iterator) {

No Clang-Tidy warnings found so I assume my comments were addressed

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/algorithms/calorimetry/SimCalorimeterHitProcessor.cc:25

• std::ranges::find is used in this file, but its declaration is provided by the <algorithm> header per the C++ standard. Relying on transitive includes (or <ranges>) is non-portable across standard library implementations; include <algorithm> explicitly.

#include <cmath>
#include <cstddef>
#include <cstdlib>
#include <functional>
#include <gsl/pointers>
#include <limits>
#include <ranges>
#include <stdexcept>

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:307

• The new promptDecayPDGs-based primary selection changes physics-facing behavior (e.g. preferring stable decay products over prompt resonances), but the existing unit test src/tests/algorithms_test/calorimetry_CalorimeterClusterRecoCoG.cc does not cover this prompt-decay case. Add a test that builds a prompt-decay chain (e.g. pi0 -> gamma gamma) and asserts get_primary returns the stable daughter when the parent PDG is in cfg.promptDecayPDGs.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };

  for (auto iterator = chain.rbegin(); iterator != chain.rend(); ++iterator) {
    if (!iterator->isAvailable()) {
      continue;
    }
    const bool isPrompt = is_prompt_decay_particle(*iterator);

    if (isPrompt) {
      continue;
    }

    return *iterator;
  }

src/algorithms/calorimetry/ImagingClusterReco.cc:342

• get_primary constructs chain for every contribution. Reserving a small capacity avoids repeated allocations in hot paths while keeping the logic unchanged.

  std::vector<edm4hep::MCParticle> chain;
  chain.push_back(current);

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:279

• get_primary builds a new chain vector per contribution; reserving a small capacity reduces allocator churn in this hot loop without changing behavior.

This issue also appears on line 291 of the same file.

  std::vector<edm4hep::MCParticle> chain;
  chain.push_back(current);

src/algorithms/calorimetry/SimCalorimeterHitProcessor.cc:66

• lookup_primary allocates a new std::vector for every contribution. This function is called inside per-hit/per-contribution loops, so avoiding a heap allocation here can reduce overhead; reserving a small capacity is a low-risk improvement.

  const edm4hep::MCParticle original = current;
  std::vector<edm4hep::MCParticle> chain{current};

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:307

• This change introduces new behavior (skipping configured prompt-decay PDGs when selecting the “primary” MCParticle), but the existing CalorimeterClusterRecoCoG unit test doesn’t cover it. Please add a focused test case where a contribution’s ancestor is a prompt-decay particle (e.g., pi0) and verify the association uses the first non-prompt particle (e.g., photon) when promptDecayPDGs includes 111.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };

  for (auto iterator = chain.rbegin(); iterator != chain.rend(); ++iterator) {
    if (!iterator->isAvailable()) {
      continue;
    }
    const bool isPrompt = is_prompt_decay_particle(*iterator);

    if (isPrompt) {
      continue;
    }

    return *iterator;
  }

src/algorithms/calorimetry/ImagingClusterReco.cc:334

• get_primary now selects the first ancestor that is not in promptDecayPDGs, which is different from “first stable particle” wording in the PR title/description. Adding a short comment here would make the selection criteria explicit (and avoid future confusion about what “primary” means).

edm4hep::MCParticle
ImagingClusterReco::get_primary(const edm4hep::CaloHitContribution& contrib) const {

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/algorithms/calorimetry/CalorimeterClusterRecoCoGConfig.h:30

• The default promptDecayPDGs list is duplicated in several calorimetry configs. To keep behavior consistent across algorithms, consider centralizing the default list in one shared constant/header and reusing it here (and in the other configs).

  bool enableEtaBounds = false;
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};

src/algorithms/calorimetry/SimCalorimeterHitProcessor.cc:58

• The new primary-selection logic now exists in multiple places (lookup_primary here plus get_primary in ImagingClusterReco and CalorimeterClusterRecoCoG). This duplication risks subtle divergence (e.g. if the prompt-decay list or traversal rules change). Consider moving the implementation to a shared utility (e.g. under src/algorithms/calorimetry/ or a common utils header) and calling it from all algorithms.

// Lookup primary MCParticle @TODO this should be a shared utility function in the edm4xxx
// libraries
edm4hep::MCParticle lookup_primary(const edm4hep::CaloHitContribution& contrib,
                                   const std::vector<int>& promptDecayPDGs) {

src/algorithms/calorimetry/SimCalorimeterHitProcessorConfig.h:33

• The default promptDecayPDGs list is duplicated across multiple calorimetry config structs (also in ImagingClusterRecoConfig and CalorimeterClusterRecoCoGConfig). Keeping these defaults in sync will be error-prone as the list evolves; consider centralizing the default PDG list in a shared header/constant and referencing it from each config.

  double timeWindow{100 * edm4eic::unit::ns};
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};

src/algorithms/calorimetry/ImagingClusterRecoConfig.h:12

• promptDecayPDGs has the same default list as other calorimetry configs (e.g. SimCalorimeterHitProcessorConfig, CalorimeterClusterRecoCoGConfig). To avoid future drift, consider defining the default prompt-decay PDGs once (shared constant/header) and reusing it here.

  int trackStopLayer = 9;
  std::vector<int> promptDecayPDGs{111, 221, 331, 310, 3122};

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:295

• This change adds prompt-decay skipping in primary selection (via promptDecayPDGs), but the existing CoG unit test only covers a generator-status parent/child chain and does not validate the new prompt-decay behavior (e.g. 111 -> 22 should yield 22 as the primary). Please add/extend a unit test to cover at least one prompt-decay PDG scenario to prevent regressions.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/algorithms/calorimetry/ImagingClusterReco.cc:367

• Local variable isPrompt uses camelCase while the surrounding code in this file predominantly uses snake_case (e.g. is_prompt_decay_particle, do_assoc, cl_layers). Renaming improves consistency and readability.

    const bool isPrompt = is_prompt_decay_particle(*iterator);

    if (isPrompt) {
      continue;

src/algorithms/calorimetry/CalorimeterClusterRecoCoG.cc:304

• Local variable isPrompt uses camelCase while the surrounding code in this file predominantly uses snake_case (e.g. is_prompt_decay_particle, logWeightBase_Eref, enableEtaBounds). Renaming improves consistency and readability.

    const bool isPrompt = is_prompt_decay_particle(*iterator);

    if (isPrompt) {
      continue;

src/algorithms/calorimetry/ImagingClusterReco.cc:357

• The prompt-decay-aware primary selection logic is now duplicated in three places (SimCalorimeterHitProcessor.cc:57-88, ImagingClusterReco.cc:334-372, CalorimeterClusterRecoCoG.cc:270-310). This increases the risk of future divergence (e.g. if the prompt PDG list semantics or traversal changes) and makes bugfixes harder to propagate consistently.

Consider factoring this into a shared helper (e.g. a small utility function taking CaloHitContribution + promptDecayPDGs) and reusing it in all three algorithms.

  const auto is_prompt_decay_particle = [this](const edm4hep::MCParticle& particle) {
    return std::ranges::find(m_cfg.promptDecayPDGs, std::abs(particle.getPDG())) !=
           m_cfg.promptDecayPDGs.end();
  };