Skip to content
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

Add Flux Overview to basics.md #1579

Merged
merged 17 commits into from
Apr 29, 2021
Merged

Add Flux Overview to basics.md #1579

merged 17 commits into from
Apr 29, 2021

Conversation

batate
Copy link
Contributor

@batate batate commented Apr 17, 2021

As requested in issues/comments, this documentation PR adds an overview for Flux:

  • I described the overall process that Flux uses: data/model/train/verify.
  • I created an example that simulates a single Wx+b line.
  • I made some minor tweaks to the other documentation.
  • I structured the outline headings so that this model can be easily skipped

If you like this direction, I can add:

  • A Flux quick start that defines a prediction of an XOR operator (one example, around 10-20 loc)
  • A link to quick-start directly from the overview
  • Transition into the gradient descent from the quick start

PR Checklist

  • Tests are added
  • Entry in NEWS.md
  • [ x] Documentation, if applicable
  • API changes require approval from a committer (different from the author, if applicable)

Copy link
Member

@DhairyaLGandhi DhairyaLGandhi left a comment

Choose a reason for hiding this comment

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

Haven't gone through the PR fully, but left a couple thoughts. Thanks for looking into this! This might want to be it's own little section, because we want to make sure the flow of the document is natural and targeted.

docs/src/models/basics.md Outdated Show resolved Hide resolved
docs/src/models/basics.md Outdated Show resolved Hide resolved
Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on! A quick first pass from me:

docs/src/models/basics.md Outdated Show resolved Hide resolved
docs/src/models/basics.md Outdated Show resolved Hide resolved
docs/src/models/basics.md Outdated Show resolved Hide resolved
docs/src/models/basics.md Outdated Show resolved Hide resolved
docs/src/models/basics.md Show resolved Hide resolved
docs/src/models/basics.md Outdated Show resolved Hide resolved
@batate
Copy link
Contributor Author

batate commented Apr 21, 2021

I am almost done with these updates, and another tweak to extract this flow into another section. I also want to do a Flux quickstart for those already familiar with ML math and Julia, a rapid introduction of all Flux concepts and a quick pointer to the model zoo. Sorry it's going slowly; my writing schedule is intense.

Can someone guide me through where to use doctests, and what the markdown should look like if I want to show Julia console output, but not in a doctest? Right now, anything with random content breaks, as do the predictions. Rather than:

jldoctest basics

what should I use to mark up code blocks?

@DhairyaLGandhi
Copy link
Member

