Skip to content

feat: reject edge clusters in track fit#4128

Merged
osbornjd merged 3 commits intosPHENIX-Collaboration:masterfrom
osbornjd:reject_cluster_edges
Jan 21, 2026
Merged

feat: reject edge clusters in track fit#4128
osbornjd merged 3 commits intosPHENIX-Collaboration:masterfrom
osbornjd:reject_cluster_edges

Conversation

@osbornjd
Copy link
Copy Markdown
Contributor

@osbornjd osbornjd commented Jan 20, 2026 •
edited by coderabbitai bot
Loading

Per @bogui56 , this PR rejects any cluster on the edge from the track fit and creates an API that allows you to set it from the macro level. Let's see what it does per jenkins

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work for users)
  • Requiring change in macros repository (Please provide links to the macros pull request in the last section)
  • I am a member of GitHub organization of sPHENIX Collaboration, EIC, or ECCE (contact Chris Pinkenburg to join)

What kind of change does this PR introduce? (Bug fix, feature, ...)

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

Edge Cluster Rejection in Track Fitting

Motivation / Context

  • Provide an option to exclude clusters flagged as "edge" during Acts-based track fitting to improve fit robustness/quality where edge clusters are unreliable. The behavior is configurable from the macro level so users can enable/adjust rejection without changing core code.

Key changes

  • API:
    • PHActsTrkFitter::setClusterEdgeRejection(int edge) — new public setter; stores m_cluster_edge_rejection.
    • MakeSourceLinks::set_cluster_edge_rejection(int edge) — new public setter; stores m_cluster_edge_rejection.
    • Added m_cluster_edge_rejection members (default 0) to both classes.
  • Behavior:
    • MakeSourceLinks now skips TPC clusters whose cluster->getEdge() > m_cluster_edge_rejection during source-link construction (checks at MakeSourceLinks.cc lines shown in diff: cluster->getEdge() conditions).
    • PHActsTrkFitter forwards its m_cluster_edge_rejection to MakeSourceLinks (makeSourceLinks.set_cluster_edge_rejection(...) in PHActsTrkFitter.cc).
  • Other:
    • Minor defensive/logging improvements and refactorings in MakeSourceLinks (null checks, verbosity-controlled diagnostics, surface/transform handling).
    • Small addition in InitRun to populate chi2Cuts for the outlier finder (m_outlierFinder.chi2Cuts).

Potential risk areas

  • Reconstruction behavior: Changing which clusters are used in fits will affect track efficiency, resolution, and derived physics quantities. Validation on simulation and data is required (efficiency, fake rates, residuals).
  • Default behavior / backward compatibility: Default m_cluster_edge_rejection = 0 changes behavior only if clusters have getEdge() > 0; but verify expected default across workflows.
  • Performance: Per-cluster edge checks are inexpensive, but any additional per-cluster work or altered hit counts could change timing in high-multiplicity events — measure if needed.
  • Thread-safety / concurrency: MakeSourceLinks mutates the transient transform map (replaceTransform) per cluster to apply TPC crossing/distortion corrections. If this transient map is shared between threads or reused across events without proper synchronization/cloning, there is a potential concurrency risk when used in multi-threaded fitting. Confirm transformMapTransient lifetime and per-thread semantics.
  • Scope: Current rejection is implemented for TPC clusters only (TrkrDefs::tpcId). Ensure logic does not unintentionally affect other subsystems.

Possible future improvements

  • Make edge thresholds configurable per layer/side/subsystem instead of a single global integer.
  • Extend/reuse the mechanism to apply edge-rejection for other detectors (MVTX, INTT, Micromegas) if appropriate.
  • Add unit/integration tests to assert expected source-link counts and fitter behavior for representative seeds (enable CI/Jenkins checks).
  • Add profiling if needed and optimize filtering if it becomes a bottleneck.
  • Consider richer cluster quality metrics (combined edge + signal/charge/shape) for more selective rejection.

