noamaanMulla-03/transformers-issues huggingface dataset:
router_logits not getting populated on  output_router_logits=True for GptOssForCausalLM
Author: qrdlgit
Pull request: no
Labels: name: bug

### System Info

On kaggle - https://www.kaggle.com/code/kaggleqrdl/router-logits-visualization?scriptVersionId=255623848 see https://cookbook.openai.com/articles/gpt-oss/run-transformers and https://github.com/huggingface/transformers/pull/39940

- `transformers` version: 4.56.0.dev0
- Platform: Linux-6.6.56+-x86_64-with-glibc2.35
- Python version: 3.11.13
- Huggingface_hub version: 0.34.4
- Safetensors version: 0.5.3
- Accelerate version: 1.8.1
- Accelerate config: 	not found
- DeepSpeed version: not installed
- PyTorch version (accelerator?): 2.8.0+cu128 (CUDA)
- Tensorflow version (GPU?): 2.18.0 (True)
- Flax version (CPU?/GPU?/TPU?): 0.10.6 (gpu)
- Jax version: 0.5.2
- JaxLib version: 0.5.1
- Using distributed or parallel set-up in script?: yes
- Using GPU in script?: yes
- GPU type: Tesla T4






### Who can help?

@SunMarc @MekkCyber (tagging as they are in the kernels pr for this model)

### Information

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

