Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "no bias" layers #3882

Merged
merged 9 commits into from Aug 21, 2017
Merged

Add support for "no bias" layers #3882

merged 9 commits into from Aug 21, 2017

Conversation

AlexDBlack
Copy link
Contributor

@AlexDBlack AlexDBlack commented Aug 18, 2017

Should be good to go as-is.

Adds a .hasBias(boolean) config option (default: true, though false for embedding) for the following layers:

  • DenseLayer, EmbeddingLayer
  • Output (OutputLayer, RnnOutputLayer, CenterLossOutputLayer)
  • Convolutional (ConvolutionLayer, Convolution1DLayer)

Of note:

  • Config, parameter initializer, forward/backward passes updated to support this
  • Added tests + gradient checks
  • CuDNN convolutional layer was also tested (using dummy/all 0s 'bias' array)
  • EmbeddingLayer: now defaults to having no biases (i.e., hasBias=false by default)

@tomthetrainer
Copy link

@AlexDBlack

I tried searching for papers on the importance of this and was not able to find anything. Bias is not a particular discriminating search pattern for papers on this.

Can you answer some quick questions at some point before or after this is merged so I can see that some documentation is written.

  1. Is there a Paper that describes why this feature is useful?
  2. Just in general why is it useful.

No need for burdensome research just a quick brain dump would help.

Thanks

@AlexDBlack
Copy link
Contributor Author

@tomthetrainer I haven't really seen anything in the literature on "no bias" layers. For most users (99% of the time) they will use layers with biases (which is the default). The only exception there is embedding layer (which now defaults to noBias = true) which is more in line with how word2vec operates.

The main reason we are adding this is for model import support from Keras.

@maxpumperla
Copy link
Contributor

@AlexDBlack it's not all that important, but I would vote for quickly refactoring noBias to useBias or hasBias. Two reasons:
a) it would be closer to keras, which, as you say, is the main reason we do this.
b) it's cognitively easier to avoid double negatives (current default is noBias false). It's trivial on its own, but when you build complex models you want to keep arguments as intuitive as possible.

@AlexDBlack
Copy link
Contributor Author

@maxpumperla yeah, that thought did cross my mind. I was on the fence about that when implementing it. I'll switch to hasBias I think (useBias could be misinterpreted as meaning the bias exists but isn't used... hasBias is the most unambiguous option IMO).

@AlexDBlack
Copy link
Contributor Author

I've switched noBias to hasBias, should be good to go.
Unless anyone wants to review, this can probably be merged.

Copy link
Contributor

@maxpumperla maxpumperla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlexDBlack AlexDBlack merged commit 34ae96c into master Aug 21, 2017
@AlexDBlack AlexDBlack deleted the ab_nobias branch August 21, 2017 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants