stared / livelossplot

Live training loss plot in Jupyter Notebook for Keras, PyTorch and others
https://p.migdal.pl/livelossplot
MIT License
1.29k stars 142 forks source link

Parsing of loss name raises a TypeError when loss is a dict #27

Closed AlFontal closed 5 years ago

AlFontal commented 5 years ago

I am working in a multitask model where I am passing a dictionary of losses and metrics for each task in model_compile(). It is a mixture of classification and regression tasks so I use custom variants of MSE and binary crossentropy. There is no issue when I pass each of these custom losses in a model as a single value, but I get an error when I try to use your PlotLossesKeras as callback and a dictionary of losses.

I checked the code and I think I found what causes the issue in keras_plot.py:

 if isinstance(self.model.loss, list):
    losses = self.model.loss
elif isinstance(self.model.loss, dict):
    losses = self.model.loss.values()
else:
# by far the most common scenario
    losses = [self.model.loss]

loss_name = loss2name(losses[0])

The last line raises a type error when losses is of type dict_values since it does not support indexing. In any case, the purpose of this line is to get a name for the general loss, which at least in my case (and I believe in most cases where a dictionary of losses will be passed) does not really make sense and a generic term like 'loss' would make more sense. (In my case this value is the equivalent of the sum of all the individual losses of each task).

My solution was just to change this line to:

loss_name = loss2name(losses[0]) if isinstance(losses, list) else 'loss'

PD: Your little package is great, I have been using for a good while and it adds a neat functionality to the networks I train in my jupyter notebooks!

AlFontal commented 5 years ago

Never mind, I just realized that I am using an old version and that the TypeError has been solved by doing:

elif isinstance(self.model.loss, dict):
    losses = list(self.model.loss.values())

The name of the global loss is still taken from the first value though, which although a very minor issue, is quite misleading in cases like mine.

stared commented 5 years ago

@AlFontal Thanks for bringing this issue., and sorry for my late reply. Does it work right now (as of v0.2.3)?