Allow GradientAccumulationPlugin to be configured from AcceleratorConfig
Author: fabianlim
Pull request: yes

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Fixes #29425. Also please refer to the accompanying PR https://github.com/huggingface/accelerate/pull/2531 which implements an extra control `sync_each_batch` for `GradientAccumulationPlugin`. Before these changes, `GradientAccumulationPlugin` is configured by `Trainer` with a fixed set of hardcodes. This PR allows the user to set `sync_each_batch` if memory issues are faced when using FSDP with `no_sync`.

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [x] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [x] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [x] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@muellerzr

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker and @younesbelkada
- vision models: @amyeroberts
- speech models: @sanchit-gandhi
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @gante
- pipelines: @Narsil
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @muellerzr and @pacman100

Integrations:

- deepspeed: HF Trainer/Accelerate: @pacman100
- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization (bitsandbytes, autogpt): @SunMarc and @younesbelkada

Documentation: @stevhliu and @MKhalusova

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->


Comments:
> Hello @fabianlim , Thank you for the PR of Accelerate and this for reducing the memory usage with FSDP by forcing gradient synchronization at each step.
An overall comment: What if we change the default in Accelerate to not enable no_sync when preparing FSDP model. That way user would not have to pass an extra argument.

@pacman100 thanks for the message. I considered this, but decided not too, as i did some experiments with smaller models like llama7b and saw that turning the flag on or off made very little difference in terms of peak memory consumption. This is probably the case that it was activation dominated. 

Thus I concluded that this is use case dependent, and not a good idea to set it to `True` by default. It wouldnt be so good for the llama7b user to now explicity `sync_each_batch=False` to improve the throughput. (and its probably more insiduous because the user wouldnt be aware that its turned on, and that the training time could actually be faster at no impact if it were turned off) 

> thanks for the message. I considered this, but decided not too, as i did some experiments with smaller models like llama7b and saw that turning the flag on or off made very little difference in terms of peak memory consumption. This is probably the case that it was activation dominated.

> Thus I concluded that this is use case dependent, and not a good idea to set it to True by default. It wouldnt be so good for the llama7b user to now explicity sync_each_batch=False to improve the throughput. (and its probably more insiduous because the user wouldnt be aware that its turned on, and that the training time could actually be faster at no impact if it were turned off)

Got it, makes sense and thank you for all the details. Then maybe more documentation on the Accelerate docs and Transformers docs would help when this flag makes a difference. I think the info about negligible peak memory change for 7B model should be added to the Accelerate docs.

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_29589). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

> Thanks! We're getting closer.
> Also, no need to force-push your commits, we squash the commit history (and force-push makes it harder for us to track things)

Got it. Yes I mostly have a force-push habit to not bloat my commits. However I did it once to rebase, because I noticed the `tests_torch` were failing due to daily PR merges. 

After my latest changes the tests are failing again, but I shall refrain from rebasing for now. 

> Got it, makes sense and thank you for all the details. Then maybe more documentation on the Accelerate docs and Transformers docs would help when this flag makes a difference. I think the info about negligible peak memory change for 7B model should be added to the Accelerate docs.

@pacman100  how about [this addition to the accelerate docs](https://github.com/fabianlim/accelerate/commit/15e9ec69683a1c036e09df0bf4fdfcafb271af24). I decided to have the example about llama13b instead of 7b. The PR has already been merged so this will have to be another PR.



(You may also need to rebase from main for the test failures)

~> (You may also need to rebase from main for the test failures)~

~@muellerzr  should I rebase now or wait till the end until we resolve most of the changes? because if I rebase I will need to force push and that makes it harder to track?~ 
I pulled main's changes in, but probably one more pull is needed after https://github.com/huggingface/transformers/pull/29647 is merged.

I agree that conditionals are not preferred in tests, and there are other ways like using [`parameterized`](https://github.com/huggingface/transformers/blob/f738ab3b5d30e30c43a4c3d00ca8939f8a4d4427/docs/source/en/testing.md?plain=1#L631-L641), but we need the rework the `self.assert` lines to check conditionally (e.g., if `grad_accum_kwargs` is not specified, etc etc.). Also we will need some conditionals on how to populate the dicts going into the `@parameterized` decorator.  So it does come with its own complexities as well. 

Yes conditionals generally not best practice, but in this case I feel the usage it quite minor and feel that it does not affect readability very much. 

I have incorporated your suggestions to follow the FSDP style of `require_fsdp`.  In an attempt to be more consistent i have also moved `GRAD_ACCUM_KWARGS_VERSION_AVAILABLE` to the top of the file. I have tried to put comments. Note there is [one more use of `require_accelerate`](https://github.com/huggingface/transformers/blob/f738ab3b5d30e30c43a4c3d00ca8939f8a4d4427/tests/trainer/test_trainer.py#L1602) in the same `test_trainer.py` file that I cannot replace, because it changes the logic.

@fabianlim you'll need to run `make style; make quality;` to fix the style tests

its been a pleasure @muellerzr! i have just pulled the latest changes from `main`!

Hi @amyeroberts looking forward to your review! If there is anything I can address please feel free to let me know. FYI: @muellerzr 

@amyeroberts I pulled main again have [updated the code](https://github.com/huggingface/transformers/pull/29589/commits/968d4154750c9041bacd077f0855aaf476057d6d)  to conform to @muellerzr's changes in  https://github.com/huggingface/transformers/pull/29779