Remove redundant is_valid field and use same prefix for containers in…

… disp
cta-observatory · Nov 6, 2023 · 0ebff9d · 0ebff9d
1 parent 3c4f140
commit 0ebff9d
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 16 deletions.
diff --git a/ctapipe/containers.py b/ctapipe/containers.py
@@ -988,10 +988,11 @@ class DispContainer(Container):
     Standard output of disp reconstruction algorithms for origin reconstruction
     """
 
-    default_prefix = "disp_parameter"
+    default_prefix = "disp"
 
-    norm = Field(nan * u.deg, "reconstructed value for disp", unit=u.deg)
-    is_valid = Field(False, "true if the predictions are valid")
+    parameter = Field(
+        nan * u.deg, "reconstructed value for disp (= sign * norm)", unit=u.deg
+    )
 
 
 class ReconstructedContainer(Container):

diff --git a/ctapipe/reco/sklearn.py b/ctapipe/reco/sklearn.py
@@ -493,7 +493,7 @@ class DispReconstructor(Reconstructor):
     Predict absolute value and sign for disp origin reconstruction for each telescope.
     """
 
-    target = "true_norm"
+    target = "true_disp"
 
     prefix = traits.Unicode(default_value="disp", allow_none=False).tag(config=True)
 
@@ -722,16 +722,15 @@ def __call__(self, event: ArrayEventContainer) -> None:
                     )
             else:
                 disp_container = DispContainer(
-                    norm=u.Quantity(np.nan, self.unit),
-                    is_valid=False,
+                    parameter=u.Quantity(np.nan, self.unit),
                 )
                 altaz_container = ReconstructedGeometryContainer(
                     alt=u.Quantity(np.nan, u.deg, copy=False),
                     az=u.Quantity(np.nan, u.deg, copy=False),
                     is_valid=False,
                 )
 
-            disp_container.prefix = f"{self.prefix}_parameter"
+            disp_container.prefix = f"{self.prefix}_tel"
             altaz_container.prefix = f"{self.prefix}_tel"
             event.dl2.tel[tel_id].disp[self.prefix] = disp_container
             event.dl2.tel[tel_id].geometry[self.prefix] = altaz_container
@@ -763,18 +762,12 @@ def predict_table(self, key, table: Table) -> Dict[ReconstructionProperty, Table
         valid = self.quality_query.get_table_mask(table)
         disp[valid], is_valid[valid] = self._predict(key, table[valid])
 
-        disp_result = Table(
-            {
-                f"{self.prefix}_parameter_norm": disp,
-                f"{self.prefix}_parameter_is_valid": is_valid,
-            }
-        )
+        disp_result = Table({f"{self.prefix}_tel_parameter": disp})
         add_defaults_and_meta(
             disp_result,
             DispContainer,
-            prefix=f"{self.prefix}_parameter",
-            # disp is always per telescope, so no need to add the prefix
-            add_tel_prefix=False,
+            prefix=self.prefix,
+            add_tel_prefix=True,
         )
 
         psi = table["hillas_psi"].quantity.to_value(u.rad)

diff --git a/docs/changes/2443.optimization.rst b/docs/changes/2443.optimization.rst
@@ -0,0 +1,3 @@
+Remove redundant ``is_valid`` field in ``DispContainer`` and rename the remaining field.
+Use the same prefix for both containers filled by ``DispReconstructor``.
+Fix default name of ``DispReconstructor`` target column.