-
Notifications
You must be signed in to change notification settings - Fork 48
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
build(deps): Upgrade accelerate requirement to allow version 1.0.0 #371
Conversation
Signed-off-by: Will Johnson <[email protected]>
Signed-off-by: Will Johnson <[email protected]>
Thanks for making a pull request! 😃 |
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.
Change looks good to me, none of the other accelerate changes from the changelog look to affect fms-hf-tuning repo but good to check with fms-acceleration repo. Thanks for testing multi-GPU tuning and inference as this was the issue we saw with previous versions
@@ -14,7 +14,7 @@ fsdp_config: | |||
fsdp_auto_wrap_policy: TRANSFORMER_BASED_WRAP | |||
|
|||
# this controls the FSDP pipelining | |||
fsdp_backward_prefetch_policy: BACKWARD_PRE # set to BACKWARD_PRE for the most time-efficient pipeline | |||
fsdp_backward_prefetch: BACKWARD_PRE # set to BACKWARD_PRE for the most time-efficient pipeline |
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.
fsdp_backward_prefetch: BACKWARD_PRE # set to BACKWARD_PRE for the most time-efficient pipeline | |
fsdp_backward_prefetch: BACKWARD_PRE # set to BACKWARD_PRE for the most time-efficient pipeline | |
fsdp_backward_prefetch_policy: BACKWARD_PRE # keep for backward compatiblity for accelerate < 1.0 |
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.
I think it works with both keys. I have tried on my side. Maybe you can also confirm
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.
This is a good solution, thanks Fabian! Can confirm tuning works on accelerate version 1.0.0 with both.
Signed-off-by: Will Johnson <[email protected]>
Description of the change
Updates the requirements on accelerate to permit the latest version.
Accelerate version 1.0.0 release notes
Variable
--fsdp_backward_prefetch_policy
became outdated, replaced with new variable--fsdp_backward_prefetch
to fix unit tests.Related issue number
closes #372
How to verify the PR
Was the PR tested