Skip to content

Make EventSeeder run before primary generation#1617

Draft
andresailer wants to merge 1 commit into
AIDASoft:masterfrom
andresailer:seedFirst
Draft

Make EventSeeder run before primary generation#1617
andresailer wants to merge 1 commit into
AIDASoft:masterfrom
andresailer:seedFirst

Conversation

@andresailer

Copy link
Copy Markdown
Member

If I understand correctly, the callback order from GeneratePrimaries and beginOfEvent is controlled by Geant4?

So this first registers the eventSeeder to be called by the generate primary call, and then because it is registered as a call, and not "adopted" as a sequence by our GeneratePrimaries code, I change the order in which calls and sequences are called. I have no idea if that will have bad side effects... And one should expect calls to come after sequences?

Since "adopt" claims ownership of the sequence, I didn't want to pass the eventSeeder into it.

BEGINRELEASENOTES

ENDRELEASENOTES

@github-actions

Copy link
Copy Markdown

Test Results

   18 files     18 suites   6h 13m 13s ⏱️
  357 tests   357 ✅ 0 💤 0 ❌
3 143 runs  3 143 ✅ 0 💤 0 ❌

Results for commit ca9c070.

@andresailer

Copy link
Copy Markdown
Member Author

@MarkusFrankATcernch What do you think of this?

Can I instead put a sequence to be adopted into the GeneratePrimaries step?

@MarkusFrankATcernch

Copy link
Copy Markdown
Contributor

Looks quite elegant.

But why still register the callback

Geant4Action::eventAction().callAtBegin(this,&Geant4EventSeed::beginEvent);

To me it looks like this callback is called also by void Geant4EventSeed::setSeedForPrimaries(G4Event* evt)
One call should be enough. no ?

The other possibility you mention I like more:

I think GeneratePrimaries is already a sequence:

class Geant4GeneratorActionSequence : public Geant4Action {

Maybe this one also needs a callAtBegin similar to

void callAtBegin(Q* p, void (T::*f)(const G4Event*)) {
to inject entries to the front of the queue like e.g. the event action sequence.
and then:

/// Generator callback
void Geant4GeneratorActionSequence::operator()(G4Event* event) {
  if ( context()->kernel().processEvents() )  {

    m_calls_at_begin(event);

    m_actors(&Geant4GeneratorAction::operator(), event);
    m_calls(event);
    return;
  }
  throw DD4hep_Stop_Processing();
}

or alternatively allow to inject a Gent4GeneratorAction (aka the event seeder) into the front of m_actors.
Then callAtBegin becomes:

/// Add an actor responding to all callbacks. Sequence takes ownership.
void Geant4GeneratorActionSequence::callAtBegin(Geant4GeneratorAction* action) {
  if (action) {
    G4AutoLock protection_lock(&action_mutex);
    action->addRef();
    m_actors.add_front(action);
    return;
  }
  except("Attempt to add invalid actor!");
}

To me this possibility looks more natural than tweaking the event action sequence.

@andresailer

Copy link
Copy Markdown
Member Author

I only thought of this on the walk to work today and I have to check: when the generator action reads from a file, it also reads the metadate, also the run and event number, and that should be used by the event seeder, so that in that case we actually have to call it after we read an event from the file. But for the particle gun, we have to call it before the particle gun.

I think injecting this to the front of the action sequence is better, I'll give this a go. Thanks!

@andresailer

Copy link
Copy Markdown
Member Author

I think I found that we don't pick up the event or run id from the input file at all. So calling the eventSeeder first would be fine, but maybe we have to fix that we pick up the event and runID from the input file

wdconinc added a commit to eic/npsim that referenced this pull request Jul 2, 2026
### Briefly, what does this PR introduce? Please link to any relevant
presentations or discussions.
This PR adds `--random.enableEventSeed` to the npsim CI tests to ensure
we end up simulating the same events even when the internal event random
number consumption changes. Mostly aspirational until we pick up
AIDASoft/DD4hep#1617 (at least for gun)...

### What is the urgency of this PR?
- [ ] High (please describe reason below)
- [x] Medium
- [ ] Low

### What kind of change does this PR introduce?
- [x] Bug fix (issue: ensure events are reproducible)
- [ ] New feature (issue #__)
- [ ] Optimization (issue #__)
- [ ] Updated documentation
- [ ] other: __

### Please check if any of the following apply
- [ ] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Question about random seed reproducibility when detector XML is modified

2 participants