Skip to content

fix: stabilize tracker digi raw hit ordering - #2805

Open
wdconinc wants to merge 3 commits into
mainfrom
wdconinc-fix-digi-determinism
Open

fix: stabilize tracker digi raw hit ordering#2805
wdconinc wants to merge 3 commits into
mainfrom
wdconinc-fix-digi-determinism

Conversation

@wdconinc

Copy link
Copy Markdown
Contributor

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

This PR makes raw-hit emission deterministic in SiliconTrackerDigi and MPGDTrackerDigi by preserving the existing unordered_map-based hit aggregation and sorting the final output cell IDs in ascending order before emitting raw hits, links, and associations.

This addresses one specific MT reproducibility issue in tracking digitization, but it is not sufficient by itself to resolve all determinism issues in the broader CKF unfiltered path. Additional nondeterministic ordering or aggregation points may still exist elsewhere in the chain.

Computationally, the change adds a sort over the unique output cell IDs after aggregation. That changes the final emission step from hash-iteration order to O(N log N) ordering over the number of unique cells, while leaving the main accumulation logic unchanged. Expected impact is small because the extra work is limited to the already-reduced set of unique output channels per event.

What is the urgency of this PR?

  • High (please describe reason below)
  • Medium
  • Low

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • 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.

AI was used to inspect the two digi algorithms, implement the minimal deterministic-ordering fix, run a targeted build/test in the EIC container environment, and draft this PR description.

Copilot AI review requested due to automatic review settings July 29, 2026 17:46
wdconinc and others added 3 commits July 29, 2026 12:49
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

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.

Pull request overview

This PR addresses a multi-thread reproducibility issue in tracker digitization by making raw-hit emission deterministic in the SiliconTrackerDigi and MPGDTrackerDigi algorithms. It keeps the existing unordered_map aggregation behavior but stabilizes the final emission ordering by sorting the aggregated output cell IDs before creating RawTrackerHits and their corresponding links/associations.

Changes:

  • Collect aggregated output cell IDs into a vector and std::ranges::sort them before emitting raw hits in SiliconTrackerDigi.
  • Apply the same sorted-emission approach to each cell_hit_map in MPGDTrackerDigi, ensuring deterministic raw hit emission order.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/algorithms/digi/SiliconTrackerDigi.cc Sorts aggregated cell IDs before emitting raw hits and building link/association collections.
src/algorithms/digi/MPGDTrackerDigi.cc Sorts aggregated strip/cell IDs before emitting raw hits and building link/association collections.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/algorithms/digi/SiliconTrackerDigi.cc
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants