noamaanMulla-03/transformers-issues huggingface dataset:
[FIX] `offload_weight()` takes from 3 to 4 positional arguments but 5 were given
Author: faaany
Pull request: yes

## What does this PR do?
2 tests fail with the following messages:
```bash
FAILED tests/test_modeling_utils.py::ModelUtilsTest::test_from_pretrained_disk_offload_derived_to_base_model - TypeError: offload_weight() takes from 3 to 4 positional arguments but 5 were given
FAILED tests/test_modeling_utils.py::ModelUtilsTest::test_from_pretrained_disk_offload_task_model - TypeError: offload_weight() takes from 3 to 4 positional arguments but 5 were given
```
Below is the Traceback:
```bash
Traceback (most recent call last):
  File "/mnt/disk4/fanlilin/transformers/fanli.py", line 42, in <module>
    new_model_with_offload = AutoModel.from_pretrained(
  File "/mnt/disk4/fanlilin/transformers/src/transformers/models/auto/auto_factory.py", line 563, in from_pretrained
    return model_class.from_pretrained(
  File "/mnt/disk4/fanlilin/transformers/src/transformers/modeling_utils.py", line 3507, in from_pretrained
    ) = cls._load_pretrained_model(
  File "/mnt/disk4/fanlilin/transformers/src/transformers/modeling_utils.py", line 3932, in _load_pretrained_model
    new_error_msgs, offload_index, state_dict_index = _load_state_dict_into_meta_model(
  File "/mnt/disk4/fanlilin/transformers/src/transformers/modeling_utils.py", line 798, in _load_state_dict_into_meta_model
    state_dict_index = offload_weight(param, param_name, model, state_dict_folder, state_dict_index)
TypeError: offload_weight() takes from 3 to 4 positional arguments but 5 were given
```

I remove the additional `model` argument and also change `require_torch_accelerator` to `require_torch_gpu`. Because  `0` implied `cuda:0` and on non-nv-gpu devices, I would get `AssertionError: Torch not compiled with CUDA enabled`.

@younesbelkada @SunMarc 


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_29457). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[tests] enable test_pipeline_accelerate_top_p on XPU   
Author: faaany
Pull request: yes

## What does this PR do?
Is there any particular reason why we use `device_map="auto"` in this test `test_pipeline_accelerate_top_p`? If not, I would suggest using the device-agnostic variable `torch_device` instead of `device_map="auto"` just like in all other tests e.g. `test_small_model_fp16`. 

Another reason is that the `device_map="auto"` mechanism is still not mature on XPU, causing the model to be loaded on the CPU, rather than on XPU. With this fix, `test_pipeline_accelerate_top_p` will definitively work on all devices. Below is an evidence for XPU:

BEFORE
```bash
================================================== short test summary info ===================================================
SKIPPED [1] tests/pipelines/test_pipelines_text_generation.py:221: test requires TensorFlow
SKIPPED [1] tests/pipelines/test_pipelines_text_generation.py:378: test requires CUDA
SKIPPED [1] tests/pipelines/test_pipelines_text_generation.py:180: test requires TensorFlow
FAILED tests/pipelines/test_pipelines_text_generation.py::TextGenerationPipelineTests::test_small_model_fp16 - RuntimeError: could not execute a primitive
===================================== 1 failed, 5 passed, 3 skipped, 8 warnings in 7.18s =====================================
```

AFTER

```bash
================================================== short test summary info ===================================================
SKIPPED [1] tests/pipelines/test_pipelines_text_generation.py:221: test requires TensorFlow
SKIPPED [1] tests/pipelines/test_pipelines_text_generation.py:378: test requires CUDA
SKIPPED [1] tests/pipelines/test_pipelines_text_generation.py:180: test requires TensorFlow
========================================== 6 passed, 3 skipped, 8 warnings in 7.52s ==========================================
```

Pls have a review, thx! @Narsil @ArthurZucker 

Comments:
> I think in this one we want to test accelerate support with sampling so it's probably on purpose!

Could you elaborate more on the relationship between top_p sampling and device? I don't see the need to use "device_map='auto'" in this case, e.g. if I change "device_map='auto'" to "device=torch_device", will the test on top_p sampling fail?   

> The name of the test is `test_pipeline_accelerate_top_p` to make sure we test `accelerate` dispatching automatically. I thought testing `"auto"` made sense but you're right here, we can test any device, as long as we make sure we test the accelerate support here!

