noamaanMulla-03/transformers-issues huggingface dataset:
[fix] LlavaNextProcessor '_get_unpadded_features' method
Author: laurentd-lunit
Pull request: yes

# What does this PR do?

Fixes [33261 (issue)](https://github.com/huggingface/transformers/issues/33261)
The typo in `LlavaNextProcessor`'s  `_get_unpadded_features` method can lead to size mismatch between the number of image tokens computed by the processor and the number of image tokens resulting of the vision tower and then leads to an error when applying the `masked_scatter` operation ([here](https://github.com/huggingface/transformers/blob/main/src/transformers/models/llava_next/modeling_llava_next.py#L881))


## 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?
- [x] 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.
- [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).
- [x] Did you write any new necessary tests?


## Who can review?

I believe @zucchini-nlp would be the right person to tag here.


Comments:
The tests didn't show the issue probably because we usually have a square image, instead of 2-1 or 1-2 resolution. @laurentd-lunit can you add a test for this one pls?

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

@zucchini-nlp @LysandreJik Thanks for the reviews! I added a test too. `check_repository_consistency` checks fail now but this is related to other changes than those I made so not sure if I should fix it here?

@laurentd-lunit can you run `make fix-copies`? Yes, seems like the PR recently merged didn't rebase on `main` before it was merged

cool, thanks for fixing the copies, will merge the PR now
wait 15m before SSH into runner workflow stops
Author: ydshieh
Pull request: yes

# What does this PR do?

Let's make it wait a bit longer (instead of the current 5m) - to reduce the chance we have to re-trigger it if we forget to connect within 5m.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33300). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[InstructBLIP] qformer_tokenizer is required input
Author: amyeroberts
Pull request: yes

# What does this PR do?

At the moment, `InstructBLIP` doesn't have `qformer_tokenizer` listed as one of its attributes. This means that the processor is not currently compatible with tests using the `ProcessorTesterMixin`, as the component won't be automatically loaded when constructing the processor. 

As the assumption that `processor.attributes` lists all the processing classes for the processor I believe is a reasonable one, I've modified the processor here to include it. 

This does require some hacky logic for when the processor is saved, to avoid overwriting the tokenizer configs and still saving the qformer_tokenizer configs to a seaprate subfolder. 

For future processors, I think it would be better for us to handle this more cleanly such that many processing classes of the same type can be bundled and saved together easily and automatically. For example, having all the qformer_tokenzer files saved with a `qformer_` prefix on the top level. This would enable e.g. having image processors and video processors saved together too. 

This isn't possible for this processor because of backward compatibility, but we can consider what it would look like in the future. 


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

LMK when you want a review Amy!

@LysandreJik Yes please! Would also like a review from @zucchini-nlp, who I discussed this with and who has been doing lots of work with the processors recently

@zucchini-nlp Done! Added processor tests for InstructBlipVideoProcessor too
Multi agents with manager
Author: aymeric-roucher
Pull request: yes
Labels: name: Agents

# What does this PR do?

Add native support for multi-agents in a tree-like hierarchic structure, by allowing an agent to manage other agents and call them as if they were tools.

cc @LysandreJik and @plaggy 

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

@aymeric-roucher did you consider wrapping managed agents as tools, the way you did with Gaia? Would passing a separate list of managed agents be beneficial in some way? 

@plaggy passing the list of agents as tools works as done on GAIA, but it leads to confusion between managed agents and tools, which leads to errors.
For instance if you use a search agent and name it `search_agent`, when building tool descriptions, `search_agent` being amongst tools means that your manager agent will understand it should be called the same way as tools. So instead of giving it detailed and complex requests as it would do for an agent (something like "Give me the 3rd player listed on the Denver Nuggets homepage from the 15th of October"), it will give it minimal requests as for a normal google search : "3rd player Denver Nuggets", and thus get suboptimal results. This bad behaviour persists even if you prompt it correctly in the description like" this is not a tool but an agent, give it detailed explanations"

This is why passing managed agents as a different argument than tools is mandatory IMO.

> @plaggy passing the list of agents as tools works as done on GAIA, but it leads to confusion between managed agents and tools, which leads to errors. For instance if you use a search agent and name it `search_agent`, when building tool descriptions, `search_agent` being amongst tools means that your manager agent will understand it should be called the same way as tools. So instead of giving it detailed and complex requests as it would do for an agent (something like "Give me the 3rd player listed on the Denver Nuggets homepage from the 15th of October"), it will give it minimal requests as for a normal google search : "3rd player Denver Nuggets", and thus get suboptimal results. This bad behaviour persists even if you prompt it correctly in the description like" this is not a tool but an agent, give it detailed explanations"
> 
> This is why passing managed agents as a different argument than tools is mandatory IMO.

@aymeric-roucher is the idea to provide agents-as-tools a verbose description of the task in a prompt so they'd perform better? 
The implementation makes sense to me! Left a couple of comments

@aymeric-roucher let me know when you'd like me to review this PR!

@LysandreJik this is ready for review! 🤗
File `agents.py` is becoming quite big with the addition of planning and ManagedAgent : at some point we might have to export both these components to other files.
Out-of-Index Error when training by `Qwen2VLFlashAttention2`
Author: heli-qi
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.45.0.dev0
- Platform: Linux-5.15.0-94-generic-x86_64-with-glibc2.35
- Python version: 3.10.14
- Huggingface_hub version: 0.24.5
- Safetensors version: 0.4.4
- Accelerate version: 0.33.0
- Accelerate config:    not found
- PyTorch version (GPU?): 2.2.0+cu121 (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>
- GPU type: NVIDIA GeForce RTX 4090 D


### Who can help?

@ArthurZucker @amyeroberts

### 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
Hi, I'm finetuning the newly-released `Qwen2VLForConditionalGeneration` model by LoRA. I'm building the model by
```python
Qwen2VLForConditionalGeneration.from_pretrained(
    "Qwen/Qwen2-VL-7B-Instruct", attn_implementation="flash_attention_2", torch_dtype=torch.float16
)
```
I found `attn_implementation="flash_attention_2"` activates `Qwen2VLFlashAttention2` which will throw a out-of-index error during training. When I switch to `attn_implementation="sdpa"`, the error does not come out and training goes smoothly. 

After some time of debugging, I located that the problem comes from [this line](https://github.com/huggingface/transformers/blob/main/src/transformers/models/qwen2_vl/modeling_qwen2_vl.py#L638) where `rotary_seq_len` does not properly reflect the length of the input sequence but rather the real length minus 1. I modified this line to `rotary_seq_len = cache_position[-1] + 1` in my local transformers offline package and it turns out that the training with `flash_attention_2` goes smoothly.

My input batch to the model is as follow:
```
batch
    input_ids: Tensor (B, seq_len)
    attention_mask: Tensor (B, seq_len)
    labels: Tensor (B, seq_len)
    pixel_values: Tensor (B, res_h, res_w)  # res_h and res_w are the shape of image after processor()
    image_grid_thw: Tensor (B, 3)
```
I suspect that my input batch to the model has the correct shape, so I'm wondering whether my tiny workaround is the optimal solution to the problem. I really appreciate it if you could tell me some better solutions.

### Expected behavior

As Reproduction section. Thanks for your patience for my issue.

Comments:
Will be fixed by https://github.com/huggingface/transformers/pull/33161 🤗
Add a warning to the chat template docs about the tool_calls format
Author: Rocketknight1
Pull request: yes

Add a warning because a lot of users apparently expected the OpenAI API!

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

cc @LysandreJik !
Add new documentation page for advanced agent usage
Author: aymeric-roucher
Pull request: yes
Labels: name: Agents

# What does this PR do?

Create a new doc page for advanced agent usage.

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

This is ready for merge after https://github.com/huggingface/transformers/pull/32687.
[BUG] fix upper nltk version
Author: ylacombe
Pull request: yes

# What does this PR do?

NLTK recent [3.9 release](https://github.com/nltk/nltk/blob/6c3aff6fedc6f34f6c1ea58cf58398ec961bbe3e/ChangeLog#L1-L13) breaks FastSpeechConformer2 tests on our CI.

More on this:
- I've managed to pinpoint the [precise commit](https://github.com/nltk/nltk/commit/8669576e753f4d701ef5d7a7f941f27c3dc43fc7)
- It could probably be solved by upgrading [this snippet](https://github.com/Kyubyong/g2p/blob/c6439c274c42b9724a7fee1dc07ca6a4c68a0538/g2p_en/g2p.py#L20-L23) in g2P with `averaged_perceptron_tagger -> averaged_perceptron_tagger_eng`, but the library hasn't been updated in 5 years
- Another way we could handle it is to manually run:
```py
>>> import nltk
>>> nltk.download('averaged_perceptron_tagger_eng')
```
- I preferred to pin NLTK version instead, WDYT @ydshieh @LysandreJik ?


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

# What does this PR do?

On top of #32830, make the newly added module importable

## Before submitting
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [x] 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.


## 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.

@amyeroberts
@gante

Comments:
Use this PR to iterate on module impartiality and autodoc 

Squeezed into #32830
[trainer] fractional epoch 
Author: stas00
Pull request: no

Running 
```
export BS=20; rm -r output_dir; CUDA_VISIBLE_DEVICES=0,1 PYTHONPATH=../../src USE_TF=0   ./finetune_trainer.py --model_name_or_path t5-small --output_dir output_dir --adam_eps 1e-06 --data_dir wmt_en_ro --do_eval --do_predict --do_train --evaluation_strategy=steps --freeze_embeds --label_smoothing 0.1 --learning_rate 3e-5 --logging_first_step --logging_steps 1000 --max_source_length 128 --max_target_length 128 --num_train_epochs 1 --overwrite_output_dir --per_device_eval_batch_size $BS --per_device_train_batch_size $BS --predict_with_generate --eval_steps 25000  --sortish_sampler --task translation_en_to_ro --test_max_target_length 128 --val_max_target_length 128 --warmup_steps 5 --n_train 100 --n_val 100 --n_test 100  --fp16 --save_steps 1
```
on master, gives:
```
{'loss': 14.846837043762207, 'learning_rate': 6e-06, 'epoch': 0.3333333333333333} 
```
epoch can't be fractional.


Comments:
The fraction (and float) `'epoch': 0.3333333333333333` comes from here:

https://github.com/huggingface/transformers/blob/1c19b423bf274a465f95725a79819bf82f71329e/src/transformers/trainer.py#L899

@sgugger - is this by design 

or should it be `ceil`: `'epoch': 1`
```
                    self.state.epoch = math.ceil( epoch + (step + 1) / steps_in_epoch)
```
or `floor`:  `'epoch': 0`
```
                    self.state.epoch = math.floor( epoch + (step + 1) / steps_in_epoch) 
```

`'epoch': 0.3333333333333333` is telling me it's somewhere in the first epoch but isn't done yet?

Perhaps it's just fine, it's just very odd to see epoch not being an int.

Thanks.


This is not a bug, it's completely normal. See the [dcoumentation](https://huggingface.co/transformers/main_classes/callback.html#trainerstate) of `TrainerState.epoch`.

Ah, OK. Some rounding then perhaps - `0.3333333333333333` is just too loud. 2 decimals?

Sure, there is no formatting at all for those results, but we can add some.

Anything else to format while I'm at it? loss I guess - 4 decimals, right?

`{'loss': 14.846837043762207, 'learning_rate': 6e-06, 'epoch': 0.3333333333333333} `

I don't see why not.

num_train_epochs=1 and 'epoch': 0.3333333333333333 can that happen ? In my case also the epochs stop at 0.57 and I don't get any print later?
Fix: Fix `FalconMamba` training issues due to incompatible kernels
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fixes training instability issues that users are facing with FalconMamba architecture. This is due to the fact that we did not upstreamed the modified kernels into mamba-ssm library.

To reproduce, you can try out this snippet:

```python
import torch
from transformers import AutoModelForCausalLM, AutoTokenizer, FalconMambaForCausalLM

model_id = "tiiuae/falcon-mamba-7b"
text = "Hello today"

model = AutoModelForCausalLM.from_pretrained(model_id, device_map="auto", torch_dtype=torch.bfloat16)
tok = AutoTokenizer.from_pretrained(model_id)

inputs = tok(text, return_tensors="pt").to(0)

with torch.no_grad():
    logits = torch.argmax(model(**inputs).logits, dim=-1)
    
print(tok.batch_decode(logits))

model.train()
lm_logits = model(**inputs).logits
next_token = torch.argmax(lm_logits, dim=-1)
    
print(tok.batch_decode(next_token))
loss = (1 - lm_logits).mean()
loss.backward()
```
That will output complete gibberish in the main branch. This is because we use the kernels that do not take into account the layer normalization of B, C and dt layers of Mamba.

There is an ongoing PR: https://github.com/state-spaces/mamba/pull/543 - I also propose to upstream the changes here in case that PR will not get merged as it will require users to install mamba-ssm from a certain version

The added kernel is a copy paste of the existing kernel that depends on triton, causal-conv1d and einops, and is not imported by transformers at all unless users excplictly import it.

Fixes https://github.com/huggingface/transformers/issues/33234

cc @ArthurZucker @molbap @vasqu 

Comments:
I can confirm the new slow tests pass on my end (on single GPU it was failing due to OOM, I just changed it to multi-GPU and it fixed it)

<img width="1327" alt="Screenshot 2024-09-05 at 11 25 49 AM" src="https://github.com/user-attachments/assets/7aa7243a-e5e8-40d5-9c5b-3e5277f488f4">


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

Thanks for the PR 🤗 Glad to see you around in the OS world!
Added optimizer adam mini
Author: Ashwanth369
Pull request: yes

Implements Adam Mini algorithm as introduced in [Adam-mini: Use Fewer Learning Rates To Gain More] (https://arxiv.org/abs/2406.16793).

Inspired from https://github.com/zyushun/Adam-mini


Comments:
cc @muellerzr @SunMarc 

cc @muellerzr  @SunMarc 

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

@muellerzr 

@muellerzr 

@muellerzr could you review the changes

@muellerzr 

@amyeroberts completed the mentioned changes. Could you review them

@amyeroberts 

@amyeroberts could you check the new test case and let me know if that works?

Hi, all. Thanks for implementing Adam-mini into Transformers!

I am one of the authors of the Adam-mini paper. Adam-mini is now available on PyPI (i.e., pip install adam_mini). We may update the optimizer in the future, so we prefer to support AdamW in Transformers by importing Adam-mini directly. We already have such an implementation. Could you provide some tips for us? Should I create a new pull request?

Hi all,

I have discussed this with my collaborators, and we have a proposal. We suggest naming the optimizer in this pull request 'Adam_mini_HF' because we think it is really cool to make it compatible with `accelerate`. At the same time, I will create a new pull request that supports using Adam-mini directly via importing Adam-mini directly.

My code for this version of Adam-mini is available [here](https://github.com/liziniu/transformers/tree/adam_mini) and is ready for a new pull request.

Key code sections are highlighted below:

https://github.com/liziniu/transformers/blob/adam_mini/src/transformers/trainer.py#L1125-L1129

https://github.com/liziniu/transformers/blob/adam_mini/src/transformers/trainer.py#L1221-L1244

Best,
Ziniu

@liziniu sounds great! I will go ahead and change the name to "Adam_mini_HF". @amyeroberts does this work?


@Ashwanth369 @liziniu in this case, I think the more canonical integration would be to use the optimizer from the source library and just use like e.g. [like for GrokAdamW](https://github.com/huggingface/transformers/blob/47b096412da9cbeb9351806e9f0eb70a693b2859/src/transformers/trainer.py#L1458). I don't have a strong opinion here - maybe @muellerzr can weigh in on what the best balance is in terms of functionality v.s. simplicity
ONNX export may fail for Hiera due to `math.sqrt` and python type `int` casts
Author: xenova
Pull request: no

Similar to a previous issue (https://github.com/huggingface/transformers/pull/31311) we had with vision models. cc @merveenoyan @amyeroberts @NielsRogge 

Offending lines:

https://github.com/huggingface/transformers/blob/5c1027bf09717f664b579e01cbb8ec3ef5aeb140/src/transformers/models/hiera/modeling_hiera.py#L340

https://github.com/huggingface/transformers/blob/5c1027bf09717f664b579e01cbb8ec3ef5aeb140/src/transformers/models/hiera/modeling_hiera.py#L344


We should probably abstract `interpolate_pos_encoding`, since this is reused across many different vision architectures.

Comments:
Hi @xenova, there are more issues with the current implementation of interpolate_pos_encoding
 - https://github.com/huggingface/transformers/issues/32410#issuecomment-2293359788
 
Maybe it's better to rewrite interpolation using `size` argument instead of `scale` to avoid errors described in the issue above, will it be compatible with ONNX export? WDYT?

> Maybe it's better to rewrite interpolation using size argument instead of scale to avoid errors described in the issue above, will it be compatible with ONNX export? WDYT?

Yes I agree! Some other issues/PRs where this is discussed:
- https://github.com/YvanYin/Metric3D/issues/117#issue-2364404569
- https://github.com/huggingface/optimum/pull/1580
- https://github.com/huggingface/transformers/pull/30065

I'll try draft a PR for this soon.
Update SECURITY.md
Author: Michellehbn
Pull request: yes

updated reporting a vulnerability section

# What does this PR do?

Updates to security page with details on submitting a vulnerability. 

Comments:
Thx!

Feel free to merge when you have the approvals you're looking for

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