TrainingArguments shouldn't set ACCELERATE_MIXED_PRECISION env variable
Author: Thytu
Pull request: no
Labels: name: trainer

name: WIP

name: bug

### Feature request

When a mixed precision data type (such as `fp16`) is provided to `TrainingArguments`, it currently sets the `ACCELERATE_MIXED_PRECISION` ([cf](https://github.com/huggingface/transformers/blob/main/src/transformers/training_args.py#L1610)) environment variable to this mixed precision data type value.

Consequently, the `Trainer` class uses this mixed precision setting even when `TrainingArguments` are not explicitly provided as `args=training_args`.

The expected behavior should be that `Trainer` does not use mixed precision unless it's explicitly stated in the `TrainingArguments` provided as `args=training_args`.

CC: @muellerzr & @pacman100

### Motivation

I encountered this behavior while debugging a model that was producing `nan`s as output logits during training.

To troubleshoot this issue, I opted to stop providing `TrainingArguments` to `Trainer` in order to minimize potential sources of error but unfortunately that did not solved the problem.

After extensive investigation, this is how I discovered that `TrainingArguments` automatically sets `Trainer`'s mixed precision, even when not explicitly provided as an argument.

To prevent others from encountering the same debugging process, I advocate for mixed precision to be set only when explicitly required by the user by providing `TrainingArguments` to `Trainer`.

### Your contribution

I'm interested in opening a pull request to address this issue. This a good opportunity for me to learn more about the codebase.

Comments:
Hi @Thytu, thanks for digging into this! 

Indeed, this doesn't seem like a desirable behaviour. In fact, I'm not sure we should be setting global properties like this at all. Happy to review a PR, if you'd like to address this! 

There's a bit more nuance involved here with it than a blanket yes/no, as we do rely on it being global for Accelerate if used.

For example, if a user uses `accelerate launch` with a config that specifies `bf16`, this is why we use that env variable to do said configuration.

That being said, it should be done at the `Trainer` level I think as a better way, I was looking at that a bit this morning trying to come up with alternatives. 

Edit: actually. Thats after the fact. Why we set the global variable might be accelerate related. I'll see how hard it is for us to remove it from the transformer-related stuff without breaking.

As @amyeroberts said, feel free to open a PR migrating that logic to inside the Trainer itself :) 

Thank you both for your reply :)

Indeed, I saw that accelerate does rely on it, so simply removing the env var won't work.

With pleasure to work on a PR to address this issue. I should have some free time this weekend to work on it, though I can't provide a specific ETA at the moment.

I'll keep you updated on my progress!

Hey, I started diving into `transformers`' and `accelerate`'s source doc and few questions came to my mind:

* I couldn't found a test that verifies that AMP is activated when asked by the user. Is there any so I can use to be sure I did not break anything?
* I see that `ACCELERATE_MIXED_PRECISION` env var is set only if `TrainerArguments.half_precision_backend != "apex"`([cf](https://github.com/huggingface/transformers/blob/main/src/transformers/training_args.py#L1604)). Why the need to handle it differently in case the used backend is `apex`? Is it because `apex` is using its own arguments in `TrainerArguments` like `fp16_opt_level`? 

**Status:** Here is a draft of the PR [transformers#29562](https://github.com/huggingface/transformers/pull/29562)

----

### What's happening

> [!NOTE]
> No need to read this trace for anyone familiar with `transformers`' source code. It's simply a way for me to get an understanding of the situation and the various relationships among the objects involved.

At the init of the `Trainer` object, the method `create_accelerator_and_postprocess` is called ([here](https://github.com/huggingface/transformers/blob/main/src/transformers/trainer.py#L369)) which initialize the `Trainer.accelerator` attribute as an `Accelerator` object ([here](https://github.com/huggingface/transformers/blob/main/src/transformers/trainer.py#L4102)).

The `Accelerator` class itself retrieves the `AcceleratorState` singleton as `self.state` ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/accelerator.py#L371)). This singleton uses the `TODO` env var ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/state.py#L783-L787)) to set  `AcceleratorState._mixed_precision` as `fp16` ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/state.py#L808)) that can be retrieve using the `AcceleratorState.mixed_precision` method ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/state.py#L890-L891)).

Still in `Accelerator`'s `__init__` method, `Accelerator.state.mixed_precision` is used to know to set `Accelerator.native_amp` as `True` ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/accelerator.py#L448)) used by `Accelerator.prepare_model` ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/accelerator.py#L1325)) to know if it needs to use mixed precision.

Once in `Trainer._inner_training_loop`, the method `Accelerator.prepare` is called ([here](https://github.com/huggingface/transformers/blob/main/src/transformers/trainer.py#L1779)) which itself will call the `Accelerator.prepare_one` method with `first_pass` set as `True` ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/accelerator.py#L1262)), then calling `Accelerator.prepare_model` ([here](https://github.com/huggingface/accelerate/blob/main/src/accelerate/accelerator.py#L1138)) setting the model in mixed precision.

> [!WARNING]
> Here I described the trace when training the model but obviously same happens for the data and for evaluation/prediction.