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

update docs #1602

Merged
merged 17 commits into from
Sep 17, 2024
Merged

update docs #1602

merged 17 commits into from
Sep 17, 2024

Conversation

felipemello1
Copy link
Contributor

@felipemello1 felipemello1 commented Sep 16, 2024

Context

What is the purpose of this PR? Is it to

  • add a new feature
  • fix a bug
  • update tests and/or documentation
  • other (please add here)

Update:

  • Recipe overview
  • LoRA
  • torchtune.models

Removed unnecessary paragraphs so its more lean

Test plan

Built the docs after the changes and double checked

Copy link

pytorch-bot bot commented Sep 16, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchtune/1602

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit d5d5986 with merge base 6b55c1d (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 16, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.81%. Comparing base (6b55c1d) to head (0423644).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1602       +/-   ##
===========================================
- Coverage   72.27%   26.81%   -45.46%     
===========================================
  Files         290      290               
  Lines       14552    14570       +18     
===========================================
- Hits        10517     3907     -6610     
- Misses       4035    10663     +6628     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@felipemello1 felipemello1 changed the base branch from main to release/0.3.0 September 17, 2024 00:46
@felipemello1 felipemello1 changed the base branch from release/0.3.0 to main September 17, 2024 00:46
@felipemello1 felipemello1 changed the title WIP update docs overview update docs Sep 17, 2024
@felipemello1 felipemello1 marked this pull request as ready for review September 17, 2024 01:56

mistral
-------

All models from `Mistral AI family <https://mistral.ai/technology/#models>`_.

Request Access on `Hugging Face <https://huggingface.co/mistralai/Mistral-7B-v0.3>`__.
Important: You need to request access on `Hugging Face <https://huggingface.co/mistralai/Mistral-7B-v0.1>`__ to download this model.
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did we land on this? Why not just use 0.2 if the arch is the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i can run it and see if loss goes down, but thats not the most robust test

Copy link
Contributor

Choose a reason for hiding this comment

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

Eh np, fine to just keep it as is for now

.. autosummary::
:toctree: generated/
:nosignatures:
.. .. autosummary::
Copy link
Contributor

Choose a reason for hiding this comment

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

So these we will just expose later on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense. Joe was against it in the past, and Philip suggested the same. While we are not ready for MM, it doesnt make much sense to expose it, IMO

@@ -5,11 +5,10 @@ LoRA Single Device Finetuning
=============================

This recipe supports finetuning on next-token prediction tasks using parameter efficient fine-tuning techniques (PEFT)
such as `LoRA <https://arxiv.org/abs/2106.09685>`_ and `QLoRA <https://arxiv.org/abs/2305.14314>`_. These techniques
such as :ref:`glossary_lora` and :ref:`glossary_qlora`. These techniques
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: why not keep the links to the original papers here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we have pages for them, explaining the details, how to use it, the these pages have hte link for the papers.


For a deeper understanding of the different levers you can pull when using this recipe,
see our documentation for the different PEFT training paradigms we support:

* :ref:`glossary_lora`
* :ref:`glossary_qlora`

Many of our other memory optimization features can be used in this recipe, too:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit would at least keep this first sentence as a lead-in the the "You can learn more about all of our memory optimization features". Otherwise I agree that it's nice to drop the full list here though

Copy link
Contributor

@ebsmothers ebsmothers left a comment

Choose a reason for hiding this comment

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

Left a bunch of small comments, but no major concerns

Co-authored-by: ebsmothers <ebs@meta.com>
docs/source/recipes/recipes_overview.rst Outdated Show resolved Hide resolved
docs/source/recipes/recipes_overview.rst Outdated Show resolved Hide resolved
using a variety of :ref:`memory optimization features <memory_optimization_overview_label>`. Our fine-tuning recipes support all of our models and all our dataset types.
This includes continued pre-training, and various supervised funetuning paradigms, which can be customized through our datasets. Check out our
:ref:`dataset tutorial <dataset_tutorial_label>` for more information.
Our recipes include:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh Felipe you minimalist

@SalmanMohammadi
Copy link
Collaborator

LGTM, some small nits pls address.

felipemello1 and others added 7 commits September 17, 2024 10:50
Co-authored-by: Salman Mohammadi <salman.mohammadi@outlook.com>
Co-authored-by: Salman Mohammadi <salman.mohammadi@outlook.com>
Co-authored-by: Salman Mohammadi <salman.mohammadi@outlook.com>
Co-authored-by: ebsmothers <ebs@meta.com>
Co-authored-by: ebsmothers <ebs@meta.com>
Co-authored-by: ebsmothers <ebs@meta.com>
@felipemello1 felipemello1 merged commit 74b99fe into pytorch:main Sep 17, 2024
17 checks passed
@felipemello1 felipemello1 deleted the update_docs branch September 17, 2024 18:26
ebsmothers added a commit that referenced this pull request Sep 17, 2024
Co-authored-by: Felipe Mello <felipemello@fb.com>
Co-authored-by: ebsmothers <ebs@meta.com>
Co-authored-by: Salman Mohammadi <salman.mohammadi@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants