Define a "unique event ID" different from event counter #1447
Conversation
This file contains 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
Experiment frameworks expect to have more than 4 billion globally unique event identifiers, and these aren't run sequentially in a single simulation. Define a "unique event ID" as UID-like integer only for reseeding and not for tracking.
sethrj
added
bug
We may want to rename our internal "event ID" to avoid confusion with the external ones. |
Test summary 3 279 files 5 076 suites 3m 30s ⏱️ Results for commit 4342e42. |
pcanal
approved these changes
Oct 14, 2024
amandalund
reviewed
Oct 14, 2024
| @@ -199,7 +198,7 @@ void LocalTransporter::Push(G4Track const& g4track) | |||
| * back to Geant4. | |||
| */ | |||
| track.track_id = TrackId{track_counter_++}; | |||
| track.event_id = event_id_; | |||
| track.event_id = EventId{0}; | |||
There was a problem hiding this comment.
So this is just temporary, and the plan is to set the Celeritas event ID from a global counter and to reset the (single, local) sim data track counter at the start of a new event? And I guess until we do that our track IDs won't be meaningful...
There was a problem hiding this comment.
Yep, the next PR will include a function to reset these at every event. Track IDs aren't currently very meaningful anyway since they're nondeterministic.
sethrj
added a commit
to sethrj/celeritas
that referenced
this pull request
Oct 15, 2024
…oject#1447) * Use a longer integer for event reseeding Experiment frameworks expect to have more than 4 billion globally unique event identifiers, and these aren't run sequentially in a single simulation. Define a "unique event ID" as UID-like integer only for reseeding and not for tracking. * Force event ID to be zero
sethrj
added a commit
to sethrj/celeritas
that referenced
this pull request
Oct 15, 2024
…oject#1447) * Use a longer integer for event reseeding Experiment frameworks expect to have more than 4 billion globally unique event identifiers, and these aren't run sequentially in a single simulation. Define a "unique event ID" as UID-like integer only for reseeding and not for tracking. * Force event ID to be zero
sethrj
added a commit
to sethrj/celeritas
that referenced
this pull request
Oct 15, 2024
…oject#1447) * Use a longer integer for event reseeding Experiment frameworks expect to have more than 4 billion globally unique event identifiers, and these aren't run sequentially in a single simulation. Define a "unique event ID" as UID-like integer only for reseeding and not for tracking. * Force event ID to be zero
sethrj
added a commit
to sethrj/celeritas
that referenced
this pull request
Oct 15, 2024
…oject#1447) * Use a longer integer for event reseeding Experiment frameworks expect to have more than 4 billion globally unique event identifiers, and these aren't run sequentially in a single simulation. Define a "unique event ID" as UID-like integer only for reseeding and not for tracking. * Force event ID to be zero
This was referenced Oct 15, 2024
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.
ATLAS integration is currently failing because the framework is now setting the Geant4 event ID based on the global event context, whose event number is a globally unique identifier. In contrast, Celeritas expected the event number to be a counter (and a thread-local one at that). Since there are far more than 4 billion total events simulated across all runs and all simulations, and each event should have a unique random sequence, we need to define a special long integer for seeding.
Since the unique event ID (aka the event ID from geant4) doesn't necessarily correlate to the event counter, I'm forcing the EventId that we use for the primary (used for track counting) to zero. This may hurt "debuggability" but means that we can eliminate the "max events" from Geant4 integration. A follow-on PR will add support a special mode for resetting the track counter at the beginning of each event (and having a single "global" event ID) rather than using a unique event-ID-based slot.
A follow-on pull request should map our internal "event ID" (which is consecutive from zero and local to a thread) to the external "event ID" (which is some arbitrary integer used to reference an event, whether from ATLAS or a HepMC3 file etc) for user output.
See #1144 , #1233