Skip to content

Major KFParticle alteration #4332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cdean-github wants to merge 9 commits into
base: master
Choose a base branch
from
+369 −262
Open

Major KFParticle alteration #4332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4741a7b
CD: Better truth matching
cdean-github Jun 3, 2026
5d6acc1
CD: Better truth matching
cdean-github Jun 3, 2026
8a9daa9
Merge branch 'master' of github.com:sPHENIX-Collaboration/coresoftware
cdean-github Jun 12, 2026
e58b940
CD: New KFP methods for variable calculation
cdean-github Jul 2, 2026
6f8ee41
CD: Patched DCA calculation in KFParticle
cdean-github Jul 2, 2026
bdedb9e
Update offline/packages/KFParticle_sPHENIX/KFParticle_eventReconstruc…
cdean-github Jul 2, 2026
d712e8a
Update offline/packages/KFParticle_sPHENIX/KFParticle_Tools.cc
cdean-github Jul 2, 2026
881bc72
CD: CPP check suggestions
cdean-github Jul 2, 2026
4104405
Merge branch 'master' of github.com:cdean-github/coresoftware
cdean-github Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 41 additions & 30 deletions offline/packages/HFTrackEfficiency/HFTrackEfficiency.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ int HFTrackEfficiency::process_event(PHCompositeNode *topNode)
}
}

if (!m_svtx_evalstack)
{
m_svtx_evalstack = new SvtxEvalStack(topNode);
trackeval = m_svtx_evalstack->get_track_eval();
}
m_svtx_evalstack->next_event(topNode);
/*
m_dst_truth_reco_map = findNode::getClass<PHG4ParticleSvtxMap>(topNode, "PHG4ParticleSvtxMap");
if (m_dst_truth_reco_map)
{
Expand All @@ -129,7 +136,7 @@ int HFTrackEfficiency::process_event(PHCompositeNode *topNode)
std::cout << __FILE__ << ": PHG4ParticleSvtxMap not found, reverting to true matching by momentum relations. Truth matching will be less accurate" << std::endl;
}
}

*/
if (m_decay_descriptor.empty() && !m_decayMap->empty())
{
getDecayDescriptor();
Expand Down Expand Up @@ -239,11 +246,12 @@ bool HFTrackEfficiency::findTracks(PHCompositeNode *topNode, Decay decay)
}

int index = -1;
PHG4Particle *daughterG4 {nullptr};

