Improve column names for disp

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):

ctapipe/io/datawriter.py

@@ -48,8 +48,11 @@ def _get_tel_index(event, tel_id):
 #   (meaning readers need to update scripts)
 # - increase the minor number if new columns or datasets are added
 # - increase the patch number if there is a small bugfix to the model.
-DATA_MODEL_VERSION = "v5.0.0"
+DATA_MODEL_VERSION = "v5.1.0"
 DATA_MODEL_CHANGE_HISTORY = """
+- v5.1.0: - Remove redundant 'is_valid' column in ``DispContainer``.
+          - Rename content of ``DispContainer`` from 'norm' to 'parameter' and use the same
+            default prefix ('disp') for all containers filled by ``DispReconstructor``.
 - v5.0.0: - Change DL2 telescope-wise container prefixes from {algorithm}_tel to {algorithm}_tel_{kind}.
             As of now, this only changes 'tel_distance' to 'tel_impact_distance'
 - v4.0.0: - Changed how ctapipe-specific metadata is stored in hdf5 attributes.
@@ -58,8 +61,8 @@ def _get_tel_index(event, tel_id):
             and true parameters.
           - Telescope Impact Parameters were added.
           - Effective focal length and nominal focal length are both included
-            in the optics description now. Moved `TelescopeDescription.type`
-            to `OpticsDescription.size_type`. Added `OpticsDescription.reflector_shape`.
+            in the optics description now. Moved ``TelescopeDescription.type``
+            to ``OpticsDescription.size_type``. Added ``OpticsDescription.reflector_shape``.
           - n_samples, n_samples_long, n_channels and n_pixels are now part
             of CameraReadout.
           - The reference_location (EarthLocation origin of the telescope coordinates)

ctapipe/io/hdf5eventsource.py

@@ -16,6 +16,7 @@
     CameraHillasParametersContainer,
     CameraTimingParametersContainer,
     ConcentrationContainer,
+    DispContainer,
     DL1CameraContainer,
     EventIndexContainer,
     HillasParametersContainer,
@@ -64,12 +65,14 @@
     "geometry": ReconstructedGeometryContainer,
     "classification": ParticleClassificationContainer,
     "impact": TelescopeImpactParameterContainer,
+    "disp": DispContainer,
 }
 
 
 COMPATIBLE_DATA_MODEL_VERSIONS = [
     "v4.0.0",
     "v5.0.0",
+    "v5.1.0",
 ]
 
 

ctapipe/io/tests/test_hdf5eventsource.py

@@ -28,7 +28,7 @@ def test_is_compatible(compatible_file, request):
 def test_metadata(dl1_file):
     with HDF5EventSource(input_url=dl1_file) as source:
         assert source.is_simulation
-        assert source.datamodel_version == (5, 0, 0)
+        assert source.datamodel_version == (5, 1, 0)
         assert set(source.datalevels) == {
             DataLevel.DL1_IMAGES,
             DataLevel.DL1_PARAMETERS,

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)
 
@@ -690,12 +690,10 @@ def __call__(self, event: ArrayEventContainer) -> None:
 
             if passes_quality_checks:
                 disp, valid = self._predict(self.subarray.tel[tel_id], table)
-                disp_container = DispContainer(
-                    norm=disp[0],
-                    is_valid=valid[0],
-                )
 
                 if valid:
+                    disp_container = DispContainer(parameter=disp[0])
+
                     hillas = event.dl1.tel[tel_id].parameters.hillas
                     psi = hillas.psi.to_value(u.rad)
 
@@ -715,23 +713,25 @@ def __call__(self, event: ArrayEventContainer) -> None:
                     )
 
                 else:
+                    disp_container = DispContainer(
+                        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,
                     )
             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 +763,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)

ctapipe/tools/tests/test_apply_models.py

@@ -208,9 +208,8 @@ def test_apply_all(
         assert f"{prefix_disp}_tel_alt" in tel_events.colnames
         assert f"{prefix_disp}_tel_az" in tel_events.colnames
         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
+        assert f"{prefix_disp}_tel_parameter" in tel_events.colnames
+        assert f"{prefix_disp}_parameter" not in tel_events.colnames
 
         # check that the "--no-dl1-parameters" option worked
         assert "hillas_intensity" not in tel_events.colnames

ctapipe/tools/tests/test_process_ml.py

@@ -161,8 +161,7 @@ def test_process_apply_disp(
     run_tool(tool, argv=argv, cwd=tmp_path, raises=True)
 
     tel_events = read_table(output, "/dl2/event/telescope/disp/disp/tel_004")
-    assert "disp_parameter_norm" in tel_events.colnames
-    assert "disp_parameter_is_valid" in tel_events.colnames
+    assert "disp_tel_parameter" in tel_events.colnames
 
     tel_events = read_table(output, "/dl2/event/telescope/geometry/disp/tel_004")
     assert "disp_tel_alt" in tel_events.colnames

docs/changes/2443.datamodel.rst

@@ -0,0 +1,6 @@
+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.
+
+Let ``HDF5EventSource`` load ``DispContainer``.