SSDDigitizer: reuse TF1, hoist per-hit lookups, cleanup#305
Draft
gavinsdavies wants to merge 1 commit into
Draft
SSDDigitizer: reuse TF1, hoist per-hit lookups, cleanup#305gavinsdavies wants to merge 1 commit into
gavinsdavies wants to merge 1 commit into
Conversation
Also remove 'using namespace emph;', drop commented-out debug/legacy blocks, and fix tab/space indentation in the touched regions (review feedback).
4 tasks
Collaborator
|
✔️ CI build for EMPHATIC Succeeded on slf7 for maxopt -- details available through the CI dashboard |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #303.
Fixes (all in
Digitization/SSDDigitizer_module.cc)TF1("func","gaus",...)was heap-allocated per hit inSimulateChargeSharing(and deleted at the end of the branch). Now created once inbeginJoband reused viaSetRange/SetParameters— removes per-hit allocation and gROOT name-registration churn.TF1::Integralis stateless given range+parameters, so results are identical. (Note: issue Broad-sweep code review: bugs, performance, and modernization findings (June 2026) #303 originally called this a leak; the original code diddelete func— the cost was churn, not a leak.)art::ServiceHandle<ChannelMapService>was constructed inside per-hit branches (twice); now fetched once at the top of the function. Redundant inner re-declarations ofechan/dchanremoved (their fields are fully re-set before each use).adcRange(): theallowedADCtable was astd::vectorrebuilt on every call (per hit); now astatic const std::array<float, 8>.Cleanup (review feedback)
Removed
using namespace emph;(everything lives insidenamespace emph;DEFINE_ART_MODULEis fully qualified), dropped the commented-out legacyadcRangeand debugstd::coutblocks, and fixed tab/space indentation in the touched regions.Verification
No numerical logic, thresholds, or outputs changed. No local build environment and no repo CI — a collaborator build before undrafting would be appreciated.
🤖 Assisted by Claude Code (claude-fable-5)