Normal code blocks and triple backtick (```) blocks should do

@DhairyaLGandhi
Copy link
Member

We should typically only have doctests for features from the FluxML ecosystem and have predictable outcomes (so nothing that uses random output, for example)

Documenter understands to parse julia> as a simulated repl session.

@batate
Copy link
Contributor Author

batate commented Apr 21, 2021

So

```julia>

and not

```julia> basics

?

@DhairyaLGandhi
Copy link
Member

Yup

@batate
Copy link
Contributor Author

batate commented Apr 22, 2021

I make the suggested changes, and broke the overview into its own section. Eventually, the overview will contain

Flux Overview

Flux Gentle Introduction (this prose)

Flux Quick Start (a much quicker overview and specific links to appropriate quick start resources)

Copy link
Member

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

Looking pretty good. One suggestion I have is to add a section on "decoding" the output of a model (i.e. the difference between loss and accuracy). The current problem in the tutorial being regression may not be best suited to this. So, we could save this for a different tutorial.

docs/src/models/overview.md Outdated Show resolved Hide resolved
docs/src/models/overview.md Outdated Show resolved Hide resolved
docs/src/models/overview.md Outdated Show resolved Hide resolved
batate and others added 4 commits April 22, 2021 14:11
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
@batate
Copy link
Contributor Author

batate commented Apr 22, 2021

Looking pretty good. One suggestion I have is to add a section on "decoding" the output of a model (i.e. the difference between loss and accuracy). The current problem in the tutorial being regression may not be best suited to this. So, we could save this for a different tutorial.

Great idea. I agree that it belongs elsewhere.

@batate
Copy link
Contributor Author

batate commented Apr 23, 2021

This review flow is unfamiliar to me. Let me know if I need to do anything else. Thanks for your patience.

@darsnack
Copy link
Member

darsnack commented Apr 23, 2021

Looks ready to merge, but I just wanted clarification on the

```julia>

syntax. I've never seen that before. If I understand the desired behavior correctly, then the syntax you want is @repl

@batate
Copy link
Contributor Author

batate commented Apr 24, 2021

Looks ready to merge, but I just wanted clarification on the

syntax. I've never seen that before. If I understand the desired behavior correctly, then the syntax you want is @repl

Great. This doc system is new to me. I adjusted the pull request to look like the examples I found here ... see the repl examples within the file.

I did not want to use @repl because that would run the examples again, leading to new values, and potentially rendering the prose invalid (e.g. a loss that goes up for one epoch). So I landed on the compromise of julia instead of @repl. It works OK, for the most part. Hope that's OK.

@DhairyaLGandhi
Copy link
Member

DhairyaLGandhi commented Apr 24, 2021

Just wrapping it in the code blocks is fine, just need the Julia prompt to be on the new line, no need for the at-repl annotation, that's picked up automatically

docs/src/models/basics.md Outdated Show resolved Hide resolved
docs/src/models/overview.md Outdated Show resolved Hide resolved
docs/src/models/overview.md Outdated Show resolved Hide resolved
docs/src/models/overview.md Outdated Show resolved Hide resolved
@darsnack
Copy link
Member

Yeah, we don't use @repl in the Flux docs. What you have right now works. I left some final comments.

batate and others added 3 commits April 24, 2021 12:54
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
@doppioandante
Copy link

doppioandante commented Apr 24, 2021

As a newbie to Flux and machine learning in general, thanks for this. I skimmed through it and it is pretty clear.

Copy link
Contributor Author

@batate batate left a comment

Choose a reason for hiding this comment

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

I dropped the julia annotations (julia to ) and added a review.

When this one is ready, I am happy to take on a quick start if we like the direction of these docs.

docs/src/models/overview.md Outdated Show resolved Hide resolved
docs/src/models/overview.md Outdated Show resolved Hide resolved
batate and others added 2 commits April 27, 2021 13:05
Co-authored-by: Kyle Daruwalla <daruwalla.k.public@icloud.com>
@batate batate requested a review from darsnack April 28, 2021 18:36
darsnack
darsnack previously approved these changes Apr 28, 2021
Copy link
Member

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

Great, thanks for your work @batate! Will merge once the checks pass.

Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Couple last-minute comments from me, if you don't mind. Mostly following up on earlier comments that I didn't get notifications for when they became "outdated" in the churn.

Now, let's verify the predictions:

```
julia> predict(y_test)
Copy link
Member

Choose a reason for hiding this comment

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

Last minute comment, sorry! #1579 (comment)

Copy link
Contributor Author

@batate batate Apr 29, 2021

Choose a reason for hiding this comment

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

I took another shot at the prose because our discussions are a clue that the readers will also have trouble with the concept. Here's the main bit:


julia> model.b
1-element Array{Float64,1}:
 0.0

Under the hood, a dense layer is a struct with fields W and b. W represents a weight and b represents a bias. There's another way to think about a model. In Flux, models are conceptually predictive functions:

julia> predict = Dense(1, 1)

Dense(1, 1) also implements the function σ(Wx+b) where W and b are the weights and biases. σ is an activation function (more on activations later). Our model has one weight and one bias, but typical models will have many more. Think of weights and biases as knobs and levers Flux can use to tune predictions. Activation functions are transformations that tailor models to your needs.


let me know if this is takes us in a better direction.

docs/src/models/overview.md Show resolved Hide resolved
@batate
Copy link
Contributor Author

batate commented Apr 29, 2021

Great, thanks for your work @batate! Will merge once the checks pass.

You are quite welcome. I plan to be around for a while, and this looked like a good place to cut my teeth on the process and the community. Any idea why the build failed? It didn't look like it had anything to do with the commit.

@darsnack
Copy link
Member

Not sure, but the tests are passing now.

Copy link
Member

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

I think we settled on a good compromise for now. We can fine tune at a later date as needed.

Will merge once the checks pass.

@darsnack
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 29, 2021

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants