FSDP Reshard-only Post-Backward Multi-Grad Hooks Need to be Conditionally Re-Registered

[speediedan]

🐛 Describe the bug

In order to support common fine-tuning patterns wherein a subset of model parameters are thawed after n fine-tuning steps, FSDP reshard-only post-backward multi-grad hooks need to be conditionally re-registered as normal post-backward hooks (and attached to AccumulateGrad).

Absent the proposed changes below (or a functionally similar implementation), thawing model parameters after n fine-tuning steps triggers a SystemError on run_backward:

./test_fsdp_fine_tune.py::TestFSDPFineTune::test_backward_reshard_hooks Failed with Error: Process 1 exited with error code 10 and exception:
Traceback (most recent call last):
  File "/home/speediedan/repos/pytorch/torch/testing/_internal/common_distributed.py", line 657, in run_test
    getattr(self, test_name)()
  File "/home/speediedan/repos/pytorch/torch/testing/_internal/common_distributed.py", line 543, in wrapper
    fn()
  File "/home/speediedan/repos/pytorch/torch/testing/_internal/common_distributed.py", line 174, in wrapper
    return func(*args, **kwargs)
  File "/home/speediedan/repos/pytorch/test/distributed/fsdp/test_fsdp_fine_tune.py", line 61, in test_backward_reshard_hooks
    self.run_subtests(
  File "/home/speediedan/repos/pytorch/torch/testing/_internal/common_fsdp.py", line 895, in run_subtests
    return run_subtests(self, *args, **kwargs)
  File "/home/speediedan/repos/pytorch/torch/testing/_internal/common_fsdp.py", line 847, in run_subtests
    test_fn(*test_args, **test_kwargs, **subtest_kwargs)
  File "/home/speediedan/repos/pytorch/test/distributed/fsdp/test_fsdp_fine_tune.py", line 114, in _test_backward_reshard_hooks
    output.sum().backward()
  File "/home/speediedan/repos/pytorch/torch/_tensor.py", line 491, in backward
    torch.autograd.backward(
  File "/home/speediedan/repos/pytorch/torch/autograd/__init__.py", line 204, in backward
    Variable._execution_engine.run_backward(  # Calls into the C++ engine to run the backward pass
SystemError: <built-in method run_backward of torch._C._EngineBase object at 0x7f9defadf8b0> returned NULL without setting an exception

To reproduce

Update the existing test_fsdp_fine_tune.py::TestFSDPFineTune::test_backward_reshard_hooks test as follows:

        with mock.patch(
            "torch.distributed.fsdp._runtime_utils._post_backward_reshard",
            _post_backward_reshard_with_count,
        ):
            for step_num in range(2):
                if step_num == 1:
                    post_backward_reshard_count = 0
                    # changes requires grad to simulate post-backward hook re-registration behavior after n steps
                    seq[4]._flat_param.requires_grad_(True)
                inp = torch.randn((8, 5), device="cuda", requires_grad=inp_requires_grad)
                output = seq(inp)
                if step_num == 1:
                    seq.zero_grad()
                # note here that with the proposed update to ` _register_post_backward_hooks`, the previous
                # `requires_grad=False` path multi-grad post backward hook will have been removed and replaced with
                # appropriate `AccumulateGrad` object, otherwise, a `SystemError` will be thrown here on `step_num == 1`
                output.sum().backward()
                # If the input does not require gradient, then the 0th frozen
                # linear gets resharded in the catch-all reshard since we cannot
                # register an autograd hook on it
                expected_post_backward_reshard_count = (
                    self.NUM_LINEARS if inp_requires_grad else self.NUM_LINEARS - 1
                )
                self.assertEqual(
                    post_backward_reshard_count, expected_post_backward_reshard_count
                )

Potential resolution

While I will be away the next couple weeks and so unfortunately won't be able to submit/collaborate on any PR associated with this issue in that time, one possible fix is to patch _register_post_backward_hooks as follows:

        already_registered = hasattr(flat_param, "_post_backward_hook_state")
        # if already_registered or not flat_param.requires_grad:
        #    continue
        if not flat_param.requires_grad:
            continue
        if already_registered:
            flat_param._post_backward_hook_state.handles[-1].remove()
            delattr(flat_param, "_post_backward_hook_state")
        # Get the `AccumulateGrad` object

After applying that fix, the extended test above should pass again.

I opened this issue as a bug given the prevalence of the aforementioned fine-tuning pattern but feel free to re-label it as an enhancement request if you think that is a more appropriate classification. Thanks again for all your continued contributions, the PyTorch Distributed team is awesome!! 🚀 🎉

@awgu @rohan-varma

Versions

PyTorch version: 2.1.0a0+git3a823e4
Is debug build: False
CUDA used to build PyTorch: 12.1
ROCM used to build PyTorch: N/A

OS: Ubuntu 20.04.6 LTS (x86_64)
GCC version: (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
Clang version: Could not collect
CMake version: version 3.26.4
Libc version: glibc-2.31

Python version: 3.10.11 (main, May 16 2023, 00:28:57) [GCC 11.2.0] (64-bit runtime)
Python platform: Linux-5.4.0-152-generic-x86_64-with-glibc2.31
Is CUDA available: True
CUDA runtime version: 12.1.105
CUDA_MODULE_LOADING set to: LAZY
GPU models and configuration:
GPU 0: NVIDIA GeForce RTX 4090
GPU 1: NVIDIA GeForce RTX 2070 SUPER

Nvidia driver version: 530.41.03
cuDNN version: Probably one of the following:
/usr/lib/x86_64-linux-gnu/libcudnn.so.8.9.2
/usr/lib/x86_64-linux-gnu/libcudnn_adv_infer.so.8.9.2
/usr/lib/x86_64-linux-gnu/libcudnn_adv_train.so.8.9.2
/usr/lib/x86_64-linux-gnu/libcudnn_cnn_infer.so.8.9.2
/usr/lib/x86_64-linux-gnu/libcudnn_cnn_train.so.8.9.2
/usr/lib/x86_64-linux-gnu/libcudnn_ops_infer.so.8.9.2
/usr/lib/x86_64-linux-gnu/libcudnn_ops_train.so.8.9.2
/usr/local/cuda-10.2/targets/x86_64-linux/lib/libcudnn.so
HIP runtime version: N/A
MIOpen runtime version: N/A
Is XNNPACK available: True

CPU:
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
Address sizes: 43 bits physical, 48 bits virtual
CPU(s): 24
On-line CPU(s) list: 0-23
Thread(s) per core: 2
Core(s) per socket: 12
Socket(s): 1
NUMA node(s): 1
Vendor ID: AuthenticAMD
CPU family: 23
Model: 1
Model name: AMD Ryzen Threadripper 1920X 12-Core Processor
Stepping: 1
Frequency boost: enabled
CPU MHz: 1997.986
CPU max MHz: 3500.0000
CPU min MHz: 2200.0000
BogoMIPS: 6986.08
Virtualization: AMD-V
L1d cache: 384 KiB
L1i cache: 768 KiB
L2 cache: 6 MiB
L3 cache: 32 MiB
NUMA node0 CPU(s): 0-23
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Retbleed: Vulnerable
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl and seccomp
Vulnerability Spectre v1: Mitigation; usercopy/swapgs barriers and __user pointer sanitization
Vulnerability Spectre v2: Mitigation; Retpolines, IBPB conditional, STIBP disabled, RSB filling, PBRSB-eIBRS Not affected
Vulnerability Srbds: Not affected
Vulnerability Tsx async abort: Not affected
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc cpuid extd_apicid amd_dcm aperfmperf pni pclmulqdq monitor ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw skinit wdt tce topoext perfctr_core perfctr_nb bpext perfctr_llc mwaitx cpb hw_pstate sme ssbd sev ibpb vmmcall fsgsbase bmi1 avx2 smep bmi2 rdseed adx smap clflushopt sha_ni xsaveopt xsavec xgetbv1 xsaves clzero irperf xsaveerptr arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload vgif overflow_recov succor smca

Versions of relevant libraries:
[pip3] mypy==0.981
[pip3] mypy-extensions==0.4.3
[pip3] numpy==1.25.0
[pip3] torch==2.1.0a0+git3a823e4
[conda] blas 1.0 mkl
[conda] magma-cuda121 2.6.1 1 pytorch
[conda] mkl 2023.1.0 h6d00ec8_46342
[conda] mkl-include 2023.1.0 h06a4308_46342
[conda] mkl-service 2.4.0 py310h5eee18b_1
[conda] mkl_fft 1.3.6 py310h1128e8f_1
[conda] mkl_random 1.2.2 py310h1128e8f_1
[conda] numpy 1.25.0 py310h5f9d8c6_0
[conda] numpy-base 1.25.0 py310hb5e798b_0
[conda] torch 2.1.0a0+git3a823e4 dev_0

cc @zhaojuanmao @mrshenli @rohan-varma @awgu