PR #35438 introduced a new bug
Author: techkang
Pull request: no
Labels: name: bug

### System Info

(base) MBP-HD6JD9Q599-2052 :: ~/code/transformers ‹main*› % transformers-cli env                                                   1 ↵

Copy-and-paste the text below in your GitHub issue and FILL OUT the two last points.

- `transformers` version: 4.49.0.dev0
- Platform: macOS-14.6.1-arm64-arm-64bit
- Python version: 3.12.4
- Huggingface_hub version: 0.27.1
- Safetensors version: 0.4.3
- Accelerate version: 1.2.1
- Accelerate config:    not found
- PyTorch version (GPU?): 2.5.1 (False)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using distributed or parallel set-up in script?: <fill in>


### Who can help?

@muellerzr @hiyouga @ArthurZucker @SunMarc 

### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [X] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

```
export RUN_SLOW=True
pytest tests/trainer/test_trainer.py::TrainerIntegrationPrerunTest::test_gradient_accumulation_loss_alignment_with_loss_func

======================================================= short test summary info =======================================================
FAILED tests/trainer/test_trainer.py::TrainerIntegrationPrerunTest::test_gradient_accumulation_loss_alignment_with_loss_func - AssertionError: 3.0949999999999998 not less than 0.01 : Difference 3.0949999999999998 is not within 0.01
=================================================== 1 failed, 2 warnings in 54.91s ====================================================
```

### Expected behavior

Test Passed.

Comments:
PR link: https://github.com/huggingface/transformers/pull/35438

I think the PR: https://github.com/huggingface/transformers/pull/35438 should be reverted and the proper way to fix the bug mentioned in the PR is as follows.

In the following code, `loss` is scaled when `num_items_in_batch is None`.
https://github.com/huggingface/transformers/blob/main/src/transformers/trainer.py#L3712-L3713

But the true meaning of this code is to scale loss when GA bug fix is not performed. This is not identical to `num_items_in_batch is None` after recent PRs. So it should be changed to 
```
if not self.model_accepts_loss_kwargs and self.compute_loss_func is None
```

Thanks! Would you like to make a PR for this? Else I can do so today

@muellerzr Thanks for reply. I will open a PR today.

Hi @techkang , it has been an evidence that #35121 introduces bug making the loss of the Qwen2VL model incorrect through our rigorous experiments in #35438 . I think we should not only focus on the model with loss function but also pay attention to the models without `loss_kwargs`. There should be a solution that let both the two conditions work instead of simply reverting our fix.  cc @muellerzr 

Hi @hiyouga , https://github.com/huggingface/transformers/pull/35121 indeed introduced a bug but I don't think https://github.com/huggingface/transformers/pull/35438 is the proper way to fix it. Can you try to varify the Qwen2VL loss by new PR: https://github.com/huggingface/transformers/pull/35651?

@techkang Yep, the new PR looks better to me, let us perform some experiments on it