From ff7390bbebedfa32899746b9823f828e9c9a82c7 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Tue, 19 Sep 2023 15:04:31 +0200 Subject: [PATCH] Use intmin for invalid pixel positions, allow empty arguments --- ctapipe/instrument/camera/geometry.py | 18 ++++++------------ .../instrument/camera/tests/test_geometry.py | 12 +++++++++++- docs/changes/2397.api.rst | 8 ++++++++ 3 files changed, 25 insertions(+), 13 deletions(-) create mode 100644 docs/changes/2397.api.rst diff --git a/ctapipe/instrument/camera/geometry.py b/ctapipe/instrument/camera/geometry.py index dd55b14425b..1d83a1bab60 100644 --- a/ctapipe/instrument/camera/geometry.py +++ b/ctapipe/instrument/camera/geometry.py @@ -954,12 +954,13 @@ def position_to_pix_index(self, x, y): pix_indices: Pixel index or array of pixel indices. Returns -1 if position falls outside camera """ - if not self._all_pixel_areas_equal: logger.warning( " Method not implemented for cameras with varying pixel sizes" ) unit = x.unit + scalar = x.ndim == 0 + points_searched = np.dstack([x.to_value(unit), y.to_value(unit)]) circum_rad = self._pixel_circumradius[0].to_value(unit) kdtree = self._kdtree @@ -969,8 +970,9 @@ def position_to_pix_index(self, x, y): del dist pix_indices = pix_indices.flatten() + invalid = np.iinfo(pix_indices.dtype).min # 1. Mark all points outside pixel circumeference as lying outside camera - pix_indices[pix_indices == self.n_pixels] = -1 + pix_indices[pix_indices == self.n_pixels] = invalid # 2. Accurate check for the remaing cases (within circumference, but still outside # camera). It is first checked if any border pixel numbers are returned. @@ -1006,17 +1008,9 @@ def position_to_pix_index(self, x, y): ) del dist_check if index_check != insidepix_index: - pix_indices[index] = -1 - - # print warning: - for index in np.where(pix_indices == -1)[0]: - logger.warning( - " Coordinate ({} m, {} m) lies outside camera".format( - points_searched[0][index, 0], points_searched[0][index, 1] - ) - ) + pix_indices[index] = invalid - return pix_indices if len(pix_indices) > 1 else pix_indices[0] + return np.squeeze(pix_indices) if scalar else pix_indices @staticmethod def simtel_shape_to_type(pixel_shape): diff --git a/ctapipe/instrument/camera/tests/test_geometry.py b/ctapipe/instrument/camera/tests/test_geometry.py index 10410e0efc6..8e3a7545d95 100644 --- a/ctapipe/instrument/camera/tests/test_geometry.py +++ b/ctapipe/instrument/camera/tests/test_geometry.py @@ -68,10 +68,20 @@ def test_load_lst_camera(prod5_lst): def test_position_to_pix_index(prod5_lst): """test that we can lookup a pixel from a coordinate""" + geometry = prod5_lst.camera.geometry + x, y = (0.80 * u.m, 0.79 * u.m) - pix_id = prod5_lst.camera.geometry.position_to_pix_index(x, y) + + pix_id = geometry.position_to_pix_index(x, y) + assert pix_id == 1575 + pix_ids = geometry.position_to_pix_index([0.8, 0.8] * u.m, [0.79, 0.79] * u.m) + np.testing.assert_array_equal(pix_ids, [1575, 1575]) + + assert len(geometry.position_to_pix_index([] * u.m, [] * u.m)) == 0 + assert geometry.position_to_pix_index(5 * u.m, 5 * u.m) == np.iinfo(int).min + def test_find_neighbor_pixels(): """test basic neighbor functionality""" diff --git a/docs/changes/2397.api.rst b/docs/changes/2397.api.rst new file mode 100644 index 00000000000..45b64781bb8 --- /dev/null +++ b/docs/changes/2397.api.rst @@ -0,0 +1,8 @@ +``CameraGeometry.position_to_pix_index`` will now return the minimum integer value for invalid +pixel coordinates instead of -1 due to the danger of using -1 as an index in python accessing +the last element of a data array for invalid pixels. +The function will now also no longer raise an error if the arguments are empty arrays and instead +just return an empty index array. +The function will also no longer log a warning in case of coordinates that do not match a camera pixel. +The function is very low-level and if not finding a pixel at the tested position warrants a warning or +is expected will depend on the calling code.