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.

Sign up for GitHub

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

Jump to bottom

add flags to readme #2003

Merged
merged 12 commits into from
Nov 18, 2024
Merged

add flags to readme #2003

merged 12 commits into from
Nov 18, 2024

Conversation

felipemello1
Copy link
Contributor

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)

image

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Nov 13, 2024 •
edited
Loading

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit 1789d43 with merge base d5c54f3 (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 Nov 13, 2024
RdoubleA
RdoubleA approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@RdoubleA RdoubleA left a comment

Choose a reason for hiding this comment

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

This is beautiful. Can we get this table on our memory optimizations page on our live docs?

README.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.88%. Comparing base (51b31c8) to head (c95d2f8).
Report is 6 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #2003       +/-   ##
===========================================
+ Coverage   24.78%   64.88%   +40.10%     
===========================================
  Files         318      318               
  Lines       17620    17683       +63     
===========================================
+ Hits         4367    11474     +7107     
+ Misses      13253     6209     -7044

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

ebsmothers
ebsmothers reviewed Nov 14, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 123 to 124
| [LoRA](https://pytorch.org/torchtune/main/tutorials/lora_finetune.html) | 9.16 | -47.8% | 8355 | 16.2% |
| [QLoRA](https://pytorch.org/torchtune/main/tutorials/qlora_finetune.html) | 5.28 | -42.4% | 8228 | -1.5% |
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed already but make sure to update these links. Also (related to my other comment) the compile link does not point to a place that demonstrates its usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the link used in compile is a pytorch tutorial. We don't have a link for compile. I could change it our utility function "compile_model". But since we have the example below with the command for QLoRA, and also it is present in every config, i think it should be ok. Do you disagree?

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 will update it for LoRA and QLoRA

ebsmothers
ebsmothers reviewed Nov 18, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
ebsmothers
ebsmothers reviewed Nov 18, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@felipemello1 felipemello1 merged commit 1814feb into pytorch:main Nov 18, 2024
5 checks passed
@felipemello1 felipemello1 deleted the add_flags_to_readme branch November 18, 2024 21:35
ebsmothers
ebsmothers reviewed Nov 18, 2024
View reviewed changes
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.

Thanks for adding this!

@ebsmothers ebsmothers mentioned this pull request Nov 26, 2024
Closed
44 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebsmothers ebsmothers ebsmothers left review comments

@RdoubleA RdoubleA RdoubleA approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@felipemello1 @codecov-commenter @ebsmothers @RdoubleA @facebook-github-bot