cta-observatory / cta-lstchain

LST prototype testbench chain
https://cta-observatory.github.io/cta-lstchain/
BSD 3-Clause "New" or "Revised" License
26 stars 77 forks source link

NSB tunning : pre-generated NSB only waveforms and a new class #1274

Closed gabemery closed 3 months ago

gabemery commented 4 months ago

In order to implement the possibility to pre-generate nsb waveforms to add nsb to events instead of generating new nsb waveforms on the fly, a new class was added.

The class WaveformNsbTunner take a new config file entry pre_computed_multiplicity which can be set to 0 to generate nsb on the fly. Or to any int to pre-generate pre_computed_multiplicity X number of pixels X number of gains waveforms.

Closes #1273

To do before removing draft :

Some processing duration to illustrate the gain on a single (proton) MC file : +50% NSB, no pre-generation : 1min 39s +50% NSB, 10X pre-generation : 1min 28s +1000% NSB, no pre-generation : 3min 18s +1000% NSB, 10X pre-generation : 1min 31s

Results of the tuning are compatible between using pre-generated waveforms or on the fly generation. Results are also the same as the previous implementation. See the signal less pixel charges for a data runs vs MC without and with tuning. image

Unrelated to this PR , but the injection of 55% nominal NSB here is a bit higher than automatically extracted using calculate_required_additional_nsb (~40%). This will need to be investigated independently.

vuillaut commented 4 months ago

Hi Grabiel, all,

Won't this negatively affect image-based methods that do not use cleaning?

gabemery commented 4 months ago

Hi @vuillaut. You mean that using correlated nsb injection could lead to issues? Or are you speaking of something else?

vuillaut commented 4 months ago

Hi @vuillaut. You mean that using correlated nsb injection could lead to issues? Or are you speaking of something else?

Hi. Yes exactly. I understand that after cleaning, most of the injected nsb will anyway be removed so re-using nsb waveforms should not be a problem but I am afraid it won't be the case for methods using uncleaned images.

gabemery commented 4 months ago

It also concerned me, but with the current implementation I think it should not be a massive issue because:

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.44%. Comparing base (ef91c64) to head (15e9093).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1274 +/- ## ========================================== + Coverage 73.35% 73.44% +0.08% ========================================== Files 134 134 Lines 14117 14161 +44 ========================================== + Hits 10356 10400 +44 Misses 3761 3761 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.