Add missing value support to SoftmaxLossLayer

[longjon]

This PR adds an option, missing_value, which allows some specified label to indicate that loss should not be computed at the corresponding location.

This creates an issue for normalization. To match the existing normalization, we should normalize by the number of labels actually present. However, this means that the weight of each label depends on the number of missing values in each batch, which may be undesirable. For this reason an extra boolean, normalize, is added; if normalize is false, normalization occurs only over the batch size.

Ideally this would be done in a way that can be used uniformly across different loss layers, but this is the form it exists in now.

Not yet well tested.

[longjon]

This is currently passing, but there were some odd nondeterministic failures in the CPU_ONLY/CMake case, see: https://s3.amazonaws.com/archive.travis-ci.org/jobs/45421159/log.txt. No idea what's up with that.

[bhack]

@longjon What kind of failures? Do you mean tests on db?

[longjon]

@bhack oops, the link isn't to the failed test, but updated to the passing test... NetTest was failing nondeterministically. This was due to a bug that should be fixed now (an uninitialized member variable).

I also noticed some test sketchiness while debugging this, will make another PR.

[sguada]

I don't think this is safe, since the parameter is optional and doesn't have default value

[longjon]

Yes, it's safe, see https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#fields and https://developers.google.com/protocol-buffers/docs/proto#optional. It might be a good idea to add a comment to make this clear.

[sguada]

For int the default value is 0

Sergio

2014-12-30 13:56 GMT-08:00 Jon Long notifications@github.com:

In src/caffe/layers/softmax_loss_layer.cpp
#1654 (diff):

@@ -17,6 +17,11 @@ void SoftmaxWithLossLayer::LayerSetUp(
softmax_top_vec_.clear();
softmax_top_vec_.push_back(&prob_);
softmax_layer_->SetUp(softmax_bottom_vec_, softmax_top_vec_);
+

• has_missing_values_ =
• this->layer_param_.softmax_loss_param().has_missing_value();
• missing_value_ = this->layer_param_.softmax_loss_param().missing_value();

Yes, it's safe, see
https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#fields
and https://developers.google.com/protocol-buffers/docs/proto#optional.
It might be a good idea to add a comment to make this clear.

—
Reply to this email directly or view it on GitHub
https://github.com/BVLC/caffe/pull/1654/files#r22366612.

[longjon]

Yes, I know. has_missing_values_ gets set to false, and missing_value_ gets set to zero, but that value has no effect. Anyway I think I'll just add the unnecessary check, since it's less of a mental speed bump.

[sguada]

@longjon I think it will be great if we could define a typical default -1 for missing_value (better called ignore_label), that could be used for other losses including Multi-Label

[longjon]

I agree that ignore_label is a better, more descriptive name; I've updated accordingly. I don't agree that -1 labels should be ignored by default; I think it's better to ignore nothing by default, and be explicit when ignoring certain labels. Erroneously providing -1 is not terribly hard to imagine; e.g., this is exactly what happened before #1661 which I just posted. Being explicit also means that nets are better self-documented, and avoids "magic numbers".

[sguada]

What do you think about renaming the new param softmax_loss_param to ignore_label_param that way could be used by other the Loss layers? I used a fixed value to ignore labels in #523, but this will be better.

[longjon]

I agree that it makes sense to use a more generic name, but then what about the normalize option? Adding yet another param block for that seems messy. Since that option can also apply to other loss layers, maybe both should be in a generic loss_param?

[sguada]

That sounds good to me.

On Tuesday, December 30, 2014, Jon Long notifications@github.com wrote:

I agree that it makes sense to use a more generic name, but then what
about the normalize option? Adding yet another param block for that seems
messy. Since that option can also apply to other loss layers, maybe both
should be in a generic loss_param?

—
Reply to this email directly or view it on GitHub
#1654 (comment).

Sergio

[shelhamer]

Needs tests (just check loss and gradients for a toy input and truth pair) and docs, then merge.

[longjon]

I've added a doc comment describing the additional parameters, and simple tests that check the gradient with an ignore label set and with normalize set to false, so this should be ready pending any further comments.

[shelhamer]

@longjon the docs and gradient tests look good, but whether the ignored labels do not actually contribute the loss or not deserves coverage. Once that test is in, let's merge.

[longjon]

Indeed, the pure gradient checks leave something out. I've added a simple test that checks that labels are being ignored.

[shelhamer]

Thanks Jon!