Added a flatten module #22245
Added a flatten module #22245
Conversation
Chillee
force-pushed
the
addflatten
branch
2 times, most recently
from
5e9fff8 to
6f0deb0
Compare
fmassa
left a comment
fmassa
left a comment
There was a problem hiding this comment.
This looks good, thanks!
Can you also add an entry in the documentation in https://github.com/pytorch/pytorch/blob/master/docs/source/nn.rst
pytorchbot
added
the
module: docs
|
Done. Hopefully I added it to the |
torch/nn/modules/flatten.py
Outdated
|
|
||
| Shape: | ||
| - Input: :math:`(N, *dims) | ||
| - Output: :math:`(N, product of dims)` |
There was a problem hiding this comment.
It's not entirely clear to me what's desired here. If we want to be completely accurate we could do (N, \prod_{i=\text{start\_dim}}^{\text{end\_dim+1}} \text{dims[i]}, *dims[end_dim+1:]), which renders like
.
I'd also appreciate other suggestions on how to write this.
There was a problem hiding this comment.
Even mentioning N is not quite right. The default arguments works for batched tensors, but it doesn't have to be batched. Since you already mentioned what the two dims are, instead of saying the general case, can you just say that explicitly, for the default arguments, what the input and output shapes are?
There was a problem hiding this comment.
Sorry I'm not sure I understand your comment. If I were to say "explicitly, for the default arguments, what the input and output shapes are", I'd write what I had originally + a note that this is for the default arguments. Is that correct?
|
I've updated the docs - I hope I interpreted your comment correctly @ssnl |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@Chillee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@Chillee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
It seems that 'nn.Flatten' missed in '.pyi' file, Pycharm can't find it' |
Summary: Generated with `stubgen` and moved from `out/flatten.pyi` to `flatten.pyi.in`. #22245 (comment) Pull Request resolved: #24459 Differential Revision: D16881182 Pulled By: ezyang fbshipit-source-id: 5e25fad55f169b5a58ab7522b583d7c923314d4d
|
The documentation seems to be empty in |
|
@ptrblck whoops I think the |
Summary: This PR implements a feature extension discussed in #41516. I followed this other PR #22245 to add this other module. While I was at it, I also added `extra_repr()` method in `Flatten` which was missing. I see there are no unit tests for these modules. Should I add those too? If so, what is the best place I should place these? Pull Request resolved: #41564 Reviewed By: gchanan Differential Revision: D22636766 Pulled By: albanD fbshipit-source-id: f9efdefd3ffe7d9af9482087625344af8f990943
Summary: I've noticed when PR #22245 introduced `nn.Flatten`, the docstring had a bug where it wouldn't render properly on the web, and this PR addresses that. Additionally, it adds a unit test for this module. **Actual**  **Expected**  Pull Request resolved: #42084 Reviewed By: mrshenli Differential Revision: D22756662 Pulled By: ngimel fbshipit-source-id: 60c58c18c9a68854533196ed6b9e9fb0d4f83520
#2118
I'm not sure I'm doing it correctly, so I'll add tests if we decide that it's roughly correct.