noamaanMulla-03/transformers-issues huggingface dataset:
Fix Bark logits processors device misplacement
Author: ylacombe
Pull request: yes

# What does this PR do?
Fix some Bark failing tests where the logits processors' tensors were misplaced on CPU!
It was introduced by #29804 but not flagged because of the Bark custom generation!

Slow tests are now all green on a A100!

cc @ydshieh and @amyeroberts  

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

> It was introduced by https://github.com/huggingface/transformers/pull/29804 but not flagged because of the Bark custom generation!

@ylacombe Does this mean that Bark's generation isn't properly tested? 

> @ylacombe Does this mean that Bark's generation isn't properly tested?

It's not at the moment, we've discussed the reasons why in [this thread](https://github.com/huggingface/transformers/pull/25372#discussion_r1287026171). It might make sense to revisit this decision once the standard generation method is refactored, WDYT ?

Thank you @ylacombe !

@amyeroberts  There is still `BarkModelIntegrationTests` and it catches the bug :-) which leads to this PR.
[DOCS] - Model outputs of RecurrentGemmaCausalLM doesn't align with the documentation
Author: godjw
Pull request: no

### System Info

Latest docs on https://huggingface.co/docs/transformers/main/model_doc/recurrent_gemma#transformers.RecurrentGemmaForCausalLM

### Who can help?

@ArthurZucker, @younesbelkada, and @stevhliu 

### Information

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

### Tasks

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

### Reproduction

