noamaanMulla-03/transformers-issues huggingface dataset:
Ignore non-causal mask in more cases with SDPA
Author: fxmarty
Pull request: yes

Fixes https://github.com/huggingface/transformers/issues/30095. In the non-causal case, we can ignore the mask even for `key_value_length != tgt_len` as we will never have any fully masked row and will never hit the SDPA's mem-efficient attention backend issue.

Depends on https://github.com/huggingface/transformers/pull/28802

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30138). 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.

This is probably fixed on main
`_prepare_4d_attention_mask_for_sdpa` is not for causal attention but claims...
Author: minostauros
Pull request: no

... SDPA causal mask generation may be wrong for the mask generation.

https://github.com/huggingface/transformers/blob/76fa17c1663a0efeca7208c20579833365584889/src/transformers/modeling_attn_mask_utils.py#L421-L433

Will it be safe to just return `None` for the `else:` case?

For causal attention, we can just use `_prepare_4d_causal_attention_mask_for_sdpa`

Related issues:
https://github.com/pytorch/pytorch/issues/108108
https://github.com/Dao-AILab/flash-attention/commit/9e5e8bc91e30af5cdc321362b553f6c0da332e30
https://github.com/huggingface/transformers/pull/28802

Comments:
cc @fxmarty 

Hi @minostauros, thank you for the report.

> ... SDPA causal mask generation may be wrong for the mask generation.

`_prepare_4d_attention_mask_for_sdpa` does not handle causal masks. However,

> Will it be safe to just return None for the else: case?

Yes, good catch, I'll fix that! This is a somewhat unlikely case though, where one would use past key values for typically encoder-type of models. How did you run into this case?

> This is a somewhat unlikely case though, where one would use past key values for typically encoder-type of models. How did you run into this case?

I didn't run into the specific section but I was just reviewing #28802 and was trying to add flash-attention-2 to BERT (BLIP-2 variant of BERT to be exact).
Thanks for confirmation!

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.

This issue will be closed by https://github.com/huggingface/transformers/pull/30138
🚨 [Mistral and friends] Update MLP
Author: NielsRogge
Pull request: yes

# What does this PR do?

This PR is a tiny cosmetic change, no need to store the entire config and replace a one-letter variable name.

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

# What does this PR do?

Fix or skip `test_compile_static_cache` on Nvida T4 GPU. Those tests **are never run on T4** until we switch to torch 2.3.

- For LLama: just update the output values for T4.
- For Mistral: `torch.compile` failed. Let's skip it for T4 and focus on A10 CI (important model testing CI)
- (Gemma don't need any update)

cc @ArthurZucker @zhenglongjiepheonix 

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

Hi @ArthurZucker any comment?
Cohere: Fix copied from
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fix the wrongly set copied from statement leading to the cookie cutter not parsing the class correctly

cc @ArthurZucker 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31213). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Set greater_is_better to False if metric_for_best_model ends with "loss"
Author: miivanov90
Pull request: yes

Prior to this PR greater_is_better (used for determining if the best checkpoint, for example) would be set to False if users didn't set it explicitly and either metric_for_best_model is unset or it's one of "loss" or "eval_loss".

This PR simplifies the logic to simply check if metric_for_best_model ends with loss.

Background: Despite having read the documentation a while back I forgot about this behavior and updated my script to evaluate against multiple datasets, for each one I compute the "eval_dataset-name_loss" metric. You can imagine my surprise that after 7h of training I realized that the worst model was saved and all work was lost :)

@ArthurZucker @muellerzr

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

Hey! I was wondering if there were questions/concerns with this small refactor/fix?

@muellerzr
Fix GPU OOM for `mistral.py::Mask4DTestHard`
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix GPU OOM for `mistral.py::Mask4DTestHard`

```
tests/models/mistral/test_modeling_mistral.py::Mask4DTestHard::test_partial_stacked_causal_mask
(line 1159)  torch.cuda.OutOfMemoryError: CUDA out of memory. Tried to allocate 224.00 MiB. GPU
tests/models/mistral/test_modeling_mistral.py::Mask4DTestHard::test_stacked_causal_mask
(line 1159)  torch.cuda.OutOfMemoryError: CUDA out of memory. Tried to allocate 224.00 MiB. GPU
```

