Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error in ctapipe-process in case telescope event is missing true image #2659

Merged
merged 9 commits into from
Dec 4, 2024
10 changes: 10 additions & 0 deletions docs/changes/2659.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Fix error in ``ctapipe-process`` when in the middle of a simtel file
that has true images available, a telescope event is missing the true image.
This can happen rarely in case a telescope triggered on pure NSB or
is oversaturated to the point where the true pe didn't fit into memory constraints.

The error was due to the ``DataWriter`` trying to write a ``None`` into an
already setup table for the true images.

The ``SimTelEventSource`` will now create an invalid true image filled with ``-1``
for such events.
48 changes: 45 additions & 3 deletions src/ctapipe/io/simteleventsource.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@
"MirrorClass",
]

# default for MAX_PHOTOELECTRONS in simtel_array is 2_500_000
# so this should be a safe limit to distinguish "dim image true missing"
# from "extremely bright true image missing", see issue #2344
MISSING_IMAGE_BRIGHTNESS_LIMIT = 1_000_000

# Mapping of SimTelArray Calibration trigger types to EventType:
# from simtelarray: type Dark (0), pedestal (1), in-lid LED (2) or laser/LED (3+) data.
SIMTEL_TO_CTA_EVENT_TYPE = {
Expand Down Expand Up @@ -570,6 +575,7 @@
self.file_, kind=self.atmosphere_profile_choice
)

self._has_true_image = None
self.log.debug(f"Using gain selector {self.gain_selector}")

def __exit__(self, exc_type, exc_val, exc_tb):
Expand Down Expand Up @@ -812,7 +818,7 @@
self.log.warning(msg)
warnings.warn(msg)

def _generate_events(self):

Check failure on line 821 in src/ctapipe/io/simteleventsource.py

View check run for this annotation

CTAO-DPPS-SonarQube / ctapipe Sonarqube Results

src/ctapipe/io/simteleventsource.py#L821

Refactor this function to reduce its Cognitive Complexity from 46 to the 15 allowed.
# for events without event_id, we use negative event_ids
pseudo_event_id = 0

Expand Down Expand Up @@ -861,6 +867,20 @@
impact_distances = np.full(len(self.subarray), np.nan) * u.m

for tel_id, telescope_event in telescope_events.items():
if tel_id not in trigger.tels_with_trigger:
# skip additional telescopes that have data but did not
# participate in the subarray trigger decision for this stereo event
# see #2660 for details
self.log.warning(
"Encountered telescope event not present in"
" stereo trigger information, skipping."
" event_id = %d, tel_id = %d, tels_with_trigger: %s",
event_id,
tel_id,
trigger.tels_with_trigger,
)
continue

adc_samples = telescope_event.get("adc_samples")
if adc_samples is None:
adc_samples = telescope_event["adc_sums"][:, :, np.newaxis]
Expand All @@ -871,6 +891,30 @@
.get(tel_id - 1, {})
.get("photoelectrons", None)
)
true_image_sum = true_image_sums[
self.telescope_indices_original[tel_id]
]

if self._has_true_image is None:
self._has_true_image = true_image is not None

if self._has_true_image and true_image is None:
if true_image_sum > MISSING_IMAGE_BRIGHTNESS_LIMIT:
self.log.warning(
"Encountered extremely bright telescope event with missing true_image in"
"file that has true images: event_id = %d, tel_id = %d."
"event might be truncated, skipping telescope event",
event_id,
tel_id,
)
continue
else:
self.log.info(
"telescope event event_id = %d, tel_id = %d is missing true image",
event_id,
tel_id,
)
true_image = np.full(n_pixels, -1, dtype=np.int32)

if data.simulation is not None:
if data.simulation.shower is not None:
Expand All @@ -887,9 +931,7 @@
)

data.simulation.tel[tel_id] = SimulatedCameraContainer(
true_image_sum=true_image_sums[
self.telescope_indices_original[tel_id]
],
true_image_sum=true_image_sum,
true_image=true_image,
impact=impact_container,
)
Expand Down
4 changes: 3 additions & 1 deletion src/ctapipe/io/tableloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ def _join_telescope_events(table1, table2):
def _merge_table_same_index(table1, table2, index_keys, fallback_join_type="left"):
"""Merge two tables assuming their primary keys are identical"""
if len(table1) != len(table2):
raise ValueError("Tables must have identical length")
raise ValueError(
f"Tables must have identical length, {len(table1)} != {len(table2)}"
)

if len(table1) == 0:
return table1
Expand Down
19 changes: 10 additions & 9 deletions src/ctapipe/io/tests/test_datawriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,16 @@ def test_roundtrip(tmpdir: Path):

# make sure it is readable by the event source and matches the images