In the documentation for [RecurrentGemmaForCausalLM](https://huggingface.co/docs/transformers/main/model_doc/recurrent_gemma#transformers.RecurrentGemmaForCausalLM)'s forward function, the docs read: 

> Returns [transformers.modeling_outputs.CausalLMOutput](https://huggingface.co/docs/transformers/main/en/main_classes/output#transformers.modeling_outputs.CausalLMOutput) or tuple(torch.FloatTensor)
with 
attentions (tuple(torch.FloatTensor), optional, returned when output_attentions=True is passed or when config.output_attentions=True) — Tuple of torch.FloatTensor (one for each layer) of shape (batch_size, num_heads, sequence_length, sequence_length).

However, examining the source code for RecurrentGemma and RecurrentGemmaForCausalLM reveals that the model does not accept `output_attentions=True` and, contrary to the documentation, does not return any attention values. The relevant sections of the source code can be found here:
https://github.com/huggingface/transformers/blob/47735f5f0f2752500d115d2f6bd57816032599b6/src/transformers/models/recurrent_gemma/modeling_recurrent_gemma.py#L744-L747

https://github.com/huggingface/transformers/blob/47735f5f0f2752500d115d2f6bd57816032599b6/src/transformers/models/recurrent_gemma/modeling_recurrent_gemma.py#L895-L899

### Expected behavior

- Modification of the document or
- Modify RecurrentGemma to handle the argument `output_attentions=True`

Comments:
Hey! Would you like to open a PR for this? 🤗 
Documentation should be modified IMO unless people ask for output attentions logic, which is a feature request

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.
Added attention layers to wrap for fsdp
Author: alvations
Pull request: yes

# What does this PR do?

This PR defines the `fsdp_transformer_layer_cls_to_wrap` value in the Mistral config. This way user can easily load the config to figure out what values to use for FSDP, e.g. 

```
from transformers import AutoConfig
c = AutoConfig.from_pretrained("mistralai/Mistral-7B-v0.1")
c.fsdp_transformer_layer_cls_to_wrap
```

[out]:

```
MistralDecoderLayer
```

**Context:** Users has been asking when and which layer to wrap, there shouldn't be a need to load the model to figure it out by going through the state_dict of model summary,  

**Fixes:** https://discuss.huggingface.co/t/accelerate-fsdp-config-prompts/21262/3

Currently, this information is also available through https://github.com/huggingface/transformers/blob/main/src/transformers/models/mistral/modeling_mistral.py#L809 


## Who can review?

Models:

- text models: @ArthurZucker and @younesbelkada

Integrations:

- deepspeed: HF Trainer/Accelerate: @pacman100

Documentation: @stevhliu and @MKhalusova




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

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.
Rename misnamed image processor test files
Author: amyeroberts
Pull request: yes

# What does this PR do?

3 test files had names which didn't match library patterns -- they had the pattern `test_image_processor_xxx.py` instead of `test_image_processing_xxx.py`

Files renamed: 
* tests/models/llava_next/test_image_processor_llava_next.py      
* tests/models/owlv2/test_image_processor_owlv2.py                
* tests/models/siglip/test_image_processor_siglip.py

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31430). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Generate: fix `tokenizer` being popped twice
Author: gante
Pull request: yes

# What does this PR do?

See title :) Because the tokenizer was popped twice from `kwargs` in `generate`, the example [here](https://huggingface.co/docs/transformers/internal/generation_utils#transformers.StopStringCriteria) was failing -- `tokenizer` was `None` even when it was passed.

Found while replying to https://github.com/huggingface/transformers/issues/26959

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

## What does this PR do?
This PR is to enable the `TestDeepSpeedModelZoo` tests to work on a non-CUDA accelerator.

@amyeroberts and @ydshieh 

Comments:
Let's get a second 👍  from @ydshieh the test master to confirm this is OK on our CI runs 

> Let's get a second 👍 from @ydshieh the test master to confirm this is OK on our CI runs

Sure!

I further updated my code to get the real device count. But the CI seems to not use the latest transformers version. @ydshieh is this something intended? 

@faaany . It uses the  latest dev. transformers version (i.e. this PR).

The problem is that, some variables like `BACKEND_DEVICE_COUNT` is only defined when `torch` is available. But we have other CI environment where only `Tensorflow` or only `Flax` is installed without `torch`. 

Let me check what to do with it.

```
if is_torch_available():
    # Mappings from device names to callable functions to support device agnostic
    # testing.
    BACKEND_MANUAL_SEED = {"cuda": torch.cuda.manual_seed, "cpu": torch.manual_seed, "default": torch.manual_seed}
    BACKEND_EMPTY_CACHE = {"cuda": torch.cuda.empty_cache, "cpu": None, "default": None}
    BACKEND_DEVICE_COUNT = {"cuda": torch.cuda.device_count, "cpu": lambda: 0, "default": lambda: 1}
```

Could you add

```python
if is_torch_available():
    # Mappings from device names to callable functions to support device agnostic
    # testing.
    BACKEND_MANUAL_SEED = {"cuda": torch.cuda.manual_seed, "cpu": torch.manual_seed, "default": torch.manual_seed}
    BACKEND_EMPTY_CACHE = {"cuda": torch.cuda.empty_cache, "cpu": None, "default": None}
    BACKEND_DEVICE_COUNT = {"cuda": torch.cuda.device_count, "cpu": lambda: 0, "default": lambda: 1}
```
to
```python
if is_torch_available():
    # Mappings from device names to callable functions to support device agnostic
    # testing.
    BACKEND_MANUAL_SEED = {"cuda": torch.cuda.manual_seed, "cpu": torch.manual_seed, "default": torch.manual_seed}
    BACKEND_EMPTY_CACHE = {"cuda": torch.cuda.empty_cache, "cpu": None, "default": None}
    BACKEND_DEVICE_COUNT = {"cuda": torch.cuda.device_count, "cpu": lambda: 0, "default": lambda: 1}
else:
    BACKEND_MANUAL_SEED = {"default": None}
    BACKEND_EMPTY_CACHE = {"default": None}
    BACKEND_DEVICE_COUNT = {"default": lambda: 0}
```
and see how it goes? 🙏  thank you in advance

@ydshieh yes, it works, thx for the suggestion! The CI still doesn't run through, but I think it is not caused by this PR.  

Yeah, rebase on main to include #31407 would make CI green.

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31402). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Set seed for M4T retain grad test
Author: ylacombe
Pull request: yes

# What does this PR do?

The speech encoder attentions of M4T can be None with a non-zero probability when training=True. Instead of skipping the test, this PR fixes the seed so that the first layer is never skipped: `test_retain_grad_hidden_states_attentions`.

I've ran it multiple times and it passed every time. 

cc @amyeroberts 

Fixes #28036

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31419). 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 review @amyeroberts ! Let me know when I can merge!
Support multiple validation datasets when `dataloader_persistent_workers=True`
Author: bastienlc
Pull request: yes

# What does this PR do?

Fixes #30527 

This PR adds support for multiple validation datasets in `transformers.Trainer` when `dataloader_persistent_workers=True`. The current behavior is that all validations will be done on the first validation dataset.

I had to pass the `validation_dataset_name` to the `evaluate` and `get_eval_dataloader` methods so that the right validation dataloader could be retrieved.

## Who can review?

@muellerzr

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

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

@muellerzr I like the sound of this solution :) 

Hi, sure that would be better, something like what I just pushed ?

