Fix FusedAdam empty tensor handling#3212
Conversation
Greptile SummaryThis PR fixes empty tensor handling in fused optimizer multi-tensor launches. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (6): Last reviewed commit: "Move empty tensor filtering into MultiTe..." | Re-trigger Greptile |
2306b6e to
0b4b00b
Compare
Signed-off-by: Jingyue Wu <wujingyue@gmail.com>
0b4b00b to
69337e7
Compare
| from torch.distributed._tensor import DTensor | ||
|
|
||
|
|
||
| def filter_empty_tensor_lists(tensor_lists): |
There was a problem hiding this comment.
Should we call this from MultiTensorApply unconditionally? I'm not sure whether filtering is safe for every optimizer.
There was a problem hiding this comment.
Yes, agreed, I dont think there is any harm applying this filter directly in the MultiTensorApply class instead of putting it in each optimizer. Could we make that change @wujingyue ?
There was a problem hiding this comment.
Non blocking comment, given that we have just have optimizers exposed in TE
vthumbe1503
left a comment
There was a problem hiding this comment.
LGTM. Just one minor review comment
|
/te-ci pytorch |
Signed-off-by: Jingyue Wu <wujingyue@gmail.com>
7558e37 to
a6baf76
Compare
|
/te-ci |
|
FYI, CI has an unrelated build error: |
Summary
multi_tensor_applymulti_tensor_applycheck so zero-chunk tensors fail with a clear error if they reach the launcherRoot cause
multi_tensor_applydrives launch bookkeeping from the per-tensor chunk loop. A zero-sized tensor has zero chunks, so if an empty tensor appears at the end of a group, pending work for earlier tensors can remain unlaunched and FusedAdam silently skips the update.The generic
MultiTensorApplywrapper still preserves caller-visible slot semantics. Adam and SGD opt into filtering because empty tensor slots are no-op optimizer updates.Fixes #3207
Validation
NVTE_FRAMEWORK=pytorch NVTE_CUDA_ARCHS=86 NVTE_WITH_NCCL_EP=0 NVTE_SKIP_SUBMODULE_CHECKS_DURING_BUILD=1 MAX_JOBS=4 NVTE_BUILD_THREADS_PER_JOB=1 python setup.py build_ext --inplacepython -m py_compile transformer_engine/pytorch/optimizers/fused_adam.py transformer_engine/pytorch/optimizers/fused_sgd.py transformer_engine/pytorch/optimizers/multi_tensor_apply.pyPYTHONPATH=/tmp/TransformerEngine pytest -q tests/pytorch/test_fused_optimizer.py::TestFusedAdam::test_empty_param_at_end_of_group tests/pytorch/test_fused_optimizer.py::TestFusedSGD::test_empty_param_at_end_of_groupte.optimizers.FusedAdamandte.optimizers.FusedSGD: both completedstep()andtorch.cuda.synchronize()te 0.0009999871253967285,torch 0.0009999871253967285git diff --check