Skip to content

Avoid multi-loopers generation when using embedding - #16015

Merged
sawenzel merged 2 commits into
AliceO2Group:devfrom
jackal1-66:loopersembedding
Oct 30, 2026
Merged

sawenzel merged 2 commits into
AliceO2Group:devfrom
jackal1-66:loopersembedding

Conversation

@jackal1-66

Copy link
Copy Markdown
Collaborator

With embedding, the background and the signal simulation each cover the whole timeframe with TPC loopers, so the loopers reach the digits basically twice. A collision of the collision context is now owned by a single source, and only its owner generates its loopers. A protection has been added in the Generator class not to make generators that do not own collisions (rare case) enable the loopers flag. The applied fix has been validated against multiple embedding configurations (both with and without replacement) and with the standard simulations as well. The issue is tracked by https://its.cern.ch/jira/browse/O2-7272 and was discovered in https://its.cern.ch/jira/browse/O2-6781. A simple veto to the signal was also investigated, but the generated loopers were incorrect in the time intervals both in replace mode and when the background events were reused.

@sawenzel

Copy link
Copy Markdown
Collaborator

Thanks, this is a good proposition. Before taking a decision, could you take a look at the alternative of doing this in O2DPG: AliceO2Group/O2DPG#2524? It needs no new O2 functionality, keeps the choice in the configuration, and avoids the extra collision-context bookkeeping in the generator. I would be interested in your view on whether it covers the cases you validated.

@jackal1-66

Copy link
Copy Markdown
Collaborator Author

I checked your PR and unfortunately it fixes the issues we have only partly. It does remove the double-counting, as expected, however it reduces the number of counted loopers both in the signal replacement and in the background events reuse scenarios (I launched a few tests on this to make sure). In the replacement scenario this is clear because the signal event doesn't contain loopers, as they are only generated by the background, in the reuse one instead the background does not cover the interaction time frame correctly: the old formula for the time interval gives event i the interval between collisions i and i+1, which is right only when the transport has one event per collision, so when we reuse background only the first intervals are considered and they are replayed n times. The loopers of the background here cover a fraction of the time span, and the fraction depends on nb/ns. I would still open your PR as well, but with the modification I suggested there, as it provides a better treatment of the GenTPCLoopers parameters.

Comment thread Generators/src/TPCLoopers.cxx Resolved
Comment thread Generators/src/TPCLoopers.cxx Resolved
Comment thread Generators/src/TPCLoopers.cxx Resolved
Comment thread Generators/src/Generator.cxx Resolved
sawenzel added a commit to sawenzel/O2DPG that referenced this pull request Oct 30, 2026
This applies the reviewer's suggestion so that the workflow works together with the ownership of collisions in O2.

- The signal no longer gets GenTPCLoopers.loopersVeto=true unless loopers are switched off.
- Both transports take GenTPCLoopers.colsys from -colBkg.

AliceO2Group/AliceO2#16015

Co-authored-by: Marco Giacalone <mgiacalo@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sawenzel

Copy link
Copy Markdown
Collaborator

I agree and we should merge this PR then. Just consider the few additional points inline. If you take the first one, could you rerun your embedding tests, in particular @0:e1 with background reuse?

@jackal1-66

Copy link
Copy Markdown
Collaborator Author

Thanks, I integrated all your suggestions. I had overlooked the early loopers in the timeframe using the background as base. I relaunched everything with the last commit in (including @0:e1) and it seems fine. This can be merged together with your PR now when the CI is green.

@jackal1-66
jackal1-66 requested a review from sawenzel October 30, 2026 17:18
@sawenzel
sawenzel enabled auto-merge (rebase) October 30, 2026 21:56
@sawenzel
sawenzel merged commit 1f4567d into AliceO2Group:dev Oct 30, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants