Merge pull request #2361 from cta-observatory/fix_single_pixel_area

Improve docstring and validation of parameters in CameraGeometry

ctapipe/image/tests/test_hillas.py

@@ -223,7 +223,7 @@ def test_straight_line_width_0():
                     pix_x=x * u.m,
                     pix_y=y * u.m,
                     pix_type="hexagonal",
-                    pix_area=1 * u.m**2,
+                    pix_area=np.full(len(pix_id), 1.0) * u.m**2,
                 )
 
                 img = rng.poisson(5, size=len(long))
@@ -243,7 +243,7 @@ def test_single_pixel():
         pix_x=x.ravel() * u.cm,
         pix_y=y.ravel() * u.cm,
         pix_type="rectangular",
-        pix_area=1 * u.cm**2,
+        pix_area=np.full(9, 1.0) * u.cm**2,
     )
 
     image = np.zeros((3, 3))

ctapipe/instrument/camera/geometry.py

@@ -88,25 +88,32 @@ class CameraGeometry:
 
     Parameters
     ----------
-    self : type
-        description
     name : str
-         Camera name (e.g. NectarCam, LSTCam, ...)
-    pix_id : array(int)
+         Camera name (e.g. "NectarCam", "LSTCam", ...)
+    pix_id : np.ndarray[int]
         pixels id numbers
-    pix_x : array with units
+    pix_x : u.Quantity
         position of each pixel (x-coordinate)
-    pix_y : array with units
+    pix_y : u.Quantity
         position of each pixel (y-coordinate)
-    pix_area : array(float)
-        surface area of each pixel, if None will be calculated
-    neighbors : list(arrays)
-        adjacency list for each pixel
-    pix_type : string
+    pix_area : u.Quantity
+        surface area of each pixel
+    pix_type : PixelShape
         either 'rectangular' or 'hexagonal'
