Skip to content

fix: avoid copying map entries in digi cell_hit_map iteration#2428

Open
Copilot wants to merge 6 commits into
mainfrom
copilot/fix-cell-hit-map-iteration
Open

fix: avoid copying map entries in digi cell_hit_map iteration#2428
Copilot wants to merge 6 commits into
mainfrom
copilot/fix-cell-hit-map-iteration

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 6, 2026

Iteration over cell_hit_map was copying each std::pair<uint64_t, MutableRawTrackerHit> unnecessarily.

Changes:

  • SiliconTrackerDigi.cc: iterate by const& with structured bindings
  • MPGDTrackerDigi.cc: iterate by const& with structured bindings

Before:

for (auto item : cell_hit_map) {
  raw_hits->push_back(item.second);
  if (item.first == sim_hit.getCellID()) {
    // ...
  }
}

After:

for (const auto& [cell_id, hit] : cell_hit_map) {
  raw_hits->push_back(hit);
  if (cell_id == sim_hit.getCellID()) {
    // ...
  }
}
Original prompt

This section details on the original issue you should resolve

<issue_title>cell_hit_map in MGPD and SiTracker digi loops with copy, not ref</issue_title>
<issue_description>for (auto item : cell_hit_map) copies each entry (including the MutableRawTrackerHit) unnecessarily. Iterating by const& (ideally with structured bindings) avoids extra copies and makes the intent clearer.

Originally posted by @copilot in #2410 (comment)
</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Fix cell_hit_map iteration to use const references digi: avoid copying map entries in cell_hit_map iteration Feb 6, 2026
Copilot AI requested a review from wdconinc February 6, 2026 04:50
@wdconinc wdconinc added the pre-commit.ci autofix Enable pre-commit.ci autofixes even for bot accounts label Feb 6, 2026
@pre-commit-ci pre-commit-ci Bot removed the pre-commit.ci autofix Enable pre-commit.ci autofixes even for bot accounts label Feb 6, 2026
wdconinc

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI requested a review from wdconinc February 6, 2026 04:56
@wdconinc wdconinc marked this pull request as ready for review February 6, 2026 04:59
Copilot AI review requested due to automatic review settings February 6, 2026 04:59
@wdconinc wdconinc enabled auto-merge February 6, 2026 04:59
@wdconinc wdconinc changed the title digi: avoid copying map entries in cell_hit_map iteration fix: avoid copying map entries in digi cell_hit_map iteration Feb 6, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 aims to improve performance by avoiding unnecessary copying of map entries during iteration in tracker digitization algorithms. The changes replace value-based iteration (auto item) with const reference iteration using structured bindings (const auto& [cell_id, raw_hit]) for the cell_hit_map loops.

Changes:

  • Modified iteration pattern to use const references with structured bindings instead of copying map entries
  • Updated variable references from item.first and item.second to clearer cell_id and raw_hit names

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 Changed cell_hit_map iteration from copying to const reference with structured bindings
src/algorithms/digi/MPGDTrackerDigi.cc Changed cell_hit_map iteration from copying to const reference with structured bindings

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

Comment thread src/algorithms/digi/SiliconTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
wdconinc pushed a commit that referenced this pull request Feb 6, 2026
…#2429)

This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/21739227632.
Please merge this PR into the branch
`copilot/fix-cell-hit-map-iteration`
to resolve failures in PR #2428.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
wdconinc pushed a commit that referenced this pull request Feb 6, 2026
…#2429)

This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/21739227632.
Please merge this PR into the branch
`copilot/fix-cell-hit-map-iteration`
to resolve failures in PR #2428.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wdconinc wdconinc force-pushed the copilot/fix-cell-hit-map-iteration branch from 92e3413 to edd630b Compare February 6, 2026 20:15
Copilot AI and others added 3 commits February 14, 2026 17:38
…ndings

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
epic-capybara and others added 2 commits February 14, 2026 17:40
…#2429)

This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/21739227632.
Please merge this PR into the branch
`copilot/fix-cell-hit-map-iteration`
to resolve failures in PR #2428.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wdconinc wdconinc force-pushed the copilot/fix-cell-hit-map-iteration branch from e08ba27 to d8bb9c1 Compare February 14, 2026 23:40
@wdconinc wdconinc requested review from a team and rahmans1 and removed request for a team February 14, 2026 23:41
Comment thread src/algorithms/digi/SiliconTrackerDigi.cc Outdated
Comment thread src/algorithms/digi/MPGDTrackerDigi.cc Outdated
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.

cell_hit_map in MGPD and SiTracker digi loops with copy, not ref

4 participants