-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Enhance Distributed Fused Adam #1832
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
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
… copy after all-gather. Signed-off-by: Wil Kong <alpha0422@gmail.com>
Signed-off-by: Wil Kong <alpha0422@gmail.com>
Call unscale_grads within step if grad scaler is provided. Revert grad clipping logic. Signed-off-by: Tim Moon <tmoon@nvidia.com>
f9f2061 to
d9c0765
Compare
apex/contrib/csrc/optimizers/multi_tensor_distopt_adam_kernel.cu
Outdated
Show resolved
Hide resolved
apex/contrib/csrc/optimizers/multi_tensor_distopt_adam_kernel.cu
Outdated
Show resolved
Hide resolved
timmoon10
left a comment
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.
Overall looks good to me aside from some stylistic suggestions.
apex/contrib/csrc/optimizers/multi_tensor_distopt_adam_kernel.cu
Outdated
Show resolved
Hide resolved
Co-authored-by: Tim Moon <4406448+timmoon10@users.noreply.github.com>
Co-authored-by: Tim Moon <4406448+timmoon10@users.noreply.github.com>
This reverts commit 857e8f4.
crcrpar
left a comment
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.
could you add a test case for capturable?
crcrpar
left a comment
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.
agreed on @alpha0422 will add a test in a follow-up.
Continue of #1794.
This PR enhances distributed fused adam by:
@timmoon10 @crcrpar Please help review, thanks.