-    pix_rotation : value convertable to an `astropy.coordinates.Angle`
-        rotation angle with unit (e.g. 12 * u.deg), or "12d"
-    cam_rotation : overall camera rotation with units
+    pix_rotation : u.Quantity[angle]
+        rotation of the pixels, global value for all pixels
+    cam_rotation : u.Quantity[angle]
+        rotation of the camera. All coordinates will be interpreted
+        to be rotated by this angle with respect to the definition of the
+        frame the pixel coordinates are defined in.
+    neighbors : list[list[int]]
+        adjacency list for each pixel
+        If not given, will be build from the pixel width and distances
+        between pixels.
+    apply_derotation : bool
+        If true, rotate the pixel coordinates, so that cam_rotation is 0.
+    frame : coordinate frame
+        Frame in which the pixel coordinates are defined (after applying cam_rotation)
     """
 
     CURRENT_TAB_VERSION = "2.0"
@@ -120,43 +127,60 @@ def __init__(
         pix_y,
         pix_area,
         pix_type,
-        pix_rotation="0d",
-        cam_rotation="0d",
+        pix_rotation=0 * u.deg,
+        cam_rotation=0 * u.deg,
         neighbors=None,
         apply_derotation=True,
         frame=None,
+        _validate=True,
     ):
-        if pix_x.ndim != 1 or pix_y.ndim != 1:
-            raise ValueError(
-                f"Pixel coordinates must be 1 dimensional, got {pix_x.ndim}"
-            )
+        self.name = name
+        self.n_pixels = len(pix_id)
+        self.unit = pix_x.unit
+        self.pix_id = np.array(pix_id)
 
-        assert len(pix_x) == len(pix_y), "pix_x and pix_y must have same length"
+        if _validate:
+            self.pix_id = np.array(self.pix_id)
 
-        if isinstance(pix_type, str):
-            pix_type = PixelShape.from_string(pix_type)
-        elif not isinstance(pix_type, PixelShape):
-            raise TypeError(
-                f"pix_type most be a PixelShape or the name of a PixelShape, got {pix_type}"
-            )
+            if self.pix_id.ndim != 1:
+                raise ValueError(
+                    f"Pixel coordinates must be 1 dimensional, got {pix_id.ndim}"
+                )
 
-        self.n_pixels = len(pix_x)
-        self.name = name
-        self.pix_id = pix_id
-        self.pix_x = pix_x
-        self.pix_y = pix_y
-        self.pix_area = pix_area
-        self.pix_type = pix_type
+            shape = (self.n_pixels,)
 
-        if not isinstance(pix_rotation, Angle):
-            pix_rotation = Angle(pix_rotation)
+            if pix_x.shape != shape:
+                raise ValueError(
+                    f"pix_x has wrong shape: {pix_x.shape}, expected {shape}"
+                )
+            if pix_y.shape != shape:
+                raise ValueError(
+                    f"pix_y has wrong shape: {pix_y.shape}, expected {shape}"
+                )
+            if pix_area.shape != shape:
+                raise ValueError(
+                    f"pix_area has wrong shape: {pix_area.shape}, expected {shape}"
+                )
+
+            if isinstance(pix_type, str):
+                pix_type = PixelShape.from_string(pix_type)
+            elif not isinstance(pix_type, PixelShape):
+                raise TypeError(
+                    f"pix_type must be a PixelShape or the name of a PixelShape, got {pix_type}"
+                )
 
-        if not isinstance(cam_rotation, Angle):
-            cam_rotation = Angle(cam_rotation)
+            if not isinstance(pix_rotation, Angle):
+                pix_rotation = Angle(pix_rotation)
 
+            if not isinstance(cam_rotation, Angle):
+                cam_rotation = Angle(cam_rotation)
+
+        self.pix_x = pix_x
+        self.pix_y = pix_y.to(self.unit)
+        self.pix_area = pix_area.to(self.unit**2)
+        self.pix_type = pix_type
         self.pix_rotation = pix_rotation
         self.cam_rotation = cam_rotation
-
         self._neighbors = neighbors
         self.frame = frame
 
@@ -330,6 +354,7 @@ def __getitem__(self, slice_):
             cam_rotation=self.cam_rotation,
             neighbors=None,
             apply_derotation=False,
+            _validate=False,
         )
 
     @lazyproperty

ctapipe/instrument/camera/tests/test_geometry.py

@@ -75,15 +75,21 @@ def test_position_to_pix_index(prod5_lst):
 
 def test_find_neighbor_pixels():
     """test basic neighbor functionality"""
-    n_pixels = 5
+    n_pixels_grid = 5
     x, y = u.Quantity(
-        np.meshgrid(np.linspace(-5, 5, n_pixels), np.linspace(-5, 5, n_pixels)), u.cm
+        np.meshgrid(
+            np.linspace(-5, 5, n_pixels_grid), np.linspace(-5, 5, n_pixels_grid)
+        ),
+        u.cm,
     )
+    x = x.ravel()
+    y = y.ravel()
+    n_pixels = len(x)
 
     geom = CameraGeometry(
         "test",
         pix_id=np.arange(n_pixels),
-        pix_area=u.Quantity(4, u.cm**2),
+        pix_area=u.Quantity(np.full(n_pixels, 4), u.cm**2),
         pix_x=x.ravel(),
         pix_y=y.ravel(),
         pix_type="rectangular",
@@ -238,12 +244,13 @@ def test_rectangle_patch_neighbors():
     """ " test that a simple rectangular camera has the expected neighbors"""
     pix_x = np.array([-1.1, 0.1, 0.9, -1, 0, 1, -0.9, -0.1, 1.1]) * u.m
     pix_y = np.array([1.1, 1, 0.9, -0.1, 0, 0.1, -0.9, -1, -1.1]) * u.m
+    pix_area = np.full(len(pix_x), 0.01) * u.m**2
     cam = CameraGeometry(
         name="testcam",
         pix_id=np.arange(pix_x.size),
         pix_x=pix_x,
         pix_y=pix_y,
-        pix_area=None,
+        pix_area=pix_area,
         pix_type="rectangular",
     )
 

docs/changes/2361.maintenance.rst

@@ -0,0 +1 @@
+Improve docstring and validation of parameters of ``CameraGeometry``.