Allow get attributes on DDP similar to FSDP

[mayank31398]

FSDP implements the following logic but its missing from DDP.
This PR adds an equivalent function for the same.

    def __getattr__(self, name: str) -> Any:
        """Forward missing attributes to the wrapped module."""
        try:
            return super().__getattr__(name)  # defer to nn.Module's logic
        except AttributeError:
            return getattr(self._fsdp_wrapped_module, name)

cc @mrshenli @pritamdamania87 @zhaojuanmao @satgera @gqchen @aazzolini @osalpekar @jiayisuse @H-Huang @kwen2501 @awgu @penguinwu @fegin @XilunWu @wanchaol @fduwjj @wz337 @tianyu-l @wconstab @yf225 @chauhang @d4l3k

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/128620

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• NVIDIA runners have a blacklog of 2000+ jobs

❌ 3 New Failures, 14 Unrelated Failures

As of commit b32c632 with merge base c63ccea ():

NEW FAILURES - The following jobs have failed:

• linux-binary-libtorch-pre-cxx11 / libtorch-cpu-shared-with-deps-pre-cxx11-build / build (gh)
  Error downloading packages:
• trunk / macos-py3-arm64-mps / test (mps, 1, 1, macos-m1-13) (gh)
  test_mps.py::TestMPS::test_mps_allocator_module
• trunk / macos-py3-arm64-mps / test (mps, 1, 1, macos-m1-14) (gh)
  test_mps.py::TestMPS::test_mps_allocator_module

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

