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
62 stars 265 forks source link

Fix StereoTrigger non-deterministically discarding LST-1 in prod6 files #2552

Closed maxnoe closed 1 month ago

maxnoe commented 2 months ago

This was one of the weirder bugs I have ever tracked down. The symptom was that in consecutive runs of ctapipe-process on the same input file, sometimes LST-1 was missing from the output completely.

Tracking it down to the SoftwareTrigger class, which used a mixture of using TelescopeDescription and its string representation for storing and looking up things.

The dict _ids_by_type was the culprit, its content depended on the hash of str(TelescopeDescription), which was random, since hash of strings are random for security reasons in python.

ctao-dpps-sonarqube[bot] commented 2 months ago

Failed

Analysis Details

1 Issue

Coverage and Duplications

Project ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs

View in SonarQube

Tobychev commented 2 months ago

Can you think of a test that could catch this problem?

Telescopes being silently dropped from the trigger is quite serious, and worrying it was not caught for two years.

maxnoe commented 2 months ago

This condition only happens with the new configuration of Prod 6, before all 4 LSTs were identical.

We haven't really worked on anything Prod 6 yet, it's not even finished simulating.

maxnoe commented 1 month ago

I'd prefer to fixing this the way I do it here as the other change would require us to do a 0.22 and adapt configuration files.

With the fix at it is now, we can just publish an 0.21.1 without adapting configuration files.

I will add a test that tests for the bug.

ctao-dpps-sonarqube[bot] commented 1 month ago

Passed

Analysis Details

1 Issue

Coverage and Duplications

Project ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs

View in SonarQube

maxnoe commented 1 month ago

Rebased and changelog added, please approve again @Tobychev

ctao-dpps-sonarqube[bot] commented 1 month ago

Passed

Analysis Details

1 Issue

Coverage and Duplications

Project ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs

View in SonarQube