diff --git a/ctapipe/containers.py b/ctapipe/containers.py index 3fbb88a762c..d94f39eea99 100644 --- 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/io/datawriter.py b/ctapipe/io/datawriter.py index 3a5cc8f00dd..398e4acbb2f 100644 --- a/ctapipe/io/datawriter.py +++ b/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) diff --git a/ctapipe/io/hdf5eventsource.py b/ctapipe/io/hdf5eventsource.py index c7bda1f9b7d..9b7e533d06e 100644 --- a/ctapipe/io/hdf5eventsource.py +++ b/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", ] diff --git a/ctapipe/io/tests/test_hdf5eventsource.py b/ctapipe/io/tests/test_hdf5eventsource.py index bae1eb1359b..d64e2ed7902 100644 --- a/ctapipe/io/tests/test_hdf5eventsource.py +++ b/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, diff --git a/ctapipe/reco/sklearn.py b/ctapipe/reco/sklearn.py index 1ae0bc78158..8d5d7fe9bd9 100644 --- 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) @@ -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,6 +713,9 @@ 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), @@ -722,8 +723,7 @@ 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), @@ -731,7 +731,7 @@ def __call__(self, event: ArrayEventContainer) -> None: 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) diff --git a/ctapipe/tools/tests/test_apply_models.py b/ctapipe/tools/tests/test_apply_models.py index 32b0e4bcc86..1075cb13bc3 100644 --- a/ctapipe/tools/tests/test_apply_models.py +++ b/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 diff --git a/ctapipe/tools/tests/test_process_ml.py b/ctapipe/tools/tests/test_process_ml.py index a4fb75d428d..6a25f503aa5 100644 --- a/ctapipe/tools/tests/test_process_ml.py +++ b/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 diff --git a/docs/changes/2443.datamodel.rst b/docs/changes/2443.datamodel.rst new file mode 100644 index 00000000000..0d97889347e --- /dev/null +++ b/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``.