-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update example scripts #307
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
Conversation
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments! I was thinking the base example should be left as is to be as simple as possible, and we could add a "complete_nlp_example" that contains the checkpoint and sharding, so that a reader only wanting a base training loop is not too lost. Wdyt?
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple comments more on the step skipping!
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I think we could add a mention in the examples README on why there are two different scripts for each modality.
@@ -149,10 +149,6 @@ def training_function(config, args): | |||
model, optimizer, train_dataloader, eval_dataloader, lr_scheduler | |||
) | |||
|
|||
# Instantiate learning rate scheduler after preparing the training dataloader as the prepare method | |||
# may change its length. | |||
lr_scheduler = OneCycleLR(optimizer=optimizer, max_lr=lr, epochs=num_epochs, steps_per_epoch=len(train_dataloader)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Start of updating the example scripts to use both the save/load_state and tracking