Repository navigation
Avoid multi-loopers generation when using embedding - #16015
Conversation
|
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. |
|
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. |
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>
|
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 |
|
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. |
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.