for (unsigned int i = 1; i < decay.size(); ++i)
{
m_dst_track = nullptr;
int truth_ID = -1;
//int truth_ID = -1;
Comment on lines +249 to +254

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='offline/packages/HFTrackEfficiency/HFTrackEfficiency.cc'

echo '--- file size ---'
wc -l "$file"

echo '--- outline ---'
ast-grep outline "$file" --view expanded || true

echo '--- relevant sections ---'
sed -n '230,470p' "$file" | cat -n

echo '--- search truth_ID declarations/usages ---'
rg -n '\btruth_ID\b' "$file" || true

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 11893

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='offline/packages/HFTrackEfficiency/HFTrackEfficiency.cc'

echo '--- around line 390-445 ---'
sed -n '390,445p' "$file" | cat -n

echo '--- whole repo search for truth_ID ---'
rg -n '\btruth_ID\b' offline/packages/HFTrackEfficiency || true

echo '--- class member search for trackeval / truth-reco map context ---'
rg -n 'class HFTrackEfficiency|truth_ID|m_dst_truth_reco_map|m_truthInfo|trackeval' offline/packages/HFTrackEfficiency -g '!*.o' || true

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 7187

Reset daughterG4 per daughter and restore truth_ID

  • daughterG4 needs to be cleared at the start of each daughter iteration, and the m_nDaughters == 2 scan should break on the first match; otherwise trackeval->best_track_from(daughterG4) can use the last truth particle seen instead of the matched daughter.
  • truth_ID is still assigned in the m_nDaughters == 3 path, but its declaration is commented out in this scope, so this file will not compile.


if (std::find(std::begin(trackableParticles), std::end(trackableParticles),
std::abs(decay[i].second)) != std::end(trackableParticles))
Expand All @@ -270,31 +278,33 @@ bool HFTrackEfficiency::findTracks(PHCompositeNode *topNode, Decay decay)
m_secondary_vtx_z = thisVtx->point3d().z();

// We need the G4 ID, not the HepMC ID to use the truth/reco map
if (m_dst_truth_reco_map)
//if (m_dst_truth_reco_map)
//{
PHG4TruthInfoContainer::ConstRange range = m_truthInfo->GetParticleRange();

for (PHG4TruthInfoContainer::ConstIterator iter = range.first; iter != range.second; ++iter)
{
PHG4TruthInfoContainer::ConstRange range = m_truthInfo->GetParticleRange();
//PHG4Particle *daughterG4 = iter->second;
daughterG4 = iter->second;

for (PHG4TruthInfoContainer::ConstIterator iter = range.first; iter != range.second; ++iter)
if (std::abs(daughterG4->get_px() - daughterTrueLV->x()) <= 5e-3 &&
std::abs(daughterG4->get_py() - daughterTrueLV->y()) <= 5e-3 &&
std::abs(daughterG4->get_pz() - daughterTrueLV->z()) <= 5e-3 && daughterG4->get_pid() == decay[i].second)
{
PHG4Particle *daughterG4 = iter->second;

if (std::abs(daughterG4->get_px() - daughterTrueLV->x()) <= 5e-3 &&
std::abs(daughterG4->get_py() - daughterTrueLV->y()) <= 5e-3 &&
std::abs(daughterG4->get_pz() - daughterTrueLV->z()) <= 5e-3 && daughterG4->get_pid() == decay[i].second)
{
truth_ID = daughterG4->get_track_id();
break;
}
//truth_ID = daughterG4->get_track_id();
break;
}
}
//}
}
else
{
PHG4TruthInfoContainer::ConstRange range = m_truthInfo->GetParticleRange();

for (PHG4TruthInfoContainer::ConstIterator iter = range.first; iter != range.second; ++iter)
{
PHG4Particle *daughterG4 = iter->second;
//PHG4Particle *daughterG4 = iter->second;
daughterG4 = iter->second;
Comment on lines +281 to +307

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Possible unguarded null dereference of m_truthInfo.

m_truthInfo is allowed to remain nullptr (only a warning is logged at line 108-115, no abort). This branch now unconditionally calls m_truthInfo->GetParticleRange() where previously (per the summary) this path was gated by if (m_dst_truth_reco_map). If a "G4TruthInfo" node is ever missing on an event, this will crash.


PHG4Particle *motherG4 = nullptr;
if (daughterG4->get_parent_id() != 0)
Expand Down Expand Up @@ -337,8 +347,8 @@ bool HFTrackEfficiency::findTracks(PHCompositeNode *topNode, Decay decay)
m_secondary_vtx_y = thisVtx->get_y();
m_secondary_vtx_z = thisVtx->get_z();

m_true_track_PID[index] = daughterG4->get_pid();
truth_ID = daughterG4->get_track_id();
m_true_track_PID[index] = daughterG4->get_pid();
//truth_ID = daughterG4->get_track_id();

delete mother3Vector;
}
Expand Down Expand Up @@ -420,20 +430,21 @@ bool HFTrackEfficiency::findTracks(PHCompositeNode *topNode, Decay decay)
m_min_true_track_pT = std::min(m_true_track_pT[index], m_min_true_track_pT);
m_max_true_track_pT = std::max(m_true_track_pT[index], m_max_true_track_pT);

if (m_dst_truth_reco_map && truth_ID >= 0)
//if (m_dst_truth_reco_map && truth_ID >= 0)
if (trackeval && daughterG4)
{
std::map<float, std::set<unsigned int>> reco_set = m_dst_truth_reco_map->get(truth_ID);
if (reco_set.empty())
{
continue;
}
const auto &best_weight = reco_set.rbegin();
if (best_weight->second.empty())
{
continue;
}
unsigned int best_reco_id = *best_weight->second.rbegin();
m_dst_track = m_input_trackMap->get(best_reco_id);
//std::map<float, std::set<unsigned int>> reco_set = m_dst_truth_reco_map->get(truth_ID);
//if (reco_set.empty())
//{
// continue;
//}
//const auto &best_weight = reco_set.rbegin();
//if (best_weight->second.empty())
//{
// continue;
//}
//unsigned int best_reco_id = *best_weight->second.rbegin();
m_dst_track = trackeval->best_track_from(daughterG4);//m_input_trackMap->get(best_reco_id);
if (m_dst_track)
{
m_used_truth_reco_map[index] = true;
Expand Down
6 changes: 5 additions & 1 deletion offline/packages/HFTrackEfficiency/HFTrackEfficiency.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#define HFTRACKEFFICIENCY_H

#include <fun4all/SubsysReco.h>
#include <g4eval/SvtxEvalStack.h>
#include <g4eval/SvtxTrackEval.h>

#include <limits>
#include <string>
Expand Down Expand Up @@ -54,7 +56,9 @@ class HFTrackEfficiency : public SubsysReco
PHHepMCGenEventMap *m_geneventmap{nullptr};
PHHepMCGenEvent *m_genevt{nullptr};

PHG4ParticleSvtxMap *m_dst_truth_reco_map{nullptr};
SvtxEvalStack *m_svtx_evalstack{nullptr};
SvtxTrackEval *trackeval{nullptr};
//PHG4ParticleSvtxMap *m_dst_truth_reco_map{nullptr};
Comment on lines +59 to +61

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the target header and implementation first.
git ls-files 'offline/packages/HFTrackEfficiency/*' | sed -n '1,20p'

echo
echo '--- header outline ---'
ast-grep outline offline/packages/HFTrackEfficiency/HFTrackEfficiency.h --view expanded

echo
echo '--- implementation outline ---'
ast-grep outline offline/packages/HFTrackEfficiency/HFTrackEfficiency.cc --view expanded

echo
echo '--- relevant header lines ---'
sed -n '1,160p' offline/packages/HFTrackEfficiency/HFTrackEfficiency.h | nl -ba | sed -n '1,140p'

echo
echo '--- relevant implementation lines ---'
sed -n '1,260p' offline/packages/HFTrackEfficiency/HFTrackEfficiency.cc | nl -ba | sed -n '1,240p'

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 691

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check for any destructor / cleanup / ownership-related code paths.
rg -n "HFTrackEfficiency::~HFTrackEfficiency|delete m_svtx_evalstack|m_svtx_evalstack|unique_ptr|SvtxEvalStack" offline/packages/HFTrackEfficiency

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 774

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- header excerpt ---'
sed -n '1,140p' offline/packages/HFTrackEfficiency/HFTrackEfficiency.h | cat -n

echo
echo '--- implementation excerpt ---'
sed -n '100,140p' offline/packages/HFTrackEfficiency/HFTrackEfficiency.cc | cat -n

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 7731

Replace m_svtx_evalstack with RAII ownership

m_svtx_evalstack is allocated with new in process_event, and HFTrackEfficiency only has a default destructor, so the stack is never released. Please switch this member to std::unique_ptr<SvtxEvalStack> (and include <memory>) to make ownership explicit and avoid a job-lifetime leak.

Source: Path instructions


DecayFinderContainerBase *m_decayMap{nullptr};
std::string m_df_module_name;
Expand Down
3 changes: 2 additions & 1 deletion offline/packages/HFTrackEfficiency/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ libhftrackefficiency_la_LIBADD = \
-ldecayfinder_io \
-ltrackbase_historic_io \
-lphg4hit \
-lphhepmc
-lphhepmc \
-lg4eval

################################################
# linking tests
Expand Down
Loading