Following the discussion in slack:
vfdev :
In some sense currently we have probably a protection as metrics are computed in
evaluator that calls with torch.no_grad() so there is no graph built
even after in metric's update methods. However in general case we need
to reconsider that...
Jason Kriss :
Ah yea, I guess since we do support accumulating metrics on a trainer (and not just an evaluator) we need to handle this.
I must have missed this discussion, can one of you briefly describe the problem + the proposed solution?
cc @vfdev-5 @jasonkriss
@alykhantejani I was just wondering whether we should apply detach to y_pred in metrics before computing some cumulative variables. I was trying to add EpochMetric and when y_pred are cumulated without detaching we update the graph etc..
Just to add on to what @vfdev-5 is saying... this really only applies when calculating metrics in a training engine (where there is no call to torch.no_grad).
Is help wanted for this issue? Can I help?
Is help wanted for this issue? Can I help?
@anmolsjoshi Thanks ! Yes, you can go ahead with this issue. The idea is to incorporate torch.no_grad somewhere in Metrics a "less intrusive" manner.
EDIT: Disregard comment below.
@vfdev-5 I wanted to clarify a few things before I got started.
Is this needed for situations where metrics are calculated on each iteration during the training process? And not at the end of an epoch as shown here
If so, do you have an example of using Metrics in that way?
@vfdev-5 I tried a few different methods to incorporate torch.no_grad in Metrics. I thought about creating a new function in Metrics, which detached y_pred, it would work as shown below.
def iteration_completed(self, engine):
output = self._detach_output(self._output_transform(engine.state.output))
self.update(output)
def detach_output(self, output):
y_pred, y = output
y_pred = y_pred.detach()
return y_pred, y
Any thoughts?
This method worked well, but I'm still going to look into other options.
@anmolsjoshi i think you can try to use decorator torch.no_grad() on update metric function... https://pytorch.org/docs/stable/autograd.html#locally-disable-grad
And check which metric class really accumulates tensors with grads.
@anmolsjoshi nice example! Actually, I'm not against applying @torch.no_grad at every derived class:
from abc import ABCMeta, abstractmethod
import torch
class Metric(object):
__metaclass__ = ABCMeta
def __init__(self):
self.num = 0
@abstractmethod
def update(self, output):
pass
class ExampleMetric(Metric):
@torch.no_grad()
def update(self, output):
y_pred, y = output
mul = 2*y_pred
print (mul)
y = torch.zeros(4, requires_grad=False)
y_pred = torch.zeros(4, requires_grad=True)
outputs = [y_pred, y]
metric = ExampleMetric()
metric.update(outputs)
you're right update is overridden and base method is not called and decorator is not applied.
However as we really use iteration_completed to update metrics we can put the decorator over this method:
from ignite.engine import State
from abc import ABCMeta, abstractmethod
import torch
class Metric(object):
__metaclass__ = ABCMeta
def __init__(self, output_transform=lambda x: x):
self._output_transform = output_transform
@abstractmethod
def update(self, output):
pass
@torch.no_grad()
def iteration_completed(self, engine):
output = self._output_transform(engine.state.output)
self.update(output)
class ExampleMetric(Metric):
def update(self, output):
y_pred, y = output
mul = 2*y_pred
print (mul)
y = torch.zeros(4, requires_grad=False)
y_pred = torch.zeros(4, requires_grad=True)
outputs = [y_pred, y]
engine = Engine(lambda e, b: None)
engine.state = State()
engine.state.output = outputs
metric = ExampleMetric()
metric.iteration_completed(engine)
@vfdev-5 thanks for pointing that out! Sorry I'm a novice, trying to learn more!
Not that I wouldn't mind sending a commit of something like this, but since you did the leg work for this (above example), did you want to take over this issue/commit?
@anmolsjoshi no, I let you submit a PR with the code and with tests, please, if you do not mind :)
@vfdev-5 thanks for the opportunity!
Closed by #236