job run page

https://github.com/huggingface/transformers/actions/runs/9343352211/job/25712939733

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31212). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Trainer do model generation during evaluation loop
Author: szxiangjn
Pull request: no

### Feature request

Current trainer only supports teacher-forcing generation for computing evaluation loss but not auto-regressive generation for other metrics. Seq2SeqTrainer supports this but seems that it only accepts encoder-decoder models like T5 instead of GPT-style (decoder-only) models. Would this feature be added in the future?

### Motivation

I am training a decoder-only model and want to use model.generate to evaluate it during training.

### Your contribution

I haven't investigated deeply into Trainer code.

Comments:
You can write your own subclass of the Trainer, it's not supported and we don't plan on adding it.

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.

Why should this not be supported? I think it makes sense to have a predict_with_generate option.
Are there any examples showing how to use the `metric_for_best_model` of class `TrainingArguments`?
Author: yc1999
Pull request: no

<img width="567" alt="截屏2021-06-22 下午5 49 14" src="https://user-images.githubusercontent.com/37548571/122903624-2a128c80-d382-11eb-9400-4bb884d6c0c6.png">

In the [documents](https://huggingface.co/transformers/master/main_classes/trainer.html#trainingarguments), it says one can pass a `str` to `metric_for_best_model `.I am kind of confused why this can work. For example, can I just set `metric_for_best_model="accuracy"`, and it will compute accuracy itself? And if I have my own metric, how can I customize it? Thank you😄




Comments:
Hello, thanks for opening an issue! We try to keep the github issues for bugs/feature requests.
Could you ask your question on the [forum](https://discuss.huggingface.co) instead?

Pinging @sgugger 

Thanks!

The metrics need to be computed and have a name that is in what your `compute_metrics` function returns.

> The metrics need to be computed and have a name that is in what your `compute_metrics` function returns.

this works for me，thank you~

@sgugger are there standard metrics included? Can you use the ones from your evaluator?
Pass device in Logits Processor's init
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

This PR adds the ability to pass in device when initializing `LogitsProcessors` and is one more step towards `compile` compatibility. 

## 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.

@gante 

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

1. Cool, I did not notice that
2 and 3. Okay, thought we need it for consistency like we had with other new args in public classes. Will remove it and rebase later

Not stale

This PR now can be reviewed. Rebased main and updated the changes. All the tests from `RUN_SLOW=1 pytest tests/generation` are passing on my end

@gante Ah I forgot whisper is encoder-decoder. Oke, now it infers device from one of the inputs passed by the user. 


How could the bot come 🤣 anyways on it!

Oke, rebased main and the unnecessary formatting is removed. Will merge as I guess we don't need to add warnings :)
Observing weird downwards jump in loss after checkpoint reloading in a DDP setting
Author: jubueche
Pull request: no
Labels: name: trainer

### System Info

- `transformers` version: 4.39.1
- Platform: Linux-3.10.0-1160.76.1.el7.x86_64-x86_64-with-glibc2.17
- Python version: 3.10.14
- Huggingface_hub version: 0.21.4
- Safetensors version: 0.4.2
- Accelerate version: 0.30.0.dev0
- Accelerate config:    not found
- PyTorch version (GPU?): 2.4.0.dev20240322 (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?: True, 8 GPUs
- Using distributed or parallel set-up in script?: DDP


### Who can help?

@muellerz @pacman100  

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

We are observing a jump downwards in the training loss when we train a model (Roberta). This problem is best described by the two following plots:

<img width="1356" alt="Screenshot 2024-04-26 at 15 38 18" src="https://github.com/huggingface/transformers/assets/30778073/e0c3c387-9d3b-4550-a3d2-f22fbe144fd2">

We observe that the magnitude of the drop after re-loading the checkpoint is directly proportional to the amount of time of the previous run. 
Our first (and still only) guess was that there is some averaging going on. But debugging through the code on 1 GPU, I don't see any averaging effect: we add `1 / gradient_acc_steps * loss` until we call `optimizer.step()`. Afterwards, we reset the loss that we report. I don't see the issue there.

### Expected behavior

The loss should go down with smooth transitions between loading checkpoints.

Comments:
@jubueche Hi, jubueche , have you got some new ideas on why this happen? I meet the same problem, with loss like the pic when full fine-tune llama2:
<img width="523" alt="image" src="https://github.com/huggingface/transformers/assets/111731547/4a49b098-a0f5-4136-8388-146da2778fc3">
<img width="1088" alt="image" src="https://github.com/huggingface/transformers/assets/111731547/b927c9ad-acdf-4638-9889-07bc4b542363">


No, still no idea. We had to move on and are not actively working on identifying this problem anymore :( 

@jubueche Thanks for the reply!  I'll keep trying to figure out.

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.
Fix sentence fragment within test comments
Author: DomHudson
Pull request: yes

# What does this PR do?
Within https://github.com/huggingface/transformers/pull/1764, I accidentally left a meaningless sentence fragment within the new tests' comment blocks.

These have now been copied around the codebase several times. This clears them up.

## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).

## 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.

Tagging @stevhliu and @MKhalusova as the suggested engineers for documentation.

Comments:
Video-LLaVa: handle any number of frames
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

As per title, gives users freedom to sample any number of frames and tune the model with it. The inference will not generate quality text if we sample more than 8 frames, so it's only for those who want to tune with longer videos.

All video-llava tests are passing locally. 

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

# What does this PR do?

Protect the torch imports in a pipeline tests broken by #30996 (my bad)


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31227). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
PEFT: Properly warn users when PEFT is not installed
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fixes: https://github.com/huggingface/transformers/issues/31214

Updates the PEFT model loading logic to properly warn users when PEFT is not installed in their env and try to load an adapter model

Context: https://huggingface.slack.com/archives/C04L3MWLE6B/p1717430298492739

cc @amyeroberts @osanseviero @BenjaminBossan 



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

Hmm we discussed offline with @BenjaminBossan offline, this is trickier than what I thought - see for reference: https://huggingface.slack.com/archives/C04L3MWLE6B/p1717491255295879?thread_ts=1717430298.492739&cid=C04L3MWLE6B - moving forward users should just be aware that they need to correctly follow the docs by installing PEFT before trying to load a PEFT model !
[`TokenizationUtils`] add support for `split_special_tokens` 
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Adds support for `split_special_tokens` for fast models as well 
- [ ] deprecate `split_special_tokens` for `encode_special_tokens` for API consistency
- [ ] make sure this is saved and used not only as kwargs but also the attribute
- [ ] add some tests
- [ ] add some docs

Comments:
Wip, not urgent! Can already be done but it's not save / loaded
[Feature Extractors] Fix kwargs to pre-trained
Author: sanchit-gandhi
Pull request: yes

# What does this PR do?

It was reported by @osanseviero that when instantiating a feature extractor from pre-trained, setting kwargs occasionally leads to incorrect behaviour. This is particularly the case for Whisper, where setting the argument `feature_size` can have no effect on the number of Mel-bins:

```python
from transformers import WhisperFeatureExtractor

# initialise from kwargs -> mel filters matches feature size
feature_extractor = WhisperFeatureExtractor(feature_size=100)
print(f"From kwargs: feature size {feature_extractor.feature_size}, mel filters {feature_extractor.mel_filters.shape[1]}")

# initialise from pre-trained with kwargs -> mel filters not updated to new feature size
feature_extractor = WhisperFeatureExtractor.from_pretrained("openai/whisper-tiny", feature_size=100)
print(f"From pre-trained: feature size {feature_extractor.feature_size}, mel filters {feature_extractor.mel_filters.shape[1]}")
```
**Print Output:**
```
From kwargs: feature size 100, mel filters 100
From pre-trained: feature size 100, mel filters 80
```

This is because of the order in which we set the args in the feature extractor. We first pass all the arguments that we have saved in the `preprocessor_config.json` file: https://github.com/huggingface/transformers/blob/8127f39624f587bdb04d55ab655df1753de7720a/src/transformers/feature_extraction_utils.py#L569

And subsequently override any kwargs that we got from the user: https://github.com/huggingface/transformers/blob/8127f39624f587bdb04d55ab655df1753de7720a/src/transformers/feature_extraction_utils.py#L573-L575

The problem with this method is that for Whisper, we set some attributes based on the values of others in the init. E.g. we set `mel_filters` based on the value of `feature_size`:
https://github.com/huggingface/transformers/blob/8127f39624f587bdb04d55ab655df1753de7720a/src/transformers/models/whisper/feature_extraction_whisper.py#L87-L89

These **won't** be updated properly with our current method, since we're only updating the attribute `feature_size`, and not subsequently recomputing the correct `mel_filters`.

The simple fix is to give priority to the user kwargs and ensure these are passed to the init.

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

Well done for this change.  I was hitting the same issue with transformers 4.39.3 package - with transformers 4.41.0 I can use kwargs as expected.  Thanks.
Blip: Deprecate `BlipModel`
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fixes: https://github.com/huggingface/transformers/issues/31199

As pointed out in https://github.com/huggingface/transformers/issues/28034 and https://github.com/huggingface/transformers/issues/25024 , specifically here: https://github.com/huggingface/transformers/issues/28034#issuecomment-1859735216 `BlipModel` is a copy-pasta from CLIPModel and cannot be used as it is using blip weights on the Hub. We should deprecate this class as it bring a lot of confusion to users and progressively remove it from the docs, then remove the `BlipModel` class itself in the future. 

cc @amyeroberts 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31235). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Some weights of BlipModel were not initialized from the model checkpoint at Salesforce/blip-image-captioning-base.
Author: aivolcano
Pull request: no

### System Info

Some weights of BlipModel were not initialized from the model checkpoint at Salesforce/blip-image-captioning-base. This bug is thrown when I run the offical code in https://huggingface.co/docs/transformers/v4.41.3/en/model_doc/blip#transformers.BlipModel
![截图 2024-06-03 16-03-57](https://github.com/huggingface/transformers/assets/68730894/6be49972-c218-4669-8120-7aae4c82d9ba)

below is the offical code. 
```python
from PIL import Image
import requests
from transformers import AutoProcessor, BlipModel

model = BlipModel.from_pretrained("Salesforce/blip-image-captioning-base")
processor = AutoProcessor.from_pretrained("Salesforce/blip-image-captioning-base")

url = "http://images.cocodataset.org/val2017/000000039769.jpg"
image = Image.open(requests.get(url, stream=True).raw)

inputs = processor(
    text=["a photo of a cat", "a photo of a dog"], images=image, return_tensors="pt", padding=True
)

outputs = model(**inputs)
logits_per_image = outputs.logits_per_image  # this is the image-text similarity score
probs = logits_per_image.softmax(dim=1)  # we can take the softmax to get the label probabilities
``` 
 @ArthurZucker and @younesbelkada @amyeroberts

### Who can help?

 @ArthurZucker and @younesbelkada @amyeroberts

### Information

- [X] 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

from PIL import Image
import requests
from transformers import AutoProcessor, BlipModel

model = BlipModel.from_pretrained("Salesforce/blip-image-captioning-base")
processor = AutoProcessor.from_pretrained("Salesforce/blip-image-captioning-base")

url = "http://images.cocodataset.org/val2017/000000039769.jpg"
image = Image.open(requests.get(url, stream=True).raw)

inputs = processor(
    text=["a photo of a cat", "a photo of a dog"], images=image, return_tensors="pt", padding=True
)

outputs = model(**inputs)
logits_per_image = outputs.logits_per_image  # this is the image-text similarity score
probs = logits_per_image.softmax(dim=1)  # we can take the softmax to get the label probabilities

### Expected behavior

The model weight will be initialized correctly when running the official code 

Comments:
Hi @aivolcano , it is a known issue with `BlipModel` because this class isn't compatible right now with weights that are on the HF hub and there are already two issues closed regarding this. Please have a look at them as these may contain the solutions for your issue as well. Here they are: #28034  #25024 
And I think this class will be removed from the docs and deprecated.

Cheers!
Move out common backbone config param validation
Author: amyeroberts
Pull request: yes

# What does this PR do?

Moves out the repetitive config validation for backbone config params to one place 

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