noamaanMulla-03/transformers-issues huggingface dataset:
refactor: change default block_size
Author: pphuc25
Pull request: yes

Hi,
As mentioned in issue #26069, I have created a new PR with the aim of modifying the block_size for all files. The goal is to set the block size = min(1024, config.max_position_embeddings). This change is intended to ensure synchronization and prevent errors that can occur when the block size exceeds the maximum position embeddings value.
I would like to cc @sanchit-gandhi to review my PR, thank you so much

Comments:
however, I have some confuse because why I can not pass the test case

thank you @ArthurZucker, but I have some confuse why I just change 1024 to min(1024, config.max_position_embeddings) and docs and then I can not pass the test case, still stuck, can you help me

Hey @pphuc25 - don't worry, your PR is perfect! All you need to do is rebase onto `main`:

```
git fetch upstream
git rebase upstream main
```

And then force push:
```
git commit -m "rebase" --allow-empty
git push -f origin flax_min_block_size
```

This should fix your CI issues! The test that's failing isn't related to your PR and was fixed yesterday on `main`.

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_26229). All of your documentation changes will be reflected on that endpoint.

thanks @sanchit-gandhi so much on very helpful information, but this seem to still bug.

Hmm could you try running: `make fix-copies` and pushing? Does this change the code for you at all?

Thank you for really helpful support @sanchit-gandhi, this help me so much and gain me more knowledge, thank you really much.

but @sanchit-gandhi, I think you should review the code to merge PR

Shall we merge this one @ArthurZucker since it protects against an edge case, and then add follow-up features to your own codebase / the HF Hub @pphuc25?

Sure!
[`CI-Quality`] Main is red following `Docstring check (#26052)` 
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Fixes the doc of mistral

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_26592). All of your documentation changes will be reflected on that endpoint.
[Mistral] Update config docstring
Author: sanchit-gandhi
Pull request: yes

# What does this PR do?

Runs `make fix-copies` to correct the Mistral config after the PR #26052, and subsequently fills out the missing docstring args.


Comments:
#26592 is all green 👼🏻 

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_26593). All of your documentation changes will be reflected on that endpoint.
feat: change the flow of data preprocess and avoid bug in remove columns
Author: pphuc25
Pull request: yes

# What does this PR do?
I change the data flow of prepare_dataset function, make a case to avoid remove `speech` columns

While examining the 'wav2vec2' workflow, I noticed that the `prepare_dataset` function typically takes the path of audio files and converts them into audio arrays. However, I believe this approach may not be ideal for several reasons:
- Not all data entries contain a `path` column, or the `path` column may not always be correctly populated (e.g., in the case of 'vivos' data). When attempting to use this code with such data, errors can occur.
- This process is somewhat redundant, especially in cases like 'common voice' datasets, where we already have the audio data stored in the `audio` column. In these instances, it would be more efficient to directly pass the audio array to the `speech` column.

To address these issues, I've adjusted the data flow to accept the audio file path as an input column, ensuring that the sampling rate matches the feature extractor's requirements. Additionally, I've created a list of columns to exclude during data processing to prevent inadvertently removing the 'speech' column."

I would like cc @sanchit-gandhi to review my PR.

