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

Added bias vector and regularizers #44

Merged
merged 2 commits into from
Feb 22, 2022
Merged

Added bias vector and regularizers #44

merged 2 commits into from
Feb 22, 2022

Conversation

vsheska
Copy link
Contributor

@vsheska vsheska commented Jul 26, 2021

Added options to include a bias vector, and regularizers to the kernels

if bias:
assert np.isclose(diff, 0.22)
else:
assert np.isclose(diff, 0.2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do these target numbers (e.g. 0.2) come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

Figured it out. They're based on the default L1 amount of regularization, and the sizes and values of the respective kernels and bias term. I've redone the test to compute their values based on all those terms.

@hunse hunse force-pushed the bias-regularizers branch from 40d7395 to 4e7cd65 Compare February 17, 2022 18:30
@hunse
Copy link
Contributor

hunse commented Feb 17, 2022

I've made a few modifications. The main one is that the bias is no longer a subsidiary of the kernel, because a user may want to have a bias for the recurrent kernel when they have the encoding kernel turned off.

I also added a separate initialization and regularization term for the bias, since these are typically treated separately (e.g. in Dense layers), and one usually wants a different initializer for the biases (i.e. zeros) than for the weights. The use_bias argument is now a little redundant, in that we could accomplish this by just having the user set bias_initializer to None if they don't want a bias (this is what we do with the kernel). However, I kept it because we want the bias to be off by default, but if the user wants to turn the bias on, it's nice to have a sensible default for the initializer (i.e. zeros). Otherwise, any user that wants to turn the bias on would need to know what initializer they want to use (though we could help them out in the docstring by suggesting "zeros").

@vsheska
Copy link
Contributor Author

vsheska commented Feb 18, 2022

Changes are looking good to me!

@hunse hunse force-pushed the bias-regularizers branch from 1ba777e to d8ade78 Compare February 22, 2022 15:49
@hunse hunse force-pushed the bias-regularizers branch from d8ade78 to ea6053c Compare February 22, 2022 17:18
@hunse hunse merged commit ea6053c into master Feb 22, 2022
@hunse hunse deleted the bias-regularizers branch February 22, 2022 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants