Fix broken telescope id string, Fixes #1058

ctapipe/image/muon/muon_reco_functions.py

@@ -35,8 +35,8 @@ def analyze_muon_event(event):
 
     """
 
-    names = ['LST:LSTCam', 'MST:NectarCam', 'MST:FlashCam', 'MST-SCT:SCTCam',
-             '1M:DigiCam', 'GCT:CHEC', 'ASTRI:ASTRICam', 'ASTRI:CHEC']
+    names = ['LST_LST_LSTCam', 'MST_MST_NectarCam', 'MST_MST_FlashCam', 'MST_SCT_SCTCam',
+             'SST_1M_DigiCam', 'SST_GCT_CHEC', 'SST_ASTRI_ASTRICam', 'SST_ASTRI_CHEC']
     tail_cuts = [(5, 7), (5, 7), (10, 12), (5, 7),
                 (5, 7), (5, 7), (5, 7), (5, 7)]  # 10, 12?
     impact = [(0.2, 0.9), (0.1, 0.95), (0.2, 0.9), (0.2, 0.9),

ctapipe/instrument/subarray.py

@@ -12,7 +12,9 @@
 from astropy.table import Table
 
 import ctapipe
+
 from ..coordinates import GroundFrame
+from . import TelescopeDescription
 
 
 class SubarrayDescription:
@@ -125,6 +127,37 @@ def tel_indices(self):
         lists based on tel_ids into fixed-length arrays"""
         return {tel_id: ii for ii, tel_id in enumerate(self.tels.keys())}
 
+    @property
+    def tel_index_array(self):
+        """ 
+        returns an expanded array that maps tel_id to tel_index. I.e. for a given
+        telescope, this array maps the tel_id to a flat index starting at 0 for
+        the first telescope. `tel_index = tel_id_to_index_array[tel_id]`
+        If the tel_ids are not contiguous, gaps will be filled in by -1. 
+        For a more compact representation use the `tel_indices` 
+        """
+        idx = np.zeros(np.max(self.tel_ids)+1, dtype=int) - 1 # start with -1
+        for key,val in self.tel_indices.items():
+            idx[key] = val
+        return idx
+
+    def tel_ids_to_indices(self, tel_ids):
+        """maps a telescope id (or array of them) to flat indices
+
+        Parameters
+        ----------
+        tel_ids : int or List[int]
+            array of tel IDs
+
+        Returns
+        -------
+        np.array:
+            array of corresponding tel indices
+        """
+        tel_ids = np.asanyarray(tel_ids).ravel()
+        index_map = self.tel_index_array
+        return index_map[tel_ids]
+
     @property
     def footprint(self):
         """area of smallest circle containing array on ground"""
@@ -259,26 +292,31 @@ def peek(self):
     @property
     def telescope_types(self):
         """ list of telescope types in the array"""
-        return [t.type + ':' + t.camera.cam_id for t in set(self.tel.values())]
+        return list({tel for tel in self.tel.values()})
 
     @property
     def camera_types(self):
         """ list of camera types in the array """
-        return [t.camera.cam_id for t in set(self.tel.values())]
+        return list({tel.camera for tel in self.tel.values()})
 
     @property
     def optics_types(self):
         """ list of optics types in the array """
-        return [t.optics for t in set(self.tel.values())]
+        return list({tel.optics for tel in self.tel.values()})
 
     def get_tel_ids_for_type(self, tel_type):
         """
         return list of tel_ids that have the given tel_type
 
         Parameters
         ----------
-        tel_type: str
+        tel_type: str or TelescopeDescription
            telescope type string (e.g. 'MST:NectarCam')
 
         """
-        return [id for id, descr in self.tels.items() if str(descr) == tel_type]
+        if isinstance(tel_type, TelescopeDescription):
+            tel_str = str(tel_type)
+        else:
+            tel_str = tel_type
+
+        return [id for id, descr in self.tels.items() if str(descr) == tel_str]

ctapipe/instrument/telescope.py

@@ -85,18 +85,25 @@ def from_name(cls, optics_name, camera_name):
         camera = CameraGeometry.from_name(camera_name)
         optics = OpticsDescription.from_name(optics_name)
 
-        t = guess_telescope(camera.n_pixels, optics.equivalent_focal_length)
+        tel_type='unknown'
+        tel_name='unknown'
+        try:
+            t = guess_telescope(camera.n_pixels, optics.equivalent_focal_length)
+            tel_type = t.type
+            tel_name = t.name
+        except ValueError:
+            pass # couldn't detect name
 
