pytorch#816 detach metric (pytorch#827)

* add detach() for metric * add test for detach() * add is_attached() * update tests * Update metric.py * Update test_metric.py * Update metric.py * selfregister of lambda in metrics for detach issue * improvment : empty _child, fill _child when attach * replace child impl by weakref * remove debug * fix bug * dont detach - weakref engine is only used for is_detach() * Update ignite/metrics/metrics_lambda.py * Update ignite/metrics/metrics_lambda.py Co-Authored-By: vfdev <vfdev.5@gmail.com> * Update ignite/metrics/metrics_lambda.py Co-Authored-By: vfdev <vfdev.5@gmail.com> * replace weakref * docs improvment * Update metrics_lambda.py * Update metric.py Co-authored-by: Desroziers <sylvain.desroziers@ifpen.fr> Co-authored-by: vfdev <vfdev.5@gmail.com>
cajanond · Mar 17, 2020 · 59ca2f8 · 59ca2f8
1 parent 981e9c4
commit 59ca2f8
Show file tree

Hide file tree

Showing 4 changed files with 170 additions and 3 deletions.
diff --git a/ignite/metrics/metric.py b/ignite/metrics/metric.py
@@ -142,12 +142,70 @@ def completed(self, engine: Engine, name: str) -> None:
         engine.state.metrics[name] = result
 
     def attach(self, engine: Engine, name: str) -> None:
+        """
+        Attaches current metric to provided engine. On the end of engine's run,
+        `engine.state.metrics` dictionary will contain computed metric's value under provided name.
+
+        Args:
+            engine (Engine): the engine to which the metric must be attached
+            name (str): the name of the metric to attach
+
+        Example:
+
+        .. code-block:: python
+
+            metric = ...
+            metric.attach(engine, "mymetric")
+
+            assert "mymetric" in engine.run(data).metrics
+
+            assert metric.is_attached(engine)
+        """
         engine.add_event_handler(Events.EPOCH_COMPLETED, self.completed, name)
         if not engine.has_event_handler(self.started, Events.EPOCH_STARTED):
             engine.add_event_handler(Events.EPOCH_STARTED, self.started)
         if not engine.has_event_handler(self.iteration_completed, Events.ITERATION_COMPLETED):
             engine.add_event_handler(Events.ITERATION_COMPLETED, self.iteration_completed)
 
+    def detach(self, engine: Engine) -> None:
+        """
+        Detaches current metric from the engine and no metric's computation is done during the run.
+        This method in conjunction with :meth:`~ignite.metrics.Metric.attach` can be useful if several
+        metrics need to be computed with different periods. For example, one metric is computed every training epoch
+        and another metric (e.g. more expensive one) is done every n-th training epoch.
+
+        Args:
+            engine (Engine): the engine from which the metric must be detached
+
+        Example:
+
+        .. code-block:: python
+
+            metric = ...
+            engine = ...
+            metric.detach(engine)
+
+            assert "mymetric" not in engine.run(data).metrics
+
+            assert not metric.is_attached(engine)
+        """
+        if engine.has_event_handler(self.completed, Events.EPOCH_COMPLETED):
+            engine.remove_event_handler(self.completed, Events.EPOCH_COMPLETED)
+        if engine.has_event_handler(self.started, Events.EPOCH_STARTED):
+            engine.remove_event_handler(self.started, Events.EPOCH_STARTED)
+        if engine.has_event_handler(self.iteration_completed, Events.ITERATION_COMPLETED):
+            engine.remove_event_handler(self.iteration_completed, Events.ITERATION_COMPLETED)
+
+    def is_attached(self, engine: Engine) -> bool:
+        """
+        Checks if current metric is attached to provided engine. If attached, metric's computed
+        value is written to `engine.state.metrics` dictionary.
+
+        Args:
+            engine (Engine): the engine checked from which the metric should be attached
+        """
+        return engine.has_event_handler(self.completed, Events.EPOCH_COMPLETED)
+
     def __add__(self, other):
         from ignite.metrics import MetricsLambda
 

diff --git a/ignite/metrics/metrics_lambda.py b/ignite/metrics/metrics_lambda.py
@@ -15,8 +15,8 @@ class MetricsLambda(Metric):
 
     When update, this metric does not recursively update the metrics
     it depends on. When reset, all its dependency metrics would be
-    resetted. When attach, all its dependencies would be automatically
-    attached.
+    resetted. When attach, all its dependency metrics would be attached
+    automatically (but partially, e.g `is_attached()` will return False).
 
     Args:
         f (callable): the function that defines the computation
@@ -37,12 +37,36 @@ def Fbeta(r, p, beta):
         F2 = MetricsLambda(Fbeta, recall, precision, 2)
         F3 = MetricsLambda(Fbeta, recall, precision, 3)
         F4 = MetricsLambda(Fbeta, recall, precision, 4)
