Skip to content

Documentation headings & sections #2056

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

Merged
merged 9 commits into from
Sep 19, 2022
Merged

Documentation headings & sections #2056

merged 9 commits into from
Sep 19, 2022

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Aug 29, 2022

As suggested here #2055 (comment) this tries to organise the sections of the documentation a bit more logically.

Nothing is removed. The only new content is a section listing Zygote.jl as a package which Flux re-exports.

@@ -0,0 +1,47 @@
## Model Building
Copy link
Member Author

@mcabbott mcabbott Aug 29, 2022

Choose a reason for hiding this comment

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

I put outputsize on its own page, as I don't know what it lives with. But I'm sure nobody found it under utilities.

This is the heading it had, I think, but clearly not quite right here. Now changed to "Size Propagation" but not sure that's perfect either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now called "shape inference"

Comment on lines 1 to 9
## Callback Helpers

```@docs
Flux.throttle
Flux.stop
Flux.skip
```

## Patience Helpers
Copy link
Member Author

@mcabbott mcabbott Aug 29, 2022

Choose a reason for hiding this comment

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

These were in utilities, now their own page. Some are going away in 0.14.

Comment on lines +1 to 3
# Random Weight Initialisation

Flux initialises convolutional layers and recurrent cells with `glorot_uniform` by default.
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe the file ought to be renamed. Utilities has a long list of rand variants, which could live here or in Model Tools below?

Comment on lines -37 to -38
"Datasets" => "datasets.md",
"Community" => "community.md"
Copy link
Member Author

Choose a reason for hiding this comment

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

Datasets became one line in the "ecosystem" page.

Community I put on the first page, after "learning flux"

@github-actions
Copy link
Contributor

Once the build has completed, you can preview any updated documentation at this URL: https://fluxml.ai/Flux.jl/previews/PR2056/ in ~20 minutes

Comment on lines 24 to +26
"Handling Data" => [
"One-Hot Encoding with OneHotArrays.jl" => "data/onehot.md",
"Working with data using MLUtils.jl" => "data/mlutils.md"
"MLUtils.jl" => "data/mlutils.md",
"OneHotArrays.jl" => "data/onehot.md",
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I'm trying to (1) shorten these to fit on one line, and (2) put the package Name.jl to make such pages of "foreign API" visually distinct from pages about Flux itself.

I hope that "Working with data" is clear enough from "Handling Data" just above. The actual titles on the pages remain as before.

Copy link
Member Author

Choose a reason for hiding this comment

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

New on the right:
Screenshot 2022-08-29 at 19 26 02

docs/make.jl Outdated
pages = [
"Home" => "index.md",
"Building Models" => [
"Overview" => "models/overview.md",
"Basics" => "models/basics.md",
"Recurrence" => "models/recurrence.md",
"Model Reference" => "models/layers.md",
"Layer Reference" => "models/layers.md",
"Loss Functions" => "models/losses.md",
"Regularisation" => "models/regularisation.md",
"Advanced Model Building" => "models/advanced.md",
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe "Advanced Model Building" should be more like "Custom Layers"? That's what it describes.

It also has a section "Freezing Layer Parameters" which I think should move to the Training heading below. (And will be scrapped when Params is removed.)

@Saransh-cpp
Copy link
Member

I had something similar planned in my proposal, and this PR does all of it (even more than the proposed plan) 😅

If it helps, this was the structure I had in mind when I was submitting a proposal - https://whimsical.com/documentation-A3YtpKeqH6ZHopZYNdgBLc

(Note: I was unsure about how the API references and re-exported packages should be placed, and I was very new to the FluxML ecosystem back then.

For instance I imagined the datasets to live under a "Data" heading, along with OneHotArrays and utility functions, but the datasets look good in the ecosystem page too!)

@mcabbott
Copy link
Member Author

mcabbott commented Sep 1, 2022

Cool. I don't think this is perfect, but it's a step towards a less random organisation... without editing much text.

I just saw https://github.com/JuliaComputing/MultiDocumenter.jl, possibly from you somewhere. That might be a nice alternative to having the complete list of NNlib functions included in Flux's docs. This complete list seems likely to go stale, better to have only one, inside NNlib, etc. In which case I'm not sure what happens to these sections --- having activation functions listed here makes some sense, deserves a page still? But should the section titled NNlib.jl be deleted entirely, or become just a selection (e.g. softmax) with a note to look upward for the rest?

@Saransh-cpp
Copy link
Member

MultiDocumenter.jl will definitely be very helpful in increasing the accessibility of docs. I think NNlib.jl should eventually become a selection, and the users could be guided to NNlib's docs for more information (one click away).

@mcabbott
Copy link
Member Author

mcabbott commented Sep 1, 2022

Now I found your slack message. Sounds like MultiDocumenter.jl is not quite ready to for prime-time?

Cool. Zygote.jl is already just a selection.

Maybe we should merge this & then iterate? "Model Tools" sounds a bit generic but I couldn't think of a better idea... and IMO it's useful to group things. What should happen at the intro I'm not sure, maybe we need a "Getting Started" separate from "Building Models"...

@Saransh-cpp
Copy link
Member

Sounds like MultiDocumenter.jl is not quite ready to for prime-time?

Not ready yet :(

Maybe we should merge this & then iterate? "Model Tools" sounds a bit generic but I couldn't think of a better idea... and IMO it's useful to group things.

Yes! We should merge this in. This looks very good to me. We might need to take care of the failing cross-checks.

What should happen at the intro I'm not sure, maybe we need a "Getting Started" separate from "Building Models"...

This sounds good! We can have both the pages in this way?

@mcabbott
Copy link
Member Author

Shall we do this? And then try to figure out how to organise the intro / getting started bit.

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

This looks good! This does add some 404 cross-references, but majority of them do not originate from Flux. I tried pushing the fixed Flux cross-references but I don't have the rights.

Originating from Flux

┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "train!")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter
┌ Warning: invalid local link: unresolved path in models\functors.md        
│   link.text =
│    1-element Vector{Any}:                                                 r\1bI4T\src\Writers\HTMLWriter.jl:2081
│     "Advanced Model Building and Customisation"
│   link.url = "@ref Advanced-Model-Building-and-Customisation"
└ @ Documenter.Writers.HTMLWriter

Originating from Zygote

┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "pullback")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "jacobian")
│   link.url = "zygote.html#Zygote.jacobian-Tuple{Any, Vararg{Any, N} where N}"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "withgradient")
│   link.url = "zygote.html#Zygote.withgradient-Tuple{Any, Vararg{Any, N} where N}"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "pullback")
│   link.url = "training/@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "withjacobian")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "hessian")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "hessian_reverse")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "frule")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "NoTangent()")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "RuleConfig")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "frule")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "@scalar_rule")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in training\zygote.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "RuleConfig")
│   link.url = "@ref"
└ @ Documenter.Writers.HTMLWriter 

Originating from OneHotArrays (these have been fixed but are not a part of any tagged release)

┌ Warning: invalid local link: unresolved path in models\losses.md
│   link.text =
│    1-element Vector{Any}:
│    1-element Vector{Any}:
│     Markdown.Code("", "onehot")
│   link.url = "onehot.html#OneHotArrays.onehot"                            r\1bI4T\src\Writers\HTMLWriter.jl:2081
└ @ Documenter.Writers.HTMLWriter C:\Users\Saransh\.julia\packages\Documenter\1bI4T\src\Writers\HTMLWriter.jl:2081
┌ Warning: invalid local link: unresolved path in data\onehot.md
│   link.text =
│    1-element Vector{Any}:
│     Markdown.Code("", "onehotbatch")                                      r\1bI4T\src\Writers\HTMLWriter.jl:2081
│   link.url = "onehot.html#OneHotArrays.onehotbatch"
└ @ Documenter.Writers.HTMLWriter 

These can be fixed in another PR!

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

(Edit: Oops, I think my review was posted twice)

@mcabbott
Copy link
Member Author

I haven't thought much about the links. Are the "foreign" docstrings linking to functions not documented here, or are the links just not being translated / scoped correctly?

Anyway maybe we merge, and then look at next steps...

@mcabbott mcabbott merged commit 0cbab9e into master Sep 19, 2022
@mcabbott mcabbott deleted the doc_sections branch September 19, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants