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 Flax Dropout guide #2675

Merged
merged 1 commit into from
Dec 20, 2022
Merged

Add Flax Dropout guide #2675

merged 1 commit into from
Dec 20, 2022

Conversation

8bitmp3
Copy link
Collaborator

@8bitmp3 8bitmp3 commented Dec 1, 2022

  • Add a dedicated Dropout guide.
  • Covers PRNG key usage, model creation (with nn.Module and nn.Dropout), initialization, the forward pass, and the training step (with TrainStep).
  • Includes examples from flax/examples like a Transformer-based model trained on the WMT dataset (with dropout and attention dropout).

Shorten the Flax - The Sharp Bits by migrating some of the dropout instructions to the new guide. (To be done in a separate PR)

Live preview: https://flax--2675.org.readthedocs.build/en/2675/guides/dropout.html

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@marcvanzee
Copy link
Collaborator

Adding @cgarciae as a reviewer too since he's been thinking about this guide as well.

@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 1, 2022

Thanks @marcvanzee 👍

@cgarciae Your feedback would be super welcome 👍

Copy link
Collaborator

@marcvanzee marcvanzee left a comment

Choose a reason for hiding this comment

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

Great work! I think this will be an awesome tutorial, just needs a bit of work to get it really perfect.

docs/notebooks/flax_sharp_bits.md Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 5, 2022

@marcvanzee @cgarciae Thanks. PTAL 👍

@8bitmp3 8bitmp3 added the Priority: P1 - soon Response within 5 business days. Resolution within 30 days. (Assignee required) label Dec 5, 2022
Copy link
Collaborator

@cgarciae cgarciae left a comment

Choose a reason for hiding this comment

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

PTAL

docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
docs/guides/dropout.rst Outdated Show resolved Hide resolved
@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 9, 2022

Thanks for the help @cgarciae 👍

@marcvanzee some of @cgarciae's review comments got stuck in "pending", they are out now (see above). We have gone over them and the new commits are on the way. Some changes will involve some changes to your code suggestions.

@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 13, 2022

@marcvanzee fyi working on adding the training step

docs/guides/dropout.rst Outdated Show resolved Hide resolved
@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 15, 2022

Reviewed the code and text with @marcvanzee. Committed and pushed the changes to the branch. cc @cgarciae. Amendments to the Sharp Bits will be in a separate PR.

@8bitmp3 8bitmp3 closed this Dec 15, 2022
@8bitmp3 8bitmp3 reopened this Dec 15, 2022
docs/guides/dropout.rst Outdated Show resolved Hide resolved
@marcvanzee
Copy link
Collaborator

LGTM -- thanks a lot for this @8bitmp3, looks really awesome!! 🥳

(Please fix the doc build failures)

@marcvanzee marcvanzee self-requested a review December 15, 2022 12:02
@8bitmp3 8bitmp3 closed this Dec 16, 2022
@8bitmp3 8bitmp3 reopened this Dec 16, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2022

Codecov Report

Merging #2675 (c44cef0) into main (6d5bc2a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2675   +/-   ##
=======================================
  Coverage   81.21%   81.21%           
=======================================
  Files          53       53           
  Lines        5605     5605           
=======================================
  Hits         4552     4552           
  Misses       1053     1053           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 17, 2022

All checks ✔️ @cgarciae PTAL. Thank you.

Live preview: https://flax--2675.org.readthedocs.build/en/2675/guides/dropout.html (build successful @marcvanzee).

@8bitmp3 8bitmp3 changed the title Add Applying dropout guide Add Flax Dropout guide Dec 17, 2022
docs/guides/dropout.rst Outdated Show resolved Hide resolved
@8bitmp3
Copy link
Collaborator Author

8bitmp3 commented Dec 19, 2022

@cgarciae PTAL.

Copy link
Collaborator

@cgarciae cgarciae left a comment

Choose a reason for hiding this comment

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

Thanks @8bitmp3! Looks great, lets merge this!

@copybara-service copybara-service bot merged commit d659145 into google:main Dec 20, 2022
@8bitmp3 8bitmp3 deleted the dropout-guide branch December 20, 2022 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: P1 - soon Response within 5 business days. Resolution within 30 days. (Assignee required) pull ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants