Repository: cta-observatory/ctapipe
Pull request #2440 (2023-10-31 10:16:11)

Fix unwanted default columns in disp output


Patch:
diff --git a/ctapipe/reco/sklearn.py b/ctapipe/reco/sklearn.py
index 976ecb60..1ae0bc78 100644
--- a/ctapipe/reco/sklearn.py
+++ b/ctapipe/reco/sklearn.py
@@ -416,7 +416,7 @@ class EnergyRegressor(SKLearnRegressionReconstructor):
             result,
             ReconstructedEnergyContainer,
             prefix=self.prefix,
-            stereo=False,
+            add_tel_prefix=True,
         )
         return {ReconstructionProperty.ENERGY: result}
 
@@ -480,7 +480,10 @@ class ParticleClassifier(SKLearnClassificationReconstructor):
             }
         )
         add_defaults_and_meta(
-            result, ParticleClassificationContainer, prefix=self.prefix, stereo=False
+            result,
+            ParticleClassificationContainer,
+            prefix=self.prefix,
+            add_tel_prefix=True,
         )
         return {ReconstructionProperty.PARTICLE_TYPE: result}
 
@@ -770,7 +773,8 @@ class DispReconstructor(Reconstructor):
             disp_result,
             DispContainer,
             prefix=f"{self.prefix}_parameter",
-            stereo=False,
+            # disp is always per telescope, so no need to add the prefix
+            add_tel_prefix=False,
         )
 
         psi = table["hillas_psi"].quantity.to_value(u.rad)
@@ -797,7 +801,7 @@ class DispReconstructor(Reconstructor):
             altaz_result,
             ReconstructedGeometryContainer,
             prefix=self.prefix,
