From b30083730d7ca31f5df0abf6f57ec373da998b5c Mon Sep 17 00:00:00 2001 From: Jonas Hackfeld Date: Fri, 21 Mar 2025 17:12:52 +0100 Subject: [PATCH 1/3] Overwrite stereo_combiner of loaded reconstrucor - Init new reconstructor with parent=self - Not working yet with DispReconstructor bc of different arguments --- src/ctapipe/tools/apply_models.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/ctapipe/tools/apply_models.py b/src/ctapipe/tools/apply_models.py index 3caff55fed1..d66d2422b3c 100644 --- a/src/ctapipe/tools/apply_models.py +++ b/src/ctapipe/tools/apply_models.py @@ -149,7 +149,20 @@ def setup(self): self._reconstructors = [] for path in self.reconstructor_paths: - r = Reconstructor.read(path, parent=self, subarray=self.loader.subarray) + r = Reconstructor.read( + path, + parent=self, + subarray=self.loader.subarray, + ) + # DispReconstructor hat norm_cls und sign_cls - keine model_cls + r.stereo_combiner = Reconstructor.from_name( + r.__class__.__name__, + subarray=self.loader.subarray, + parent=self, + prefix=r.prefix, + model_cls=r.model_cls, + ).stereo_combiner + if self.n_jobs: r.n_jobs = self.n_jobs self._reconstructors.append(r) From 67d91fcf44db632ec5d989055c5b4425b79f1868 Mon Sep 17 00:00:00 2001 From: Jonas Hackfeld Date: Mon, 24 Mar 2025 14:11:03 +0100 Subject: [PATCH 2/3] Make StereoCombiner configurable in ApplyModels --- src/ctapipe/tools/apply_models.py | 9 +++++++-- src/ctapipe/tools/tests/test_apply_models.py | 21 +++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/ctapipe/tools/apply_models.py b/src/ctapipe/tools/apply_models.py index d66d2422b3c..2946f571c26 100644 --- a/src/ctapipe/tools/apply_models.py +++ b/src/ctapipe/tools/apply_models.py @@ -154,13 +154,18 @@ def setup(self): parent=self, subarray=self.loader.subarray, ) - # DispReconstructor hat norm_cls und sign_cls - keine model_cls + + # Init new Reconstructor with config parameters and overwrite the StereoCombiner + model_keys = ["model_cls", "norm_cls", "sign_cls"] + model_kwargs = { + key: getattr(r, key) for key in model_keys if hasattr(r, key) + } r.stereo_combiner = Reconstructor.from_name( r.__class__.__name__, subarray=self.loader.subarray, parent=self, prefix=r.prefix, - model_cls=r.model_cls, + **model_kwargs, ).stereo_combiner if self.n_jobs: diff --git a/src/ctapipe/tools/tests/test_apply_models.py b/src/ctapipe/tools/tests/test_apply_models.py index 6958a3b5812..a4f57ec8822 100644 --- a/src/ctapipe/tools/tests/test_apply_models.py +++ b/src/ctapipe/tools/tests/test_apply_models.py @@ -1,5 +1,6 @@ import numpy as np import pytest +from numpy.testing import assert_allclose from ctapipe.containers import ( EventIndexContainer, @@ -31,7 +32,7 @@ def test_apply_energy_regressor( f"--input={input_path}", f"--output={output_path}", f"--reconstructor={energy_regressor_path}", - "--StereoMeanCombiner.weights=konrad", + "--StereoMeanCombiner.weights=intensity", "--chunk-size=5", # small chunksize so we test multiple chunks for the test file ], raises=True, @@ -73,6 +74,24 @@ def test_apply_energy_regressor( assert f"{prefix}_tel_is_valid" in tel_events.colnames assert "hillas_intensity" in tel_events.colnames + event_id = 301 + valid_mask = tel_events[tel_events["event_id"] == event_id][ + f"{prefix}_tel_is_valid" + ] + event_energy = np.average( + tel_events[tel_events["event_id"] == event_id][f"{prefix}_tel_energy"][ + valid_mask + ], + weights=tel_events[tel_events["event_id"] == event_id]["hillas_intensity"][ + valid_mask + ], + ) + assert_allclose( + table[table["event_id"] == event_id][f"{prefix}_energy"].value, + event_energy, + atol=1e-7, + ) + trigger = read_table(output_path, "/dl1/event/subarray/trigger") energy = read_table(output_path, "/dl2/event/subarray/energy/ExtraTreesRegressor") From 9ae85ca9a79be29d7c0d97730044891d3a4ffbeb Mon Sep 17 00:00:00 2001 From: Jonas Hackfeld Date: Mon, 24 Mar 2025 14:31:27 +0100 Subject: [PATCH 3/3] Adding changelog --- docs/changes/2724.bugfix.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 docs/changes/2724.bugfix.rst diff --git a/docs/changes/2724.bugfix.rst b/docs/changes/2724.bugfix.rst new file mode 100644 index 00000000000..a72de9d34f6 --- /dev/null +++ b/docs/changes/2724.bugfix.rst @@ -0,0 +1,3 @@ +Make ``StereoCombiner`` configurable in ``ApplyModels``. +Loaded joblib-pickled ``Reconstructor``s are already instantiated, which +leads to a non-configurable ``StereoCombiner``.