Skip to content

Call G4 user tracking actions when offloading#2363

Closed
rahmans1 wants to merge 21 commits into
celeritas-project:developfrom
rahmans1:pr1-tracking-callbacks
Closed

Call G4 user tracking actions when offloading#2363
rahmans1 wants to merge 21 commits into
celeritas-project:developfrom
rahmans1:pr1-tracking-callbacks

Conversation

@rahmans1

@rahmans1 rahmans1 commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Title

Fire Pre/PostUserTrackingAction for offloaded primaries in Flush

Depends on: #2367 and #2368

Pre-requisite for: #2370

Description

  • Call copy_deaths() in both HitProcessor::operator() overloads; expose last_deaths() accessor
  • Add apply_death_state() helper to convert TrackDeathRecord (native units) to Geant4 CLHEP units on a G4Track
  • Fire PreUserTrackingAction (via view_initial) and PostUserTrackingAction (via apply_death_state) back-to-back in Flush() for each
    generator-level primary
  • Add flushing_tracking_actions_ guard in Push() to prevent re-offloading during Flush callbacks
  • Simplify event_manager_ initialization in Flush() to eager unconditional pattern
  • Update TrackingManager::HandOverOneTrack comment documenting deferred Pre/Post

Assisted by: Claude-Opus-4-6 and Claude-Sonnet-4-6

@rahmans1 rahmans1 requested a review from sethrj April 15, 2026 13:53
@rahmans1 rahmans1 added core Software engineering infrastructure (corecel) ai-assisted Generated/refactored substantially with agentic/LLM AI tools labels Apr 15, 2026
@github-actions

github-actions Bot commented Apr 15, 2026

Copy link
Copy Markdown

Test summary

 6 020 files   9 757 suites   18m 53s ⏱️
 2 289 tests  2 246 ✅  43 💤 0 ❌
34 212 runs  34 025 ✅ 187 💤 0 ❌

Results for commit 5f47e30.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Apr 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.02454% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.27%. Comparing base (4eea2c0) to head (5f47e30).

Files with missing lines Patch % Lines
src/celeritas/user/StepData.hh 80.76% 5 Missing ⚠️
src/celeritas/ext/detail/HitProcessor.cc 50.00% 3 Missing ⚠️
src/celeritas/user/DetectorSteps.hh 0.00% 3 Missing ⚠️
src/accel/LocalTransporter.cc 93.10% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop    #2363    +/-   ##
=========================================
  Coverage    87.26%   87.27%            
=========================================
  Files         1386     1386            
  Lines        43985    44116   +131     
  Branches     13483    13514    +31     
=========================================
+ Hits         38385    38501   +116     
- Misses        4374     4385    +11     
- Partials      1226     1230     +4     
Files with missing lines Coverage Δ
src/accel/LocalTransporter.hh 95.45% <100.00%> (+0.21%) ⬆️
src/accel/TrackingManager.cc 100.00% <ø> (ø)
src/accel/TrackingManager.hh 80.00% <100.00%> (+13.33%) ⬆️
src/celeritas/ext/GeantSd.cc 85.43% <100.00%> (+0.28%) ⬆️
src/celeritas/ext/GeantTrackReconstruction.cc 96.29% <100.00%> (+1.55%) ⬆️
src/celeritas/ext/GeantTrackReconstruction.hh 100.00% <100.00%> (ø)
src/celeritas/ext/detail/HitProcessor.hh 100.00% <100.00%> (ø)
src/celeritas/user/DetectorSteps.cc 100.00% <100.00%> (+1.58%) ⬆️
src/celeritas/user/StepInterface.hh 100.00% <100.00%> (ø)
src/celeritas/user/detail/StepGatherExecutor.hh 100.00% <100.00%> (ø)
... and 5 more

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sethrj sethrj marked this pull request as ready for review April 15, 2026 14:53

@sethrj sethrj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I need @drbenmorgan or @whokion or another G4 expert to review this: I'm not sure what the consequences of pre/post user tracking for a "tracking manager" particle are. It looks like Adept does this, with an extra flag, but that also requires offloading killed tracks and additional tracking... 🤔