-        return cls(name=t.name, type=t.type, optics=optics, camera=camera)
+        return cls(name=tel_name, type=tel_type, optics=optics, camera=camera)
 
     def __str__(self):
-        return str(self.optics) + ":" + str(self.camera)
+        return f"{self.type}_{self.optics}_{self.camera}"
 
     def __repr__(self):
-        return "{}({}, type={}, optics={}, camera={})".format(
-            self.name,
-            self.type,
+        return "{}(type={}, name={}, optics={}, camera={})".format(
             self.__class__.__name__,
+            self.type,
+            self.name,
             str(self.optics),
             str(self.camera),
         )

ctapipe/instrument/tests/test_camera.py

@@ -263,3 +263,8 @@ def test_hashing():
     cam3 = CameraGeometry.from_name("ASTRICam")
 
     assert len(set([cam1, cam2, cam3])) == 2
+
+@pytest.mark.parametrize("camera_name", CameraGeometry.get_known_camera_names())
+def test_camera_from_name(camera_name):
+    camera = CameraGeometry.from_name(camera_name)
+    assert str(camera) == camera_name

ctapipe/instrument/tests/test_optics.py

@@ -5,6 +5,7 @@
 
 def test_guess_optics():
     from ctapipe.instrument import guess_telescope
+
     answer = guess_telescope(1855, 28.0 * u.m)
 
     od = OpticsDescription.from_name(answer.name)
@@ -15,18 +16,26 @@ def test_guess_optics():
 
 def test_construct_optics():
     OpticsDescription(
-        name='test',
+        name="test",
         num_mirrors=1,
         num_mirror_tiles=100,
-        mirror_area=u.Quantity(550, u.m**2),
+        mirror_area=u.Quantity(550, u.m ** 2),
         equivalent_focal_length=u.Quantity(10, u.m),
     )
 
     with pytest.raises(TypeError):
         OpticsDescription(
-            name='test',
+            name="test",
             num_mirrors=1,
             num_mirror_tiles=100,
             mirror_area=550,
             equivalent_focal_length=10,
         )
+
+
+@pytest.mark.parametrize("optics_name", OpticsDescription.get_known_optics_names())
+def test_optics_from_name(optics_name):
+    optics = OpticsDescription.from_name(optics_name)
+    assert optics.equivalent_focal_length > 0
+    # make sure the string rep gives back the name:
+    assert str(optics) == optics_name

ctapipe/instrument/tests/test_subarray.py

@@ -2,30 +2,26 @@
 from astropy import units as u
 from astropy.coordinates import SkyCoord
 
-from ctapipe.instrument import (
-    SubarrayDescription,
-    TelescopeDescription,
-)
+from ctapipe.instrument import (CameraGeometry, OpticsDescription,
+                                SubarrayDescription, TelescopeDescription)
 
 
-def test_subarray_description():
+def example_subarray(n_tels=10):
     pos = {}
     tel = {}
-    n_tels = 10
 
     for tel_id in range(1, n_tels + 1):
         tel[tel_id] = TelescopeDescription.from_name(
-            optics_name="MST",
-            camera_name="NectarCam",
+            optics_name="MST", camera_name="NectarCam"
         )
         pos[tel_id] = np.random.uniform(-100, 100, size=3) * u.m
 
-    sub = SubarrayDescription(
-        "test array",
-        tel_positions=pos,
-        tel_descriptions=tel
-    )
+    return SubarrayDescription("test array", tel_positions=pos, tel_descriptions=tel)
 
+
+def test_subarray_description():
+    n_tels = 10
+    sub = example_subarray(n_tels)
     sub.peek()
 
     assert len(sub.telescope_types) == 1
@@ -36,12 +32,14 @@ def test_subarray_description():
     assert sub.tel_ids[0] == 1
     assert sub.tel[1].camera is not None
     assert 0 not in sub.tel  # check that there is no tel 0 (1 is first above)
-    assert len(sub.to_table()) == n_tels
     assert len(sub.camera_types) == 1  # only 1 camera type
-    assert sub.camera_types[0] == 'NectarCam'
+    assert isinstance(sub.camera_types[0], CameraGeometry)
+    assert isinstance(sub.telescope_types[0], TelescopeDescription)
+    assert isinstance(sub.optics_types[0], OpticsDescription)
+    assert len(sub.telescope_types) == 1  # only have one type in this array
+    assert len(sub.optics_types) == 1  # only have one type in this array
+    assert len(sub.camera_types) == 1  # only have one type in this array
     assert sub.optics_types[0].equivalent_focal_length.to_value(u.m) == 16.0
-    assert sub.telescope_types[0] == 'MST:NectarCam'
-    assert sub.tel_coords
     assert isinstance(sub.tel_coords, SkyCoord)
     assert len(sub.tel_coords) == n_tels
 
@@ -51,9 +49,38 @@ def test_subarray_description():
     assert subsub.tel_indices[6] == 3
     assert subsub.tel_ids[3] == 6
 
-    assert len(sub.to_table(kind='optics')) == 1
+    assert len(sub.to_table(kind="optics")) == 1
+    assert sub.telescope_types[0] == sub.tel[1]
+
+
+def test_to_table(example_event):
+    sub: SubarrayDescription = example_event.inst.subarray
+
+    assert len(sub.to_table(kind="subarray")) == sub.num_tels
+    assert len(sub.to_table(kind="optics")) == len(sub.optics_types)
+
+
+def test_tel_indexing(example_event):
+    """ Check that we can convert between telescope_id and telescope_index """
+    sub: SubarrayDescription = example_event.inst.subarray
+
+    assert sub.tel_indices[1] == 0  # first tel_id is in slot 0
+    for tel_id in sub.tel_ids:
+        assert sub.tel_index_array[tel_id] == sub.tel_indices[tel_id]
+
+    assert sub.tel_ids_to_indices(1) == 0
+    assert np.all(sub.tel_ids_to_indices([1, 2, 3]) == np.array([0, 1, 2]))
+
 
+def test_get_tel_ids_for_type(example_event):
+    """ 
+    check that we can get a list of telescope ids by a telescope type, which can
+    be passed by string or TelscopeDescription instance 
+    """
+    sub: SubarrayDescription = example_event.inst.subarray
 
+    types = sub.telescope_types
 
-if __name__ == '__main__':
-    test_subarray_description()
+    for teltype in types:
+        assert len(sub.get_tel_ids_for_type(teltype)) > 0
+        assert len(sub.get_tel_ids_for_type(str(teltype))) > 0

ctapipe/instrument/tests/test_telescope.py

@@ -1,22 +1,44 @@
+import itertools
+
+import pytest
+
+from ctapipe.instrument.camera import CameraGeometry
+from ctapipe.instrument.optics import OpticsDescription
+from ctapipe.instrument.telescope import TelescopeDescription
+
+
 def test_hash():
-    from ctapipe.instrument.telescope import TelescopeDescription
-    from ctapipe.instrument.optics import OpticsDescription
-    from ctapipe.instrument.camera import CameraGeometry
 
-    types = ['LST', 'MST', 'SST']
-    names = ['LST', 'MST', 'SST-1M']
-    cameras = ['LSTCam', 'FlashCam', 'DigiCam']
+    types = ["LST", "MST", "SST"]
+    names = ["LST", "MST", "SST-1M"]
+    cameras = ["LSTCam", "FlashCam", "DigiCam"]
 
     telescopes = []
     for name, type, camera in zip(names, types, cameras):
         for i in range(3):
 
-            telescopes.append(TelescopeDescription(
-                name=name,
-                type=type,
-                optics=OpticsDescription.from_name(name),
-                camera=CameraGeometry.from_name(camera)
-            ))
+            telescopes.append(
+                TelescopeDescription(
+                    name=name,
+                    type=type,
+                    optics=OpticsDescription.from_name(name),
+                    camera=CameraGeometry.from_name(camera),
+                )
+            )
 
     assert len(telescopes) == 9
     assert len(set(telescopes)) == 3
+
+
+optics_names = OpticsDescription.get_known_optics_names()
+camera_names = CameraGeometry.get_known_camera_names()
+
+@pytest.mark.parametrize("camera_name", camera_names)
+@pytest.mark.parametrize("optics_name", optics_names)
+def test_telescope_from_name(optics_name, camera_name):
+    tel = TelescopeDescription.from_name(optics_name, camera_name)
+    assert optics_name in str(tel)
+    assert camera_name in str(tel)
+    assert tel.camera.pix_x.shape[0] > 0
+    assert tel.optics.equivalent_focal_length.to("m") > 0
+    assert tel.type in ['MST', 'SST', 'LST', 'unknown']