done, thanks for the review!
[tests] use `torch_device` instead of `auto` for model testing 
Author: faaany
Pull request: yes

## What does this PR do?
When running test cases under the models folder on XPU, I found that many model tests fail at the same test `test_model_parallel_beam_search`, e.g. 
```bash
FAILED tests/models/bigbird_pegasus/test_modeling_bigbird_pegasus.py::BigBirdPegasusModelTest::test_model_parallel_beam_search - RuntimeError: Expected all tensors to be on the same device, but found at least two devices, cpu and xpu:0! (when che
```
This is because `device_map="auto"` is used. As elaborated in [this](https://github.com/huggingface/transformers/pull/29309) PR, the device_map="auto" mechanism is still not mature on XPU, causing model to be loaded on CPU, rather than on XPU.

If there is no particular reason for using `auto`, I would suggest using `torch_device` instead `auto`, because `torch_device` is more specific than `auto` and we don't have the need to use `auto`, e.g. large model inference, in our tests anyway. WDYT? @ArthurZucker 

Comments:
Hi @faaany - thanks for opening this PR! Using device_map="auto" is necessary for this test - it's checking that beam search works when the model is split across devices. If it doesn't work with XPU, then you can add a skip with unittest e.g.

```py
if "xpu" in torch_device:
    return unittest.skip("device_map='auto' does not work with XPU devices")
```

Hi @amyeroberts , thanks so much for reviewing this PR! I updated my patch and used `torch_device=="xpu"` instead of `'xpu' in torch_device`, hope this is fine for you. And yes, I will remove this skip once `auto` works on XPU.  

> Thanks for handling!
> 
> Just a small nit

done, thx!
[tests] add the missing `require_sacremoses` decorator 
Author: faaany
Pull request: yes

## What does this PR do?
When running:
```bash
python -m pytest -rA tests/models/biogpt
```
I got a lot of failed tests due to uninstalled package:
```bash
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_special_tokens_mask_input_pairs - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_token_type_ids - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_tokenize_special_tokens - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_tokenizer_slow_store_full_signature - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_tokenizers_common_ids_setters - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_tokenizers_common_properties - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_tokenizers_special_tokens_properties_unset_0 - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
FAILED tests/models/biogpt/test_tokenization_biogpt.py::BioGptTokenizationTest::test_tokenizers_special_tokens_properties_unset_1 - ImportError: You need to install sacremoses to use BioGptTokenizer. See https://pypi.org/project/sacremoses/ for installation.
=============================== 49 failed, 132 passed, 57 skipped, 19 warnings in 85.18s (0:01:25) ===============================
```
The reason lies at [this](https://github.com/huggingface/transformers/blob/d45f47ab7f7c31991bb98a0302ded59ab6adac31/src/transformers/models/biogpt/tokenization_biogpt.py#L118) line. Instead of letting tests fail, we should add a decorator to skip these tests if the package is not installed.

Comments:
Hi @faaany, there's a failing test, which I don't think is related to this PR but I haven't seen on other PRs. Could you try rebasing onto main to see if this resolves the issue?

> Hi @faaany, there's a failing test, which I don't think is related to this PR but I haven't seen on other PRs. Could you try rebasing onto main to see if this resolves the issue?

done and yes, the failed test is not related to this PR.
[tests] make `test_trainer_log_level_replica` to run on accelerators with more than 2 devices
Author: faaany
Pull request: yes

## What does this PR do?
When I run the following test on NV A100 with 7 available cards, 
```bash
RUN_SLOW=1 pytest -rA tests/extended/test_trainer_ext.py::TestTrainerExt
```
I got 2 failing tests:
```bash
================================================= short test summary info =================================================
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_run_seq2seq
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_run_seq2seq_ddp
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_run_seq2seq_dp
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_0_base
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_3_mixed
SKIPPED [1] tests/extended/test_trainer_ext.py:107: test requires apex
SKIPPED [1] tests/extended/test_trainer_ext.py:174: test requires bitsandbytes and torch
SKIPPED [1] tests/extended/test_trainer_ext.py:93: test requires 0 or 1 accelerator
FAILED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_1_low - AssertionError: 7 != 2
FAILED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_2_high - AssertionError: 6 != 1
```
This is because the variable `n_matches` is hard-coded and assumes that there should be 2 cards available on the device. This PR fixes this issue by setting `n_gpus_to_use` to 2 for this test. 

Below is the tests results after the fix:
```bash
=================================================== short test summary info ====================================================
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_run_seq2seq
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_run_seq2seq_ddp
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_run_seq2seq_dp
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_0_base
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_1_low
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_2_high
PASSED tests/extended/test_trainer_ext.py::TestTrainerExt::test_trainer_log_level_replica_3_mixed
SKIPPED [1] tests/extended/test_trainer_ext.py:109: test requires apex
SKIPPED [1] tests/extended/test_trainer_ext.py:182: test requires bitsandbytes and torch
SKIPPED [1] tests/extended/test_trainer_ext.py:95: test requires 0 or 1 accelerator
==================================== 7 passed, 3 skipped, 21 warnings in 143.86s (0:02:23) ====================================
```

Could you help review this PR? Thanks a lot! 
@amyeroberts

Comments:
[doc] fix some typos and add `xpu` to the testing documentation
Author: faaany
Pull request: yes

## What does this PR do?
the same as the title suggests.

## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] 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.
- [ ] 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).
- [ ] Did you write any new necessary tests?

## Who can review?
@amyeroberts, @stevhliu and @MKhalusova


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_29894). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[tests] make 2 tests device-agnostic 
Author: faaany
Pull request: yes