Or maybe we'd be better off hashing the evaluation dataset like `datasets` does for caching ? That would avoid having to check multiple cases.

@bastienlc The current state LGTM. Let's get @muellerzr's opinion though - as he know's the trainer better than me :) 

Great! cc @amyeroberts for final review :)

Thanks for the reviews !

I added some tests to check the behavior of the `get_eval_dataloader` method, as it's the main modification and initial source of the bug. I'm not sure what tests we could add for the evaluate method though ?
Bugfix convert_llama_weights_to_hf.py
Author: helson73
Pull request: yes

# What does this PR do?
Division with n_heads_kv_local is duplicated inside permute function, so removed.

Comments:
cc @ArthurZucker 

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.
LlavaNextProcessor.__init__() got an unexpected keyword argument 'image_token'
Author: M-Fannilla
Pull request: no

### System Info

transformers 4.41.2
Python 3.10.12
Nvidia A40 / Runpod.io

Whe working with LlavaNextProcessor I get errors that were not there.

Error:
LlavaNextProcessor.__init__() got an unexpected keyword argument 'image_token'

Which previously did not happen (~week ago).


### Who can help?

_No response_

### Information

- [X] The official example scripts
- [x] My own modified scripts

### Reproduction

model_name = "llava-hf/llava-v1.6-vicuna-7b-hf"
processor = LlavaNextProcessor.from_pretrained(
    model_name, padding_side='left'
)
model = LlavaNextForConditionalGeneration.from_pretrained(
    model_name, torch_dtype=torch.float16, low_cpu_mem_usage=True
).to("cuda").eval()


### Expected behavior

I would expect to initiate the objects properly

Comments:
@M-Fannilla hi, I just updates llava-weights in the hub which caused the error. I will revert the changes soon

@zucchini-nlp Great, Thanks!

@M-Fannilla should be working now, closing the issue as resolved! 

There is new issue:

Failed to import transformers.models.llama.modeling_llama because of the following error (look up to see its traceback):
/usr/local/lib/python3.10/dist-packages/flash_attn_2_cuda.cpython-310-x86_64-linux-gnu.so: undefined symbol: _ZN2at4_ops5zeros4callEN3c108ArrayRefINS2_6SymIntEEENS2_8optionalINS2_10ScalarTypeEEENS6_INS2_6LayoutEEENS6_INS2_6DeviceEEENS6_IbEE


I did not have this one before.

@M-Fannilla this one isn't related to the model checkpoint, but rather to the installed packages you have. Seems like flash-attn wasn't installed properly or has some dependency issues. Try to uninstall flash-attn and load again

If it doesn't work feel free to open a new issue :)
Agents: Improve python interpreter
Author: aymeric-roucher
Pull request: yes
Labels: name: Agents

# What does this PR do?

Extend the Python Interpreter for Agents by:
- improving error logging by displaying the offending line
- adding support for a few statements: With, Assert.

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

@ArthurZucker this is what we discussed: I've added clarifying comments to ease the review.
Donut: fix `generate` call from local path
Author: gante
Pull request: yes

# What does this PR do?

Donut has custom logic in `generate`. Typically, we can detect certain architectures through `self.__class__.__name__`, to then trigger custom logic. However, to detect donut's original checkpoints, whose `model_type` and `architectures` is incorrectly defined (e.g. [here](https://huggingface.co/naver-clova-ix/donut-base/blob/a959cf33c20e09215873e338299c900f57047c61/config.json#L3)), we can't rely on `self.__class__.__name__`.

The custom logic that was in place relied on the hub path, but that doesn't work for local file paths. This PR fixes it by clawing at the [only piece of text saying "donut" in the original config](https://huggingface.co/naver-clova-ix/donut-base/blob/a959cf33c20e09215873e338299c900f57047c61/config.json#L138) 👀 

cc @nbroad1881 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31470). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Make "tool_use" the default chat template key when tools are passed
Author: Rocketknight1
Pull request: yes

We've started to see models with multiple templates, keyed by name. The standard name (or at least the standard I'm trying to enforce) is that the template for tool use is called `tool_use`.

This PR updates the chat template logic so that if there's a template named "tool_use" and the user has passed tools, then that template will be used as the default.

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

Will do!

