[optim] Adam defaults to fused when CUDA + differentiable=False #90865
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/90865
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 5df2847: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
janeyx99
added
release notes: nn
janeyx99
force-pushed
the
adam-default-fused
branch
3 times, most recently
from
c292b78
to
e379ec5
Compare
janeyx99
force-pushed
the
adam-default-fused
branch
from
e379ec5
to
5df2847
Compare
janeyx99
changed the title
| @@ -105,9 +105,12 @@ class Adam(Optimizer): | |||
| capturable (bool, optional): whether this instance is safe to capture in a CUDA graph. | |||
| Passing True can impair ungraphed performance, so if you don't intend to | |||
| graph capture this instance, leave it False (default: False) | |||
| fused (bool, optional): whether fused implementation of optimizer is used. | |||
| fused (bool, optional): whether the fused implementation (CUDA only) is used. | |||
There was a problem hiding this comment.
nit: it is not really cuda-only? You could use it for CPU. And other device like xla/mps/xpu might benefit from it as well.
Or you prefer to mention this for now and update it when we get more implementations?
There was a problem hiding this comment.
Thanks for the review! I thought it was CUDA only since fused_adam only has a CUDA dispatch, but yea we can update this when there are more implementations.
https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/native_functions.yaml#L14471-L14476
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 additional jobs have failed, first few of them are: build Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -f "build ios failure not related" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
…#91896) following up to #90865 and #92048 Pull Request resolved: #91896 Approved by: https://github.com/albanD
…g to fused (#92181) A "fix" following #90865. Realized that fused is not compatible with torch.jit.is_scripting() when looking at a later line. Took the opportunity to make the code cleaner/slightly more performant (with the extends) as well. Pull Request resolved: #92181 Approved by: https://github.com/albanD
|
@janeyx99 what would be the expected behavior when |
Step 1 in faster default optimizers.
Preliminary benchmarks show gaps in improvement on CUDA for BERT_pytorch and resnet18:
