BxCppDev / Bayeux

Core Persistency, Geometry and Data Processing C++ Library for Particle and Nuclear Physics Experiments
GNU General Public License v3.0
4 stars 9 forks source link

bug in time_slicer_generator.cc due to a typo #58

Closed emchauve closed 4 years ago

emchauve commented 4 years ago

correct a typo in time_slicer_generator.cc : all particles of the delayed events must have their time shifted negatively from the first delayed time and not positively !

pfranchini commented 4 years ago

Hi @fmauger, could be possible for you tag a 3.4.2 at least with this patch included. I have few users who might get great advantage from a proper 207Bi simulation and reconstruction. Thanks

fmauger commented 4 years ago

I'm working on it but there are other accumulated modifications to validate before, particularly compatibility with new versions of third party software and new decay generator system. I'll do my best. We also need to make sure Falaise is also upgraded in turn.

pfranchini commented 4 years ago

Hi Francois,

thanks for taking care of this.

In the meantime, for the Bismuth reconstruction in quite easy for a Falaise user to patch the wrong sign in the generator,

cheers,

Paolo

On 03/06/2020 21:52, François Mauger wrote:

I'm working on it but there are other accumulated modifications to validate before, particularly compatibility with new versions of third party software and new decay generator system. I'll do my best. We also need to make sure Falaise is also upgraded in turn.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/BxCppDev/Bayeux/pull/58#issuecomment-638454950, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIU7CTFNSFKINGIJWYAI52DRU2ZX5ANCNFSM4M3PIWEQ.

emchauve commented 3 years ago

Waking up this thread after realising that this fix was not enough to make work the Bi207 decay with time slicer generator in Falaise ! In fact, the primary event which is given to GEANT4 has the original time for each particle with large values which is restored by doing the sum of the event time (which contains the big time offset) to the particle time (line 666-669 of bxmctools/src/g4/primary_generator.cc) :

        double particle_time = event_time;
        if (genbb_particle.has_time()) {
          particle_time += genbb_particle.get_time();
        }

This time is then propagated into tracker and calo hit.

In fact, we should either :

In both cases, Falaise will be able to process reconstruct, because it is consider the delta time particle_time-event_time for separating prompt and delayed (alpha) signals.

pfranchini commented 3 years ago

Glad you fixed it! Thanks!

fmauger commented 2 years ago

I had a deeper look in the code (original version then the 'fixed' one) and I think the original code was ok. The purpose of this generator is to split an event in two independent events with some new time reference for the event identified as "delayed" (in the second time window). This has nothing to do with delayed alpha (from BiPo214 for example) and delayed alpha reconstruction within Falaise.

So when you propose to "restore the "typo" and do not apply the particle time offset by the next line delayed_event.shift_particles_time(-first_delayed_time)" I disagree. It is clear that the following line:

"delayed_event.set_time(working_event.get_time() - first_delayed_time);" must be restored to the original: "delayed_event.set_time(working_event.get_time() + first_delayed_time);"

But we must also shift individual particle times relatively to this new time reference: "delayed_event.shift_particles_time(-first_delayed_time)"

The result of this operation is to provide TWO independent prompt events from an original one which is supposed to contain 2 groups of primary particles packed within two distinct time windows. These two resulting events are supposed to be processed as prompt events in Falaise reconstruction software.

However I have to check another part of the library to make sure there is no mistake while shifting the particle times...