fix layerSetUp of scale_layer to not add bias blob when already present

[bwilbertz]

In case a net is loaded, which has already be initialized and contains valid blob data, the layerSetUp of the scale_layer would add the blob for the bias term a second time (resulting in three blobs instead of only two).

This PR only adds the blob, if it was not present before.

@jeffdonahue I think the original code of the scale layer was written by you. Could you have a look, if this fix does not miss any case which you had in mind.

Thanks,

Benedikt

[jeffdonahue]

Thanks for the fix @bwilbertz. Can't this happen for the scale parameter as well, though? It seems like you could just skip all parameter setup whenever this->blobs_.size() > 0 as done in InnerProductLayer, for example. (I could easily be misunderstanding the problem though.)

[bwilbertz]

The thing is a bit tricky here, because we derive from bottom.size(), if we should learn the scale parameter or receive it externally (via bottom[1], c.f. #3591). Therefore we cannot just use this->blobs_.size() > 0 like in InnerProductLayer.

Having understood a bit more the case bottom.size() == 2, I think that my PR only solved the case of a learned scale parameter (bottom.size() == 1), and that we have to change the condition

if ( this->blobs_.size() < 2 )

into

if ( this->blobs_.size() + bottom.size() < 3 )

in order to treat both cases correctly.

If you agree, I would update the PR accordingly.

Regarding the initialization of the scale blob: I think, this is OK like it is.

[bwilbertz]

@jeffdonahue I have updated the PR according to my last comment. Did you have any chance to look at it?

Thanks,

Benedikt

[jeffdonahue]

rm leading/trailing whitespace (use if (x) rather than if ( x ))

[jeffdonahue]

Ok, thanks for the explanation and fix @bwilbertz -- this makes sense now. It seems a bit fragile and difficult to understand, and would ideally have unit tests, but that's unfortunately just kind of the state of things across Caffe right now... Maybe you could add a comment in at least one of the cases, e.g., for the if case, // Bias param not already initialized or a bottom.. Please address the minor whitespace issues to follow our conventions, then I can merge this.

[bwilbertz]

@jeffdonahue PR is updated.

Thanks,

Benedikt

[bwilbertz]

Hi @jeffdonahue. Is there anything I can improve to get this PR merged?

Thanks,
Benedikt

[jeffdonahue]

Nope -- thanks for the fixes, and apologies for the delay!