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

[maxnoe]

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.

[kosack]

Why not just use the telescope type to find all LSTs? That is what it was designed for, as it would be "LST" for any variant of LST, even if the camera or the optics version changes. e.g.:

lst_types = [ttype for ttype in subarray.telescope_types if ttype.type=='LST']
ids = subarray.get_tel_ids(lst_types)

The tel_str should change if the version of the optics is upgraded or the camera version changes, but it's still an LST. So in the future of we have upgrades or design changes that create LST_Gen2_LSTCam or LST_LST_LSTCamV2 they would still both have type=LST, but the telescope string would reflect the changed version,

[maxnoe]

At lest in our current definition of the type, that would result in issues with prod6 because the two MAGIC telescopes (which are right between MSTs and LSTs size wise) are also type == "LST":

LST_MAGIC1_MAGIC1

and the configuration system uses lookups by the full telescope description string, which are matched with the glob expression

[kosack]

maybe just a config variable that accept a list of telescopes would be the most robust then, even if it's more work and has to be specified for each Prod. That would fully decouple the reliance on knowing the telescope type and which ones are compatible with each other.

E.g.

SoftwareTrigger:
      participating_tel_ids = [1,2,3,4]   

That's also more flexible for future trigger configurations, where e.g. one of the LSTs is not connected since it is being commissioned.

[maxnoe]

I agree just specifying IDs that belong to sich a trigger group is most straight forward, but for the case you mentioned I think you'd just exclude it from allowed_telescopes in the Subarray selection, right?

[kosack]

The only case I can think of is that during commissioning of a new telescope, when multiple telescopes are already on-site, we may take some data where that telescope is in mono-trigger-mode (i.e. not connected to the hardware trigger), but where we might still want to do stereo analysis or use the SWAT. In that case, you'd for example have tel_id [1,2,3] in the hardware trigger sim, but not 4, even though 4 is also an LST. I think that could still be configured with TelescopeParameters though, using the 'id' method instead of 'type'

[Tobychev]

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]

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]

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.

[maxnoe]

Rebased and changelog added, please approve again @Tobychev

[ctao-dpps-sonarqube]

Analysis Details

1 Issue

• 0 Bugs
• 0 Vulnerabilities
• 1 Code Smell

Coverage and Duplications

• 100.00% Coverage (94.20% Estimated after merge)
• 0.00% Duplicated Code (0.60% Estimated after merge)

Project ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs

View in SonarQube