Merge pull request #2958 from cta-observatory/fix_impact_type

Introduce new `needs_atmosphere_profile` property to reconstructors

Author: kosack

docs/changes/2958.maintenance.rst

@@ -0,0 +1 @@
+Introduce ``needs_atmosphere_profile`` property to ``Reconstructors``.

src/ctapipe/reco/hillas_intersection.py

@@ -100,6 +100,7 @@ class HillasIntersection(HillasGeometryReconstructor):
     ).tag(config=True)
 
     property = ReconstructionProperty.GEOMETRY
+    needs_atmosphere_profile = False
 
     def __init__(self, subarray, atmosphere_profile=None, **kwargs):
         """

src/ctapipe/reco/hillas_reconstructor.py

@@ -107,6 +107,8 @@ class that reconstructs the direction of an atmospheric shower
 
     """
 
+    needs_atmosphere_profile = False
+
     def __init__(
         self, subarray: SubarrayDescription, atmosphere_profile=None, **kwargs
     ):

src/ctapipe/reco/impact.py

@@ -136,6 +136,7 @@ class ImPACTReconstructor(HillasGeometryReconstructor):
     spe = 0.6  # Also hard code single p.e. distribution width
 
     property = ReconstructionProperty.ENERGY | ReconstructionProperty.GEOMETRY
+    needs_atmosphere_profile = True
 
     def __init__(
         self, subarray, atmosphere_profile, dummy_reconstructor=False, **kwargs

src/ctapipe/reco/reconstructor.py

@@ -89,6 +89,11 @@ def __init__(self, subarray, atmosphere_profile=None, **kwargs):
         self.quality_query = StereoQualityQuery(parent=self)
         self.atmosphere_profile = atmosphere_profile
 
+    @property
+    @abstractmethod
+    def needs_atmosphere_profile(self) -> bool:
+        """Whether this reconstructor requires ``atmosphere_profile``."""
+
     @abstractmethod
     def __call__(self, event: ArrayEventContainer):
         """

src/ctapipe/reco/shower_processor.py

@@ -67,13 +67,6 @@ def __init__(
         super().__init__(config=config, parent=parent, **kwargs)
         self.subarray = subarray
         self.atmosphere_profile = atmosphere_profile
-        if (
-            atmosphere_profile is None
-            and "ImPACTReconstrcutor" in self.reconstructor_types
-        ):
-            raise TypeError(
-                "Argument 'atmosphere_profile' can not be 'None' if 'ImPACTReconstructor' is in 'reconstructor_types'"
-            )
         self.reconstructors = [
             Reconstructor.from_name(
                 reco_type,
@@ -84,6 +77,14 @@ def __init__(
             for reco_type in self.reconstructor_types
         ]
 
+        if atmosphere_profile is None and any(
+            r.needs_atmosphere_profile for r in self.reconstructors
+        ):
+            raise TypeError(
+                "Argument 'atmosphere_profile' can not be 'None' if one of the selected "
+                "reconstructors requires an atmosphere profile."
+            )
+
     def __call__(self, event: ArrayEventContainer):
         """
         Apply all configured stereo reconstructors to the given event.

src/ctapipe/reco/sklearn.py

@@ -95,6 +95,8 @@ class SKLearnReconstructor(Reconstructor):
     #: Property predicted, overridden in subclass.
     property = None
 
+    needs_atmosphere_profile = False
+
     prefix = traits.Unicode(
         default_value=None,
         allow_none=True,
@@ -521,6 +523,8 @@ class DispReconstructor(Reconstructor):
 
     target = "true_disp"
 
+    needs_atmosphere_profile = False
+
     prefix = traits.Unicode(
         default_value="disp",
         allow_none=False,

src/ctapipe/reco/tests/test_shower_processor.py

@@ -107,3 +107,14 @@ def test_shower_processor_geometry(
         assert not isfinite(DL2a.core_y)
         assert not DL2a.is_valid
         assert not isfinite(DL2a.average_intensity)
+
+
+def test_shower_processor_requires_atmosphere_profile_for_impact(example_subarray):
+    pytest.importorskip("iminuit")
+
+    with pytest.raises(TypeError):
+        ShowerProcessor(
+            subarray=example_subarray,
+            atmosphere_profile=None,
+            reconstructor_types=["ImPACTReconstructor"],
+        )

test_plugin/ctapipe_test_plugin/__init__.py

@@ -97,6 +97,8 @@ def _generator(self):
 class PluginReconstructor(Reconstructor):
     """A plugin Reconstructor"""
 
+    needs_atmosphere_profile = False
+
     foo = traits.Bool(
         True, help="example traitlet to see that it is included in --help"
     ).tag(config=True)