for event in EventSource(output_path):
for tel_id, dl1 in event.dl1.tel.items():
original_image = events[event.count].dl1.tel[tel_id].image
read_image = dl1.image
assert np.allclose(original_image, read_image, atol=0.1)

original_peaktime = events[event.count].dl1.tel[tel_id].peak_time
read_peaktime = dl1.peak_time
assert np.allclose(original_peaktime, read_peaktime, atol=0.01)
with EventSource(output_path) as source:
for event in source:
for tel_id, dl1 in event.dl1.tel.items():
original_image = events[event.count].dl1.tel[tel_id].image
read_image = dl1.image
assert np.allclose(original_image, read_image, atol=0.1)

original_peaktime = events[event.count].dl1.tel[tel_id].peak_time
read_peaktime = dl1.peak_time
assert np.allclose(original_peaktime, read_peaktime, atol=0.01)


def test_dl1writer_no_events(tmpdir: Path):
Expand Down
24 changes: 12 additions & 12 deletions src/ctapipe/io/tests/test_hdf5.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,19 +988,19 @@ class Container1(Container):

# But when explicitly giving the prefixes, this works and order
# should not be important
reader = HDF5TableReader(path)
generator = reader.read(
"/values",
[Container1, Container1],
prefixes=["bar", "foo"],
)
with HDF5TableReader(path) as reader:
generator = reader.read(
"/values",
[Container1, Container1],
prefixes=["bar", "foo"],
)

for value in (1, 2, 3):
c1, c2 = next(generator)
assert c1.value == 5 * value
assert c1.prefix == "bar"
assert c2.value == value
assert c2.prefix == "foo"
for value in (1, 2, 3):
c1, c2 = next(generator)
assert c1.value == 5 * value
assert c1.prefix == "bar"
assert c2.value == value
assert c2.prefix == "foo"


@pytest.mark.parametrize("input_type", (str, Path, tables.File))
Expand Down
35 changes: 35 additions & 0 deletions src/ctapipe/io/tests/test_simteleventsource.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,3 +653,38 @@ def test_provenance(provenance, prod5_gamma_simtel_path):
assert len(inputs) == 1
assert inputs[0]["url"] == str(prod5_gamma_simtel_path)
assert inputs[0]["reference_meta"] is None


def test_prod6_issues():
"""Test behavior of source on file from prod6, see issues #2344 and #2660"""
input_url = "dataset://prod6_issues.simtel.zst"

events_checked_trigger = set()
events_checked_image = set()

# events with two telescope events but only one in stereo trigger in simtel
strange_trigger_events = {
1548602: 3,
2247909: 32,
3974908: 2,
4839806: 1,
}
missing_true_images = {1664106: 32}

with SimTelEventSource(input_url) as source:
for e in source:
event_id = e.index.event_id
if event_id in strange_trigger_events:
expected_tel_id = strange_trigger_events[event_id]
np.testing.assert_equal(e.trigger.tels_with_trigger, [expected_tel_id])
assert e.trigger.tel.keys() == {expected_tel_id}
assert e.r1.tel.keys() == {expected_tel_id}
events_checked_trigger.add(event_id)

if event_id in missing_true_images:
tel_id = missing_true_images[event_id]
np.testing.assert_equal(e.simulation.tel[tel_id].true_image, -1)
events_checked_image.add(event_id)

assert strange_trigger_events.keys() == events_checked_trigger
assert missing_true_images.keys() == events_checked_image
33 changes: 33 additions & 0 deletions src/ctapipe/tools/tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -535,3 +535,36 @@ def test_on_old_file(input_url, args, tmp_path):
np.testing.assert_array_equal(
finite_reco, events["HillasReconstructor_is_valid"]
)


def test_prod6_issues(tmp_path):
"""Test behavior of source on file from prod6, see issues #2344 and #2660"""
input_url = "dataset://prod6_issues.simtel.zst"
output_path = tmp_path / "test.dl1.h5"

run_tool(
ProcessorTool(),
argv=[
f"--input={input_url}",
f"--output={output_path}",
"--write-images",
"--write-showers",
"--overwrite",
],
cwd=tmp_path,
raises=True,
)

with TableLoader(output_path) as loader:
tel_events = loader.read_telescope_events()
subarray_events = loader.read_subarray_events()

trigger_counts = np.count_nonzero(subarray_events["tels_with_trigger"], axis=0)
_, tel_event_counts = np.unique(tel_events["tel_id"], return_counts=True)

mask = trigger_counts > 0
np.testing.assert_equal(trigger_counts[mask], tel_event_counts)

images = loader.read_telescope_events([32], true_images=True)
images.add_index("event_id")
np.testing.assert_array_equal(images.loc[1664106]["true_image"], -1)
Loading