noamaanMulla-03/transformers-issues huggingface dataset:
Fix `kwargs` handling in `generate_with_fallback`
Author: cifkao
Pull request: yes

# What does this PR do?

Fixes #29312.

1. changes the `pop()` to `get()` to avoid modifying `kwargs` between loop iterations,
2. makes sure a copy of `kwargs` is made as the first step in `generate_with_fallback()` to prevent any changes to it from propagating outside the method call.
3. makes sure the keys that were assigned to `generation_config` are removed from the keyword arguments to `super().generate()` (to avoid overriding the former), but this is done in a copy of `kwargs` that is not reused between iterations.

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->


## 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?

@patrickvonplaten @sanchit-gandhi @ylacombe 


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

@sanchit-gandhi Test added! 

On `main`:
```
FAILED tests/models/whisper/test_modeling_whisper.py::WhisperModelIntegrationTests::test_whisper_longform_multi_batch_beam - assert 1 == 2
```
After fix:
```
PASSED tests/models/whisper/test_modeling_whisper.py::WhisperModelIntegrationTests::test_whisper_longform_multi_batch_beam
```
All chunks except the first one ignore `num_beams` in Whisper long-form transcription
Author: cifkao
Pull request: no

### System Info

- `transformers` version: 4.39.0.dev0 (`dabe855`)

### Who can help?

@patrickvonplaten @sanchit-gandhi

### Reproduction