When I pass   output_router_logits=True to out=model(.., the out.router_logits is none.   I believe the issue is a combination of having kernels installed and being used and https://github.com/huggingface/transformers/blob/913c0a8c334afcbb42d8ee74567ff9ed0344b11d/src/transformers/models/gpt_oss/modeling_gpt_oss.py#L394C67-L394C68  

Any brief comments on the underlying technical solution to this problem would be greatly appreciated!  I have a workaround of sorts in  https://www.kaggle.com/code/kaggleqrdl/router-logits-visualization?scriptVersionId=255623848  and comments would be helpful given the tight timeline.  Also note the code for https://github.com/huggingface/transformers/blob/913c0a8c334afcbb42d8ee74567ff9ed0344b11d/src/transformers/models/gpt_oss/modeling_gpt_oss.py#L685 

I nulled that out as we're not training for this task.

### Expected behavior

out.router_logits should be populated via the OutputRecorder when kernels is used

Comments:
I've switched to using hooks, so less of an issue.

Sorry for the delay, and yes your hack works well, cc @SunMarc if you can check when you come back from vacations! 

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.
Add `logits_to_keep` to many older CausalLM models
Author: philiproeleveld
Pull request: yes

# What does this PR do?

Fixes #40984 

Adds `logits_to_keep` to many (older) `ForCausalLM` models that inherit from `GenerationMixin`.
Also consistently renames to `loss` and `logits`, and removes some code for float casting and mapping labels to the logits' device for models where that is already handled by the loss function (e.g. `gpt_neo`).


## 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?
- [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.
- [ ] 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. @Rocketknight1 

Comments:
I was wondering if I could also add this to the many seq2seq models that also inherit from `GenerationMixin`? T5 for example. In theory they would benefit when the user provides a very large `decoder_input_ids`, but that's not really what they're designed for...

I'd like some guidance on the failing test `GitModelTest::test_forward_with_logits_to_keep`. The problem is that test assumes sequence length of logits is equal to sequence length of input_ids, but that's not true for Git (because it concatenates image tokens). I see three potential fixes, but not sure what's preferable:
1. Copy the test to `GitModelTest` and modify
2. Add an exception to `GenerationTesterMixin`
3. Skip it (If so, what reason do I give?)

@philiproeleveld I'd just skip it in that case! For the reason you can say that the sequence lengths are not equivalent and so the test doesn't work.

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

**[For maintainers]** Suggested jobs to run (before merge)

run-slow: bart, bert, bert_generation, big_bird, bigbird_pegasus, biogpt, blenderbot, blenderbot_small, blip, bloom, camembert, chameleon, codegen, cpmant, ctrl, data2vec

Merging! Thanks a lot!
Adding `logits_to_keep` to older models
Author: philiproeleveld
Pull request: no
Labels: name: Good First Issue

Hi, while working with BioGPT I came across #39016 to not compute logits of the entire sequence when it's not needed. I was wondering if you'd be open to a PR applying the same improvement to BioGPT and, while I'm at it, any other `GenerationMixin` models that would benefit from this. I understand some of these models might be a bit obsolete and some could benefit from a refactor to modular instead, but I figure while they're there it's worth doing.

Comments:
Yes, I think we could support this, and it should be easy enough to do, following the pattern in the GPT PR or newer models that already support it. Feel free to ping me if anyone makes a PR for this!

Hi, I’d like to work on this issue as part of my first contribution.  
My plan is to:
- Add the `logits_to_keep` argument to the forward methods of the older models that currently don’t support it.
- Update the logits computation to slice accordingly.
- Add/extend unit tests to verify that `logits_to_keep` works as expected.

Please let me know if there are any specific models or guidelines I should follow before starting. 


yo i wanna take this up

@VIVEK-MARRI @Namit24 I hope it's not too late to tell you I've already made a start with this actually

Hi everyone, I’d like to contribute by implementing logits_to_keep for BioGPT first, following the GPT pattern from #39016. If no one is actively working on it, I can expand to other older models as well. Let me know if that works!


hey @Rocketknight1 opened a pr but had a few doubts, firstly i haven't updated the documentation yet, secondly i was only sure that the sequence classification model can allow me to add the slicing logits logic directly, i wanted to know if there was a way more me to test out these changes i make? i tried making a main.py at the root of the repository and doing relative imports and trying to run as a module but some reason the imports with `...` failing even though i ran it with the -m flag . if there is another way please let me know so ill make the changes to each of the models in BioGPT and try them out then add another commit to my current pr. Thanks :D

Hi, I’d like to take this up as a first contribution. Can I work on it?

uh @Rocketknight1 i think you missed my question above

Hi @0x-avi sorry, I didn't manually close this issue! It got automatically closed when we merged #41270, because that issue was tagged as fixing this issue.

I'll reopen for now, and if you want to make PRs like the `biogpt` PR for other older models, feel free. Thank you!

I've made a PR to add `logits_to_keep` to a bunch of `ForCausalLM` models (including the BioGPT one). Also FYI @0x-avi
remove ray_scope and check_quantized_param
Author: SunMarc
Pull request: yes

# What does this PR do?

This PR removes a few things that we don't need in v5. 

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

#41445 moved `load_sharded_checkpoint` to the trainer utils instead of the modeling utils. This PR changes the remaining references and (hopefully) fixes the CI on main with the doc builder

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

# What does this PR do?

This function has been moved to integrations but was not deleted properly

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

vLLM found that this import was causing CUDA to be initialised early

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

merge as the failing tests is irrelevant and fixed 

[41618](https://github.com/huggingface/transformers/pull/41618)
Update a dataset reop link
Author: ydshieh
Pull request: yes

# What does this PR do?

`https://huggingface.co/kirp/kosmos2_5/resolve/main/receipt_00008.png` is no longer available (likely the author deleted it).
That is used during the PR. Let's use the official repo. now

https://huggingface.co/microsoft/kosmos-2.5/resolve/main/receipt_00008.png



Comments:
**[For maintainers]** Suggested jobs to run (before merge)

run-slow: kosmos2_5

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_41618). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
[`from_pretrained`] Small refactor `from_pretrained`: move around unrelated stuff
Author: ArthurZucker
Pull request: yes

# What does this PR do?
As we are moving towards having a dynamic weight loader, we need to have a simple `from_pretrained` call. 
This should help

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

**[For maintainers]** Suggested jobs to run (before merge)

run-slow: fbgemm_fp8, finegrained_fp8

Can you really delete `_process_model_after_weight_loading`?
I'm getting `TypeError: Can't instantiate abstract class FPQuantHfQuantizer with abstract method _process_model_after_weight_loading` on fp_quant and AQLM tests now.

Oh, fixed by  #41613, I see.
Add __iter__ to DynamicCache
Author: remi-or
Pull request: yes

Currently, DDP is broken when there is a `DynamicCache` because it has no `__iter___` method and so it cannot be concatenated after the distributed forward. This PR adds back and `__iter__` and adapts the way ddp data is consumed to properly initialize sliding windows.

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

> indeed this [PR](https://github.com/huggingface/transformers/pull/41378/files#diff-357aea2fcf1b51b2a8ad17c7d30044159b1fe1a3f6bd832cced70aca84c8be66) broke a few things regarding distributed training.

I remember fixing it (for similar reasons as in this PR), but I don't remember the following: can we instead disable cache at training time? I know there are some less-common fine-tuning strategies that use caches (prefix tuning in PEFT), but it may be wiser to simply disallow cache+DDP

At inference time, if we want scale, we won't be using these classes anyways (we want continuous batching)

> but it may be wiser to simply disallow cache+DDP

Indeed, I think for v5 it is reasonable to do this change and most users won't be impacted by this change ! cc @BenjaminBossan 

> can we instead disable cache at training time?

What exactly would that entail? Would it mean that any PEFT method that uses `past_key_values` during training would stop working?

> What exactly would that entail? Would it mean that any PEFT method that uses past_key_values during training would stop working?

Most likely, I will add an arg in trainer to allow users to change `use_cache` and the default will be `False` unless we detect it is a peft model. 

lmk what you think of that : https://github.com/huggingface/transformers/pull/41585

Yes, I strongly agree with @gante here - I don't really see the point of supporting dp/ddp on the cache, as it's inference-only anyway. If we can avoid having those somewhat awkward `__init__`, it's a win in my opinion!

Oh, I just noticed that this PR breaks this line in PEFT:

https://github.com/huggingface/peft/blob/086f187a4d89be3fbfcc3629fd833c0878dde4c0/src/peft/peft_model.py#L796

There, we initialize a `DynamicCache` instance with `ddp_cache_data` but now the expected input data format has changed. Would it be a possibility to make passing `sliding_window_tensor` optional in the data format? Otherwise, I guess we'd have to do a version check on transformers in PEFT and then insert a `None` for `sliding_window_tensor` into the `ddp_cache_data`. But other code out there could possibly also be affected by this change, so a backwards compatible init would be better IMHO.

Hey @BenjaminBossan , I am adding support for `(key, value)` tuples instead of `(sliding, key, value)` in #41612 . Once merged, please let us know if it works!
[chat template] update when "push_to_hub"
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

As discussed internally, we don't have tests for chat templates loading-pushing to hub and a few bugs were encountered recently




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

@Rocketknight1 kind ping

It got stale a bit, so ping again @Rocketknight1 . Whenever you got some time 😄 

**[For maintainers]** Suggested jobs to run (before merge)

run-slow: auto
Remove the head masking block in some vision models
Author: ydshieh
Pull request: yes

# What does this PR do?

#36545 introduced a mistake regarding the headmask

before that PR, we had some

```
        # Mask heads if we want to
        if head_mask is not None:
            attention_probs = attention_probs * head_mask
```
but after the PR, it became

```
    # Mask heads if we want to
    if attention_mask is not None:
        attn_weights = attn_weights * attention_mask
```
which is apparently wrong.

Furthermore, we already remove the support of head masking.

This PRs simply remove the blocks.

Comments:
**[For maintainers]** Suggested jobs to run (before merge)

run-slow: audio_spectrogram_transformer, deit, dinov2, dinov2_with_registers, dinov3_vit, dpt, ijepa, videomae, vit, vit_mae, vit_msn, vitpose_backbone, vivit, vjepa2, yolos

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

# What does this PR do?

FIxes failing VJEPA test. The test was failing because the video was not permuted to the correct format that torchvision expects `(T, C, H, W)`

Comments:
run-slow: vjepa

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

> Do you know what it worked before you PR? (or your PR change something regarding this channel stuff?)

My PR stopped casting all inputs to a numpy format, so when a video is already a `torch.Tensor` it started failing. It's an assumption that channel format should not be adjuested if input is in torch


run-slow: vjepa



slow CI is not running anymore 🥲  Locally it's green, so I believe it's really fixed

Did my comment break up things? :eyes: 

Lol, no, looks like slow CI isn't being triggered since yesterday night. I've had similar issues in other PRs
Fixed : Bad error message for AutoTokenizer loading Voxtral #41553 
Author: i3hz
Pull request: yes

Fixes # 41553
Added a small check in both src/transformers/models/llama/tokenization_llama_fast.py and src/transformers/models/llama/tokenization_llama.py.




Who can review?

@ArthurZucker
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.

Comments:
This PR should probably not be deleting those CUDA kernels :sweat_smile: 

Yeah that's weird I noticed that too ,but I didn't really do anything that would cause that .
https://github.com/huggingface/transformers/pull/41575/commits/90af495b8c33360f92b3fd5f79aba50f6e34a82a
This seems to be the commit that removed the kernels

Let me try a rebase, one sec.

Okay, now that I can actually review the code, I'm even more confused! Why should the Llama tokenizers require `mistral-common`? They don't use it.

Yeah that's very confusing . But I assumed since they have added model_type : llama in the [config](https://huggingface.co/mistralai/Voxtral-Mini-3B-2507/blob/main/config.json) they probably use the same tokenizer . 

**[For maintainers]** Suggested jobs to run (before merge)

run-slow: llama

Soo what exactly should I look into rn? I'm just confused lol

I'll stop trying to resolve this and just get the people who actually know about tokenizers :sweat_smile:

cc @arthurzucker @itazap

Copy pasting a bit but let's coordinate https://github.com/huggingface/transformers/issues/41553#issuecomment-3406606156
Add fast path for bidirectional mask creation to fix regression
Author: i3hz
Pull request: yes

# What does this PR do?
This PR fixes the performance regression due to bidirectional masks.

Fixes # (issue): 41566

## 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?
- [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.
- [ ] 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?

@vasqu @Cyrilvallez 


Comments:
I think you performed a bad rebase or merge, so I just force pushed to restore the branch in correct state (I was pushing on it as well)

Thanks a lot for the PR! I will commit a bit more to make it slightly more robust then we can merge!

Thanks a lot idk why the qualify test keeps failing , even tho there is no blank space D: , but i'm also blind so i won't be surprised if i missed it 

@vasqu I cleaned the PR a bit and made it more general, could you have a last look before we merge? Also, we'll need to revert the changes you made in `executorch` integration, as it won't be needed anymore!

LGTM, just a bit confused why we need another slicing :thinking: let me open a follow up PR for executorch, keeping things a bit cleaner 

Merged, thanks a lot for the PR @i3hz!
Fix quantization base class 
Author: SunMarc
Pull request: yes

# What does this PR do?

This PR fixes issues that was created from this [PR](https://github.com/huggingface/transformers/pull/41445) regarding quantization. 

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

**[For maintainers]** Suggested jobs to run (before merge)

run-slow: bnb
[docs] Duplicate entry
Author: stevhliu
Pull request: yes

Fixes duplicate entry in the `toctree` that may be breaking the `custom-doc-build` job for translations (https://github.com/huggingface/transformers/actions/runs/18381182160/job/52725830409)

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_41591). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Add Deepseek-VL2
Author: ArthurZucker
Pull request: no
Labels: name: New model

### Model description

Deepseek-VL2: 
- [paper](https://arxiv.org/abs/2412.10302) 
- [code](https://github.com/deepseek-ai/DeepSeek-VL2)
- [weights](https://huggingface.co/collections/deepseek-ai/deepseek-vl2-675c22accc456d3beb4613ab)

### Open source status

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

### Provide useful links for the implementation

_No response_

Comments:
Prerequisite is Deepseek-V2: https://github.com/huggingface/transformers/issues/35317

Hi, I’ve been assigned to add support for this model.
I’ve started looking into it and will post updates here @ArthurZucker @jadechoghari

@mariebbz are you currently working on this PR?

> [@mariebbz](https://github.com/mariebbz) are you currently working on this PR?

Hey @ArthurZucker is this model required? I'd love to work on this if its possible

@Player256 I was still working on this, but I’d be happy to collaborate if you’d like. Do you use Discord?

Hey @mariebbz I use discord. You can find me in the hf's discord server under the username: best_space_assassin

Hey! as long as there are no PR linked to this issue, anyone is free to work on this! 🤗 

@ArthurZucker I am actually working on opening a draft PR for this issue shortly.

@Player256 Hi! Thank you for your excellent work! Are you still working on this? May I ask when this would be merged into main.