# What does this PR do?
After Fix:
```bash
$ RUN_SLOW=1 TRANSFORMERS_TEST_DEVICE=xpu TRANSFORMERS_TEST_DEVICE_SPEC=spec.py pytest tests/test_modeling_utils.py::ModelUtilsTest::test_model_parallelism_gpt2 tests/models/blip_2/test_modeling_blip_2.py::Blip2ModelIntegrationTest::test_inference_t5_multi_accelerator
==================================================== test session starts =====================================================
platform linux -- Python 3.9.19, pytest-7.4.4, pluggy-1.4.0
rootdir: /soft/fanli/transformers
configfile: pyproject.toml
plugins: xdist-3.5.0, metadata-3.1.1, reportlog-0.4.0, anyio-4.2.0, excel-1.6.0, hypothesis-6.98.0, html-4.1.1, timeout-2.3.1, env-1.1.3
collected 2 items                                                                                                            

tests/test_modeling_utils.py .                                                                                         [ 50%]
tests/models/blip_2/test_modeling_blip_2.py .                                                                          [100%]
=============================================== 2 passed, 5 warnings in 11.19s ===============================================
```

## Who can review?
@ArthurZucker


Comments:
It's still draft tho, should we merge? 

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

Hi @ArthurZucker, thanks for the review! Yes, let's merge it.
[tests] add the missing `require_torch_multi_gpu` flag
Author: faaany
Pull request: yes

## What does this PR do?
For Non-GPU environment, `test_multi_gpu_data_parallel_forward` should be skipped instead of failing. 

## 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?
- [ ] 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.
- [ ] 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).
- [ ] Did you write any new necessary tests?


## Who can review?

@ArthurZucker and @amyeroberts


Comments:
[tests] add `require_bitsandbytes` marker
Author: faaany
Pull request: yes

# What does this PR do?
The below 2 tests fails because I haven't installed bitsandbytes in my environment: 
```bash
FAILED tests/models/persimmon/test_modeling_persimmon.py::PersimmonIntegrationTest::test_model_8b_chat_greedy_generation - ImportError: Using `bitsandbytes` 8-bit quantization requires Accelerate: `pip install accelerate` and the latest version of bitsandbytes: `pip install -i https://pypi.org/simple/ bitsandbytes`
FAILED tests/models/persimmon/test_modeling_persimmon.py::PersimmonIntegrationTest::test_model_8b_chat_logits - ImportError: Using `bitsandbytes` 8-bit quantization requires Accelerate: `pip install accelerate` and the latest version of bitsandbytes: `pip install -i https://pypi.org/simple/ bitsandbytes`
```
But these 2 tests should be skipped instead of failed. This PR fixes this issue by adding the `require_bitsandbytes` marker.

## Who can review?
@ArthurZucker 

Comments:
[tests] skip tests for xpu 
Author: faaany
Pull request: yes

## What does this PR do?

This PR skips some tests for xpu, as xpu doesn't have that feature yet. 



Comments:
@amyeroberts 

> In the future we might want something more general for other accelerators s.t. we can pass in e.g. a list to configure the decorator instead of having to have one per hardware

Good pint!

@faaany I am a bit confused by the changes like

```
    @require_non_xpu
    @require_torch_gpu
