From 932b50d6bad850d54d4728abbc7dce4b760194c2 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Thu, 16 Feb 2023 18:50:43 +0100 Subject: [PATCH 1/8] Fix wrong activity meta being written into output files --- ctapipe/core/provenance.py | 5 ++--- ctapipe/io/datawriter.py | 13 +++++++++++-- ctapipe/io/metadata.py | 2 ++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ctapipe/core/provenance.py b/ctapipe/core/provenance.py index da62afa59ca..5128f0c1188 100644 --- a/ctapipe/core/provenance.py +++ b/ctapipe/core/provenance.py @@ -156,9 +156,8 @@ def activity(self, name): @property def current_activity(self): if len(self._activities) == 0: - log.debug("No activity has been started... starting a default one") - self.start_activity() - return self._activities[-1] # current activity as at the top of stack + return None + return self._activities[-1] # current activity is at the top of stack @property def finished_activities(self): diff --git a/ctapipe/io/datawriter.py b/ctapipe/io/datawriter.py index a3b9ad350a4..2ee1c9e16b3 100644 --- a/ctapipe/io/datawriter.py +++ b/ctapipe/io/datawriter.py @@ -109,7 +109,16 @@ def write_reference_metadata_headers( instrument_info: meta.Instrument instrument metadata """ - activity = PROV.current_activity.provenance + activity = PROV.current_activity + if activity is None and len(PROV.finished_activities) > 0: + # assume that we write provenance for a "just finished activity" + activity = PROV.finished_activities[-1] + + if activity is not None: + activity_meta = meta.Activity.from_provenance(activity.provenance) + else: + activity_meta = None + category = "Sim" if is_simulation else "Other" reference = meta.Reference( @@ -129,7 +138,7 @@ def write_reference_metadata_headers( subtype="", id_=",".join(str(x) for x in obs_ids), ), - activity=meta.Activity.from_provenance(activity), + activity=activity_meta, instrument=instrument_info, ) diff --git a/ctapipe/io/metadata.py b/ctapipe/io/metadata.py index b8491c17382..ee0a8018ce5 100644 --- a/ctapipe/io/metadata.py +++ b/ctapipe/io/metadata.py @@ -149,6 +149,7 @@ def from_provenance(cls, activity): type_="software", id_=activity["activity_uuid"], start_time=activity["start"]["time_utc"], + stop_time=activity["stop"]["time_utc"], software_name="ctapipe", software_version=activity["system"]["ctapipe_version"], ) @@ -157,6 +158,7 @@ def from_provenance(cls, activity): type_ = Unicode("software") id_ = Unicode() start_time = AstroTime() + stop_time = AstroTime() software_name = Unicode("unknown") software_version = Unicode("unknown") From 2133c70cadce352dd5ab0bd8ccc4227c38991630 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Fri, 10 Mar 2023 12:17:15 +0100 Subject: [PATCH 2/8] Start new activity if needed in add_{input,output,config} --- ctapipe/core/provenance.py | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/ctapipe/core/provenance.py b/ctapipe/core/provenance.py index 5128f0c1188..35f8dc6ecc6 100644 --- a/ctapipe/core/provenance.py +++ b/ctapipe/core/provenance.py @@ -86,6 +86,16 @@ def start_activity(self, activity_name=sys.executable): self._activities.append(activity) log.debug(f"started activity: {activity_name}") + def _get_current_or_start_activity(self): + if self.current_activity is None: + log.warning( + "No activity has been started yet." + " Provenance().start_activity() should be called explicitly." + ) + self.start_activity() + + return self.current_activity + def add_input_file(self, filename, role=None): """register an input to the current activity @@ -96,11 +106,12 @@ def add_input_file(self, filename, role=None): role: str role this input file satisfies (optional) """ - self.current_activity.register_input(abspath(filename), role=role) + activity = self._get_current_or_start_activity() + activity.register_input(abspath(filename), role=role) log.debug( - "added input entity '{}' to activity: '{}'".format( - filename, self.current_activity.name - ) + "added input entity '%s' to activity: '%s'", + filename, + activity.name, ) def add_output_file(self, filename, role=None): @@ -115,11 +126,12 @@ def add_output_file(self, filename, role=None): role this output file satisfies (optional) """ - self.current_activity.register_output(abspath(filename), role=role) + activity = self._get_current_or_start_activity() + activity.register_output(abspath(filename), role=role) log.debug( - "added output entity '{}' to activity: '{}'".format( - filename, self.current_activity.name - ) + "added output entity '%s' to activity: '%s'", + filename, + activity.name, ) def add_config(self, config): @@ -131,7 +143,13 @@ def add_config(self, config): config: dict configuration parameters """ - self.current_activity.register_config(config) + activity = self._get_current_or_start_activity() + activity.register_config(config) + log.debug( + "added config entity '%s' to activity: '%s'", + config, + activity.name, + ) def finish_activity(self, status="completed", activity_name=None): """end the current activity""" From 9554c46f87202e29357d3d3eaefa7670f134e1fd Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Fri, 10 Mar 2023 13:35:27 +0100 Subject: [PATCH 3/8] Insert current time as stop time when not available --- ctapipe/io/metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ctapipe/io/metadata.py b/ctapipe/io/metadata.py index ee0a8018ce5..4da4d6df0ef 100644 --- a/ctapipe/io/metadata.py +++ b/ctapipe/io/metadata.py @@ -149,7 +149,7 @@ def from_provenance(cls, activity): type_="software", id_=activity["activity_uuid"], start_time=activity["start"]["time_utc"], - stop_time=activity["stop"]["time_utc"], + stop_time=activity["stop"].get("time_utc", Time.now()), software_name="ctapipe", software_version=activity["system"]["ctapipe_version"], ) From a66bf3b0059ca97345fbeaa027a11455dbb38a6c Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Fri, 10 Mar 2023 13:48:40 +0100 Subject: [PATCH 4/8] Start activity in ctapipe-info --- ctapipe/tools/info.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ctapipe/tools/info.py b/ctapipe/tools/info.py index 9dcb2e2e920..66b28ba97c8 100644 --- a/ctapipe/tools/info.py +++ b/ctapipe/tools/info.py @@ -215,6 +215,7 @@ def _info_system(): print("\n*** ctapipe system environment ***\n") prov = Provenance() + prov.start_activity("ctapipe-info") system_prov = prov.current_activity.provenance["system"] for section in ["platform", "python"]: From 926e46c40d7f03a8bf845b7b7c26e822ffd04805 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Fri, 10 Mar 2023 15:21:32 +0100 Subject: [PATCH 5/8] Make stop_time optional --- ctapipe/io/metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ctapipe/io/metadata.py b/ctapipe/io/metadata.py index 4da4d6df0ef..b053da67998 100644 --- a/ctapipe/io/metadata.py +++ b/ctapipe/io/metadata.py @@ -158,7 +158,7 @@ def from_provenance(cls, activity): type_ = Unicode("software") id_ = Unicode() start_time = AstroTime() - stop_time = AstroTime() + stop_time = AstroTime(allow_none=True, default_value=None) software_name = Unicode("unknown") software_version = Unicode("unknown") From 929c792221203bbcbfcf3e791281fa8864dd35d9 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Fri, 10 Mar 2023 15:27:09 +0100 Subject: [PATCH 6/8] Add changelog entry --- docs/changes/2261.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changes/2261.bugfix.rst diff --git a/docs/changes/2261.bugfix.rst b/docs/changes/2261.bugfix.rst new file mode 100644 index 00000000000..19585d42470 --- /dev/null +++ b/docs/changes/2261.bugfix.rst @@ -0,0 +1 @@ +Ensure the correct activity metadata is written into output files. From 46940e61ca5eb20aeabcd204c4ef1e36950f9594 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Fri, 10 Mar 2023 17:35:37 +0100 Subject: [PATCH 7/8] Remove non-needed None check --- ctapipe/io/datawriter.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ctapipe/io/datawriter.py b/ctapipe/io/datawriter.py index 2ee1c9e16b3..d2411c9dd9b 100644 --- a/ctapipe/io/datawriter.py +++ b/ctapipe/io/datawriter.py @@ -114,13 +114,8 @@ def write_reference_metadata_headers( # assume that we write provenance for a "just finished activity" activity = PROV.finished_activities[-1] - if activity is not None: - activity_meta = meta.Activity.from_provenance(activity.provenance) - else: - activity_meta = None - + activity_meta = meta.Activity.from_provenance(activity.provenance) category = "Sim" if is_simulation else "Other" - reference = meta.Reference( contact=contact_info, product=meta.Product( From 21a1727d8acbb75e9120207a0313a52308ed83d9 Mon Sep 17 00:00:00 2001 From: Maximilian Linhoff Date: Wed, 15 Mar 2023 13:46:06 +0100 Subject: [PATCH 8/8] Change warning to info for auto-started acitvity --- ctapipe/core/provenance.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ctapipe/core/provenance.py b/ctapipe/core/provenance.py index 35f8dc6ecc6..312f43884ee 100644 --- a/ctapipe/core/provenance.py +++ b/ctapipe/core/provenance.py @@ -88,9 +88,9 @@ def start_activity(self, activity_name=sys.executable): def _get_current_or_start_activity(self): if self.current_activity is None: - log.warning( - "No activity has been started yet." - " Provenance().start_activity() should be called explicitly." + log.info( + "No activity has been explicitly started, starting new default activity." + " Consider calling Provenance().start_activity() explicitly." ) self.start_activity()