Comment thread src/accel/TrackingManager.cc Outdated
Comment thread src/accel/TrackingManager.hh Outdated
@sethrj sethrj changed the title Tracking callbacks Call G4 user tracking actions when offloading Apr 15, 2026
@rahmans1 rahmans1 requested a review from drbenmorgan April 15, 2026 18:08
Comment thread src/accel/TrackingManager.hh Outdated
return dynamic_cast<TrackingManager const*>(
track->GetParticleDefinition()->GetTrackingManager())
!= nullptr;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please move this function and docs to the .cc file to avoid leaking the G4Track and hide the implementation. Inlining isn't going to help performance or ease-of-use in this case.

@sethrj sethrj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couple of last things. Thanks!

Comment on lines +747 to +752
RecordingTrackingAction(std::vector<int>* pre,
std::vector<int>* post,
std::mutex* mutex)
: pre_(pre), post_(post), mutex_(mutex)
{
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A helper struct to contain these data (referenced via shared pointer to be even safer!) would make the ownership/usage a little clearer.

rm.Initialize();
CELER_LOG(status) << "Run 2 events";
rm.BeamOn(2);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see how the presence of SDs affects the behavior of the test?

@rahmans1 rahmans1 force-pushed the pr1-tracking-callbacks branch from f2e681b to e96c3d9 Compare April 17, 2026 22:45
@rahmans1 rahmans1 requested a review from stognini as a code owner April 17, 2026 22:45
@rahmans1 rahmans1 marked this pull request as draft April 17, 2026 23:10
@rahmans1 rahmans1 force-pushed the pr1-tracking-callbacks branch 3 times, most recently from b21f355 to ce8a307 Compare April 18, 2026 00:40
Sakib Rahman added 5 commits April 17, 2026 20:58
Add ParticleId to AcquiredData so the Celeritas particle type is preserved
alongside track ID, parent ID, user info, and creator process. Update
acquire() to accept ParticleId. Fix view() to subtract the flush-local
start_ offset from primary_id before indexing g4_track_data_, which was
incorrect after the first clear() call when start_ advances past zero.
Add for_each_primary() template that iterates all acquired primaries and
invokes a callback with the restored G4Track.

Prompt: "Store ParticleId in AcquiredData and pass it through acquire(); fix the multi-flush bug in view() where primary_id was used as a raw index without subtracting start_; add for_each_primary() to iterate all acquired primaries with their restored G4Tracks"
Assisted-by: Claude Code (claude-sonnet-4-6)
The primary_id field was missing from operator|=, operator bool, all(), and
resize() in StepSelection/StepStateDataImpl. This caused primary_id to be
silently dropped even when selected, resulting in empty primary_id vectors
in DetectorStepOutput and incorrect trackID on reconstructed G4Tracks.

Prompt: "Fix primary_id being silently dropped from step data despite being selected, causing hits to carry trackID=0"
Assisted-by: Claude Code (claude-opus-4-6)
Add selection_.primary_id = setup.track so that primary_id is collected
alongside particle type in step data. Without this, HitProcessor receives
empty primary_id vectors and cannot restore the original G4 track ID on
reconstructed tracks before calling ProcessHits.

Prompt: "Enable primary_id in GeantSd step selection so hits carry the correct G4 track ID for MC-truth association"
Assisted-by: Claude Code (claude-opus-4-6)
Update GeantTrackReconstruction tests to pass ParticleId to acquire() and
add a multi_flush_view test verifying that view() returns the correct track
after clear() advances the start_ offset.

Prompt: "Add unit tests for the new acquire(G4Track&, ParticleId) signature and verify view() flush-local indexing across multiple clear() cycles"
Assisted-by: Claude Code (claude-opus-4-6)
The acquire() signature was updated to store the particle type in
AcquiredData alongside the G4Track, but the call site in
LocalTransporter::Push was not updated. Move the particle_id lookup
before the acquire() call so the value is available to pass.

Prompt: "acquire() call-site was missing ParticleId after signature change; move particle_id lookup before acquire() so it can be passed"
Assisted-by: Claude Code (claude-sonnet-4-6)
@rahmans1 rahmans1 force-pushed the pr1-tracking-callbacks branch 7 times, most recently from 07a50e4 to 444595c Compare April 18, 2026 22:39
Sakib Rahman added 9 commits April 18, 2026 18:57
Remove the event-scoped start_ offset and init_event() from
GeantTrackReconstruction. Primary IDs are now 0-based indices directly
into g4_track_data_, eliminating the subtract-start arithmetic in
view(), acquire(), and for_each_primary(). clear() remains at the end
of Flush() so that track data is available during hit processing and
reset before the next flush. Remove the erroneous primary_id +=
G4TrackID line in Push() that corrupted the acquire index.

Prompt: "Fix CI-failing UserActionIntegration tests caused by primary_id
indexing bugs. Remove the event-scoped start_ offset and init_event()
from GeantTrackReconstruction and replace with flush-local 0-based
direct indexing into g4_track_data_. Keep clear() at end of Flush()
so data is available during hit processing. Remove the erroneous
primary_id += G4TrackID corruption in LocalTransporter::Push(). Update
multi_flush_view test to expect flush-local id=0 after clear()."

Assisted-by: Claude Code (claude-opus-4-6)
…patch

Add TrackDeathRecord struct to DetectorStepOutput to hold the terminal GPU
state (position, direction, energy, time, primary_id, particle) of killed
tracks. Add track_death flag to StepParamsData and StepInterface::Filters,
and add corresponding death_* fields to StepStateDataImpl that are
conditionally allocated when track_death is enabled.

Also add copy_deaths() template declarations (host and device
specializations) for compacting valid death records from the scratch buffer,
a deaths vector in DetectorStepOutput for the compacted results, and a
death_valid_id field in StepStateData for GPU compaction indexing.

Prompt: "Add TrackDeathRecord struct, death_* state fields, copy_deaths declarations, deaths output vector, and death_valid_id compaction index to capture the final state of GPU-killed tracks, gated on a new track_death flag in StepParamsData and StepInterface::Filters"
Assisted-by: Claude Code (claude-opus-4-6)
…yExecutor

At the post-step gather, write position, direction, energy, time, particle,
and primary ID into death_* state fields for any track whose status is killed.
Clear death_track_id at the pre-step gather so each track produces exactly one
death record. The death gather runs before the detector-filter early returns so
tracks that die outside sensitive detectors are still captured. Add
DeathScratchCopyExecutor to compact death records from full slot arrays into
contiguous scratch storage indexed by death_valid_id.

Prompt: "In StepGatherExecutor post-step, write death_* fields for killed tracks before the detector filter so tracks dying outside SDs are captured; clear death_track_id at pre-step so each track records exactly one death. Add DeathScratchCopyExecutor to compact death records into scratch storage."
Assisted-by: Claude Code (claude-sonnet-4-6)
Verify that death record fields (track_id, primary_id, particle, pos, dir,
energy, time) are allocated when track_death is set in StepParamsData and
not allocated when it is unset.

Prompt: "Test that StepStateData death fields are conditionally allocated based on the track_death flag in StepParamsData"
Assisted-by: Claude Code (claude-opus-4-6)
Add copy_deaths<MemSpace::host> that scans death_track_id and compacts valid
entries into DetectorStepOutput::deaths. Add copy_deaths<MemSpace::device>
that uses thrust::copy_if to populate death_valid_id, launches a
gather-death-scratch kernel via DeathScratchCopyExecutor, then copies each
field device-to-host and assembles TrackDeathRecord entries. Propagate the
track_death flag from StepInterface::Filters through StepParams into
StepParamsData, and enable it unconditionally in GeantSd::filters().

Prompt: "Implement copy_deaths for host (linear scan) and device (thrust compact + kernel + D2H copy), propagate track_death from StepInterface::Filters through StepParams into StepParamsData, and set track_death=true in GeantSd::filters()"
Assisted-by: Claude Code (claude-sonnet-4-6)
Replace two identical OpaqueId validity predicates (HasDetector, HasDeath)
with a single templated IsValid<T> functor. Both performed the same
static_cast<bool>() check on different OpaqueId types.

Prompt: "Deduplicate the HasDetector and HasDeath functors in
DetectorSteps.cu that both check OpaqueId validity with static_cast<bool>.
Replace them with a single generic IsValid<T> template predicate."

Assisted-by: Claude Code (claude-opus-4-6)
Replace two-pass approach (count deaths then reserve+fill) with a single
loop using push_back and aggregate initialization of TrackDeathRecord.

Prompt: "Simplify the host copy_deaths implementation in DetectorSteps.cc
from a two-pass count-then-fill loop to a single-pass push_back with
aggregate initialization of TrackDeathRecord."

Assisted-by: Claude Code (claude-opus-4-6)
Test that copy_deaths correctly compacts sparse death records (3 of 8
slots populated) into contiguous output with correct field values, and
that it clears output when death fields are not allocated.

Prompt: "Add unit tests for copy_deaths: one that populates 3 of 8 track
slots with death records and verifies compaction produces 3 correct
TrackDeathRecords, and one that verifies copy_deaths clears output when
track_death is disabled."

Assisted-by: Claude Code (claude-opus-4-6)
Disable volume_instance_ids in death_fields_allocated and copy_deaths_host
test setup since these tests set num_volume_levels=0, which triggers the
vi_depth > 0 assertion when volume_instance_ids is selected. The assertion
only fires in debug builds (CELERITAS_DEBUG=ON), which is why it passed
locally but failed in CI.

Prompt: "Fix CI failure in death record tests where this->selection()
inherits volume_instance_ids=true but num_volume_levels=0, triggering
the vi_depth > 0 assertion in StepData.hh resize. Disable
volume_instance_ids in the test-local selection since death records
do not use volume instances."

Assisted-by: Claude Code (claude-opus-4-6)
@rahmans1 rahmans1 force-pushed the pr1-tracking-callbacks branch 2 times, most recently from 2942109 to cd65c68 Compare April 18, 2026 23:48
@rahmans1 rahmans1 added the enhancement New feature or request label Apr 18, 2026
@rahmans1 rahmans1 force-pushed the pr1-tracking-callbacks branch from cd65c68 to 1604e16 Compare April 19, 2026 00:21
@rahmans1 rahmans1 marked this pull request as ready for review April 19, 2026 01:28
Sakib Rahman and others added 6 commits April 19, 2026 07:58
With multiple primaries per event, track slots can be reused across
stepper iterations within a single Flush(). The per-iteration
copy_deaths() clears and re-scans slot-indexed death records, so a
primary that dies in iteration N loses its record when the slot is
reused in iteration N+1.

Add an accumulated_deaths_ vector to HitProcessor that appends death
records from each stepper iteration. The last_deaths() accessor returns
this accumulator, and clear_deaths() resets it after consumption. Also
add the copy_deaths() call to both operator() overloads so death
gathering runs per iteration.

Prompt: "With multiple primaries per event, death records are lost when
track slots are reused across stepper iterations because copy_deaths
clears per iteration. Accumulate deaths in HitProcessor across all
iterations so every killed primary gets a death record for
PreUserTrackingAction dispatch."

Assisted-by: claude (claude-opus-4-6)
Store initial kinematic state (position, direction, energy, time) and the
G4PrimaryParticle pointer in AcquiredData at handover time. Add
restore_initial() to reconstruct the full handover state on a G4Track,
view_initial() to prepare a track for PreUserTrackingAction dispatch, and
is_generator_primary() to filter generator-level primaries from
re-offloaded secondaries.

Prompt: "Store handover-time kinematics and G4PrimaryParticle pointer so PreUserTrackingAction sees the original track state and MC-truth frameworks identify generator primaries"
Assisted-by: Claude Code (claude-opus-4-6)
Test that view_initial restores handover-time kinematics (position,
direction, energy, time) and track/parent IDs. Verify is_generator_primary
returns true for parent_id==0 and false for re-offloaded secondaries. Check
for_each_primary iterates all acquired tracks with correct restored state.

Prompt: "Add unit tests for the new GeantTrackReconstruction methods:
view_initial should restore original handover kinematics,
is_generator_primary should distinguish generator primaries from
re-offloaded secondaries by parent_id, and for_each_primary should iterate
all acquired primaries with restored track state."

Assisted-by: Claude Code (claude-opus-4-6)
This commit means that you must (please!) install pre-commit
on your development machine and run `pre-commit install --install-hooks`.
For more information, see
https://celeritas-project.github.io/celeritas/user/development/style.html#formatting

Autogenerated: https://pre-commit.ci
Add copy_deaths() calls in both HitProcessor::operator() overloads to
gather GPU terminal state alongside hit data. Expose last_deaths()
accessor on HitProcessor so LocalTransporter can iterate death records.

Add apply_death_state() helper to convert a TrackDeathRecord (Celeritas
native units) into Geant4 CLHEP units on a G4Track. In Flush(), after
hit processing, fire PreUserTrackingAction with the original handover
state (via view_initial) and PostUserTrackingAction with the GPU
terminal state (via apply_death_state) back-to-back for each
generator-level primary (parent_id == 0).

Add flushing_tracking_actions_ guard in Push() to silently ignore
re-offload attempts from user PreUserTrackingAction callbacks fired
during Flush(). Simplify event_manager_ initialization in Flush() to
an eager unconditional pattern.

Add IsTrackOffloadedToCeleritas inline helper to TrackingManager.hh
so user callbacks can distinguish offloaded tracks. Guard the include
in TrackingManagerIntegration.test.cc with G4VERSION_NUMBER >= 1100
for Geant4 < 11.0 compatibility.

Update TrackingManager::HandOverOneTrack comment to document that
Pre/Post callbacks are deferred to Flush(). Extract primary_id local
variable in HitProcessor::operator()(DetectorStepOutput, i) for
clarity. Add integration tests for Pre/Post dispatch with and without
an active sensitive detector.

Prompt: "Gather death records via copy_deaths in HitProcessor, fire Pre/PostUserTrackingAction back-to-back in Flush for generator-level primaries using view_initial and apply_death_state, add flushing_tracking_actions re-offload guard in Push, simplify event_manager init in Flush, update TrackingManager comment, add integration tests"
Assisted-by: Claude Code (claude-opus-4-6)
After consuming last_deaths() in the Pre/PostUserTrackingAction loop,
clear the accumulator so death records don't persist across flushes.

Prompt: "Clear accumulated deaths after the tracking action loop consumes
them so records from one flush don't leak into the next."

Assisted-by: claude (claude-opus-4-6)
@rahmans1 rahmans1 force-pushed the pr1-tracking-callbacks branch from 1604e16 to a49ab41 Compare April 19, 2026 18:39
The previous implementation only fired tracking action callbacks for
generator-level primaries with death records. Offloaded secondaries
(e.g. e-/gamma from hadron interactions) bypassed begin()/end()
entirely, so MC-truth frameworks never registered equivalence entries
for those tracks—causing broken particle maps at end of event.

Iterate over all acquired tracks in Flush() and fire Pre/Post for each.
A death record map provides GPU terminal state when available; tracks
without death records use their initial handover state.

Prompt: "Fire Pre/PostUserTrackingAction for all offloaded tracks, not
just generator primaries, so MC-truth frameworks get equivalence entries
for every track that bypassed normal Geant4 tracking."

Assisted-by: claude (claude-opus-4-6)
@rahmans1 rahmans1 marked this pull request as draft April 20, 2026 12:10
@rahmans1 rahmans1 closed this May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted Generated/refactored substantially with agentic/LLM AI tools core Software engineering infrastructure (corecel) enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants