Skip to content

Commit

Permalink
Merge pull request #2552 from cta-observatory/fix_stereo_trigger_prod6
Browse files Browse the repository at this point in the history
Fix StereoTrigger non-deterministically discarding LST-1 in prod6 files
  • Loading branch information
maxnoe authored May 14, 2024
2 parents 3132584 + 358955c commit 32fc311
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 10 deletions.
6 changes: 6 additions & 0 deletions docs/changes/2552.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Fix ``SoftwareTrigger`` not correctly handling different telescope
types that have the same string representation, e.g. the four LSTs
in prod6 files.

Telescopes that have the same string representation now always are treated
as one group in ``SoftwareTrigger``.
67 changes: 67 additions & 0 deletions src/ctapipe/instrument/tests/test_trigger.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import json
from copy import deepcopy

import numpy as np
import pytest
from numpy.testing import assert_equal

from ctapipe.containers import ArrayEventContainer
from ctapipe.instrument import SubarrayDescription
from ctapipe.io import EventSource


Expand Down Expand Up @@ -235,3 +237,68 @@ def test_software_trigger_simtel_process(tmp_path):
)

assert len(events_no_trigger) > len(events_trigger)


def test_different_telescope_type_same_string(subarray_prod5_paranal):
"""
Test that for a subarray that contains two different telescope types
with the same string representation, the subarray trigger is working
as expected, treating these telescopes as a single group.
"""
from ctapipe.instrument.trigger import SoftwareTrigger

lst234 = deepcopy(subarray_prod5_paranal.tel[2])
# make LST-1 slightly different from LSTs 2-4
lst1 = deepcopy(subarray_prod5_paranal.tel[1])
lst1.optics.mirror_area *= 0.9

assert lst1 != lst234

subarray = SubarrayDescription(
name="test",
tel_positions={
tel_id: subarray_prod5_paranal.positions[tel_id] for tel_id in (1, 2, 3, 4)
},
tel_descriptions={
1: lst1,
2: lst234,
3: lst234,
4: lst234,
},
reference_location=subarray_prod5_paranal.reference_location,
)

trigger = SoftwareTrigger(
subarray=subarray,
min_telescopes=2,
min_telescopes_of_type=[
("type", "*", 0),
("type", "LST*", 2),
],
)

# all four LSTs, nothing to change
event = ArrayEventContainer()
event.trigger.tels_with_trigger = [1, 2, 3, 4]
assert trigger(event)
assert_equal(event.trigger.tels_with_trigger, [1, 2, 3, 4])

# two LSTs, nothing to change
event = ArrayEventContainer()
event.trigger.tels_with_trigger = [1, 2]
assert trigger(event)
assert_equal(event.trigger.tels_with_trigger, [1, 2])

# two LSTs, nothing to change
event = ArrayEventContainer()
event.trigger.tels_with_trigger = [1, 3]
assert trigger(event)
assert_equal(event.trigger.tels_with_trigger, [1, 3])

# one LST, remove
event = ArrayEventContainer()
event.trigger.tels_with_trigger = [
1,
]
assert not trigger(event)
assert_equal(event.trigger.tels_with_trigger, [])
22 changes: 13 additions & 9 deletions src/ctapipe/instrument/trigger.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,16 @@ class SoftwareTrigger(TelescopeComponent):
def __init__(self, subarray, *args, **kwargs):
super().__init__(subarray, *args, **kwargs)

self._ids_by_type = {
str(type): set(self.subarray.get_tel_ids_for_type(type))
for type in self.subarray.telescope_types
}
# we are grouping telescopes by the str repr of the type
# this is needed since e.g. in prod6, LST-1 is slightly different
# from LST-2 to LST-4, but we still want the trigger to work with all
# LSTs
self._ids_by_type = {}
for tel in self.subarray.telescope_types:
tel_str = str(tel)
if tel_str not in self._ids_by_type:
self._ids_by_type[tel_str] = set()
self._ids_by_type[tel_str].update(self.subarray.get_tel_ids_for_type(tel))

def __call__(self, event: ArrayEventContainer) -> bool:
"""
Expand All @@ -79,24 +85,22 @@ def __call__(self, event: ArrayEventContainer) -> bool:
"""

tels_removed = set()
for tel_type in self.subarray.telescope_types:
tel_type_str = str(tel_type)
min_tels = self.min_telescopes_of_type.tel[tel_type_str]
for tel_type, tel_ids in self._ids_by_type.items():
min_tels = self.min_telescopes_of_type.tel[tel_type]

# no need to check telescopes for which we have no min requirement
if min_tels == 0:
continue

tels_with_trigger = set(event.trigger.tels_with_trigger)
tel_ids = self._ids_by_type[tel_type_str]
tels_in_event = tels_with_trigger.intersection(tel_ids)

if len(tels_in_event) < min_tels:
for tel_id in tels_in_event:
self.log.debug(
"Removing tel_id %d of type %s from event due to type requirement",
tel_id,
tel_type_str,
tel_type,
)

# remove from tels_with_trigger
Expand Down
1 change: 0 additions & 1 deletion src/ctapipe/tools/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ def start(self):
disable=not self.progress_bar,
):
self.log.debug("Processing event_id=%s", event.index.event_id)

if not self.event_type_filter(event):
continue

Expand Down

0 comments on commit 32fc311

Please sign in to comment.