noamaanMulla-03/transformers-issues huggingface dataset:
FA2 broken for Cohere2 if Optional `Mask` is not passed in `forward`
Author: Qubitium
Pull request: no
Labels: name: bug

### System Info

transformers==4.48.0.dev0 (from git+https://github.com/huggingface/transformers.git@5615a393691c81e00251e420c73e4d04c6fe22e5)

### Who can help?

@ArthurZucker @Cyrilvallez @SunMarc 

### Information

- [ ] The official example scripts
- [X] 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

Check our CI test failures:

# Gemma
https://github.com/ModelCloud/GPTQModel/actions/runs/12651906072/job/35253942521#step:12:1164

# Cohere2
https://github.com/ModelCloud/GPTQModel/actions/runs/12651906072/job/35253938235#step:12:922

We enabled FA2 by default on GPTQModel for inference of gptq quantized models and our CI tests are failing for multiple models. This looks like a regression in the `fa2` attention code where `seq_len` is never set if `mask is None`.  FA2 forward requires `seq_len`:

https://github.com/huggingface/transformers/blob/fc74e3976d090b655a8278ca1b70f784ff8b7938/src/transformers/models/cohere2/modeling_cohere2.py#L235-L270

@SunMarc  I don't think this is related to quantization and @ArthurZucker The FA2 code above is broken if `mask` is not passed or `None` as `seq_len` will never be set.  The `mask` param is explicitly declared as `Optional`.


### Expected behavior

Work and not crash.

Comments:
cohere2 flash attention 2 code is the original one from the author as you can see [here](https://github.com/huggingface/transformers/pull/35224). cohere2 model is one of the few models that code its own  `flash_attention_forward` function. Maybe @alexrs-cohere can help you fix this issue. 

Also, we are refactoring the attention https://github.com/huggingface/transformers/pull/35235, please let us know if you face any issues with other models ! 

Not entirely sure why you chose this particular commit as a version, but this does not seem to be an issue on `main`

> Not entirely sure why you chose this particular commit as a version, but this does not seem to be an issue on `main`

@Cyrilvallez  My mistake. Our CI was force checking out a commit post 4.47.1 but not the latest main since Cohere2 code was merged right after 4.47.1 release. 

So it looks like Cohere2 is the only model that still has the broken implementation code for Fa2.  

@alexrs-cohere Please check. 

https://github.com/huggingface/transformers/blob/7f7677307cfc8722ccc4680b993811098379844c/src/transformers/models/cohere2/modeling_cohere2.py#L268


Ha indeed the issue persists for Cohere2! Thanks, I'll open a PR!

Thanks for reporting this @Qubitium!

@Cyrilvallez let me know when the PR is ready and if you need any support from me!
Refactor/fix Cohere2
Author: Cyrilvallez
Pull request: yes

# What does this PR do?

Refactor Cohere2 to meet recent standards, and fix https://github.com/huggingface/transformers/issues/35547 at the same time


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

cc @ArthurZucker, and @alexrs-cohere if you want to have a look!
Exploit symmetry of covariance matrices for faster & more stable diag…
Author: bohrium
Pull request: yes

Fixes # 34570

see https://github.com/huggingface/transformers/issues/34570

@Rocketknight1

Comments:
Hi @bohrium, thanks for the fix! 

However, there's one other thing I realized, and I'm sorry I forgot about this over the Christmas break. Another user pointed out in #35075 that we don't need to explicitly generated the eigenvals at all, because we can just use a constraint check from `torch.distributions`. They never got around to making the PR, though - maybe you could change this PR to use the constraint check instead? That should be even faster than `eigvalsh`.
Trainer: update `state.num_input_tokens_seen` to use `num_items_in_batch`
Author: avishaiElmakies
Pull request: no
Labels: name: Feature request

### Feature request

Trainer has a state and inside the state there is a field called `num_input_tokens_seen`, which could be relevant for callbacks and other information.
The problem is that this field is updated using numel() which means it counts the padding as well. 
https://github.com/huggingface/transformers/blob/241c04d36867259cdf11dbb4e9d9a60f9cb65ebc/src/transformers/trainer.py#L2492-L2496

In a recent update the class calculates `num_tokens_in_batch` so it can be used for the loss calculation, this ignores padding. and can be used to give a more accurate picture for `num_input_tokens_seen`. 


__

### Motivation

`num_input_tokens_seen` is improtant information when you train a model. Having a more accurate picture for it will help people understand their training. It can also be used for callbacks (e.g. stop training after X tokens).

This will also eliminate the double calculation for num_tokens_seen (we are already calculating it once), and eliminate the discrepancy between the `num_input_tokens_seen` and  `num_tokens_in_batch`

### Your contribution

I maybe be able to do a PR for this

Comments:
I would like to work on this issue..
Proposed Solution - Replacing numel() function with num_tokens_in_batch..
A new Traditional Chinese version of the README.md for run_on_remote.py
Author: GNG1130
Pull request: yes

…e.py

# What does this PR do?

This PR adds a new Traditional Chinese version of the README file, named README_zh_hant.md, to the project. The file provides clear instructions on how to run the run_on_remote.py script, including examples and additional context for developers who prefer to work with Traditional Chinese documentation.

<!-- Remove if not applicable -->

Fixes # (issue)


## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?

Comments:
cc @stevhliu
[doc] deepspeed universal checkpoint
Author: huyiwen
Pull request: yes

# What does this PR do?

Fixes #33157 and https://github.com/microsoft/DeepSpeed/issues/5405


## 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).
- [ ] Did you write any new necessary tests?


## Who can review?

@muellerzr @stevhliu

<!-- 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:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35015). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@muellerz @stevhliu Is there anything else that needs help regarding merging this PR?
add `_supports_flex_attn = True` for models that do support it
Author: ArthurZucker
Pull request: yes

# What does this PR do?

Make sure model do support flex attention

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

This PR fixes a few issues with the ModernBERT implementation and some typos in the docs.

First, on the flash attention 2 path, ModernBERT was incorrectly always returning repadded logits without gradients (see #35386). This PR will return padded logits with gradients if no labels are passed to `ModernBertForMaskedLM` or if `repad_logits_with_grad` is set to true. We don't keep the gradient when repadding after using an internal loss function by default to save memory.

Second, this PR fixes torch.compile from being automatically set on when on CPU (#35388).

Third, I added some details to the model doc strings.

Fourth, documentation is updated to capitalize BERT in ModernBERT following the other BERT models.

cc @ArthurZucker @tomaarsen


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35404). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[`modernbert`] Add ModernBertTokenizerFast to allow for 'add_prefix_space'
Author: tomaarsen
Pull request: yes

# What does this PR do?

Experiments by the ModernBert authors show that `add_prefix_space=True` is required in some cases for proper performance. This PR adds a small TokenizerFast class that applies the same `add_prefix_space` fix as 13 other tokenizers: https://github.com/search?q=repo%3Ahuggingface%2Ftransformers+%22self.backend_tokenizer.pre_tokenizer+%3D+pre_tok_class%28**pre_tok_state%29%22&type=code

Internal discussions: https://huggingface.slack.com/archives/C07AC3MCFGE/p1736171722040829

## 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.
- [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).
- [ ] Did you write any new necessary tests?

Presumably there should be tests for this as well.

## Open questions
Should the default of `add_prefix_space` be True or False? I've used False for now as that's the default in a few other tokenizers.

## Who can review?

@ArthurZucker

ModernBert team: @warner-benjamin @bclavie @NohTow 

- Tom Aarsen


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

Looks good to me and produces the expected tokenization in my (admitedly so far limited) tests. Thanks for taking this on @tomaarsen !

Thank you for this PR since I was very frustrated with "meta space" of the tokenizer of [ModernBERT-base](https://huggingface.co/answerdotai/ModernBERT-base). And I found `bos_token_id` and `eos_token_id` in the [config.json](https://huggingface.co/answerdotai/ModernBERT-base/blob/main/config.json), which are [CLS] and [SEP] respectively. How do you think @tomaarsen?

Replaced by #35593, which applies the `add_prefix_space` fix to all tokenizers.

Thank you @tomaarsen and I agree that #35593 well works when the `pre_tokenizer` is simply `ByteLevel`. But I suspect that it does not work when the `pre_tokenizer` is a [`Sequence`](https://huggingface.co/docs/tokenizers/en/api/pre-tokenizers#tokenizers.pre_tokenizers.Sequence) including `ByteLevel`...
fix fsdp checkpointing issues
Author: pacman100
Pull request: yes

# What does this PR do?
1. FSDP loading now return the `load_result` to be given to `_issue_warnings_after_load`. Should be merged after https://github.com/huggingface/accelerate/pull/1745
2. Earlier it was wrongly saving `optimizer.pt` with FSDP, fixed it.

Comments:
_The documentation is not available anymore as the PR was closed or merged._
ModernBert: reuse GemmaRotaryEmbedding via modular + Integration tests
Author: tomaarsen
Pull request: yes

# What does this PR do?

1. Add integration tests with the now-released checkpoint + 2 checkpoints under hf-internal-testing (thanks @Xenova !)
2. Reuse GemmaRotaryEmbedding via modular functionality

Note that I based the expected values from the integration tests on the `main` implementation, not the RotaryEmbedding refactored implementation. In short: the test passes indicate that the refactor preserves the performance.

This is the refactor that I promised earlier offline.

## 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).
- [x] Did you write any new necessary tests?

## Who can review?

@ArthurZucker

cc @warner-benjamin 

- Tom Aarsen


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35459). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
PaliGemma2 Processor returns wrong labels array when <image> token is present in `text`
Author: probicheaux
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.47.0
- Platform: Linux-5.10.0-33-cloud-amd64-x86_64-with-glibc2.31
- Python version: 3.9.1
- Huggingface_hub version: 0.26.3
- Safetensors version: 0.4.3
- Accelerate version: 0.30.1
- Accelerate config:    - compute_environment: LOCAL_MACHINE
        - distributed_type: NO
        - mixed_precision: fp16
        - use_cpu: False
        - debug: False
        - num_processes: 1
        - machine_rank: 0
        - num_machines: 1
        - gpu_ids: 0
        - rdzv_backend: static
        - same_network: True
        - main_training_function: main
        - enable_cpu_affinity: False
        - downcast_bf16: no
        - tpu_use_cluster: False
        - tpu_use_sudo: False
        - tpu_env: []
- PyTorch version (GPU?): 2.2.1+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?: no
- Using GPU in script?: yes
- GPU type: Tesla T4

### Who can help?

@ArthurZucker @molbap we chatted about the last paligemma release :)

### Information

- [ ] The official example scripts
- [X] 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

Here is a script that shows the problem:
```python
from transformers import PaliGemmaForConditionalGeneration, PaliGemmaProcessor
from PIL import Image
import numpy as np

hf_token = "..."
processor = PaliGemmaProcessor.from_pretrained(
    "google/paligemma2-3b-pt-224", token=hf_token
)

text = ["How many shapes are green?"]
suffix = ["4"]

image = [Image.fromarray(np.zeros((224, 224, 3), dtype=np.uint8))]
print(
    processor(
        images=image, text=text, suffix=suffix, return_tensors="pt", padding="longest"
    ).labels
)
text = ["<image>How many shapes are green?"]
print(
    processor(
        images=image, text=text, suffix=suffix, return_tensors="pt", padding="longest"
    ).labels
)
```


### Expected behavior


As you can see, the bottom one is missing the EOS token, which leads to bad finetunes! But the processor class warns me when the `<image>` token isn't present.

Comments:
cc @zucchini-nlp as well

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.

Was resolved by #35201 , closing
Skip torchscript tests if we see a cache object produced in a test
Author: ydshieh
Pull request: yes

# What does this PR do?

As discussed offline, but let's not to be too disruptive in a one go.

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

# What does this PR do?

As per the title.
In training mode, the in-place operation would produce `RuntimeError: FakeTensor is wrapped to wrong device, found cpu, expected cuda:1`. It is however working when gradients are not required (i.e. with `generate`), which is the reason why it wasn't detected before.
I am not entirely sure what's happening behind the scene and the root of the issue, but switching to explicit operation fixes the issue.


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

thank you so much @ArthurZucker @Cyrilvallez ! I will test it out later today!
Incorrect average calculation in `Perplexity of fixed-length models`
Author: forrestdavis
Pull request: no
Labels: name: Good Second Issue

name: bug

### System Info

- `transformers` version: 4.44.0
- Platform: macOS-14.6-arm64-arm-64bit
- Python version: 3.10.14
- Huggingface_hub version: 0.24.5
- Safetensors version: 0.4.3
- Accelerate version: 0.33.0
- Accelerate config: 	not found
- PyTorch version (GPU?): 2.4.0 (False)
- 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?: No

### Who can help?

@ArthurZucker @stevhliu 

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

(This takes longer than 30s, I apologize. But the example has a lot of data and I wanted to minimally change the provided code to demonstrate the point. Note this is the pytorch version.). 

1. [The example script for finding the perplexity of fixed length](https://huggingface.co/docs/transformers/perplexity) model using strided windows does not properly calculate the average negative log-likelihood for each token aggregated over all the strided context windows. The first context window has the maximum allowable size, which is 1024. That amounts to 1023 targets. The remaining windows have 511 targets, except the final window which has 411. The way the average is calculated assumes that the same number of targets are considered in each strided context. While this results in a minor difference (roughly 0.01 higher than it should be) in the case shown, it leads to much larger issues with shorter texts. 
2. Run the following [colab script](https://drive.google.com/file/d/13jCCj85-MOw4bhRTWJ8iyFvs4ApBg9R2/view?usp=sharing) (I don't have access to GPUs on colab, so I ran this on mac with `mps` with the exact same notebook). 
3. Compare the printed `ppl` value which is `16.44` to the reported one which is `16.45`

### Expected behavior

The value of `ppl` should reflect the average negative log-likelihood for each token across the entire corpus. 

Comments:
Hey! Fill free to open a PR to fix this 🤗 

Hi guys, can you assign me this issue? It's my first (starting in Hacktoberfest!), so might need some guidance but think I can do it

Hey! we don't assign issue, but feel free to ping us on a PR that fixes it!

@ArthurZucker Create a pull request #34394 

@ArthurZucker A gentle Nudge
Please close the issue

Closing because I believe this is resolved now
Fix flaky `test_custom_4d_attention_mask`
Author: ydshieh
Pull request: yes

# What does this PR do?

`test_custom_4d_attention_mask` is flaky (at least for `Phimoe`, see a failed run [here](https://app.circleci.com/pipelines/github/huggingface/transformers/115393/workflows/439e29bc-fdff-451f-953f-66c437bed429/jobs/1546620/tests)

This PR fixes the flakyness by using the receipt we find useful in many cases.

Failure ratio: running 5000 times:

- `main`: 0.3 ~ 0.6 %
- `PR`: 0.0 %

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