-            stereo=False,
+            add_tel_prefix=True,
         )
 
         return {
diff --git a/ctapipe/reco/utils.py b/ctapipe/reco/utils.py
index bf26d948..48e14445 100644
--- a/ctapipe/reco/utils.py
+++ b/ctapipe/reco/utils.py
@@ -1,4 +1,4 @@
-def add_defaults_and_meta(table, container, prefix=None, stereo=True):
+def add_defaults_and_meta(table, container, prefix=None, add_tel_prefix=False):
     """
     Fill column descriptions and default values into table for container
 
@@ -10,21 +10,21 @@ def add_defaults_and_meta(table, container, prefix=None, stereo=True):
         the container class to add columns and descriptions to the table
     prefix : str
         prefix for the column names
-    stereo : bool
-        If False, add a ``tel_`` prefix to the column names to signal it's
+    add_tel_prefix : bool
+        If True, add a ``tel_`` prefix to the column names to signal it's
         telescope-wise quantity
     """
     if prefix is None:
         prefix = container.default_prefix
 
     for name, field in container.fields.items():
-        if not stereo and name == "telescopes":
+        if add_tel_prefix and name == "telescopes":
             continue
 
-        if stereo:
-            colname = f"{prefix}_{name}"
-        else:
+        if add_tel_prefix:
             colname = f"{prefix}_tel_{name}"
+        else:
+            colname = f"{prefix}_{name}"
 
         if colname not in table.colnames and field.default is not None:
             table[colname] = field.default
diff --git a/ctapipe/tools/train_disp_reconstructor.py b/ctapipe/tools/train_disp_reconstructor.py
index 80352a0b..7edf728d 100644
--- a/ctapipe/tools/train_disp_reconstructor.py
+++ b/ctapipe/tools/train_disp_reconstructor.py
@@ -1,6 +1,3 @@
-"""
-Tool for training the DispReconstructor
-"""
 import astropy.units as u
 import numpy as np
 
@@ -12,10 +9,6 @@ from ctapipe.io import TableLoader
 from ctapipe.reco import CrossValidator, DispReconstructor
 from ctapipe.reco.preprocessing import check_valid_rows, horizontal_to_telescope
 
-__all__ = [
-    "TrainDispReconstructor",
-]
-
 
 class TrainDispReconstructor(Tool):
     """
@@ -23,8 +16,7 @@ class TrainDispReconstructor(Tool):
 
     The tool first performs a cross validation to give an initial estimate
     on the quality of the estimation and then finally trains two models
-    (estimating ``norm(disp)`` and ``sign(disp)`` respectively) per
-    telescope type on the full dataset.
+    (|disp| and sign(disp)) per telescope type on the full dataset.
     """
 
     name = "ctapipe-train-disp-reconstructor"
@@ -63,9 +55,9 @@ class TrainDispReconstructor(Tool):
     project_disp = Bool(
         default_value=False,
         help=(
-            "If true, ``true_disp`` is the distance between shower cog and"
+            "If true, true |disp| is the distance between shower cog and"
             " the true source position along the reconstructed main shower axis."
-            "If false, ``true_disp`` is the distance between shower cog"
+            "If false, true |disp| is the distance between shower cog"
             " and the true source position."
         ),
     ).tag(config=True)
diff --git a/docs/api-reference/tools/index.rst b/docs/api-reference/tools/index.rst
index e9e9823e..0d1c2dc8 100644
--- a/docs/api-reference/tools/index.rst
+++ b/docs/api-reference/tools/index.rst
@@ -93,8 +93,5 @@ Reference/API
 .. automodapi:: ctapipe.tools.train_particle_classifier
     :no-inheritance-diagram:
 
-.. automodapi:: ctapipe.tools.train_disp_reconstructor
-   :no-inheritance-diagram:
-
 .. automodapi:: ctapipe.tools.apply_models
     :no-inheritance-diagram:
diff --git a/docs/changes/2420.bugfix.rst b/docs/changes/2420.bugfix.rst
deleted file mode 100644
index aa21eafa..00000000
--- a/docs/changes/2420.bugfix.rst
+++ /dev/null
@@ -1,1 +0,0 @@
-Add docstring for ``ctapipe-train-disp-reconstructor``.
diff --git a/docs/changes/2440.bugfix.rst b/docs/changes/2440.bugfix.rst
new file mode 100644
index 00000000..435d4288
--- /dev/null
+++ b/docs/changes/2440.bugfix.rst
@@ -0,0 +1,1 @@
+Fix additional, unwanted columns being written into disp prediction output.
diff --git a/docs/conf.py b/docs/conf.py
index b91f6f0b..4bcb2131 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -131,10 +131,14 @@ nitpick_ignore = [
     ("py:class", "traitlets.config.application.Application"),
     ("py:class", "traitlets.utils.sentinel.Sentinel"),
     ("py:class", "traitlets.traitlets.ObserveHandler"),
+    ("py:class", "dict[K, V]"),
+    ("py:class", "G"),
+    ("py:class", "t.Sequence"),
     ("py:class", "StrDict"),
     ("py:class", "ClassesType"),
     ("py:obj", "traitlets.traitlets.G"),
     ("py:obj", "traitlets.traitlets.S"),
+    ("py:obj", "traitlets.traitlets.T"),
     ("py:class", "traitlets.traitlets.T"),
     ("py:class", "re.Pattern[t.Any]"),
     ("py:class", "Sentinel"),


Test patch:
diff --git a/ctapipe/tools/tests/test_apply_models.py b/ctapipe/tools/tests/test_apply_models.py
index 13f8edb8..32b0e4bc 100644
--- a/ctapipe/tools/tests/test_apply_models.py
+++ b/ctapipe/tools/tests/test_apply_models.py
@@ -103,7 +103,6 @@ def test_apply_all(
     energy_regressor_path,
     particle_classifier_path,
     disp_reconstructor_path,
-    dl2_shower_geometry_file_lapalma,
     tmp_path,
 ):
     from ctapipe.tools.apply_models import ApplyModels
@@ -211,6 +210,7 @@ def test_apply_all(
         assert f"{prefix_disp}_tel_is_valid" in tel_events.colnames
         assert f"{prefix_disp}_parameter_norm" in tel_events.colnames
         assert f"{prefix_disp}_parameter_is_valid" in tel_events.colnames
+        assert f"{prefix_disp}_parameter_tel_is_valid" not in tel_events.colnames
 
         # check that the "--no-dl1-parameters" option worked
         assert "hillas_intensity" not in tel_events.colnames