outdims: possible enhancements #1253
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
please rebase on master, now that #1252 is merged |
…ers like `reshape`.
Co-Authored-By: Dhairya Gandhi <[email protected]>
| @@ -64,6 +64,10 @@ outdims(c::Chain, isize) = foldr(outdims, reverse(c.layers), init = isize) | |||
| # see issue https://github.com/FluxML/Flux.jl/issues/702 | |||
| # Johnny Chen -- @johnnychen94 | |||
| # only slightly changed to better handle interaction with Zygote @dsweber2 | |||
|
|
|||
| # fallback | |||
| outdims(f::Function, isize::Tuple) = size(f(ones(Float32, isize..., 1)))[1:end-1] # since we aren't care about batch dimension, we are free to just set it to 1 | |||
There was a problem hiding this comment.
why not leave f without type restrictions?
|
Bump :) |
bors bot
added a commit
that referenced
this pull request
Dec 26, 2020
1305: Updates to outdims r=CarloLucibello a=darsnack Since #1253 stalled, I tried committing to the author's branch, but I have not received a response. So, I am creating a new PR with the following changes from the previous one: - `outdims` for generic functions - Size checking for `outdims(::Dense, isize)` I also added the following additional changes - Removed type signature restrictions on `outdims` for generic functions - Added `outdims` for normalization layers - This is helpful since `BatchNorm` etc. show up in a chain or array of layers frequently when model building - Right now there is a method error - Generic functions would address this, but I think we should avoid actually evaluating the function as much as possible - Updated docs for `outdims` changes ### PR Checklist - [x] Tests are added - [ ] Entry in NEWS.md - [x] Documentation, if applicable - [x] Final review from `@dhairyagandhi96` (for API changes). Co-authored-by: Kyle Daruwalla <[email protected]> Co-authored-by: lorenzoh <[email protected]> Co-authored-by: Kyle Daruwalla <[email protected]>
bors bot
added a commit
that referenced
this pull request
Dec 26, 2020
1305: Updates to outdims r=CarloLucibello a=darsnack Since #1253 stalled, I tried committing to the author's branch, but I have not received a response. So, I am creating a new PR with the following changes from the previous one: - `outdims` for generic functions - Size checking for `outdims(::Dense, isize)` I also added the following additional changes - Removed type signature restrictions on `outdims` for generic functions - Added `outdims` for normalization layers - This is helpful since `BatchNorm` etc. show up in a chain or array of layers frequently when model building - Right now there is a method error - Generic functions would address this, but I think we should avoid actually evaluating the function as much as possible - Updated docs for `outdims` changes ### PR Checklist - [x] Tests are added - [ ] Entry in NEWS.md - [x] Documentation, if applicable - [x] Final review from `@dhairyagandhi96` (for API changes). Co-authored-by: Kyle Daruwalla <[email protected]> Co-authored-by: lorenzoh <[email protected]> Co-authored-by: Kyle Daruwalla <[email protected]>
bors bot
added a commit
that referenced
this pull request
Dec 27, 2020
1305: Updates to outdims r=CarloLucibello a=darsnack Since #1253 stalled, I tried committing to the author's branch, but I have not received a response. So, I am creating a new PR with the following changes from the previous one: - `outdims` for generic functions - Size checking for `outdims(::Dense, isize)` I also added the following additional changes - Removed type signature restrictions on `outdims` for generic functions - Added `outdims` for normalization layers - This is helpful since `BatchNorm` etc. show up in a chain or array of layers frequently when model building - Right now there is a method error - Generic functions would address this, but I think we should avoid actually evaluating the function as much as possible - Updated docs for `outdims` changes ### PR Checklist - [x] Tests are added - [ ] Entry in NEWS.md - [x] Documentation, if applicable - [x] Final review from `@dhairyagandhi96` (for API changes). Co-authored-by: Kyle Daruwalla <[email protected]> Co-authored-by: lorenzoh <[email protected]> Co-authored-by: Kyle Daruwalla <[email protected]>
bors bot
added a commit
that referenced
this pull request
Dec 30, 2020
1305: Updates to outdims r=CarloLucibello a=darsnack Since #1253 stalled, I tried committing to the author's branch, but I have not received a response. So, I am creating a new PR with the following changes from the previous one: - `outdims` for generic functions - Size checking for `outdims(::Dense, isize)` I also added the following additional changes - Removed type signature restrictions on `outdims` for generic functions - Added `outdims` for normalization layers - This is helpful since `BatchNorm` etc. show up in a chain or array of layers frequently when model building - Right now there is a method error - Generic functions would address this, but I think we should avoid actually evaluating the function as much as possible - Updated docs for `outdims` changes ### PR Checklist - [x] Tests are added - [ ] Entry in NEWS.md - [x] Documentation, if applicable - [x] Final review from `@dhairyagandhi96` (for API changes). Co-authored-by: Kyle Daruwalla <[email protected]> Co-authored-by: lorenzoh <[email protected]> Co-authored-by: Kyle Daruwalla <[email protected]>
|
Since we now have |
|
Yes, #1305 was supposed to close this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is meant to discuss and develop further improvements to
outdimsas discussed in #1086I covered @HamletWantToCode 's proposal for a fallback (still without testing) and modified the dense version to keep all dimensions, as proposed by @darsnack .
Please feel free to also contribute commits.
Good night ;-)