In `WhisperGenerationMixin.generate_with_fallback()`, the `kwargs` argument [is modified in-place](https://github.com/huggingface/transformers/blob/dabe8556686a5727f7b707099967c8ce8ff16e96/src/transformers/models/whisper/generation_whisper.py#L776) by `kwargs.pop("num_beams", 1)`, which results in the following problems:
1. Subsequent iterations of the fallback loop will use `num_beams=1` (although this has no effect provided that temperature 0 appears only once in the temperature list).
2. More importantly, since `kwargs` is passed by reference (and not via the `**kwargs` syntax), modifications to it propagate to the caller and to subsequent calls of `generate_with_fallback`, so **every chunk except for the first one will always use `num_beams=1`**.

### Expected behavior

The specified `num_beams` should be used for all chunks.

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.

This sounds like a pretty huge bug, do you have a reproducer? 

@ArthurZucker How is this for a reproducer?

```python
import datasets
from transformers import AutoProcessor, GenerationMixin, WhisperForConditionalGeneration
import numpy as np

processor = AutoProcessor.from_pretrained("openai/whisper-tiny")
model = WhisperForConditionalGeneration.from_pretrained("openai/whisper-tiny")

orig_generate = GenerationMixin.generate


def generate(self, *args, **kwargs):
    print("num_beams:", args[1].num_beams)
    return orig_generate(self, *args, **kwargs)


GenerationMixin.generate = generate

ds = datasets.load_dataset(
    "google/fleurs", "en_us", split="test", trust_remote_code=True
)
ds = ds = ds.cast_column("audio", datasets.Audio(sampling_rate=16000))
raw_audio = np.concatenate([x["array"].astype(np.float32) for x in ds[:16]["audio"]])

inputs = processor(
    [raw_audio],
    return_tensors="pt",
    truncation=False,
    padding="longest",
    return_attention_mask=True,
    sampling_rate=16_000,
)

model.generate(
    **inputs,
    num_beams=2,
    task="transcribe",
    language="en",
)
```

Before fix:
```
num_beams: 2
num_beams: 1
num_beams: 1
num_beams: 1
num_beams: 1
num_beams: 1
```

After fix:
```
num_beams: 2
num_beams: 2
num_beams: 2
num_beams: 2
num_beams: 2
num_beams: 2
```

I've tried to modify one of the long-form transcription tests to check that `GenerationMixin.generate()` is being called with the correct arguments, but I haven't figured out how to mock it out correctly.

IT's perfect, thanks!
Incorrect no-speech probability in `WhisperNoSpeechDetection` when `language` is set and `num_beams > 1`
Author: cifkao
Pull request: no

### System Info

- `transformers` version: 4.39.0.dev0 (`c8d9840`)

### Who can help?

@patrickvonplaten @sanchit-gandhi

### Reproduction

 In [`WhisperNoSpeechDetection`](https://github.com/huggingface/transformers/blob/c8d98405a8f7b0e5d07391b671dcc61bb9d7bad5/src/transformers/generation/logits_process.py#L1893), in some cases, the model is run again inside the logits processor to get the **logits**, but then, if `self.is_scores_logprobs == True`, those logits are run through `exp()` instead of `softmax()`, resulting in incorrect (unnormalized) probabilities (possibly `> 1`).

This scenario arises when the model is called with `language` set and with `num_beams > 1`.

### Expected behavior

The no-speech probability should be the same regardless of the value of `num_beams`.

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.

@ylacombe

Thanks for the ping @cifkao, [I've answered](https://github.com/huggingface/transformers/pull/29248#issuecomment-2029358885) directly on the PR !
Fix vipllava for generation
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

When working on [this PR](https://github.com/huggingface/transformers/pull/29853), it was found that VipLlava fails when generating with kv cache, because of incorrectly indexing `past_kv_length`. This PR fixes it by indexing the past length as `-2`. Other Llava models work correctly.




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

Yes, the `tests/models/vipllava/test_modeling_vipllava.py::VipLlavaForConditionalGenerationIntegrationTest::test_small_model_integration_test` should have caught it. Also, soon we will have all GenerationTesterMixin tests running for vision language models, to catch this type of errors without slow CI.

Yeah, that's because other models index only the first head dim in [this line ](https://github.com/huggingface/transformers/blob/ae49b218c3d718df90d8e4a109016450fb8f0632/src/transformers/models/llava/modeling_llava.py#L442), while VipLlava indexes the first head.

I found why we need this head indexing hack and tried generating with padded inputs, I did not see any difference between indexing first head or first head dimension.

Made Llava model code consistent and ran all tests (+slow). Some that were failing do not have anything to do with the current changes, I confirmed they were failing long time before

reverted changes back and rebased main, can be merged now
Fix slow tests for important models to be compatible with A10 runners
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fixes all slow tests failure for the A10 runner job that I am creating in https://github.com/huggingface/transformers/pull/29235

cc @ydshieh 

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

I will close this one, push the final changes to a new branch and open a new PR.

------------------------------------------------------------------------------------------------------------------------

@ArthurZucker told me to take over this PR and finalize it, but I am not able to push the changes. I think I have everything configured, but still get permission denied.

If we want to move forward without waiting @younesbelkada back, we can close this one and open a new one. Let me know what you prefer.
Don't have audio in this URL
Author: emmanuelwithme
Pull request: no

### Feature request

For this tutorial https://huggingface.co/docs/transformers/v4.39.1/en/pipeline_tutorial.
Some code can not run.
Because this url doesn't have audio. https://huggingface.co/datasets/sanchit-gandhi/librispeech_long/resolve/main/audio.wav


### Motivation

As above

### Your contribution

As above

Comments:
cc @stevhliu
[docs] Fix audio file
Author: stevhliu
Pull request: yes

Fixes #29917 (audio file was no longer available so part of the tutorial didn't work).

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

# What does this PR do?

I have been seeing this errors for a few days (when `make quality`) and seems like there is no PR fixing it. I decided to make it a separate PR, to merge quickly independently of other changes.


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

I noticed now that it does not fail in every `make quality`. Guess that depends on what local changes were made 🤔 

The bug here is that the import is done from incorrect place, because the `requires_backend` should be imported from `utils`. It is already done so for all image processors in library. I am not sure how the Superpoint model tests passed when adding the model, but currently trying to get the processor as below fails in `main`

`processor = AutoImageProcessor.from_pretrained("magic-leap-community/superpoint")`


@gante this can be merged
Make clearer about zero_init requirements
Author: muellerzr
Pull request: yes

# What does this PR do?

@abhishekkrthakur pointed out that he was facing some confusing issues on when trying to use zero-init + model initialization, and he was initializing the model before `TrainingArguments` were made, so zero-init couldn't be setup. 

There's no real way to *catch* this, so instead it flags beforehand that we need to instantiate the `TrainingArguments` beforehand in the docstring. The exception of this is if a user used a configured `accelerate launch`, which this PR also checks for/has logic for


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#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.

@pacman100 @amyeroberts 


Comments:
Thank you for adding this. It will hopefully save a lot of people a lot of time :) 

@pacman100 the other option I'm considering: 

Could we realistically check for this from `accelerate launch` via environmental variables set for zero init? This way the catch works for people not using `accelerate launch`, but we can do it ourselves if not

(Whether we raise an explicit error, or doing it behind the scenes I'm iffy on, I'd rather raise an err)

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

Thanks @amyeroberts, I've tweaked it so now it lives under its own new func, and we call both in order since it's only needed in two places

@amyeroberts @pacman100 should be good for a last review. Post discussions offline it's best to just do an RTFM, since we can't reliably check beforehand and currently rely on having numerous examples instead
[`Main CIs`] Fix the red cis 
Author: ArthurZucker
Pull request: yes

# What does this PR do?
The fetcher did not trigger tests

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

Just checked that failing tests pass, cc @ydshieh no idea why the fetcher found the whisper tests, then ignored it

what is the original PR? 🙏
BASED 
Author: axelmagn
Pull request: no
Labels: name: New model

### Model description

BASED is an attention model which combines sliding window attention and global linear attention to capture similar dependencies to transformers in a subquadratic model.  

It outperforms other similar models such as Mamba.



### Open source status

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

### Provide useful links for the implementation

- [GitHub Repository](https://github.com/HazyResearch/based)
- Model Weights: [360m](https://huggingface.co/hazyresearch/based-360m), [1b](https://huggingface.co/hazyresearch/based-1b)

Comments:
Hi I'm curious if it's possible to add in these models. Is there anything I can do to speed it along?
Is adamw_torch the same as torch.optim.adamw ?
Author: RonanKMcGovern
Pull request: no

### System Info

I see adamw_torch is the name of an optimiser in the docs but can't find details about it.

### Who can help?

@muellerzr and @pacman100

### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [X] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

I'm looking to wrap the torch_adamw optimizer and am wondering if it's torch.optim.adamw that I need to wrap

### Expected behavior

see above

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.
Idefics - AttentionMasks wrongly set with padding='longest'
Author: VictorSanh
Pull request: no
Labels: name: Good Second Issue

### System Info

transformers==4.36.2


### Reproduction

Reported by https://huggingface.co/VishnuSuganth
https://huggingface.co/HuggingFaceM4/idefics-9b-instruct/discussions/11


Comments:
Cc @ArthurZucker @younesbelkada 

Might be a tokenization issue will have a look 

Is anyone working on this issue? If not, would it be something a new contributor could look at?

I think the issue may be how `unpadded_seq_len` is calculated here: https://github.com/huggingface/transformers/blob/main/src/transformers/models/idefics/processing_idefics.py#L347-L354

Dumping the parameters inside this loop we can see a mismatch between the case of `padding=False` and `padding='longest'` for the smaller input in their repro:
```
Calling processor() with padding=False
--------------------------------------------------
Param Values:
text=[1, 4911, 29901, 1724, 338, 297, 445, 1967, 29973, 32000, 32001, 32000]
decoded_text=
 User: What is in this image?<fake_token_around_image><image><fake_token_around_image>
unpadded_seq_len=12
start=8
```
```
Calling processor() with padding='longest'
--------------------------------------------------
Param Values:
text=[0, 0, 0, 0, 0, 0, 0, 0, 1, 4911, 29901, 1724, 338, 297, 445, 1967, 29973, 32000, 32001, 32000]
decoded_text=<unk><unk><unk><unk><unk><unk><unk><unk>
 User: What is in this image?<fake_token_around_image><image><fake_token_around_image>
unpadded_seq_len=20
start=0
```

If using `<unk>` as the padding token (https://huggingface.co/HuggingFaceM4/idefics-9b-instruct/blob/main/tokenizer_config.json#L64) is deliberate, could this issue be solved by simply omitting the padding token from the calculation of `unpadded_seq_len`, or is this problem more complex?

After looking closer at `IdeficsProcessor`, I have a (possibly overconfident) idea of what's going on.

IIUC, it seems like this code is currently **always** hand-performing an additional padding pass after the tokenizer performs it's padding, according to this code: https://github.com/huggingface/transformers/blob/main/src/transformers/models/idefics/processing_idefics.py#L348-L354

And since the output tensors are stacked together [in this code](https://github.com/huggingface/transformers/blob/main/src/transformers/models/idefics/processing_idefics.py#L372-L374), they all have to be of the same size, i.e. padded or otherwise normalized.

In this case, I am not sure if `padding` should default to `False`, and instead should default to `longest`.

^ Tried my hand at a fix in the pr above #29449

Thanks I'll review in a bit!
TF: GPT2 with native embedding layers
Author: gante
Pull request: yes

# What does this PR do?

This PR continues the (paused) goal of deprecating our custom TF embedding layers and related code. Previously, we have converted encoder-decoder models (e.g. [here](https://github.com/huggingface/transformers/pull/19263)), removing `TFSharedEmbeddings` there and making the necessary adaptations.

In this PR, I make the necessary adaptations for GPT2. The goal is for you, the reviewers, to raise objections in this PR :D All slow tests for TF GPT2 are passing.


Then, the following sequence of PRs will be opened:
1. Remove `TFSharedEmbeddings` from the other decoder-only models
2. Remove other uses of `TFSharedEmbeddings` in the codebase (e.g. in tests)
3. Remove `resize_token_embeddings` and all related functions (it is only used to resize our models' embeddings instantiated with `TFSharedEmbeddings`)
4. Remove the slow decorator from `test_save_load_after_resize_token_embeddings`, which will be fixed as a consequence of these changes 🙌 

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

I noticed this change one year later, while doing a demo of LoRA in the embedding layer :-D  Nice, but I am always afraid of doing tensorflow operations outside of keras layers. Usually masks are lost, as well as any subtle things that depend of chaining layers. It is not a problem in transformers because the -100 trick is the standard mask and the loss is aware of it.
FIX: Fix 8bit serialization tests with safetensors refactor
Author: younesbelkada
Pull request: yes

# What does this PR do?

In bnb 8bit model's state dict, we include some str fields, such as the weight format (`row` or `column`) for each weight. 
To reproduce:
```python
import tempfile
from transformers import AutoModelForCausalLM

model_id = "gpt2"

model = AutoModelForCausalLM.from_pretrained(model_id, load_in_8bit=True)

with tempfile.TemporaryDirectory() as tmp_dir:
    model.save_pretrained(tmp_dir)
```
This PR fixes this issue by making sure the non-tensor objects pointers are dealt when processing the state dict

cc @Narsil @Titus-von-Koeller @ArthurZucker 

Fixes failing bnb serialization tests in: https://github.com/huggingface/transformers/actions/runs/8470920358


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

Closing in favor of #30051
Add `whisper` to `IMPORTANT_MODELS`
Author: ydshieh
Pull request: yes

# What does this PR do?

See https://github.com/huggingface/transformers/pull/30018#issuecomment-2034960969

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

# What does this PR do?

This starts to fail after #29473

See https://github.com/huggingface/transformers/pull/29473#pullrequestreview-1945687810

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

As per title. This is needed for the ONNX export of Musicgen e.g. for transformers.js

This removes an important warning:
```
/home/fxmarty/hf_internship/transformers/src/transformers/models/encodec/modeling_encodec.py:121: TracerWarning: Converting a tensor to a Python float might cause the trace to be incorrect. We can't record the data flow of Python values, so this value will be treated as a constant in the future. This means that the trace might not generalize to other inputs!
  ideal_length = (math.ceil(n_frames) - 1) * stride + (kernel_size - padding_total)
```
where a padding length was previously hard-coded and applied here https://github.com/huggingface/transformers/blob/f01e1609bf4dba146d1347c1368c8c49df8636f6/src/transformers/models/encodec/modeling_encodec.py#L149-L160

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