Note on AI analysis

  • This summary was generated with the help of AI; AI-based diff summaries can be mistaken or miss subtle semantics. Please review the implementation (the exact getEdge() condition, default value, and transformMap usage) before relying on this text for critical decisions.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 20, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

Adds cluster-edge-rejection configuration and threading from PHActsTrkFitter into MakeSourceLinks, plus defensive checks, expanded logging, minor API additions (setters, member), operator<< spacing normalization, and a const-correctness change to resetTransientTransformMap.

Changes

Cohort / File(s) Summary
Track fitter — interface & init
offline/packages/trackreco/PHActsTrkFitter.h, offline/packages/trackreco/PHActsTrkFitter.cc		 Added public setter setClusterEdgeRejection(int) and private m_cluster_edge_rejection. Forwarded edge-rejection value to MakeSourceLinks in track loop. Assigned chi2Cuts to m_outlierFinder.chi2Cuts in InitRun.
Source-link implementation & header
offline/packages/trackreco/MakeSourceLinks.h, offline/packages/trackreco/MakeSourceLinks.cc		 Added set_cluster_edge_rejection(int) and private int m_cluster_edge_rejection (default 0). Applied edge-rejection logic for TPC clusters, added null-check guards and expanded verbosity diagnostics, standardized position/surface transforms and unit handling, switched some temporaries to pointers/emplace_back, reordered includes, and adjusted formatting/bracing.
Operator overloads & minor API change
offline/packages/trackreco/MakeSourceLinks.cc		 Normalized stream operator signatures to operator<< (no space). No changes to exported MakeSourceLinks method signatures except added setter.
Const-correctness change
offline/packages/trackreco/MakeSourceLinks.h		 resetTransientTransformMap(...) declaration changed to be const.
Formatting & non-functional edits
offline/packages/trackreco/MakeSourceLinks.*		 Reordered includes, indentation/spacing normalization, and minor logging-condition refactorings; no behavioral changes beyond those listed above.
✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Jan 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
offline/packages/trackreco/MakeSourceLinks.cc (1)

359-425: Filter edge clusters before _clusterMover.processTrack to prevent biasing the fitted trajectory.

The mover fits a circle and line through input cluster positions to calculate corrections for all clusters. Edge clusters included in this fit will bias these parameters, degrading corrections for good clusters. Edge filtering must occur in the first loop before global_raw is passed to the mover, not after.

Proposed fix 
   for (auto clusIter = track->begin_cluster_keys();
        clusIter != track->end_cluster_keys();
        ++clusIter)
   {
     auto key = *clusIter;
     auto cluster = clusterContainer->findCluster(key);
     if (!cluster)
     {
       ...
       continue;
     }
+
+    if (cluster->getEdge() > m_cluster_edge_rejection)
+    {
+      continue;
+    }

     /// Make a safety check for clusters that couldn't be attached
     /// to a surface
     auto surf = tGeometry->maps().getSurface(key, cluster);
     if (!surf)
     {
       continue;
     }
     ...
     global_raw.emplace_back(std::make_pair(key, global));
   }

@sphenix-jenkins-ci
Copy link
Copy Markdown

sphenix-jenkins-ci bot commented Jan 20, 2026

Build & test report

Report for commit 147009b249d97bd8d5ef7d60f25800e833763e49:
Jenkins passed

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@sphenix-jenkins-ci
Copy link
Copy Markdown

sphenix-jenkins-ci bot commented Jan 21, 2026

Build & test report

Report for commit 6ce581e35841fffaf9e131986ede27c969427ed3:
Jenkins passed

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@sphenix-jenkins-ci
Copy link
Copy Markdown

sphenix-jenkins-ci bot commented Jan 21, 2026

Build & test report

Report for commit 435537f4adb086205857a252f97ee8911af17037:
Jenkins passed

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@osbornjd osbornjd merged commit 76b38ec into sPHENIX-Collaboration:master Jan 21, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@osbornjd