@amyeroberts I added some opinionated text where I explain this and try to dissuade people from writing templates this way (I might eventually chase down the cases where people are doing it and ban it as a feature, we'll see!)
Streamlining bnb quantizer interfaces [bnb]
Author: poedator
Pull request: yes

This is a follow-up to #26610 .

The Bnb classes require few additional calls in the middle of `_load_state_dict_into_meta_model()`. This PR makes the class interface comparable to that of GPTQ and AWQ:
- creating buffers to load quantization params in `_process_model_before_weight_loading()`
- load weights from state dict normally
- use `_process_model_after_weight_loading()` to combine weights with the buffer values into the quantized weights.

Benefits:
- more uniform class interfaces
- fewer calls from `from_pretrained()` to the quantizer.

So far I did this for `bnb.8bit`.
4bit is doable but is a bit more tricky.

@younesbelkada @SunMarc , what do you think of this idea?

Comments:
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

[FOR INFO:] 
This PR was put on hold because of compatibility issues with the bnb 4-bit formats.
When writing the original code for 4bit serialization I used names like weight.absmax for the saved quantized components. Now they are not fit as buffer names because of the dot. 
My temptation is to rename them to something like  weight__absmax  and load as buffers, but this means incompatibility with bnb 4bit models created in the past few months.
Fix single letter stop strings
Author: Rocketknight1
Pull request: yes

Our `StopStringCriteria` has a bug when all stop strings are one letter long. This results in a case where no stop strings have any `valid_positions`, as this variable only tracks internal positions, and this causes a `max()` to fail later.

This code fixes the issue by ensuring that `max()` takes a minimum value of `1`. It also adds a test for single-letter stop strings!

Fixes #31435

cc @amyeroberts @zucchini-nlp @gante 

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

> Our StopStringCriteria has a bug when all stop strings are one letter long. This results in a case where no stop strings have any valid_positions, as this variable only tracks internal positions, and this causes a max() to fail later.

@Rocketknight1 Could you explain this a bit more? Why does this result in no stop strings having any `valid_positions`? i.e. what does "this variable only tracks internal positions" mean? 

@amyeroberts Sorry for the confusion! The way this works is that we precompute two things for every `(token, stop_string)` pair: The length of the token's overlap with the **end** of the stop string, which we use to begin matches, and the positions inside the stop string where the token can match that don't overlap with the end.

The `max()` in the code was implicitly assuming that at least one `(token, stop_string)` pair would have a valid non-end position (a position inside the string that the token matches that does not overlap with the end of the stop string), but there are cases when this isn't true, and the most common one is when all the stop strings are a single character, which can only have end-matches.

@lapp0 that issue was fixed today (https://github.com/huggingface/transformers/pull/31427), if you rebase on `main` and apply this PR on top of it it should work 🤗
Update chat template docs and bump Jinja version
Author: Rocketknight1
Pull request: yes

This PR makes some updates to the chat template docs. Specifically, I put much more emphasis on using trimming blocks like `{{-` and `{%-` to let you write neater, indented code, and I also add a section on making your template compatible with non-Python implementations of Jinja. This PR also bumps the minimum Jinja version to 3.1, which is the earliest version that supports the filters we recommend in these docs (and have started to use in our templates), such as `|items`.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31455). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Slow tokenizer-loading when many tokens (~17k) are added by a user.
Author: jaeminSon
Pull request: no

### System Info

Ubuntu 20.04.2 LTS
transformers==4.40.1

### Who can help?

@ArthurZucker

### Information

- [ ] The official example scripts
- [X] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [X] My own task or dataset (give details below)

### Reproduction

1. add many tokens to tokenizer using tokenizer.add_tokens method
2. save it
3. load it (takes too long time)

reason: this list-comprehension takes long time 

https://github.com/huggingface/transformers/blob/2b9e252b16396c926dad0e3c31802b4af8004e93/src/transformers/tokenization_utils_fast.py#L173 

using vs-code debugger, when the list-comprehension omits comparison, such as the following, it is fast.

```
tokens_to_add = [
            token
            for index, token in sorted(added_tokens_decoder.items(), key=lambda x: x[0])
        ]

```

There is comment saying 

```
# The following logic will be replace with a single add_tokens once a fix is pushed to tokenizers
# allows converting a slow -> fast, non-legacy: if the `tokenizer.json` does not have all the added tokens
# uses the information stored in `added_tokens_decoder`.
# this is costly for fast tokenizers as we re-compute the regex again. But not all tokens are added tokens
```

Any plan to replace this code?
        


### Expected behavior

Faster tokenizer initialization

Comments:
Completely agree. #30574 will fix it! I just need to fix a breaking change made in `tokenizers` before I can release and should adress this

Thanks for your time addressing this issue!

Also #31404 is fixing this as well! kudos to @ydshieh