+
+    When check if the metric is attached, if one of its dependency
+    metrics is detached, the metric is considered detached too.
+
+    .. code-block:: python
+
+        engine = ...
+        precision = Precision(average=False)
+
+        aP = precision.mean()
+
+        aP.attach(engine, "aP")
+
+        assert aP.is_attached(engine)
+        # partially attached
+        assert not precision.is_attached(engine)
+
+        precision.detach(engine)
+
+        assert not aP.is_attached(engine)
+        # fully attached
+        assert not precision.is_attached(engine)
+
     """
 
     def __init__(self, f: Callable, *args, **kwargs):
         self.function = f
         self.args = args
         self.kwargs = kwargs
+        self.engine = None
         super(MetricsLambda, self).__init__(device="cpu")
 
     @reinit__is_reduced
@@ -64,17 +88,46 @@ def compute(self) -> Any:
         return self.function(*materialized, **materialized_kwargs)
 
     def _internal_attach(self, engine: Engine) -> None:
+        self.engine = engine
         for index, metric in enumerate(itertools.chain(self.args, self.kwargs.values())):
             if isinstance(metric, MetricsLambda):
                 metric._internal_attach(engine)
             elif isinstance(metric, Metric):
+                # NB : metrics is attached partially
+                # We must not use is_attached() but rather if these events exist
                 if not engine.has_event_handler(metric.started, Events.EPOCH_STARTED):
                     engine.add_event_handler(Events.EPOCH_STARTED, metric.started)
                 if not engine.has_event_handler(metric.iteration_completed, Events.ITERATION_COMPLETED):
                     engine.add_event_handler(Events.ITERATION_COMPLETED, metric.iteration_completed)
 
     def attach(self, engine: Engine, name: str) -> None:
-        # recursively attach all its dependencies
+        # recursively attach all its dependencies (partially)
         self._internal_attach(engine)
         # attach only handler on EPOCH_COMPLETED
         engine.add_event_handler(Events.EPOCH_COMPLETED, self.completed, name)
+
+    def detach(self, engine: Engine) -> None:
+        # remove from engine
+        super(MetricsLambda, self).detach(engine)
+        self.engine = None
+
+    def is_attached(self, engine: Engine) -> bool:
+        # check recursively the dependencies
+        return super(MetricsLambda, self).is_attached(engine) and self._internal_is_attached(engine)
+
+    def _internal_is_attached(self, engine: Engine) -> bool:
+        # if no engine, metrics is not attached
+        if engine is None:
+            return False
+        # check recursively if metrics are attached
+        is_detached = False
+        for metric in itertools.chain(self.args, self.kwargs.values()):
+            if isinstance(metric, MetricsLambda):
+                if not metric._internal_is_attached(engine):
+                    is_detached = True
+            elif isinstance(metric, Metric):
+                if not engine.has_event_handler(metric.started, Events.EPOCH_STARTED):
+                    is_detached = True
+                if not engine.has_event_handler(metric.iteration_completed, Events.ITERATION_COMPLETED):
+                    is_detached = True
+        return not is_detached
diff --git a/tests/ignite/metrics/test_metric.py b/tests/ignite/metrics/test_metric.py
@@ -312,6 +312,43 @@ def process_function(*args, **kwargs):
     assert m2.compute_count == 10
     assert m2.update_count == 50
 
+    assert m1.is_attached(engine)
+    assert m2.is_attached(engine)
+
+
+def test_detach():
+    class DummyMetric(Metric):
+        _required_output_keys = None
+
+        def reset(self):
+            pass
+
+        def compute(self):
+            pass
+
+        def update(self, output):
+            pass
+
+    def process_function(*args, **kwargs):
+        return 1
+
+    engine = Engine(process_function)
+    m1 = DummyMetric()
+    m2 = DummyMetric()
+    m1.attach(engine, "m1")
+    m2.attach(engine, "m2_1")
+    m2.attach(engine, "m2_2")
+    m1.detach(engine)
+    m2.detach(engine)
+    engine.run(range(10), 5)
+
+    assert 'm1' not in engine.state.metrics
+    assert 'm2_1' not in engine.state.metrics
+    assert 'm2_2' not in engine.state.metrics
+
+    assert not m1.is_attached(engine)
+    assert not m2.is_attached(engine)
+
 
 def test_integration():
     np.random.seed(1)

diff --git a/tests/ignite/metrics/test_metrics_lambda.py b/tests/ignite/metrics/test_metrics_lambda.py
@@ -52,6 +52,25 @@ def plus(this, other):
     assert engine.state.metrics["m0_plus_m1"] == 22
     assert engine.state.metrics["m2_plus_2"] == 202
 
+    # metrics are partially attached
+    assert not m0.is_attached(engine)
+    assert not m1.is_attached(engine)
+    assert not m2.is_attached(engine)
+
+    # a dependency is detached
+    m0.detach(engine)
+    # so the lambda metric is too
+    assert not m0_plus_m1.is_attached(engine)
+    # the lambda is attached again
+    m0_plus_m1.attach(engine, "m0_plus_m1")
+    assert m0_plus_m1.is_attached(engine)
+    # metrics are always partially attached
+    assert not m0.is_attached(engine)
+    m0_plus_m1.detach(engine)
+    assert not m0_plus_m1.is_attached(engine)
+    # detached (and no longer partially attached)
+    assert not m0.is_attached(engine)
+
 
 def test_metrics_lambda_reset():
     m0 = ListGatherMetric(0)