Improve docs for Conv etc.
#1391
Conversation
|
Worth pointing out that |
|
This PR is a great improvement, thanks! It's ready to be merged? |
|
I have one more commit, to simplify printing of |
|
sure |
actually I'm not sure about this, but I think it's ok since the defaults are so commonly used and intuitive for this layer |
|
Can un-do if you think it's better. But otherwise I am done here. At the moment |
|
look's fine like this, thanks bors r+ |
|
Build succeeded: |
|
Note to self: Only the docstring for the first method of https://fluxml.ai/Flux.jl/dev/models/layers/#Flux.Conv According to https://juliadocs.github.io/Documenter.jl/stable/man/syntax/#@docs-block one can give signatures, but they must be exact. So we should fix that once the signature has settled down. |
1394: remove some unused Conv constructors r=DhairyaLGandhi a=CarloLucibello I think these constructors can go, I suspect no one is using them and they weren't even tested. Docstrings need some love, I won't do it here in order to avoid conflicts with #1391 As a general rule, we consistently provide `Layer(W, b)` constructors, so no need to also have `Layer(; weight=W, bias=b)` for an arbitrary subset of the layers. ### PR Checklist - [ ] Tests are added - [x] Entry in NEWS.md - [ ] Documentation, if applicable - [ ] Final review from `@dhairyagandhi96` (for API changes). Co-authored-by: Carlo Lucibello <carlo.lucibello@gmail.com>
1394: remove some unused Conv constructors r=DhairyaLGandhi a=CarloLucibello I think these constructors can go, I suspect no one is using them and they weren't even tested. Docstrings need some love, I won't do it here in order to avoid conflicts with #1391 As a general rule, we consistently provide `Layer(W, b)` constructors, so no need to also have `Layer(; weight=W, bias=b)` for an arbitrary subset of the layers. ### PR Checklist - [ ] Tests are added - [x] Entry in NEWS.md - [ ] Documentation, if applicable - [ ] Final review from `@dhairyagandhi96` (for API changes). Co-authored-by: Carlo Lucibello <carlo.lucibello@gmail.com>
This re-writes the docstrings for the convolutional layers, as I thought they were confusing and occasionally misleading. (I guess they grew organically over time.) It adds many examples as these often seem clearer than words; hopefully they will be run as doctests but I'm not entirely sure.
No functional changes.
One small printing change, that
MaxPool((5, 5), pad = 2, stride = (5, 5))prints like this, notpad = (2, 2, 2, 2). Perhaps that should be made still less verbose. OrConvmore verbose?