-
-
Notifications
You must be signed in to change notification settings - Fork 612
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
Distinct init for kernel and recurrent #2522
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2522 +/- ##
===========================================
+ Coverage 33.40% 60.37% +26.96%
===========================================
Files 31 31
Lines 1907 1938 +31
===========================================
+ Hits 637 1170 +533
+ Misses 1270 768 -502 ☔ View full report in Codecov by Sentry. |
src/layers/recurrent.jl
Outdated
Wh::H | ||
bias::V | ||
struct RNNCell{F, I, H, V} | ||
σ::F |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
σ::F | |
σ::F |
We have two spaces indent in Flux, unfortunately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeha I figured, although https://github.com/FluxML/Flux.jl/blob/master/.JuliaFormatter.toml#L1
indent = 4
kinda confused me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, that should be fixed, or even better we should move to 4 spaces indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With 0.15 coming up that could be a good opportunity! Also at the moment if you do follow https://github.com/FluxML/Flux.jl/blob/master/CONTRIBUTING.md
julia dev/flux_format.jl --verbose .
results in formatting 80 files, so I guess it's formatting everything with 4 spaces indent
can we use |
The naming at the moment follows Flax's, while TF has |
Lux has |
makes sense then, I'll change it |
Adding different initializers options for input matrix and recurrent matrix in recurrent cells as described in #2514. Similar approaches are available in:
An example of the implementation is as follows:
PR Checklist
Tests are added(should not be necessary)