From 7c328fa5caa15bd261777f64822e0dbcb7ef21d8 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Fri, 1 Nov 2024 16:54:52 +0100 Subject: [PATCH 01/18] Adding different method of applying calibration --- src/ctapipe/calib/camera/calibrator.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index 853ba3f7da8..b0421250612 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -13,6 +13,7 @@ from ctapipe.core import TelescopeComponent from ctapipe.core.traits import ( BoolTelescopeParameter, + CaselessStrEnum, ComponentName, TelescopeParameter, ) @@ -61,6 +62,11 @@ class CameraCalibrator(TelescopeComponent): image_extractor_type: str The name of the ImageExtractor subclass to be used for image extraction + + image_calibration_type: str + The name of the method for calibrating images. + ``charge`` for charge images, ``variance`` for + variance images. """ data_volume_reducer_type = ComponentName( @@ -73,6 +79,14 @@ class CameraCalibrator(TelescopeComponent): help="Name of the ImageExtractor subclass to be used.", ).tag(config=True) + image_calibration_type = CaselessStrEnum( + ["charge", "variance"], + default_value="charge", + help=( + "Image calibration method to be used." "Options ``charge``, ``variance``" + ), + ).tag(config=True) + invalid_pixel_handler_type = ComponentName( InvalidPixelHandler, default_value="NeighborAverage", @@ -292,13 +306,21 @@ def _calibrate_dl1(self, event, tel_id): and dl1_calib.absolute_factor is not None ): if selected_gain_channel is None: - dl1.image *= dl1_calib.relative_factor / dl1_calib.absolute_factor + if self.image_calibration_type == "charge": + dl1.image *= dl1_calib.relative_factor / dl1_calib.absolute_factor + elif self.image_calibration_type == "variance": + dl1.image *= np.sqrt( + dl1_calib.relative_factor / dl1_calib.absolute_factor + ) else: corr = ( dl1_calib.relative_factor[selected_gain_channel, pixel_index] / dl1_calib.absolute_factor[selected_gain_channel, pixel_index] ) - dl1.image *= corr + if self.image_calibration_type == "charge": + dl1.image *= corr + elif self.image_calibration_type == "variance": + dl1.image *= np.sqrt(corr) # handle invalid pixels if self.invalid_pixel_handler is not None: From ceaf91b63535e396cdae21823b3c159f379fe147 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Mon, 4 Nov 2024 10:23:43 +0100 Subject: [PATCH 02/18] Adding tests for variance calibration, fixing VarianceExtractor for use with Calibrator --- src/ctapipe/calib/camera/calibrator.py | 6 +++++- .../calib/camera/tests/test_calibrator.py | 17 +++++++++++++++++ src/ctapipe/image/extractor.py | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index b0421250612..c81db8a7aab 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -297,7 +297,11 @@ def _calibrate_dl1(self, event, tel_id): ) # correct non-integer remainder of the shift if given - if self.apply_peak_time_shift.tel[tel_id] and remaining_shift is not None: + if ( + self.apply_peak_time_shift.tel[tel_id] + and remaining_shift is not None + and self.image_calibration_type == "charge" + ): dl1.peak_time -= remaining_shift # Calibrate extracted charge diff --git a/src/ctapipe/calib/camera/tests/test_calibrator.py b/src/ctapipe/calib/camera/tests/test_calibrator.py index b5a3875f62e..e3d3b2090cd 100644 --- a/src/ctapipe/calib/camera/tests/test_calibrator.py +++ b/src/ctapipe/calib/camera/tests/test_calibrator.py @@ -16,6 +16,7 @@ GlobalPeakWindowSum, LocalPeakWindowSum, NeighborPeakWindowSum, + VarianceExtractor, ) from ctapipe.image.reducer import NullDataVolumeReducer, TailCutsDataVolumeReducer @@ -67,6 +68,7 @@ def test_config(example_subarray): "TailCutsDataVolumeReducer": { "TailcutsImageCleaner": {"picture_threshold_pe": 20.0} }, + "image_calibration_type": "charge", } } ) @@ -130,6 +132,21 @@ def test_check_dl0_empty(example_event, example_subarray): assert (event.dl1.tel[tel_id].image == 2).all() +def test_dl1_variance_calib(example_event, example_subarray): + # test the calibration of variance images + tel_id = list(example_event.r0.tel)[0] + calibrator = CameraCalibrator( + subarray=example_subarray, + image_extractor=VarianceExtractor(subarray=example_subarray), + image_calibration_type="variance", + apply_waveform_time_shift=False, + ) + calibrator(example_event) + image = example_event.dl1.tel[tel_id].image + assert image is not None + assert image.shape == (1764,) + + def test_dl1_charge_calib(example_subarray): # copy because we mutate the camera, should not affect other tests subarray = deepcopy(example_subarray) diff --git a/src/ctapipe/image/extractor.py b/src/ctapipe/image/extractor.py index a8a6a71323c..6a5b9e0f03e 100644 --- a/src/ctapipe/image/extractor.py +++ b/src/ctapipe/image/extractor.py @@ -1308,7 +1308,7 @@ def __call__( self, waveforms, tel_id, selected_gain_channel, broken_pixels ) -> DL1CameraContainer: container = DL1CameraContainer( - image=np.nanvar(waveforms, dtype="float32", axis=2), + image=np.nanvar(waveforms, dtype="float32", axis=2)[0], ) container.meta["ExtractionMethod"] = str(VarianceType.WAVEFORM) return container From dd3a59f1a83915338016fcfd6ad03ae6ac5fbaf1 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Mon, 4 Nov 2024 10:26:21 +0100 Subject: [PATCH 03/18] improving docustring --- src/ctapipe/calib/camera/calibrator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index c81db8a7aab..1fbf9fe4add 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -83,7 +83,8 @@ class CameraCalibrator(TelescopeComponent): ["charge", "variance"], default_value="charge", help=( - "Image calibration method to be used." "Options ``charge``, ``variance``" + "Image calibration method to be used." + "Options are ``charge``, ``variance``" ), ).tag(config=True) From 4a5e93ba839cb6d42d521ae669a12b4775854d94 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Mon, 4 Nov 2024 11:06:28 +0100 Subject: [PATCH 04/18] Fixing error in VarianceExtractor test --- src/ctapipe/image/tests/test_extractor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctapipe/image/tests/test_extractor.py b/src/ctapipe/image/tests/test_extractor.py index 2b7fd1a5f5e..ab3ed4da6fe 100644 --- a/src/ctapipe/image/tests/test_extractor.py +++ b/src/ctapipe/image/tests/test_extractor.py @@ -290,7 +290,7 @@ def test_variance_extractor(toymodel): extractor = ImageExtractor.from_name("VarianceExtractor", subarray=subarray) variance = extractor(var_data, 0, None, None).image - np.testing.assert_allclose(variance, np.var(var_data, axis=2), rtol=1e-3) + np.testing.assert_allclose(variance, np.var(var_data, axis=2)[0], rtol=1e-3) @pytest.mark.parametrize("toymodels", camera_toymodels) From cb1929d80196482b33e6408cc4d5d96eaadfc1f1 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Mon, 4 Nov 2024 11:25:04 +0100 Subject: [PATCH 05/18] adding changelog --- docs/changes/2636.features.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 docs/changes/2636.features.rst diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst new file mode 100644 index 00000000000..643c18805b3 --- /dev/null +++ b/docs/changes/2636.features.rst @@ -0,0 +1,4 @@ +Add a trait to the CameraCalibrator called "image_calibration_type". +This trait allows to switch the method of calibration between "charge" and "variance" +"charge" is the calibration method for the existing image extractors +"variance" is the method to calibrate variance images. From e5d6fa6b3656b5ed464ae7e3a99db0faff83aa2d Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Tue, 5 Nov 2024 10:41:07 +0100 Subject: [PATCH 06/18] Addressing reviewer comments --- docs/changes/2636.features.rst | 5 +--- src/ctapipe/calib/camera/calibrator.py | 29 +++++-------------- .../calib/camera/tests/test_calibrator.py | 7 +++-- src/ctapipe/image/extractor.py | 2 +- src/ctapipe/image/tests/test_extractor.py | 2 +- 5 files changed, 14 insertions(+), 31 deletions(-) diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst index 643c18805b3..faa441c8df8 100644 --- a/docs/changes/2636.features.rst +++ b/docs/changes/2636.features.rst @@ -1,4 +1 @@ -Add a trait to the CameraCalibrator called "image_calibration_type". -This trait allows to switch the method of calibration between "charge" and "variance" -"charge" is the calibration method for the existing image extractors -"variance" is the method to calibrate variance images. +Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly variance images generated with the VarianceExtractor diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index 1fbf9fe4add..44b7fe72f2c 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -13,7 +13,6 @@ from ctapipe.core import TelescopeComponent from ctapipe.core.traits import ( BoolTelescopeParameter, - CaselessStrEnum, ComponentName, TelescopeParameter, ) @@ -62,11 +61,6 @@ class CameraCalibrator(TelescopeComponent): image_extractor_type: str The name of the ImageExtractor subclass to be used for image extraction - - image_calibration_type: str - The name of the method for calibrating images. - ``charge`` for charge images, ``variance`` for - variance images. """ data_volume_reducer_type = ComponentName( @@ -79,15 +73,6 @@ class CameraCalibrator(TelescopeComponent): help="Name of the ImageExtractor subclass to be used.", ).tag(config=True) - image_calibration_type = CaselessStrEnum( - ["charge", "variance"], - default_value="charge", - help=( - "Image calibration method to be used." - "Options are ``charge``, ``variance``" - ), - ).tag(config=True) - invalid_pixel_handler_type = ComponentName( InvalidPixelHandler, default_value="NeighborAverage", @@ -301,7 +286,7 @@ def _calibrate_dl1(self, event, tel_id): if ( self.apply_peak_time_shift.tel[tel_id] and remaining_shift is not None - and self.image_calibration_type == "charge" + and not extractor.__class__.__name__ == "VarianceExtractor" ): dl1.peak_time -= remaining_shift @@ -311,21 +296,21 @@ def _calibrate_dl1(self, event, tel_id): and dl1_calib.absolute_factor is not None ): if selected_gain_channel is None: - if self.image_calibration_type == "charge": - dl1.image *= dl1_calib.relative_factor / dl1_calib.absolute_factor - elif self.image_calibration_type == "variance": + if extractor.__class__.__name__ == "VarianceExtractor": dl1.image *= np.sqrt( dl1_calib.relative_factor / dl1_calib.absolute_factor ) + else: + dl1.image *= dl1_calib.relative_factor / dl1_calib.absolute_factor else: corr = ( dl1_calib.relative_factor[selected_gain_channel, pixel_index] / dl1_calib.absolute_factor[selected_gain_channel, pixel_index] ) - if self.image_calibration_type == "charge": - dl1.image *= corr - elif self.image_calibration_type == "variance": + if extractor.__class__.__name__ == "VarianceExtractor": dl1.image *= np.sqrt(corr) + else: + dl1.image *= corr # handle invalid pixels if self.invalid_pixel_handler is not None: diff --git a/src/ctapipe/calib/camera/tests/test_calibrator.py b/src/ctapipe/calib/camera/tests/test_calibrator.py index e3d3b2090cd..435d833adee 100644 --- a/src/ctapipe/calib/camera/tests/test_calibrator.py +++ b/src/ctapipe/calib/camera/tests/test_calibrator.py @@ -68,7 +68,6 @@ def test_config(example_subarray): "TailCutsDataVolumeReducer": { "TailcutsImageCleaner": {"picture_threshold_pe": 20.0} }, - "image_calibration_type": "charge", } } ) @@ -138,13 +137,15 @@ def test_dl1_variance_calib(example_event, example_subarray): calibrator = CameraCalibrator( subarray=example_subarray, image_extractor=VarianceExtractor(subarray=example_subarray), - image_calibration_type="variance", apply_waveform_time_shift=False, ) calibrator(example_event) image = example_event.dl1.tel[tel_id].image assert image is not None - assert image.shape == (1764,) + assert image.shape == ( + 1, + 1764, + ) def test_dl1_charge_calib(example_subarray): diff --git a/src/ctapipe/image/extractor.py b/src/ctapipe/image/extractor.py index 6a5b9e0f03e..7dfbdc7eb14 100644 --- a/src/ctapipe/image/extractor.py +++ b/src/ctapipe/image/extractor.py @@ -1308,7 +1308,7 @@ def __call__( self, waveforms, tel_id, selected_gain_channel, broken_pixels ) -> DL1CameraContainer: container = DL1CameraContainer( - image=np.nanvar(waveforms, dtype="float32", axis=2)[0], + image=np.nanvar(waveforms, dtype="float32", keepdims=False, axis=2), ) container.meta["ExtractionMethod"] = str(VarianceType.WAVEFORM) return container diff --git a/src/ctapipe/image/tests/test_extractor.py b/src/ctapipe/image/tests/test_extractor.py index ab3ed4da6fe..2b7fd1a5f5e 100644 --- a/src/ctapipe/image/tests/test_extractor.py +++ b/src/ctapipe/image/tests/test_extractor.py @@ -290,7 +290,7 @@ def test_variance_extractor(toymodel): extractor = ImageExtractor.from_name("VarianceExtractor", subarray=subarray) variance = extractor(var_data, 0, None, None).image - np.testing.assert_allclose(variance, np.var(var_data, axis=2)[0], rtol=1e-3) + np.testing.assert_allclose(variance, np.var(var_data, axis=2), rtol=1e-3) @pytest.mark.parametrize("toymodels", camera_toymodels) From f24dfed746ed24f804959439cba6530180694a9f Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Tue, 5 Nov 2024 10:43:39 +0100 Subject: [PATCH 07/18] Implementing change suggested by max --- src/ctapipe/calib/camera/calibrator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index 44b7fe72f2c..f3f2015240c 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -286,7 +286,7 @@ def _calibrate_dl1(self, event, tel_id): if ( self.apply_peak_time_shift.tel[tel_id] and remaining_shift is not None - and not extractor.__class__.__name__ == "VarianceExtractor" + and dl1.peak_time is not None ): dl1.peak_time -= remaining_shift From 9861948bf119fa6bb517cd8cfc8a869804e291a6 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Tue, 5 Nov 2024 14:41:05 +0100 Subject: [PATCH 08/18] Fixing variance calibration and adding test with LST camera --- src/ctapipe/calib/camera/calibrator.py | 4 +- .../calib/camera/tests/test_calibrator.py | 43 +++++++++++++++++++ src/ctapipe/image/extractor.py | 2 +- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index f3f2015240c..98518179247 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -297,7 +297,7 @@ def _calibrate_dl1(self, event, tel_id): ): if selected_gain_channel is None: if extractor.__class__.__name__ == "VarianceExtractor": - dl1.image *= np.sqrt( + dl1.image *= np.square( dl1_calib.relative_factor / dl1_calib.absolute_factor ) else: @@ -308,7 +308,7 @@ def _calibrate_dl1(self, event, tel_id): / dl1_calib.absolute_factor[selected_gain_channel, pixel_index] ) if extractor.__class__.__name__ == "VarianceExtractor": - dl1.image *= np.sqrt(corr) + dl1.image *= np.square(corr) else: dl1.image *= corr diff --git a/src/ctapipe/calib/camera/tests/test_calibrator.py b/src/ctapipe/calib/camera/tests/test_calibrator.py index 435d833adee..f9c2ff0f214 100644 --- a/src/ctapipe/calib/camera/tests/test_calibrator.py +++ b/src/ctapipe/calib/camera/tests/test_calibrator.py @@ -148,6 +148,49 @@ def test_dl1_variance_calib(example_event, example_subarray): ) +def test_calib_LST_camera(example_subarray): + n_channels = 2 + n_pixels = 1855 # number of pixels in LSTcam + n_samples = 100 + + random = np.random.default_rng(1) + + tel_id = 1 + y = random.normal(0, 6, (n_channels, n_pixels, n_samples)) + + absolute = random.uniform(100, 1000, (n_channels, n_pixels)).astype("float32") + y *= absolute[..., np.newaxis] + + relative = random.normal(1, 0.01, (n_channels, n_pixels)) + y /= relative[..., np.newaxis] + + pedestal = random.uniform(-4, 4, (n_channels, n_pixels)) + y += pedestal[..., np.newaxis] + + event = ArrayEventContainer() + event.dl0.tel[tel_id].waveform = y + event.calibration.tel[tel_id].dl1.pedestal_offset = pedestal + event.calibration.tel[tel_id].dl1.absolute_factor = absolute + event.calibration.tel[tel_id].dl1.relative_factor = relative + event.dl0.tel[tel_id].selected_gain_channel = None + event.r1.tel[tel_id].selected_gain_channel = None + + calibrator = CameraCalibrator( + subarray=example_subarray, + image_extractor=VarianceExtractor(subarray=example_subarray), + apply_waveform_time_shift=False, + ) + calibrator(event) + + image = event.dl1.tel[tel_id].image + + assert image is not None + assert image.shape == ( + 2, + 1855, + ) + + def test_dl1_charge_calib(example_subarray): # copy because we mutate the camera, should not affect other tests subarray = deepcopy(example_subarray) diff --git a/src/ctapipe/image/extractor.py b/src/ctapipe/image/extractor.py index 7dfbdc7eb14..a8a6a71323c 100644 --- a/src/ctapipe/image/extractor.py +++ b/src/ctapipe/image/extractor.py @@ -1308,7 +1308,7 @@ def __call__( self, waveforms, tel_id, selected_gain_channel, broken_pixels ) -> DL1CameraContainer: container = DL1CameraContainer( - image=np.nanvar(waveforms, dtype="float32", keepdims=False, axis=2), + image=np.nanvar(waveforms, dtype="float32", axis=2), ) container.meta["ExtractionMethod"] = str(VarianceType.WAVEFORM) return container From 460857a6697eed080a5cbcb9f2281b6d4c0973b1 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Wed, 6 Nov 2024 09:36:25 +0100 Subject: [PATCH 09/18] Updating changelog --- docs/changes/2636.features.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst index faa441c8df8..8a1abe8b25c 100644 --- a/docs/changes/2636.features.rst +++ b/docs/changes/2636.features.rst @@ -1 +1,3 @@ Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly variance images generated with the VarianceExtractor +if the VarianceExtractor is used for the CameraCalibrator the element-wise square of the relative and absolute gain calibration factors are applied to the image +For other image extractors the plain factors are still applied From 13f83aa48edae27c3569235fe1a9aebaa24c837b Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Wed, 6 Nov 2024 09:37:46 +0100 Subject: [PATCH 10/18] Adding verb --- docs/changes/2636.features.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst index 8a1abe8b25c..7ba3fc74e44 100644 --- a/docs/changes/2636.features.rst +++ b/docs/changes/2636.features.rst @@ -1,3 +1,3 @@ -Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly variance images generated with the VarianceExtractor +Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly calibrate variance images generated with the VarianceExtractor if the VarianceExtractor is used for the CameraCalibrator the element-wise square of the relative and absolute gain calibration factors are applied to the image For other image extractors the plain factors are still applied From e1f4e136544233f848f3602786a2e177b52c16ce Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Wed, 6 Nov 2024 10:05:41 +0100 Subject: [PATCH 11/18] Formatting changelog --- docs/changes/2636.features.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst index 7ba3fc74e44..2ff2de2aa39 100644 --- a/docs/changes/2636.features.rst +++ b/docs/changes/2636.features.rst @@ -1,3 +1,3 @@ Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly calibrate variance images generated with the VarianceExtractor -if the VarianceExtractor is used for the CameraCalibrator the element-wise square of the relative and absolute gain calibration factors are applied to the image -For other image extractors the plain factors are still applied ++ If the VarianceExtractor is used for the CameraCalibrator the element-wise square of the relative and absolute gain calibration factors are applied to the image ++ For other image extractors the plain factors are still applied From 29b9009ecc81e0f0f0e8a36cd3ee4da56f6e72e0 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Wed, 6 Nov 2024 10:07:09 +0100 Subject: [PATCH 12/18] more formatting --- docs/changes/2636.features.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst index 2ff2de2aa39..ee0ca5ba699 100644 --- a/docs/changes/2636.features.rst +++ b/docs/changes/2636.features.rst @@ -1,3 +1,3 @@ -Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly calibrate variance images generated with the VarianceExtractor ++ Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly calibrate variance images generated with the VarianceExtractor + If the VarianceExtractor is used for the CameraCalibrator the element-wise square of the relative and absolute gain calibration factors are applied to the image + For other image extractors the plain factors are still applied From 9c7d4dc2226764000e29f19ac5445f36c42255bd Mon Sep 17 00:00:00 2001 From: "mykhailo.dalchenko" Date: Wed, 6 Nov 2024 12:33:43 +0100 Subject: [PATCH 13/18] Rename changelog entry and update formatting --- docs/changes/2636.feature.rst | 3 +++ docs/changes/2636.features.rst | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 docs/changes/2636.feature.rst delete mode 100644 docs/changes/2636.features.rst diff --git a/docs/changes/2636.feature.rst b/docs/changes/2636.feature.rst new file mode 100644 index 00000000000..bc0aa0e37d0 --- /dev/null +++ b/docs/changes/2636.feature.rst @@ -0,0 +1,3 @@ +Update ``CameraCalibrator`` in ``ctapipe.calib.camera.calibrator`` allowing it to correctly calibrate variance images generated with the ``VarianceExtractor``. + - If the VarianceExtractor is used for the ``CameraCalibrator`` the element-wise square of the relative and absolute gain calibration factors are applied to the image; + - For other image extractors the plain factors are still applied. diff --git a/docs/changes/2636.features.rst b/docs/changes/2636.features.rst deleted file mode 100644 index ee0ca5ba699..00000000000 --- a/docs/changes/2636.features.rst +++ /dev/null @@ -1,3 +0,0 @@ -+ Makes changes to the CameraCalibrator in ctapipe.calib.camera.calibrator that allows it to correctly calibrate variance images generated with the VarianceExtractor -+ If the VarianceExtractor is used for the CameraCalibrator the element-wise square of the relative and absolute gain calibration factors are applied to the image -+ For other image extractors the plain factors are still applied From 86b5db58c55c69d89e74f44ed3a04ccde8694f4a Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Thu, 7 Nov 2024 13:38:56 +0100 Subject: [PATCH 14/18] Making test_dl1_variance_calib perform the test for all cameras --- src/ctapipe/calib/camera/calibrator.py | 4 ++-- .../calib/camera/tests/test_calibrator.py | 23 +++---------------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index 98518179247..180bb581e6d 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -16,7 +16,7 @@ ComponentName, TelescopeParameter, ) -from ctapipe.image.extractor import ImageExtractor +from ctapipe.image.extractor import ImageExtractor, VarianceExtractor from ctapipe.image.invalid_pixels import InvalidPixelHandler from ctapipe.image.reducer import DataVolumeReducer @@ -296,7 +296,7 @@ def _calibrate_dl1(self, event, tel_id): and dl1_calib.absolute_factor is not None ): if selected_gain_channel is None: - if extractor.__class__.__name__ == "VarianceExtractor": + if isinstance(extractor, VarianceExtractor): dl1.image *= np.square( dl1_calib.relative_factor / dl1_calib.absolute_factor ) diff --git a/src/ctapipe/calib/camera/tests/test_calibrator.py b/src/ctapipe/calib/camera/tests/test_calibrator.py index f9c2ff0f214..eefbe0bda1b 100644 --- a/src/ctapipe/calib/camera/tests/test_calibrator.py +++ b/src/ctapipe/calib/camera/tests/test_calibrator.py @@ -131,26 +131,9 @@ def test_check_dl0_empty(example_event, example_subarray): assert (event.dl1.tel[tel_id].image == 2).all() -def test_dl1_variance_calib(example_event, example_subarray): - # test the calibration of variance images - tel_id = list(example_event.r0.tel)[0] - calibrator = CameraCalibrator( - subarray=example_subarray, - image_extractor=VarianceExtractor(subarray=example_subarray), - apply_waveform_time_shift=False, - ) - calibrator(example_event) - image = example_event.dl1.tel[tel_id].image - assert image is not None - assert image.shape == ( - 1, - 1764, - ) - - -def test_calib_LST_camera(example_subarray): +def test_dl1_variance_calib(example_subarray, camera_geometry): n_channels = 2 - n_pixels = 1855 # number of pixels in LSTcam + n_pixels = len(camera_geometry) n_samples = 100 random = np.random.default_rng(1) @@ -187,7 +170,7 @@ def test_calib_LST_camera(example_subarray): assert image is not None assert image.shape == ( 2, - 1855, + len(camera_geometry), ) From 554586f2567e3d423acddbf14fae8cd4e01bef3d Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Fri, 8 Nov 2024 15:09:29 +0100 Subject: [PATCH 15/18] Switching to isinstance to test for variance extractor --- src/ctapipe/calib/camera/calibrator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index 180bb581e6d..d0c7674a4ad 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -307,7 +307,7 @@ def _calibrate_dl1(self, event, tel_id): dl1_calib.relative_factor[selected_gain_channel, pixel_index] / dl1_calib.absolute_factor[selected_gain_channel, pixel_index] ) - if extractor.__class__.__name__ == "VarianceExtractor": + if isinstance(extractor, VarianceExtractor): dl1.image *= np.square(corr) else: dl1.image *= corr From 6b3d33655d2b952626924b80c454b46e3a69ea87 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Fri, 8 Nov 2024 15:33:45 +0100 Subject: [PATCH 16/18] parametrizing the test --- .../calib/camera/tests/test_calibrator.py | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/ctapipe/calib/camera/tests/test_calibrator.py b/src/ctapipe/calib/camera/tests/test_calibrator.py index eefbe0bda1b..d06e772b4ef 100644 --- a/src/ctapipe/calib/camera/tests/test_calibrator.py +++ b/src/ctapipe/calib/camera/tests/test_calibrator.py @@ -6,6 +6,7 @@ import astropy.units as u import numpy as np +import pytest from scipy.stats import norm from traitlets.config import Config @@ -19,6 +20,8 @@ VarianceExtractor, ) from ctapipe.image.reducer import NullDataVolumeReducer, TailCutsDataVolumeReducer +from ctapipe.instrument import FromNameWarning +from ctapipe.instrument.camera.geometry import CameraGeometry def test_camera_calibrator(example_event, example_subarray): @@ -131,14 +134,15 @@ def test_check_dl0_empty(example_event, example_subarray): assert (event.dl1.tel[tel_id].image == 2).all() -def test_dl1_variance_calib(example_subarray, camera_geometry): +@pytest.mark.parametrize("camera_name", ["LSTCam", "FlashCam", "NectarCam", "CHEC"]) +def test_dl1_variance_calib(camera_name, example_subarray): + with pytest.warns(FromNameWarning): + geometry = CameraGeometry.from_name(camera_name) n_channels = 2 - n_pixels = len(camera_geometry) + n_pixels = len(geometry) n_samples = 100 random = np.random.default_rng(1) - - tel_id = 1 y = random.normal(0, 6, (n_channels, n_pixels, n_samples)) absolute = random.uniform(100, 1000, (n_channels, n_pixels)).astype("float32") @@ -150,28 +154,29 @@ def test_dl1_variance_calib(example_subarray, camera_geometry): pedestal = random.uniform(-4, 4, (n_channels, n_pixels)) y += pedestal[..., np.newaxis] - event = ArrayEventContainer() - event.dl0.tel[tel_id].waveform = y - event.calibration.tel[tel_id].dl1.pedestal_offset = pedestal - event.calibration.tel[tel_id].dl1.absolute_factor = absolute - event.calibration.tel[tel_id].dl1.relative_factor = relative - event.dl0.tel[tel_id].selected_gain_channel = None - event.r1.tel[tel_id].selected_gain_channel = None - calibrator = CameraCalibrator( subarray=example_subarray, image_extractor=VarianceExtractor(subarray=example_subarray), apply_waveform_time_shift=False, ) - calibrator(event) - image = event.dl1.tel[tel_id].image + for tel_id in example_subarray.tel_ids: + event = ArrayEventContainer() + event.dl0.tel[tel_id].waveform = y + event.calibration.tel[tel_id].dl1.pedestal_offset = pedestal + event.calibration.tel[tel_id].dl1.absolute_factor = absolute + event.calibration.tel[tel_id].dl1.relative_factor = relative + event.dl0.tel[tel_id].selected_gain_channel = None + event.r1.tel[tel_id].selected_gain_channel = None + calibrator(event) - assert image is not None - assert image.shape == ( - 2, - len(camera_geometry), - ) + image = event.dl1.tel[tel_id].image + + assert image is not None + assert image.shape == ( + 2, + len(geometry), + ) def test_dl1_charge_calib(example_subarray): From c7a8126ac9903ce356809b76358b7800fac708d3 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Fri, 8 Nov 2024 17:07:09 +0100 Subject: [PATCH 17/18] Testing all cameras through test_subarray fixture now, creating mock data for test --- .../calib/camera/tests/test_calibrator.py | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/ctapipe/calib/camera/tests/test_calibrator.py b/src/ctapipe/calib/camera/tests/test_calibrator.py index d06e772b4ef..808e29dd1d2 100644 --- a/src/ctapipe/calib/camera/tests/test_calibrator.py +++ b/src/ctapipe/calib/camera/tests/test_calibrator.py @@ -6,7 +6,6 @@ import astropy.units as u import numpy as np -import pytest from scipy.stats import norm from traitlets.config import Config @@ -20,8 +19,6 @@ VarianceExtractor, ) from ctapipe.image.reducer import NullDataVolumeReducer, TailCutsDataVolumeReducer -from ctapipe.instrument import FromNameWarning -from ctapipe.instrument.camera.geometry import CameraGeometry def test_camera_calibrator(example_event, example_subarray): @@ -134,48 +131,49 @@ def test_check_dl0_empty(example_event, example_subarray): assert (event.dl1.tel[tel_id].image == 2).all() -@pytest.mark.parametrize("camera_name", ["LSTCam", "FlashCam", "NectarCam", "CHEC"]) -def test_dl1_variance_calib(camera_name, example_subarray): - with pytest.warns(FromNameWarning): - geometry = CameraGeometry.from_name(camera_name) +def test_dl1_variance_calib(example_subarray): + calibrator = CameraCalibrator( + subarray=example_subarray, + image_extractor=VarianceExtractor(subarray=example_subarray), + apply_waveform_time_shift=False, + ) n_channels = 2 - n_pixels = len(geometry) n_samples = 100 - random = np.random.default_rng(1) - y = random.normal(0, 6, (n_channels, n_pixels, n_samples)) + event = ArrayEventContainer() - absolute = random.uniform(100, 1000, (n_channels, n_pixels)).astype("float32") - y *= absolute[..., np.newaxis] + for tel_type in example_subarray.telescope_types: + tel_id = example_subarray.get_tel_ids_for_type(tel_type)[0] + n_pixels = example_subarray.tel[tel_id].camera.geometry.n_pixels - relative = random.normal(1, 0.01, (n_channels, n_pixels)) - y /= relative[..., np.newaxis] + random = np.random.default_rng(1) + y = random.normal(0, 6, (n_channels, n_pixels, n_samples)) - pedestal = random.uniform(-4, 4, (n_channels, n_pixels)) - y += pedestal[..., np.newaxis] + absolute = random.uniform(100, 1000, (n_channels, n_pixels)).astype("float32") + y *= absolute[..., np.newaxis] - calibrator = CameraCalibrator( - subarray=example_subarray, - image_extractor=VarianceExtractor(subarray=example_subarray), - apply_waveform_time_shift=False, - ) + relative = random.normal(1, 0.01, (n_channels, n_pixels)) + y /= relative[..., np.newaxis] + + pedestal = random.uniform(-4, 4, (n_channels, n_pixels)) + y += pedestal[..., np.newaxis] - for tel_id in example_subarray.tel_ids: - event = ArrayEventContainer() event.dl0.tel[tel_id].waveform = y event.calibration.tel[tel_id].dl1.pedestal_offset = pedestal event.calibration.tel[tel_id].dl1.absolute_factor = absolute event.calibration.tel[tel_id].dl1.relative_factor = relative event.dl0.tel[tel_id].selected_gain_channel = None event.r1.tel[tel_id].selected_gain_channel = None - calibrator(event) - image = event.dl1.tel[tel_id].image + calibrator(event) + for tel_type in example_subarray.telescope_types: + tel_id = example_subarray.get_tel_ids_for_type(tel_type)[0] + image = event.dl1.tel[tel_id].image assert image is not None assert image.shape == ( 2, - len(geometry), + example_subarray.tel[tel_id].camera.geometry.n_pixels, ) From b9d7efb5d9473a2c13ba8d27166a82ef52b0d609 Mon Sep 17 00:00:00 2001 From: Christoph Toennis Date: Fri, 8 Nov 2024 18:26:46 +0100 Subject: [PATCH 18/18] refactoring VarianceExtractor check --- src/ctapipe/calib/camera/calibrator.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/ctapipe/calib/camera/calibrator.py b/src/ctapipe/calib/camera/calibrator.py index d0c7674a4ad..8b4f0c8bc15 100644 --- a/src/ctapipe/calib/camera/calibrator.py +++ b/src/ctapipe/calib/camera/calibrator.py @@ -296,21 +296,17 @@ def _calibrate_dl1(self, event, tel_id): and dl1_calib.absolute_factor is not None ): if selected_gain_channel is None: - if isinstance(extractor, VarianceExtractor): - dl1.image *= np.square( - dl1_calib.relative_factor / dl1_calib.absolute_factor - ) - else: - dl1.image *= dl1_calib.relative_factor / dl1_calib.absolute_factor + calibration = dl1_calib.relative_factor / dl1_calib.absolute_factor else: - corr = ( + calibration = ( dl1_calib.relative_factor[selected_gain_channel, pixel_index] / dl1_calib.absolute_factor[selected_gain_channel, pixel_index] ) - if isinstance(extractor, VarianceExtractor): - dl1.image *= np.square(corr) - else: - dl1.image *= corr + + if isinstance(extractor, VarianceExtractor): + calibration = calibration**2 + + dl1.image *= calibration # handle invalid pixels if self.invalid_pixel_handler is not None: