noamaanMulla-03/transformers-issues huggingface dataset:
Add require_read_token to fp8 tests
Author: MekkCyber
Pull request: yes

# What does this PR do?
Adds `require_read_token` decorator to FP8 tests

## Who can review ?
@SunMarc 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_36189). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Add functionality to save model when training unexpectedly terminates
Author: jp1924
Pull request: no
Labels: name: Feature request

### Feature request

I'm thinking of implementing it like this:
```python
try:
    trainer.train(resume_from_checkpoint=args.resume_from_checkpoint)
finally:
    trainer._save_checkpoint(trainer.model, None)
```
I want to utilize the characteristics of 'finally' to ensure that the model is saved at least once at the end, 
even if the training terminates unexpectedly.


### Motivation

Sometimes we need to terminate training unintentionally due to scheduling or various other issues. 
If the model checkpoint hasn't been saved even after training has progressed to some extent, 
all the training resources used until now are wasted.

### Your contribution

Therefore, I want to add functionality to save the model checkpoint unconditionally 
even if the process is terminated by an error or kill signal unintentionally. 
And I want to control this through train_args.

Comments:
@muellerzr 
@SunMarc

Hey @jp1924, you can save the model checkpoint every x steps or x epoch. Do you feel like this is not enough ? The thing is that the training state (model weights, optimizer ... ) might be corrupted midway if you try to save it when the training terminates unexpectedly, so I wouldn't recommend doing that. 

@SunMarc 
Well... Now that you mention it, I guess I was thinking too short.

Certainly there are situations where the save function is not sufficient.

Still, it seems like there's a good chance that that method of saving could have unintended side effects, so I'll probably not use that idea.

Thanks for the response!
4.48.1 breaks sliding window in eager attention for qwen2
Author: wizyoung
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.48.1
- Platform: Linux-5.10.0-2.0.0.2-x86_64-with-glibc2.27
- Python version: 3.9.19
- Huggingface_hub version: 0.27.1
- Safetensors version: 0.4.5
- Accelerate version: 0.34.1
- Accelerate config:    not found
- PyTorch version (GPU?): 2.5.1+cu124 (True)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using distributed or parallel set-up in script?: <fill in>
- Using GPU in script?: <fill in>


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

None

### Expected behavior

I see v4.48.1 removes the sliding_window mask generation in `_prepare_4d_causal_attention_mask_with_cache_position` but leaves `sliding_window` arguments to `attention_interface`:
```
attn_output, attn_weights = attention_interface(
            self,
            query_states,
            key_states,
            value_states,
            attention_mask,
            dropout=0.0 if not self.training else self.attention_dropout,
            scaling=self.scaling,
            sliding_window=sliding_window,  # main diff with Llama
            **kwargs,
        )
```
This is ok under fa2, but what if using eager attention? Eager attention only accepts attention_mask and does not handle sliding windows. I see this should be a bug?

Comments:
cc @arthurzucker for qwen2 / fa2

Indeed, that's an overlook from #35235, opening a PR to revert this, sorry 🤗
Revert qwen2 breaking changes related to attention refactor
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Qwen2 has some sliding window that depends on layer id, this was removed so adding it back! 

- [x] Just need to add a test before merging

Fixes #35924
Fixes #35896


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_36162). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Saving nested configs crashes in Pixtral
Author: zucchini-nlp
Pull request: no
Labels: name: bug

name: VLM

### System Info

Issue from https://huggingface.co/mistral-community/pixtral-12b/discussions/24. When saving the config `head_dim` is skipped for being same as default value but when loaded back `head_dim` is inferred as a different value

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

```python
from transformers import AutoConfig
config = AutoConfig.rom_pretrained("mistral-community/pixtral-12b")
config.save_pretrained("tmp")
config_second = config.from_pretrained("tmp")
# config != config_second, head-dim is missing and inferred incorrectly
```

### Expected behavior

Will work on it, issue here so I don't forget :)

EDIT: might also be fixed with the help of community (https://github.com/huggingface/transformers/pull/36077) 🤗 

Comments:
Resolved by https://github.com/huggingface/transformers/pull/36077
Add @require_bitsandbytes to Aria test_batched_generation
Author: ivarflakstad
Pull request: yes

I noticed this test seems to require bitsandbytes but has not been marked as such.

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

> (do you run it and see if this test will pass at the end ..?)

Well I actually expect it to be skipped.

let's merge this!
fix: Fixed CohereForAI model identifier name
Author: Sai-Suraj-27
Pull request: yes

# What does this PR do?
Tests are failing in the CI: https://github.com/huggingface/transformers/actions/runs/12759991420/job/35565237222
The exact model identifier name: https://huggingface.co/CohereForAI/c4ai-command-r7b-12-2024

## 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


Comments:
cc @ydshieh!

Sorry @Sai-Suraj-27 fixed the merged conflicts now!

> Sorry @Sai-Suraj-27 fixed the merged conflicts now!

No, problem. The changes I made initially, fixing the failing tests are later covered in this PR: https://github.com/huggingface/transformers/pull/35682 and it was merged quickly before this PR.
FIx AutoProcessor loading error
Author: JJJYmmm
Pull request: yes

# What does this PR do?

Fix #34307 and #36194


## 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?

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.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker
- vision models: @amyeroberts, @qubvel
- speech models: @ylacombe, @eustlb
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- pipelines: @Rocketknight1
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @muellerzr and @SunMarc
- chat templates: @Rocketknight1

Integrations:

- deepspeed: HF Trainer/Accelerate: @muellerzr
- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization (bitsandbytes, autogpt): @SunMarc @MekkCyber

Documentation: @stevhliu

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->


Comments:
Make `check_repository_consistency` run faster by MP
Author: ydshieh
Pull request: yes

# What does this PR do?

It takes 3 minutes on `main`. This PR simply uses `multiprocessing` to make it faster.


Comments:
Other scripts in `make fixup` would benefit from it too 🙏 

(For completeness: A few months ago we had a conversation somewhere about adding multiprocessing or not. I remember the general consensus was to leave it as a last resort, as it may lead to other issues. Personally, I think the benefits outweigh the risks :D)

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

Other scripts might be also addressed by multiprocessing, but it depends on if a script has a linear logic: `check-copies` might be a difficult one 

@gante I just made it as an argument `num_workers` , default to `1` , and set it to `4` in CircleCI's `.circleci/config.yml`
Fix `inputs` deprecation warning in automatic_speech_recognition.py
Author: asmith26
Pull request: yes

# What does this PR do?

Updates `inputs` to  `input_features` to fix the deprecation warning found [here](https://github.com/huggingface/transformers/blob/main/src/transformers/models/whisper/generation_whisper.py#L493-L499) when running a `pipeline("automatic-speech-recognition",..)`.

## Who can review?
Models:
- speech models: @ylacombe, @eustlb

Library:
- pipelines: @Rocketknight1


Comments:
cc @gante - I think this is right because we want audio input to `generate()` to be `input_features`, but I'd like to double-check before we merge!

I don't think using `input_features` is general to every existing and future ASR pipelines tbh ! You might very much find an ASR model that uses `audio_codes` or `input_ids`, for example, if it uses a residual-vector quantization scheme to quantize audios!

@Rocketknight1 @ylacombe 

Models should have a `main_input_name` attribute. [e.g. on whisper](https://github.com/huggingface/transformers/blob/763548427d028878f4d4d8fb6f0be57cc3915fbd/src/transformers/models/whisper/modeling_whisper.py#L822)

This means we can build a dictionary of inputs where the key for this one is also a variable :D

@gante that works too, but isn't the first argument to `generate` always the input features/tokens anyway? Just leaving it as an unnamed arg should also work, right?

Hi there, just thought I'd mention I still seem to be getting the same warning (I'm currently running transformers == 4.47.0.dev0). Not sure if this can be merged? Thanks again :)

Yeah, I think this one slipped through the cracks - can we review/merge it, maybe with the positional first arg? cc @ylacombe @gante 

Revisiting this PR: I don't think we should change the input to the pipeline. Instead, we should change the odd one out, i.e. remove the warning in `Whisper` :)

(`generate` is happy with `inputs=foo`)
set `test_torchscript = False` for Blip2 testing 
Author: ydshieh
Pull request: yes

# What does this PR do?

As discussed internally, let's simply not test here.

(this is failing on current main)

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

run slow: bnb

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: []
quantizations: ['quantization/bnb'] ...

run slow: bnb

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: []
quantizations: ['quantization/bnb'] ...

run slow: vit

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: ['models/vit']
quantizations: [] ...

run slow: vit

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: ['models/vit']
quantizations: [] ...

run slow: vit

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: ['models/vit']
quantizations: [] ...

run slow: bnb

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: []
quantizations: ['quantization/bnb'] ...

not a big deal but this is just to make the CI a bit more ✅ (less noise) cc @ArthurZucker
Use `args.num_workers` in `check_modular_conversion.py`
Author: ydshieh
Pull request: yes

# What does this PR do?

Another mistake I forgot to update before merge in #36175

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

# What does this PR do?

`resolved_archive_file` in `_load_pretrained_model()` appears to be optional. In my case, I was loading a model from a GGUF file and it was `None`:

```python
model = AutoModelForCausalLM.from_pretrained(train_config.model_name,
                                             device_map='auto',
                                             gguf_file='llama3.2.gguf',
                                             offload_folder='offload')
```

In that case, `archive_file` ends up being `None` and the check for safe tensors raises an error. The change guards against that case and allows loading to continue.

I thought this change was minor enough that new tests were not warranted. If you feel otherwise, happy to add one if you can point me at the right place to do it.

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

Great! I should be able to add one sometime this week

Putting together a test case was helpful. It appears this condition is only triggered when a GGUF is loaded that is configured to offload a portion of `model.state_dict()` to disk. `modeling_utils::_load_state_dict_into_meta_model()` doesn't move the loaded state into modules mapped to `disk`. With the guard on `archive_file` from this patch, execution continues until `from_pretrained()` gets around to calling `dispatch_model()`, which promptly blows up trying to offload a meta tensor to disk.

I've worked around this by forcing the entire `state_dict` to get loaded in when `gguf_path` is specified – that gets the test working and produces the expected output. I'm not sure if this defeats the point of offloading some of the modules to disk (or if I'm missing something more fundamental). 

Thoughts or suggestions? I can update the PR with what I have if seeing the necessary changes is easier.

After reflection, I think that we shouldn't allow offload with GGUF. This is because with gguf state_dict, we still have to modify the state dict to be compatible with transformers. So we can't really offload to disk.

That makes sense. Do you think that should happen in transformers or accelerate? If it’s in the model loading here, I don’t mind taking a crack at it. 

The check should be in transformers ! 

@SunMarc I modified the handling of `device_map` so when `”auto”` is specified for a GGUF file, it will attempt to remap disk offload back to the CPU. If disk is explicitly configured, a `NotImplementedError` will be raised.

There’s a test case for the explicit disk mapping but there didn’t seem to be a non-invasive way of testing the `auto` remapping, and it didn’t seem worth it to me. Let me know if you disagree or would like any further modifications. 

@SunMarc ok, I think all the comments are addressed. Let me know if any additional changes are needed

No problem! Thanks for the feedback
Albert does not use SDPA's Flash Attention since attention mask is always created
Author: gau-nernst
Pull request: no
Labels: name: bug

### System Info

NA

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

https://github.com/huggingface/transformers/blob/dd16acb8a3e93b643aa374c9fb80749f5235c1a6/src/transformers/models/albert/modeling_albert.py#L772-L773

Since attention mask is always used by `AlbertModel`, SDPA will dispatch memory-efficient kernel instead of flash attention. The inner classes (`AlbertTransformer`, `AlbertLayerGroup`, ...) seem to handle attention_mask=None correctly, so it's only the outermost class is problematic.

### Expected behavior

Flash attention should be used when attention_mask=None

Comments:
Edit: After more careful investigation, my example didn't use Flash attention because I was running it in FP32. `_prepare_4d_attention_mask_for_sdpa()` actually returns None if attention mask is all ones -> it will use flash attention as expected. Though there is a small inefficiency in creating a ones mask just to check it and return None instead.

https://github.com/huggingface/transformers/blob/dd16acb8a3e93b643aa374c9fb80749f5235c1a6/src/transformers/modeling_attn_mask_utils.py#L448-L449
How about adding a combined step and epoch feature to save_strategy?
Author: jp1924
Pull request: no
Labels: name: Feature request

### Feature request

Add epoch+steps functionality to save_strategy

### Motivation

I often set save_strategy to epoch for saving, but sometimes I need to run experiments with steps. 
Recently, I had to compare checkpoints saved at both epoch and step intervals, which required running the experiment twice and was quite cumbersome. Having a combined feature would be really helpful. What do you think?

### Your contribution

I can add the epoch+steps functionality to save_strategy.

Comments:
@SunMarc 

Thanks for the feature request. Feel free to open a PR so that we can have a look. For which reason did you need to compare checkpoint saved both at epoch and step intervals ? Also, you can just tweak save step to make sure that you also save the model at the end of an epoch. 

@SunMarc  Ah, sorry for the late reply.
Recently, I've had to work more frequently with models like LLMs that are cumbersome to evaluate during training.
So I usually do the evaluation after training is complete, but
with epochs, the model gets evaluated sparsely, and even if we choose steps as an alternative, we still need epoch evaluation, but it's too difficult to align steps with epochs.
That's when I thought about combining steps + epochs into one, which led me to create this issue.

If you think this idea is worth pursuing, let's open a PR, and if not, we can close the issue.
Whether the OutEffHop can support with Transfomers
Author: robinzixuan
Pull request: no
Labels: name: Feature request

### Feature request

We request that the option "OutEffHop" be included in the BERT, OPT, and ViT models.

### Motivation

I am the author of OutEffHop and we plan to release our OutEffHop-based model on HuggingFace. However, because we use a different activation function (not softmax) in attention, if we directly use the original structure, the model inference online will show a huge problem.

### Your contribution

We are able to supply the code for OutEffHop and assist with integrating the function into BERT and OPT. Additionally, if necessary, we can also help incorporate OutEffHop into ViT. My initial idea is to add an option in the model configuration that allows the user to choose whether to use OutEffHop. If they opt to use it, they can employ the new activation function.

Comments:
Hi @robinzixuan, thanks for opening this feature request! 

Adding a new options for existing models, especially something as specific as this is normally something we try to avoid in transformers. 

If the attention mechanism is the only thing that's different for the models, I think the easiest approach would be having an attention class e.g. similar to `BertSdpaSelfAttention` which can be selected as an option through the config. The easiest and recommended way to make a model available in `transformers` is to add the modeling code directly on the hub: https://huggingface.co/docs/transformers/custom_models

cc @ArthurZucker @younesbelkada (as this might be related to quantization [from the arxiv paper](https://arxiv.org/abs/2404.03828))  

Thank you, we are the authors of OutEffHop. I have successfully set up the OutEffHop BERT and OPT-125m versions. However, the inference API still appears to be incorrect. What should I do for that

> However, the inference API still appears to be incorrect. What should I do for that

Could you explain a little bit about the errors or incorrect behaviour you're encountering?