Comments:
Hey @pphuc25! Thanks for your enthusiasm here! As mentioned [previously](https://github.com/huggingface/transformers/pull/26516#pullrequestreview-1653115447), this examples script is not the best place to make performance optimisations, since it's currently a WIP script (or more truthfully, a 'broken' script). If you're interested in making a contribution for Flax Wav2Vec2 pre-training, I would encourage you to take a look at the issue #19588, which endeavours to correct this script by obtaining equivalence with PyTorch. We should fix this script first before making performance optimisations like the ones proposed in this PR. Thanks for your understanding.
[Wav2Vec2] Fix tokenizer set lang
Author: sanchit-gandhi
Pull request: yes

# What does this PR do?

The PR #23909 removed `unique_no_split_tokens` as an attribute of the Wav2Vec2 tokenizer, however it is required to set the language in the method `.set_target_lang`:

https://github.com/huggingface/transformers/blob/dcbfd93d7aeb14f8ff08a48866d2a68950d4c69a/src/transformers/models/wav2vec2/tokenization_wav2vec2.py#L237

Thus, calling `.set_target_lang` currently throws an error:

```python
from transformers import Wav2Vec2CTCTokenizer

tokenizer = Wav2Vec2CTCTokenizer.from_pretrained("facebook/mms-1b-all")
tokenizer.set_target_lang("spa")
```

**Output:**
```
Traceback (most recent call last):
  File "/Users/sanchitgandhi/transformers/debug_tokenizer.py", line 4, in <module>
    tokenizer.set_target_lang("spa")
  File "/Users/sanchitgandhi/transformers/src/transformers/models/wav2vec2/tokenization_wav2vec2.py", line 237, in set_target_lang
    self.unique_no_split_tokens.append(token)
AttributeError: 'Wav2Vec2CTCTokenizer' object has no attribute 'unique_no_split_tokens'
```

This PR re-instates `unique_no_split_tokens` as an attribute of the tokenizer. Note that this should already be tested for in the following test: https://github.com/huggingface/transformers/blob/dcbfd93d7aeb14f8ff08a48866d2a68950d4c69a/tests/models/wav2vec2/test_tokenization_wav2vec2.py#L798

Comments:
_The documentation is not available anymore as the PR was closed or merged._

My space suddenly had a runtime error because of this.

Sorry to hear that @andergisomon - note that running `transformers` on the latest PyPi version should bypass this error. It's only on `main` at the moment:
```
pip uninstall transformers -y
pip install --upgrade transformers
```

Thanks for the review! Applied the suggestions from your comment - look good to you?
Add # Copied from statements to audio feature extractors that use the floats_list function
Author: dg845
Pull request: yes

# What does this PR do?

This PR adds `# Copied from` statements to audio feature extractors (and other related data processing modules) that use the `floats_list` function. The [Whisper version](https://github.com/huggingface/transformers/blob/2f3ea08a077ba3133fa8a604b22436cad250b055/tests/models/whisper/test_feature_extraction_whisper.py#L42-L53) of `floats_list` is considered the "canonical" version of the function, since the CLAP model has an existing `# Copied from tests.models.whisper.test_feature_extraction_whisper.floats_list` statement for its [`floats_list`](https://github.com/huggingface/transformers/blob/2f3ea08a077ba3133fa8a604b22436cad250b055/tests/models/clap/test_feature_extraction_clap.py#L37-L38) function.

This issue was brought up in https://github.com/huggingface/transformers/pull/24799#discussion_r1325148003, https://github.com/huggingface/transformers/pull/24799#discussion_r1326652124, and the following thread.


## 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#start-contributing-pull-requests),
      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?

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.

@sanchit-gandhi
@ArthurZucker


Comments:
The `floats_list` function is typically preceded by a `global_rng = random.Random()` statement:

https://github.com/huggingface/transformers/blob/2f3ea08a077ba3133fa8a604b22436cad250b055/tests/models/whisper/test_feature_extraction_whisper.py#L39-L45

https://github.com/huggingface/transformers/pull/24799#discussion_r1325147861 suggests replacing this with something like `transformers`'s [`set_seed`](https://github.com/huggingface/transformers/blob/2f3ea08a077ba3133fa8a604b22436cad250b055/src/transformers/trainer_utils.py#L85) utility, not sure if we should consider adding a change like this in this PR.


The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_26581). All of your documentation changes will be reflected on that endpoint.
Tokenizer pad token not saved with save_pretrained
Author: jonathanasdf
Pull request: no

### System Info

works on 4.33.3 (with tokenizers==0.13.3), fails on main (with tokenizers==0.14.0)

### Who can help?

@ArthurZucker 

### 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

```
import transformers
tokenizer = transformers.AutoTokenizer.from_pretrained('stabilityai/FreeWilly2')
tokenizer.pad_token_id = 0
print(tokenizer.pad_token)
tokenizer.save_pretrained('/tmp/tok_test')
tok = transformers.AutoTokenizer.from_pretrained('/tmp/tok_test')
print(tok.pad_token)
```

### Expected behavior

expected:

```
>>> print(tok.pad_token)
<unk>
```

actual:

```
>>> print(tok.pad_token)
Using pad_token, but it is not set yet.
None
```

Comments:
Having a look now! Thanks 


Okay, quite suprisingly, the previous behaviour only saved the pad token to `special_tokens_map.json` and not in the `tokenizer_config.json`.  Thus the padding was set to `null` in both the `tokenizer_config.json` and the `tokenizer.json`. Which IMO is not good 😄 

This particular example was fixed, thanks.

However a similar example now fails

```
import transformers
tokenizer = transformers.AutoTokenizer.from_pretrained('tiiuae/falcon-40b-instruct')
tokenizer.pad_token = tokenizer.eos_token
print(tokenizer.pad_token)  # '
'
tokenizer.save_pretrained('/tmp/tok_test')
tok = transformers.AutoTokenizer.from_pretrained('/tmp/tok_test')
print(tok.pad_token)  # None!
```

Thanks I'll fix this in #26570 🤗
docs: rewrite some document
Author: pphuc25
Pull request: yes

Just doing some documents for easier to read
I would like to cc @MKhalusova to review my code.

Comments:
@MKhalusova can you help explain why my code fail the test case, this seem like no problem with my view.

Can you provide more context on what this PR is trying to achieve? 
Also, I may not be the right person to ping for a review.

I do document, there's something like missing punctuation so I add it, and also add new extension code to reduce reduntant, but this fails so at now I return extension to origin

I would like cc @amyeroberts to review my code
Add num proc load
Author: pphuc25
Pull request: yes

Hi
I find the subject of audio quite intriguing when considering its future potential. It deserves greater attention and contributions. Currently, I am utilizing this code for my primary project rather than mere testing. I am interested in incorporating the "num_proc" feature to enhance the loading of datasets, especially since the data is so big, and I require faster processing capabilities.
Thank you for review my PR.

Comments:
This repo is not maintain anymore, but I think it still should be do some trick for time efficient

Hey @pphuc25 - I think your original comment highlights that adding this feature is exactly the kind of user customisation that these scripts are intended for. They're not necessarily the most performant scripts out of the box, but open to you modifying them for your needs to improve them. In this regard, they don't need to be fully complete, since they're open for anyone to use and update! Thus, I advise you to build and iterate upon these scripts as you see fit, and publish them standalone in your own repo or release.
GPTNeoXRotaryEmbedding has a defect when using sin/cos cache
Author: underskies00
Pull request: no

### System Info

- `transformers` version: 4.31.0
- Platform: Linux-5.10.112-005.ali5000.alios7.x86_64-x86_64-with-glibc2.17
- Python version: 3.8.13
- Huggingface_hub version: 0.12.1
- PyTorch version (GPU?): 1.13.1+cu117 (False)
- Tensorflow version (GPU?): 2.11.0 (False)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>

### Who can help?

Text models:@ArthurZucker  and @younesbelkada

### 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

in transformers/models/gpt_neox/modeling_gpt_neox.py, line320:
```python
return self.cos_cached[:seq_len, ...].to(x.device), self.sin_cached[:seq_len, ...].to(x.device)
```
lack 2 dimensions before seq_lenth.
This will not cause bug, because seq_lenth is always larger than 1, but it will cause a defect use of cache by use the whole cache ever, maybe lead to poor performance when inference?

### Expected behavior

Maybe the right code should be:
```python
return self.cos_cached[:, :, :seq_len, ...].to(x.device), self.sin_cached[:, :, :seq_len, ...].to(x.device)
```

Comments:
Looks like you are right, as this is the case for models in `llama`, where we have

```python
        return (
            self.cos_cached[:, :, :seq_len, ...].to(dtype=x.dtype),
            self.sin_cached[:, :, :seq_len, ...].to(dtype=x.dtype),
        )
```

@ArthurZucker could you like to double confirm here?

Yep indeed. Would you like to open a PR @underskies00 ?

fyi @gante if there's a reason we did not merge this for GPTNeoX maybe? 

@ArthurZucker looking at the `git blame`, this has been present since the 1st commit :D Possibly flew under the radar.

Technically harmless (since seq_len is almost always > batch size), but in need of fixing.

(Actually for a simple case like this: 

```python 
from transformers import AutoTokenizer, GPTNeoXForCausalLM, AutoConfig


config = AutoConfig.from_pretrained("EleutherAI/gpt-neox-20b", num_hidden_layers = 5)
tokenizer = AutoTokenizer.from_pretrained("EleutherAI/gpt-neox-20b", )
model = GPTNeoXForCausalLM(config).cuda()

inputs = tokenizer("Hey how are you doing", return_tensors = "pt").to("cuda")

import time
start = time.time()
model.generate(**inputs, max_new_tokens = 258)
print(time.time()-start)
```
I already get 2 more seconds. It's a small model but should apply to big one as well, just reduced the number of layers

> Yep indeed. Would you like to open a PR @underskies00 ?
> 
> fyi @gante if there's a reason we did not merge this for GPTNeoX maybe?

You handle it is OK，i‘m not familiar with how to open a PR, Thanks.@ArthurZucker
Fix failing tests on `main` due to torch 2.1
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix failing tests on `main` due to torch 2.1

Comments:
_The documentation is not available anymore as the PR was closed or merged._

@michaelbenayoun Could you help us on the torch fx tests for wav2vec2/hubert with torch 2.1.

See [this internal discussion](https://huggingface.slack.com/archives/C01NE71C4F7/p1695971833833119)

But in short, it can't do
```
if attn_weights.size() != (bsz * self.num_heads, tgt_len, src_len)
```
as the corresponding proxy object has no `_metadata` attribute (with torch 2.1) but it has it with torch 2.0.
[`core`] fix silent bug `keep_in_fp32` modules
Author: younesbelkada
Pull request: yes

# What does this PR do?

Same PR as https://github.com/huggingface/transformers/pull/26484 but without any extra diff 

Before this PR we were performing a simple check if module_name in key but that lead to some modules silently converted in fp32.

For example instructblip models got their word_embedding layers converted in fp32 because _keep_in_fp32_modules includes "wo" which is contained in the string word_embedding. The fix is to check if module_name in key.split(".")

I can confirm with this PR the failing instructblip tests now pass


Comments:
_The documentation is not available anymore as the PR was closed or merged._

Before merging this PR I want to test it on T5 models and 8bit tests as this might affect them

Relevant T5 and bnb tests are passing, this PR is ready for review! 

... and tested the failing instructblip tests on the latest docker image and they pass with these changes

Thanks for the PR! Do we have a common test that could have failed in this specific instance? If not, would it be possible to work on one?

I'm a bit afraid of the repercussions of such a change without a test that ensures the modules that should be kept in fp32 actually are and that those that shouldn't are kept in their original dtype. It is fixing a silent error but also seems like it could break some silent successes  

Hi @LysandreJik - OK makes sense, I am happy to work on a common test for that - I'll ping you once this is done

Tests are passing, this is ready for another review!
#26566 swin2 sr allow in out channels
Author: marvingabler
Pull request: yes

# What does this PR do?

This PR adds the feature of accepting arbitary number of input and output channels when using the Swin2SR model. This allows to perform super resolution from greyscale (1 channel) to color (rgb), or from low resolution multi band satellite to high resolution rgb satellite.

All examples and pretrained models are running as expected based on my tests. No new dependencies have been added.

Just use it like

```python
from transformers import Swin2SRForImageSuperResolution, Swin2SRConfig
import torch

Swin2SRConfig = (
     num_channels_in=1,
     num_channels_out=3
)
model = Swin2SRForImageSuperResolution(Swin2SRConfig)

with torch.no_grad():
    # or use the image preprocessor per default
    out = model({"pixel_values":torch.randn((1,1,264,264))})

```

Fixes #26566.


## 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#start-contributing-pull-requests),
      Pull Request section?
- [x] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Yes [here](https://github.com/huggingface/transformers/issues/26566)
- [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).
- [ ] Did you write any new necessary tests? No, test where there already.


## Tagging the reviewers

- text models: @ArthurZucker and @younesbelkada
- vision models: @amyeroberts



Comments:
Good point, yes lets do that! Let me update the PR soon :)

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_26568). All of your documentation changes will be reflected on that endpoint.

Just realized that there are a couple of more changes required, as the `Swin2SRForImageSuperResolution` denormalizes based on the input images, while for the case of mapping from multiband images to single band, the mean&stds of inputs and outputs differ. Will add the changes soon.
[docs] Update to scripts building index.md
Author: MKhalusova
Pull request: yes

The `docs/index.md` file currently contains two auto-generated parts: the list of models (same as in README), and a table of models with supported frameworks. Due to the number of models available in transformers (200+), the list and the table have become quite large, and there have been internal discussions about removing the list of models from the `index.md`. 

This PR adds the following changes: 
- removes the autogenerated model list from the `index.md` and updates the script so it's no longer added
- modifies the script that generates the table to make model names links to corresponding model_doc. 

The model lists in the main README and localized READMEs remain as is. 

Comments:
_The documentation is not available anymore as the PR was closed or merged._

> Super nice, I think it's a lot cleaner like this! ✨
> 
> It looks like some models (like Llama 2, Flan T5, DiT) are missing though.

The reason they are missing is that the table is based on the model classes. And in case of the models you mentioned, Llama 2 uses the same implementation as Llama, Flan T5 uses the same implementation as T5, and DiT's architecture is equivalent to that of BEiT. 
I can check if I can automatically find models like this, if not, I'll add them as "special case" constants.

Yep, I think these models should be part of the table and we should make sure we can easily add more models there that share the same modeling file! 

I've added a dict of models that use the same config as some "base" model. This way the table is complete. 
I couldn't find this mapping anywhere else, so it's hardcoded here. 
If these models were included in `transformers_module.models.auto.configuration_auto.CONFIG_MAPPING_NAMES`, it would not need to be hardcoded here, however, that dictionary is used in so many places, I'm hesitant to modify it. 

cc @stevhliu
Add LISA Model
Author: Dev-Khant
Pull request: no
Labels: name: New model

### Model description

LISA: Reasoning Segmentation via Large Language Model 

It proposes a new segmentation task --- reasoning segmentation. The task is designed to output a segmentation mask given a complex and implicit query text.

### Open source status

- [X] The model implementation is available
- [X] The model weights are available

### Provide useful links for the implementation

Paper: https://arxiv.org/abs/2308.00692
Github: https://github.com/dvlab-research/lisa

Comments:
Hi @NielsRogge @amyeroberts @patrickvonplaten  I'm not sure if I can add this model here because its weights are present in the hub.
Weights: https://huggingface.co/xinlai/LISA-7B-v1

Let me know if I can add this. Thanks :)

Hi, sure would be a nice addition. However we could perhaps first use the model on the hub feature for this one: https://huggingface.co/docs/transformers/custom_models#sharing-custom-models.

If the model gets a lot of traction, we can then add it to the Transformers library natively

Sure, let me know if it needs to be added to the library.

Let me know whether you need any help  regarding adding the model with code on the hub.

Thanks, I'll get started with adding model on hub will let you know if I need help.