Skip to content

Fixed the nedge logic.#4320

Merged
osbornjd merged 1 commit into
sPHENIX-Collaboration:masterfrom
Ishangoel11:branch
Jun 26, 2026
Merged

Fixed the nedge logic.#4320
osbornjd merged 1 commit into
sPHENIX-Collaboration:masterfrom
Ishangoel11:branch

Conversation

@Ishangoel11

@Ishangoel11 Ishangoel11 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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)

AI-generated summaries can be wrong; please use best judgment when reviewing the change.

Motivation / context

  • Fix the edge-counter logic in the clusterizer while preserving the existing fixed-window reconstruction behavior.

Key changes

  • In TpcClusterizer.cc, FixedWindow handling in find_t_range and find_phi_range now:
    • sets the appropriate edge flags (tredge/tledge, sredge/sledge) when truncation occurs
    • increments counts.nedge only once per affected edge
    • uses edge guards (ttop_edge/ttbottom_edge, phitop_edge/phibottom_edge) to avoid double counting
  • No functional changes were reported in calc_cluster_parameter; the TrkrClusterv5/TrkrClusterv6 selection remains unchanged.

Potential risk areas

  • Reconstruction behavior changes at FixedWindow boundaries
  • Downstream code that depends on ClusterCounters edge flags or counts.nedge
  • Any implicit IO/format expectations tied to cluster counter semantics
  • Performance impact appears likely minimal, but boundary bookkeeping should be checked in validation

Possible future improvements

  • Add targeted regression tests for fixed-window boundary cases
  • Validate edge counts against representative data samples
  • Consider documenting the intended nedge/flag semantics more explicitly for future maintenance

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fe648ad9-59ca-4ddb-8113-fea2e9247dee

📥 Commits

Reviewing files that changed from the base of the PR and between d838577 and b810603.

📒 Files selected for processing (1)
  • offline/packages/tpc/TpcClusterizer.cc

📝 Walkthrough

Walkthrough

TpcClusterizer now marks which FixedWindow boundary was hit while counting truncated t and phi cluster ranges. The existing cluster-parameter selection branch is unchanged.

Changes

FixedWindow edge flag handling

Layer / File(s) Summary
t range edge flags
offline/packages/tpc/TpcClusterizer.cc
find_t_range now sets tredge or tledge on FixedWindow truncation and increments counts.nedge once per edge.
phi range edge flags
offline/packages/tpc/TpcClusterizer.cc
find_phi_range now sets sredge or sledge on FixedWindow truncation and increments counts.nedge once per edge.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sphenix-jenkins-ci

Copy link
Copy Markdown

Build & test report

Report for commit b8106036519083d59a2d75db170c9287f9d84fb2:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants