Bug fix/time window exclusions

CONTRIBUTING.md

@@ -22,3 +22,18 @@ You can also manually run the pre-commit on single files or on all files via:
 If you need to commit something even though there are errors (this should not have to be done!), then you can add the flag `--no-verify` to the `git commit` command. This will bypass the pre-commit hooks.
 
 Additional information is provided here: https://pre-commit.com/
+
+
+## Non-backward compatible changes
+
+New contributions to this repository should aim to maintain backwards compatibility, such that
+models trained with earlier version of the software may still be run in later software versions.
+However, this is not always possible. In such cases where breaking changes are required, these
+should be documented in the `__version_compatibility__` dictionary in the `egenerator.__about__`
+file. When loading saved components from disk, this dictionary is utilized to verify compatibility
+of a previously saved model and the current software version. Breaking changes may either be of
+type `global` if they affect all components and trained models of the software, or of type
+`local` in case only certain components are affected. In the latter case, a list of
+`affected_components` must be provided in the corresponding dictionary entry.
+This list contains a list of class strings of each affected component in the event-generator
+software.

egenerator/__about__.py

@@ -4,8 +4,8 @@
 __url__ = "https://github.com/icecube/event-generator"
 
 __version_major__ = 1
-__version_minor__ = 0
-__version_patch__ = 3
+__version_minor__ = 1
+__version_patch__ = 0
 __version_info__ = "-dev"
 
 __version__ = "{}.{}.{}{}".format(
@@ -14,3 +14,43 @@
     __version_patch__,
     __version_info__,
 )
+
+# A dictionary of changes that are not backwards compatible
+# with previous versions. The keys are the versions that
+# contain the breaking changes and the values contain
+# information on the changes that were made.
+# Mandatory keys are:
+#    "type": "global" or "local"
+#        "global" means that the change affects all components
+#        "local" means that the change affects only specific components
+#                and the key "affected_components" must be present.
+#
+# Example:
+# __version_compatibility__ = {
+#     "1.0.0": {
+#         "Description": "Description of the breaking change",
+#         "type": "global",
+#     },
+#     "1.0.1": {
+#         "Description": "Description of the breaking change",
+#         "type": "local",
+#         "affected_components": ["class_string1", "class_string2"],
+#     },
+# }
+__version_compatibility__ = {
+    "1.1.0": {
+        "Description": (
+            "Bugfix: Fixed a bug in the re-normalization for time "
+            "window exclusions. The bug re-normalized the individual "
+            "mixture model components instead of the whole mixture. "
+            "This bug thus lead to changes in the shape of the pulse "
+            "arrival time PDF when exclusions were used. This is now "
+            "fixed to instead re-normalize the whole mixture. "
+            "Older models will have compensated for this effect if "
+            "already trained as a mixture of multiple sources. Thus, "
+            "introducing this bugfix will lead to incompatibilities "
+            "with older models."
+        ),
+        "type": "global",
+    },
+}

egenerator/__init__.py

@@ -1,18 +1,20 @@
 from .__about__ import (
+    __version_compatibility__,
+    __version_info__,
     __version_major__,
     __version_minor__,
     __version_patch__,
-    __version_info__,
     __version__,
     __description__,
     __url__,
 )
 
 __all__ = [
+    "__version_compatibility__",
+    "__version_info__",
     "__version_major__",
     "__version_minor__",
     "__version_patch__",
-    "__version_info__",
     "__version__",
     "__description__",
     "__url__",

egenerator/ic3/visualization.py

@@ -79,6 +79,27 @@ def __init__(self, context):
         self.AddParameter(
             "n_doms_y", "Number of DOMs to plot along y-axis.", 5
         )
+        self.AddParameter(
+            "dom_pdf_kwargs",
+            "Additional keyword arguments passed on to `plot_dom_pdf`.",
+            {},
+        )
+        self.AddParameter(
+            "dom_cdf_kwargs",
+            "Additional keyword arguments passed on to `plot_dom_cdf`.",
+            {},
+        )
+        self.AddParameter(
+            "pdf_file_template",
+            "The file template name to which the PDF will be saved to",
+            "dom_pdf_{run_id:06d}_{event_id:06d}.png",
+        )
+        self.AddParameter(
+            "cdf_file_template",
+            "The file template name to which the CDF will be saved to",
+            "dom_cdf_{run_id:06d}_{event_id:06d}.png",
+        )
+        self.AddParameter("add_event_header", "Add event information.", True)
 
     def Configure(self):
         """Configures Module and loads model from file."""
@@ -92,6 +113,11 @@ def Configure(self):
         self.num_threads = self.GetParameter("num_threads")
         self.n_doms_x = self.GetParameter("n_doms_x")
         self.n_doms_y = self.GetParameter("n_doms_y")
+        self.pdf_file_template = self.GetParameter("pdf_file_template")
+        self.cdf_file_template = self.GetParameter("cdf_file_template")
+        self.dom_pdf_kwargs = self.GetParameter("dom_pdf_kwargs")
+        self.dom_cdf_kwargs = self.GetParameter("dom_cdf_kwargs")
+        self.add_event_header = self.GetParameter("add_event_header")
 
         if isinstance(self.model_names, str):
             self.model_names = [self.model_names]
@@ -158,8 +184,10 @@ def Configure(self):
             output_dir=self.output_dir,
             n_doms_x=self.n_doms_x,
             n_doms_y=self.n_doms_y,
-            pdf_file_template="dom_pdf_{run_id:06d}_{event_id:06d}.png",
-            cdf_file_template="dom_cdf_{run_id:06d}_{event_id:06d}.png",
+            pdf_file_template=self.pdf_file_template,
+            cdf_file_template=self.cdf_file_template,
+            dom_pdf_kwargs=self.dom_pdf_kwargs,
+            dom_cdf_kwargs=self.dom_cdf_kwargs,
         )
 
     def Physics(self, frame):
@@ -176,7 +204,7 @@ def Physics(self, frame):
         assert n == 1, "Currently only 1-event at a time is supported"
 
         # collect event meta data
-        if "I3EventHeader" in frame:
+        if "I3EventHeader" in frame and self.add_event_header:
             header = frame["I3EventHeader"]
             event_header = {
                 "run_id": header.run_id,

egenerator/loss/default.py

@@ -1214,7 +1214,7 @@ def normalized_dom_charge_pdf(
         """
 
         # underneath 5e-5 the log_negative_binomial function becomes unstable
-        eps = 5e-5
+        eps = 1e-7
         dtype = getattr(
             tf, self.configuration.config["config"]["float_precision"]
         )
@@ -1223,13 +1223,6 @@ def normalized_dom_charge_pdf(
         hits_true = tf.squeeze(data_batch_dict["x_dom_charge"], axis=-1)
         hits_pred = tf.squeeze(result_tensors["dom_charges"], axis=-1)
 
-        # shape: [n_batch, 1, 1]
-        event_total = tf.reduce_sum(hits_pred, axis=[1, 2], keepdims=True)
-
-        # shape: [n_batch, 86, 60]
-        dom_pdf = hits_pred / event_total
-        llh_dom = hits_true * tf.math.log(dom_pdf + eps)
-
         # throw error if this is being used with time window exclusions
         # one needs to calculate cumulative pdf from exclusion window and
         # scale up the pulse pdf by this factor
@@ -1242,6 +1235,8 @@ def normalized_dom_charge_pdf(
             ), "Model must deal with time exclusions!"
 
         # mask out dom exclusions
+        # Note that this needs to be done prior to computing `event_total`
+        # such that the PDF is properly normalized over active DOMs
         if (
             "x_dom_exclusions" in tensors.names
             and tensors.list[tensors.get_index("x_dom_exclusions")].exists
@@ -1250,7 +1245,15 @@ def normalized_dom_charge_pdf(
                 tf.squeeze(data_batch_dict["x_dom_exclusions"], axis=-1),
                 dtype=dtype,
             )
-            llh_dom = llh_dom * mask_valid
+            hits_true = hits_true * mask_valid
+            hits_pred = hits_pred * mask_valid
+
+        # shape: [n_batch, 1, 1]
+        event_total = tf.reduce_sum(hits_pred, axis=[1, 2], keepdims=True)
+
+        # shape: [n_batch, 86, 60]
+        dom_pdf = hits_pred / event_total
+        llh_dom = hits_true * tf.math.log(dom_pdf + eps)
 
         if sort_loss_terms:
             loss_terms = [

egenerator/manager/component.py

@@ -983,6 +983,60 @@ def load(self, dir_path, modified_sub_components={}, **kwargs):
                 )
             )
 
+            # check if the saved component was made with a newer version
+            # than the one currently used
+            if version_control.is_newer_version(
+                version_base=egenerator.__version__,
+                version_test=config_dict["event_generator_version"],
+            ):
+                msg = (
+                    "The saved component was created with a newer version of "
+                    "Event-Generator. Make sure the component is still "
+                    "compatible with this version!"
+                )
+                self._logger.error(msg)
+
+            # go through compatibility changes since the saved version
+            for version, info in egenerator.__version_compatibility__.items():
+                is_newer = version_control.is_newer_version(
+                    version_base=config_dict["event_generator_version"],
+                    version_test=version,
+                )
+
+                # check if this version is compatible
+                if is_newer:
+                    if info["type"] == "global":
+                        msg = "A global change was made in "
+                        msg += "Event-Generator version {!r} leading to "
+                        msg += "incompatibility with the version of this model {!r}."
+                        msg = msg.format(
+                            version,
+                            config_dict["event_generator_version"],
+                        )
+                        self._logger.fatal(msg)
+                        raise ValueError(msg)
+                    elif info["type"] == "local":
+                        if (
+                            self.configuration.class_string
+                            in info["affected_components"]
+                        ):
+                            msg = "A local change was made to the component {!r} in "
+                            msg += "Event-Generator version {!r} leading to "
+                            msg += "incompatibility with the version of this model {!r}."
+                            msg = msg.format(
+                                self.configuration.class_string,
+                                version,
+                                config_dict["event_generator_version"],
+                            )
+                            self._logger.fatal(msg)
+                            raise ValueError(msg)
+                    else:
+                        raise KeyError(
+                            "Unknown type of compatibility change: {}.".format(
+                                info["type"]
+                            )
+                        )
+
         # check if this is the correct class
         if (
             self.configuration.class_string