Repository navigation
Conversation
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
|
/te-ci |
pggPL
left a comment
There was a problem hiding this comment.
I've looked on all the changes and they all LGTM, I run subagents throughout TE to clean some mess and they suggested following changes:
With PyTorch 2.8 now required, there are a few remaining compatibility cleanups worth considering:
In
transformer_engine/pytorch/__init__.py:136–139, replace thetry/except AttributeErrorarounderror_on_nested_jit_tracewith the direct assignment. The fallback is for an API added in 2.2.In
transformer_engine/pytorch/distributed.py:24–29, importtorch.distributed._symmetric_memorydirectly and removeHAS_TORCH_SYMMETRIC, its checks at lines 1978 and 2042, and the corresponding skip intests/pytorch/distributed/test_symmetric_memory.py:69. The module exists in 2.8; retain checks for actual backend capabilities, GPU count, and peer access.In
transformer_engine/pytorch/utils.py:34–37, importcomptimedirectly and remove the_comptime is not Nonecondition at line 52. Keep thetorch.compiler.is_compiling()distinction between compile-time and eager warnings.In
tests/pytorch/distributed/ep_reference.py:44–48,132, replace the dtype-availability lookups with direct references totorch.float8_e4m3fn,torch.float8_e8m0fnu, andtorch.float4_e2m1fn_x2. These attributes exist in 2.8. Preserve the format/dtype validation and support for packeduint8NVFP4 data.For the CUDA/NVIDIA tests in
tests/pytorch/attention/test_cp_utils.py:1022,1043, remove the skips for a missingshared_memory_per_blockproperty. It is exposed by PyTorch 2.8 CUDA device properties. This is a CUDA-specific guarantee, not a guarantee for every PyTorch backend.Use the public import paths available in 2.8:
torch.distributed.tensor.DTensorintransformer_engine/pytorch/optimizers/{fused_adam,multi_tensor_apply}.py, andtorch.distributed.fsdp.fully_shardin the threetests/pytorch/distributed/fsdp2_tests/run_fsdp2_*.pyfiles andexamples/pytorch/quantized_model_init/fully_shard.py. Also update thefused_adam.py:557docstring to referencetorch.amp.GradScaler.The graph-safe RNG compatibility branches can also be simplified, but I would prefer a separate follow-up because they span RNG tracking, checkpointing, and CUDA Graph capture. The APIs checked by
graph_safe_rng_available()are present in 2.8. Remove that availability check and the unreachable old-version branches indistributed.py:95,114,142,886–901,925–927,graph.py:485,1715–1722,1743–1747, and the assertion inattention/dot_product_attention/dot_product_attention.py:2333. Preserve the explicitgraph_safe=Falsepath and RNG state save/restore behavior; API availability does not make those paths obsolete. Validate the cleanup with focused checkpointing and CUDA Graph RNG/replay tests.Optional wording cleanup: replace the obsolete “NVFusion” description in
docs/envvars.rst:409; remove the old TorchScript conditioning limitation from the docstrings inpytorch/jit.py:116,144; renametest_sanity_amp_and_nvfuserto describe AMP and internal fusions; and remove the stalebias_gelu_nvfusionreference in the commented-out FSDP code inpytorch/module/layernorm_mlp.py:1375. Keep the existing environment-variable names for compatibility. Inpytorch/tensor/float8_blockwise_tensor.py:620, retain the private-API warning but drop the outdated “tested with PyTorch 2.5+” claim; do not replace it with a new testing claim without validation.I would treat the Dynamo
countersfallbacks intests/pytorch/test_torch_compile.py:20–23andtests/pytorch/distributed/run_layer_with_overlap.py:22–25separately. The import exists in 2.8, but the test helper explicitly describes graceful handling of future private-API changes. These are not simply pre-2.8 compatibility branches. I would retain them unless we deliberately choose to fail tests when that API changes; if so, update the associated optional counter checks as well, rather than changing only the imports.
[High impact] The PR appears safe to merge. SummaryRaises the minimum supported versions to PyTorch 2.8 and JAX 0.5.3.
No actionable issues found. Tests were not run. Reviews (1) · Last reviewed commit: "Merge branch 'main' into update_minimum_..." · Reviewed by Greptile |
Description
Please include a brief summary of the changes, relevant motivation and context.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: