noamaanMulla-03/transformers-issues huggingface dataset:
PaliGemma fix attention mask for finetunes
Author: probicheaux
Pull request: yes

# What does this PR do?

Previously, the PaliGemma attention mask was a 4d attention mask of all 1s, which preempted any downstream causal mask creation by Gemma. [PaliGemma has full attention over the image tokens and the prefix of the prompt, and causal attention over the suffix.](https://github.com/google-research/big_vision/blob/main/big_vision/configs/proj/paligemma/README.md) This PR implements that logic, allowing for finetuning PaliGemma without label leakage.

(Additionally, I noticed that using the GemmaTokenizerFast led to incorrect tokenization of tokens like <loc0000> character by character, whereas using GemmaTokenizer did proper tokenization. Be sure to set `use_fast=False` when instantiating the PaliGemma processor. I'll make an issue for this observation
edit: ah, [looks like that is a known issue?](https://github.com/huggingface/transformers/issues/30824)).



## 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 @molbap @merveenoyan 



Comments:
Hey! Can you provide a reproducer for:
1. the tokenizer issues: it's probably just about the added tokens and the conversion 
2. you are right, when you finetune, you want the non-prompt text to be causal vs now everything is ones. 



I accidentally ran style on the whole repo and now the tests are failing cause I had to force push away updating like every file (இ﹏இ`｡)

I think all the style errors are in code not touched by this change

Anyways OK I updated how we check where to split using a new index in the labels. Constructing the right labels array is now getting to be sort of tricky for new users, so now the PaliGemmaProcessor returns labels in BatchEncoding for easier finetuning. This is sort of necessary because the prefix and suffix might change length during tokenization and it'd be annoying to try to find the right index of that \n. Hope y'all are ok with this idea!!!

Merged as part of https://github.com/huggingface/transformers/pull/30967
There is a probability that a bug will be triggered when tracing the llama model: torch.fx.proxy.TraceError: symbolically traced variables cannot be used as inputs to control flow
Author: sean-wade
Pull request: no

### System Info

- `transformers` version: 4.38.1
- Platform: Linux-5.3.0-28-generic-x86_64-with-glibc2.27
- Python version: 3.10.9
- Huggingface_hub version: 0.22.2
- Safetensors version: 0.4.2
- Accelerate version: 0.29.2
- Accelerate config:    not found
- PyTorch version (GPU?): 2.0.0 (True)
- Tensorflow version (GPU?): not installed (NA)
- 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?

_No response_

### 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

Just run this code several times, and some times will report the bug.

```
from transformers.utils.fx import symbolic_trace
from transformers import AutoModelForCausalLM

model = AutoModelForCausalLM.from_pretrained("llama-hf/llama-7b/")
gm = symbolic_trace(model)

```

![c1a8f5939f5902cc87bae202e898654](https://github.com/huggingface/transformers/assets/42532862/65e2c6b0-4376-4825-af70-19ca912055b8)


### Expected behavior

Expected no error.

Comments:
cc @ArthurZucker @fxmarty 

I think llama supports tracing now cc @michaelbenayoun 

Yes, @sean-wade can you try with the `main` branch and tell me if you still have the error?

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.
Push ci image
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Fix "dev" used to check the docker to use

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30982). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Fix num_hidden_layers in initialization of new model in Mamba
Author: SrGonao
Pull request: yes

# What does this PR do?
Originally, the initialization was using config.num_layers instead of config.num_hidden_layers. This fixes that.


## Fixes 
Initialization would use a parameter not in the config (num_layers)


- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).

## Who can review?

@ArthurZucker and @younesbelkada


Comments:
FIX / Docs: Minor changes in quantization docs
Author: younesbelkada
Pull request: yes

# What does this PR do?

Applies some minor changes in quantization docs 

Clarify that "serializable" stands for serializable with HF transformers

cc @SunMarc @mobicham 

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

# What does this PR do ?
This PR fixes some tests (~ 30) are that contained in the accelerate_tests flag. Recently, I ran the following workflow to have an overview of the failing tests : https://github.com/huggingface/transformers/actions/runs/9081904684 or the following CLI: `RUN_SLOW=True pytest -m accelerate_tests tests/ `
Related PR https://github.com/huggingface/transformers/pull/30808

Model fixed:
- BertModel
- ClipModel
- GPTSanJapaneseModel
- M2M100Model 
- MambaModel - same [fix](https://github.com/huggingface/transformers/pull/30418) as jamba 
- T5/MT5Model/UMT5Model - same fix since this is copied from code
- Whisper
- SiglipVisionModel for MP. As for the offload, I skipped the tests since fixing them would require to introduce a new attribute in PretrainedConfig (`_preload_module_classes` to be used in `dispatch_model`), not sure if it is worth it if this is the only model for now. 

Tests skipped:
- DbrxModel : because offload is not working and enabling it is not simple (core modeling code). However model parallelism works. 

See the run [here](https://github.com/huggingface/transformers/actions/runs/9116151177) with most of the tests passing  .  There are only 3 tests failing but I skipped them in the latest commit (siglipvisionmodel)
cc @ydshieh 


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

Just wanna to know if those are failing for quite some time or it's a recent changes (in either `transformers` or `accelerate`) causing those failures? If it's the later case, could you elaborate it a bit more?

> Just wanna to know if those are failing for quite some time or it's a recent changes (in either transformers or accelerate) causing those failures? If it's the later case, could you elaborate it a bit more?

I'm not sure how long these tests have been failing. However, most of the tests were failing for a good reason (didn't put the right `no_split_modules`, modeling code not compatible)
Idefics2 - sdpa vs flash attn 2
Author: VictorSanh
Pull request: no

### Feature request

Right now, when i do

```
Idefics2ForConditionalGeneration.from_pretrained(
    "defics2-8b",
    torch_dtype=torch.bfloat16,
    _attn_implementation="flash_attention_2",
)
```

It will use flash attention 2 for the vision tower and the perceiver, BUT it will default to sdpa for the language model.
Would it make sense to propagate the _attn_implementation to text_model too?



### Motivation

Without thinking too much, i think it would require propagating the `_attn_implementation` to the text_config so that when we initialize the text_model, it picks up that argument too

### Your contribution

NA

Comments:
Just opened a PR (#30395) addressing this. I think it will need a bit more polishing, but it achieves the main goal currently.

@zafstojano Hi, may I ask what's the differences between idefics2 perciever versus idefics1 perceiver?

Can you just make idefics2 perciever a standalone module rather than make it in a single modeling file? So that we can resue it in other arch?

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.

@VictorSanh Closing as the selected attention is correctly propagated now when creating the idefics2 model 

yes thanks!
Add a check that warmup_setps is either 0 or >= 1
Author: ymoslem
Pull request: yes

# What does this PR do?

Update training_args.py to add a check that warmup_setps is either 0 or >= 1. Otherwise, raise an error.

## 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?


- trainer: @muellerzr and @pacman100



Comments:
Hello! I am not sure what I should do to fix these errors in the tests. Thanks!

```
FAILED tests/models/dpt/test_modeling_dpt_auto_backbone.py::DPTModelTest::test_batching_equivalence - AssertionError: tensor(False) is not true : Batched and Single row outputs are not equal in DPTForDepthEstimation for key=predicted_depth. Difference=1.0.
================= 1 failed, 577 passed, 935 skipped in 56.07s ==================
```


```
#!/bin/bash -eo pipefail
python3 .circleci/parse_test_outputs.py --file tests_output.txt --fail

1 failed because `AssertionError: tensor(False) is not true ` -> Batched and Single row outputs are not equal in DPTForDepthEstimation for key=predicted_depth. Difference=1.0.
Number of failed tests: 1

Exited with code exit status 1

CircleCI received exit code 1
```

You can try rebasing from `main`

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30764). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Update 4 `MptIntegrationTests` expected outputs
Author: ydshieh
Pull request: yes
Labels: name: run-slow

# What does this PR do?

Those are failing due to the switch to torch 2.3+cu121 from torch 2.2+cu118 (where I tried and they pass).

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30989). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Remove deprecated properties in tokenization_nllb.py and tokenization_nllb_fast.py
Author: ymoslem
Pull request: yes

# What does this PR do?

This PR fixes a typo in a warning in two tokenization files for the NLLB model. It changes `adder_tokens_decoder` into `added_tokens_decoder` and improves the warning's readability.

## Before submitting
- [x] 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?

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.

- tokenizers: @ArthurZucker


Comments:
Thanks @amyeroberts! Okay, I removed the deprecated properties, but some tests failed like this one. Should I edit it too?

https://github.com/huggingface/transformers/blob/76a33a10923ccc1074917f6b6a1e719e626b7dc9/tests/models/nllb/test_tokenization_nllb.py#L370-L373

@ymoslem Yep - if we've deprecated a method, it's OK to remove from the tests here 

Hi @ArthurZucker I think it is fixed now. Please let me know if I should do something else. Thanks!

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.

Review required. 

COuld you rebase your branch to make sure we don't have any issues? 🤗 


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

Now, it is synched with the latest repository. @ArthurZucker 

can you push the merge here? 😉 

Thanks!
fix models that fail in `test_model_parallelism`
Author: faaany
Pull request: yes

## What does this PR do?
This PR fixes the following failed tests:
```bash
pytest -rA tests/models/clip/test_modeling_clip.py::CLIPModelTest::test_model_parallelism
pytest -rA tests/models/m2m_100/test_modeling_m2m_100.py::M2M100ModelTest::test_model_parallelism
pytest -rA tests/models/mamba/test_modeling_mamba.py::MambaModelTest::test_model_parallelism
pytest -rA tests/models/whisper/test_modeling_whisper.py::WhisperStandaloneDecoderModelTest::test_model_parallelism
pytest -rA tests/models/qdqbert/test_modeling_qdqbert.py::QDQBertModelTest::test_model_parallelism
pytest -rA tests/models/udop/test_modeling_udop.py::UdopModelTest::test_model_parallelism
```

@ydshieh 


Comments:
There are 2 further tests that fail:

```bash
pytest -rA tests/models/cohere/test_modeling_cohere.py::CohereModelTest::test_model_parallelism
pytest -rA tests/models/udop/test_modeling_udop.py::UdopEncoderOnlyModelTest::test_model_parallelism
``` 

Do you have an idea why they fail? I tried to fix them, but I am not very confident about my fix. So I would like to hear about your thoughts on this first. 

Hi @amyeroberts , could you help review this PR? Thank a lot!

Hi @faaany . Thanks for the fix.

There is a work #30836 which has some overlaps with this PR? Might be better to wait that merged and see if there is any remaining to fix?

> Hi @faaany . Thanks for the fix.
> 
> There is a work #30836 which has some overlaps with this PR? Might be better to wait that merged and see if there is any remaining to fix?

Yes, indeed! Sure, let's wait. Thx!

Hi @faaany That PR is merged. You can try to rebase (and resolve the conflicts) to see if there is anything remaining to commit 🙏 

Hi @ydshieh, I tested the current master branch and all failing tests got fixed in PR #30806! Thx and close this one.
DRAFT - DO NOT MERGE: Test generate efficiency
Author: younesbelkada
Pull request: yes
Labels: name: run-slow

# What does this PR do?

Just to test https://github.com/huggingface/transformers/pull/30536


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30986). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
How to inference the model with 200k length context
Author: taishan1994
Pull request: no

### Model description

I want to test Yi-34B-200k,  Although I ran through the model, as the context length increased, OOM appeared, and I wondered how I could test to 200k context length with sufficient GPU resources.

### Open source status

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

### Provide useful links for the implementation

_No response_

Comments:
Hey! Thanks for opening this issue! Would recommend you to look into the `mistral` model which should support fairly (~16K) context length with linear memory consumption. Also make sure that you did not activate gradient computation!

> ### Model description
> I want to test Yi-34B-200k, Although I ran through the model, as the context length increased, OOM appeared, and I wondered how I could test to 200k context length with sufficient GPU resources.
> 
> ### Open source status
> * [x]  The model implementation is available
> * [x]  The model weights are available
> 
> ### Provide useful links for the implementation
> _No response_

I met the same issue. Did anyone else solve that?

设置output_scores=False（一般默认是这个）和use_cache=False，应该就不会有OOM

> 设置output_scores=False（一般默认是这个）和use_cache=False，应该就不会有OOM

请问下你测的哪个模型，测到的最大长度是多少额？

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.

> > 设置output_scores=False（一般默认是这个）和use_cache=False，应该就不会有OOM
> 
> 请问下你测的哪个模型，测到的最大长度是多少额？

刚测了下128K长度，用flash attention是没啥问题的
fix: paligemma not return loss when not use suffix
Author: MrToy
Pull request: yes

# What does this PR do?

According to https://github.com/huggingface/transformers/pull/30967 
forgot to consider the case of not using suffix , training paligemma will get an error

```
ValueError: The model did not return a loss from the inputs, only the following keys: logits. For reference, the inputs it received are input_ids,attention_mask,pixel_values,labels.
```

cc @ArthurZucker @molbap 

Comments:
But you are not supposed to train without a suffix, which is why I am not sure I understand the issue. 
prefix: <image> + promt
suffix: something. 
If you want to train on the prompt then suffix = prompt. 

My scenario is a conversation via image, so finetune it based on other vllm methods, use tokenizer.apply_chat_template to generate autoregressive prompt rather than prefix + suffix. I think suffix should be optional?

No, because you specifically need the suffix. That's just how the attention mask needs to be created for the model. 

I'll let @molbap sort this out, but TLDR, you gotta make sure the attention on the text you want to train on is causal, vs the image where it's full attention and the prompt (not the suffix) should also be bidirectional. Not sure training will work well if you don't follow this

@MrToy hope we were helpful! 🤗 

Thank you，I read the paligemma's design again， causal attn indeed only exists in suffix， I will look for some trick method to format the messages, e.g. prompt="" suffix=formatted_messages

Although incorrect, but this changes is work well for my training. The model works even without unmasking the prefix part🤣

Awesome! Hope you can share your findings with the community as well! 🚀 

That's great @MrToy ! If you need to pass a custom attention mask, you may also form the inputs by yourself entirely by passing a 

```python
# assuming you have 
inputs = processor(text=formatted_messages, images=..., return_tensors="pt", padding="longest") 
# this won't set token_type_ids since you don't pass a suffix but you can do
inputs['token_type_ids'] = torch.ones_like(your_input_ids_tensor)
```
which will set all tokens that you send to be of type "1", that is, of type suffix. Then
```python
outputs = model(**inputs)
```
should also work.
add "to" function to abstract transformer pipelines
Author: kabachuha
Pull request: yes

# What does this PR do?

Adds "to" function to transformer pipelines to correlate them with Diffusion pipelines and regular Torch nn.modules.

This is extremely important especially to automation, such as offloader context managers where it's expected to send models to devices in a unified way.

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.
Do not trigger autoconversion if local_files_only
Author: Wauplin
Pull request: yes

See [slack thread](https://huggingface.slack.com/archives/C016D661PAN/p1716540187510999?thread_ts=1716484668.496509&cid=C016D661PAN) (internal).

Since https://github.com/huggingface/transformers/pull/29846 (cc @LysandreJik) if pytorch weights are loaded and safetensors ones don't exist, we trigger an autoconversion of the weights. This should not impact user's experience. This PR disable the autoconversion call if `local_files_only=True` is passed. 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31004). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
ping `uv==0.1.45`
Author: ydshieh
Pull request: yes

# What does this PR do?

pin uv==0.1.45. 

0.2.x has issue.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31006). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Trainer should throw a warning if max_sequence_length < number of tokens in dataset sample record.
Author: ronakk-google
Pull request: no
Labels: name: trainer

name: Feature request

### Feature request

If the samples in the training dataset exceeds max_sequence_length the trainer should throw a warning.

### Motivation

Without warning devs are scratching their heads trying to understand why finetuning doesn't work for them.

### Your contribution

Willing to review the change.

Comments:
cc @muellerzr