fix: model.set_requires_gradient_sync(False) should be called to turn off gradient synchronization in FSDP2

[EquationWalker]

In FSDP2, the model(FSDPModule) does not have no_sync() and instead calls model.set_requires_gradient_sync(False) to turn off gradient synchronization. See at torch.distributed.fsdp.FSDPModule.set_requires_gradient_sync

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[S1ro1]

Thanks!

[bghira]

@EquationWalker this causes surfaces a possible user-side bug:

 'list' object has no attribute 'set_requires_gradient_sync' 

did not happen before because everything inside the function silently fails through when whatever they're looking for isn't there - see the lines for DeepSpeed ZeRO 3 check earlier where it uses getattr with a default that then returns a null context.

not sure that it helps anyone to know this, or if some earlier check should occur when a user passes an unpacked list to accumulate()

[EquationWalker]

@EquationWalker this causes surfaces a possible user-side bug:

 'list' object has no attribute 'set_requires_gradient_sync' 

did not happen before because everything inside the function silently fails through when whatever they're looking for isn't there - see the lines for DeepSpeed ZeRO 3 check earlier where it uses getattr with a default that then returns a null context.

not sure that it helps anyone to know this, or if some earlier check should occur when a user passes an unpacked list to accumulate()

I think this check should occur in Accelerator.accmulate(). If the user passes a list object and we use nullcontext, the user will not know that gradient synchronization is not actually turned off. Instead, we should unpack its list object in Accelerator.accumulate() or throw an Expection when user passed a list object.