-
Notifications
You must be signed in to change notification settings - Fork 471
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
llama 3.1 has correct max_seq_len
for all versions
#2203
base: main
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchtune/2203
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
thanks for the PR, do you mind updating the docstrings too, so they include the new arg? I think that for 405B the default is wrong, it should be 131k. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2203 +/- ##
=======================================
Coverage 23.85% 23.85%
=======================================
Files 344 344
Lines 20658 20658
=======================================
Hits 4928 4928
Misses 15730 15730 ☔ View full report in Codecov by Sentry. |
This seems inconsistent with our other builders, which do not specify a max seq len parameter. If you want to change the default max seq len for llama3_1 you should use the base builder |
@felipemello1 @RdoubleA thank you for the comments, can you please check the updated implementation? I set the correct default from the HF model config for Llama 3.1 models in the |
max_seq_len
max_seq_len
for all versions
Context
What is the purpose of this PR? Is it to
Please link to any issues this PR addresses. #2202
Changelog
What are the changes made in this PR?
max_seq_len
the model was originally trained with based on the HF configTest plan
Please make sure to do each of the following if applicable to your PR. If you're unsure about any one of these just ask and we will happily help. We also have a contributing page for some guidance on contributing.
pre-commit install
)pytest tests
pytest tests -m integration_test
UX
If your function changed a public API, please add a dummy example of what the user experience will look like when calling it.
Here is a docstring example
and a tutorial example