cta-observatory / ctapipe

Low-level data processing pipeline software for CTAO or similar arrays of Imaging Atmospheric Cherenkov Telescopes
https://ctapipe.readthedocs.org
BSD 3-Clause "New" or "Revised" License
64 stars 268 forks source link

Fix peak_time estimation in FlashCamExtractor #2333

Closed clara-escanuela closed 1 year ago

clara-escanuela commented 1 year ago

After the extractor optimization, I did not check timing performance again as I only made a few changes in the code. But one of these very small changes caused a problem in the time reconstruction. The unit tests did not identify it because they were not sensitive enough. By adding a new toymodel with a time gradient we can find timing bugs better. I checked the test with the previous implementation (with that wrong line of code) and the test fails (as expected). In fact, the same test with the standard toymodel provided (random true times) passes even when time reconstruction is wrong. I think a further test can also be added to other extractors, to avoid similar problems.

maxnoe commented 1 year ago

Before: flashcam_image png

After: flashcam_image_fixed

maxnoe commented 1 year ago

@Tobychev @kosack

Could we get a second quick review here? We need to have this release to restart processing on the grid