```

Previously, it was already has `@require_torch_gpu`, how were that/those tests be triggered on a xpu device ...?

Hi @ydshieh , this test will not run on XPU. But we end up with `this test require torch gpu` as skip message, but we are not sure whether this could run on xpu or not. So `require_non_xpu` will be more specific, so that we know that this test is skipped because xpu doesn't have that feature yet.  

Ah yeah, good point! Thanks for explaining.
[tests] enable GemmaIntegrationTest on XPU 
Author: faaany
Pull request: yes

## What does this PR do?
Below are the test results on XPU. There are some tests failed, because the generated text don't exactly match the expected tests due to precision issues. We will take a look into it, but I think `GemmaIntegrationTest` should also run on non-cuda devices. 

```bash
================================================= short test summary info =================================================
PASSED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_2b_bf16
PASSED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_2b_fp16
PASSED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_2b_sdpa
SKIPPED [1] tests/models/gemma/test_modeling_gemma.py:794: test requires CUDA
SKIPPED [1] tests/models/gemma/test_modeling_gemma.py:655: test requires bitsandbytes and torch
SKIPPED [1] tests/models/gemma/test_modeling_gemma.py:632: test requires Flash Attention
SKIPPED [1] tests/models/gemma/test_modeling_gemma.py:776: test requires bitsandbytes and torch
SKIPPED [1] tests/models/gemma/test_modeling_gemma.py:674: The test will not fit our CI runners
FAILED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_2b_bf16_dola - AssertionError: Lists differ: ['Hel[64 chars] The only tool we have is a scale', 'Hi today [58 chars] of'] != ['Hel[64...
FAILED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_2b_eager - AssertionError: Lists differ: ['Hel[117 chars]ith you a very easy and simple recipe of <strong><em>Kaju Kat'] != ['Hel...
FAILED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_7b_bf16 - KeyError: None
FAILED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_7b_fp16 - AssertionError: Lists differ: ['Hel[27 chars]a 1995 4.0L 4x4. I', 'Hi today I am going to s[51 chars] 3D'] != ['Hel[27...
FAILED tests/models/gemma/test_modeling_gemma.py::GemmaIntegrationTest::test_model_7b_fp16_static_cache - AssertionError: Lists differ: ['Hel[27 chars]a 1995 4.0L 4x4. I', 'Hi today I am going to s[51 chars] 3D'] != ['Hel[27...
============================= 5 failed, 3 passed, 5 skipped, 2 warnings in 126.64s (0:02:06) ==============================
```

Comments:
@amyeroberts
[docs] add the missing huggingface hub username
Author: faaany
Pull request: yes

## What does this PR do?
After pushing a trained model to huggingface hub, we need to have the huggingface username in the model id when loading model from huggingface hub.

@stevhliu pls help review, thx!



Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33431). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[docs] add the missing tokenizer when pushing models to huggingface hub
Author: faaany
Pull request: yes

## What does this PR do?
I tried to run the documentation, but got the following failure that the tokenizer is not to be found in the hf hub. The reason is that we don't pass the `tokenizer` to `Trainer`.  So this PR fixes this issue. 

@stevhliu Pls have a review. Thx!


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33428). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[docs] refine the doc for `train with a script`
Author: faaany
Pull request: yes

## What does this PR do?
FP16 is not the only data type for mixed precision fine-tuning, e.g. XPU only supports bf16 mixed precision fine-tuning. This PR adds an additional note for XPU.

@stevhliu, pls have a review




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

Thanks for the correction!
make length an integer value before comparison
Author: faaany
Pull request: yes

## What does this PR do?
I am not really sure why `hidden_states.shape[-1]` returns a torch.Tensor value, e.g. tensor(40) on XPU in the following test:

```bash
pytest -rA tests/models/encodec/test_modeling_encodec.py::EncodecModelTest::test_torchscript_simple
```

But I think it doesn't have any downside if we make sure that length is an integer value, since max_pad is an integer for sure. 
The test passes after the fix 
```bash
====================================================== short test summary info =======================================================
PASSED tests/models/encodec/test_modeling_encodec.py::EncodecModelTest::test_torchscript_simple
========================================= 1 passed, 60120 deselected, 17 warnings in 14.22s ==========================================
```

@amyeroberts 

Comments:
> Thanks for opening this @faaany!
> 
> We shouldn't really make changes in our code because of inconsistent behaviour on the pytorch side: the same type should be returned regardless of device. Could you open an issue in the torch repo to address this?

yep, it makes sense to me. Thx for the feedback. I am closing this.