• pull / linux-focal-py3.11-clang10 / test (dynamo, 2, 3, linux.2xlarge) (gh) (disabled by #123624 but the issue was closed recently and a rebase is needed to make it pass)
  profiler/test_torch_tidy.py::TestTorchTidyProfiler::test_optimizer_parameters_sgd
• pull / linux-focal-py3.12-clang10 / test (dynamo, 2, 3, linux.2xlarge) (gh) (disabled by #123624 but the issue was closed recently and a rebase is needed to make it pass)
  profiler/test_torch_tidy.py::TestTorchTidyProfiler::test_optimizer_parameters_sgd
• pull / linux-focal-py3.8-clang10 / test (dynamo, 1, 3, linux.2xlarge) (gh) (disabled by #123624 but the issue was closed recently and a rebase is needed to make it pass)
  profiler/test_torch_tidy.py::TestTorchTidyProfiler::test_optimizer_parameters_sgd

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

• pull / linux-focal-cuda12.1-py3.10-gcc9-experimental-split-build / test (default, 1, 5, linux.4xlarge.nvidia.gpu, unstable) (gh) (#129080)
  Process completed with exit code 9.
• pull / linux-focal-cuda12.1-py3.10-gcc9-experimental-split-build / test (default, 2, 5, linux.4xlarge.nvidia.gpu, unstable) (gh) (#129080)
  Process completed with exit code 9.
• pull / linux-focal-cuda12.1-py3.10-gcc9-experimental-split-build / test (default, 3, 5, linux.4xlarge.nvidia.gpu, unstable) (gh) (#129080)
• pull / linux-focal-cuda12.1-py3.10-gcc9-experimental-split-build / test (default, 4, 5, linux.4xlarge.nvidia.gpu, unstable) (gh) (#129080)
  Process completed with exit code 9.
• pull / linux-focal-cuda12.1-py3.10-gcc9-experimental-split-build / test (default, 5, 5, linux.4xlarge.nvidia.gpu, unstable) (gh) (#129080)
  Process completed with exit code 9.
• pull / linux-focal-py3.12-clang10-experimental-split-build / test (default, 1, 3, linux.2xlarge, unstable) (gh) (#129248)
• pull / linux-focal-py3.12-clang10-experimental-split-build / test (default, 2, 3, linux.2xlarge, unstable) (gh) (#129248)
  Process completed with exit code 9.
• pull / linux-focal-py3.12-clang10-experimental-split-build / test (default, 3, 3, linux.2xlarge, unstable) (gh) (#129248)
  Process completed with exit code 9.
• pull / linux-focal-py3.12-clang10-experimental-split-build / test (dynamo, 1, 3, linux.2xlarge, unstable) (gh) (#129256)
  Process completed with exit code 9.
• pull / linux-focal-py3.12-clang10-experimental-split-build / test (dynamo, 2, 3, linux.2xlarge, unstable) (gh) (#129256)
  Process completed with exit code 9.
• pull / linux-focal-py3.12-clang10-experimental-split-build / test (dynamo, 3, 3, linux.2xlarge, unstable) (gh) (#129256)
  Process completed with exit code 9.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mayank31398 / name: Mayank Mishra (b32c632, cc8e411)

[fegin]

@awgu Does __getattr__ cause any issues for FSDP? I don't recall any. If so, I think it is okay to land the PR.

[awgu]

@fegin I think that this change is generally okay, but I wonder what is the motivation. I am guessing to make it more similar to FSDP mainly? (Otherwise, probably everyone already is doing ddp_module.module.<attr>.)

This approach of attribute forwarding can have some non-intuitive behavior in edge cases. For example, recently, I hit an issue where we try to access a property p on FullyShardedDataParallel but inside def p:, we hit a real AttributeError. In that case, the except AttributeError would misleadingly report that the wrapped module does not have attribute p.

[mayank31398]

yeah @awgu there can be non-intuitive changes
completely agree since I have hit similar problems in past.
this change is just to make it similar to FSDP API since the NO_SHARD route is slated for deprecation in FSDP.

Also, in another PR, I want to add DDP.clip_grad_norm_ similar to FSDP.clip_grad_norm_
There are existing code which could just switch between DDP and FSDP with no change with these changes.

[awgu]

DDP.clip_grad_norm_

I would caution against this. DDP should just use torch.nn.utils.clip_grad_norm_. I do not see why we should attach it onto the module just as syntactic sugar. It feels like this could exist inside the trainer (which is what many trainers I have seen do).

[mayank31398]

makes sense @awgu. thanks for the comments

[mayank31398]

I am not sure what the failing test here is @awgu
doesn't seem related to the PR

[mayank31398]

Hi, pinging again for this issue

[mayank31398]

@awgu is this still relevant?

[awgu]

This is okay with me.

[awgu]

Note for future: If when trying to get a property attribute p, p's implementation raises an AttributeError, and self.module does not have p either, then the error message here will be non-intuitive since it would say that self.module does not have the attribute p, where the useful error would be that accessing ddp_module.p raised an AttributeError when accessing .p.

[awgu]

pull / linux-focal-cuda12.1-py3.10-gcc9 / test (default, 4, 5, linux.4xlarge.nvidia.gpu) failure is unrelated (foreach ops test, whereas this is DDP)

[awgu]

@pytorchbot merge -i

[pytorchmergebot]

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

[awgu]

@pytorchbot merge -i

[pytorchmergebot]

Merge started

Your change will be merged while ignoring the following 1 checks: pull / linux-focal-cuda12.1-py3.10-gcc9 / test (default, 4, 5, linux.4xlarge.nvidia.gpu)

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

[jeanschmidt]

It seems that this PR introduced the following error: https://github.com/pytorch/pytorch/actions/runs/9720692485/job/26833212318

[jeanschmidt]

@pytorchbot revert -m "Reverting in order to see if the trunk error on inductor is fixed" -c nosignal

[pytorchmergebot]

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

[pytorchmergebot]

@mayank31398 your PR has been successfully reverted.

This PR was reopened (likely due to being reverted), so your approval was removed. Please request another review.

[mayank31398]

yikes!! @jeanschmidt
Hmm, I guess we can drop this for now.
But I think a cleaner API is needed for DDP. Currently, there are too much differences between DDP and FSDP.