noamaanMulla-03/transformers-issues huggingface dataset:
Wait for main process in _save_checkpoint to ensure best checkpoint exists
Author: ssharpe42
Pull request: yes

# What does this PR do?

In 4.50.0 a bug was introduced with a refactor of the best checkpoint process when we try to load the best model at end and  a non-main process has a null `self.state.best_model_checkpoint`. 

Before we run `_load_best_model()` there is a barrier to make sure the main process saves the model, however inside `_save_checkpoint`, this is missing. The model is saved [here](https://github.com/huggingface/transformers/blob/0b057e66b52556da3a1cbc29e2a98c0784ea9c33/src/transformers/trainer.py#L3197) and then a couple lines later the `self.state.best_model_checkpoint` is set only if the `best_checkpoint_dir` exists. When another process gets to this line first, `self.state.best_model_checkpoint` will be null. 

Finally, when training reaches the end and it is time to load the best model [here](https://github.com/huggingface/transformers/blob/0b057e66b52556da3a1cbc29e2a98c0784ea9c33/src/transformers/trainer.py#L2666) only the main process enters the if statement and the dist.barrier causes it to wait forever for process 1 to arrive which doesn't have a best_model_checkpoint.

In 4.48.3 it didn't check for existence before assigning the best model checkpoint. They refactored how this works, I can't get it to fail on a non-cube example yet. https://github.com/huggingface/transformers/blob/298b3f19303294293f7af075609481d64cb13de3/src/transformers/trainer.py#L3190 (edited)

Note: I didn't use `self.accelerator.wait_for_everyone()` since it doesn't seem to cover the sagemaker case, but I could make a wait function in the trainer since it is used twice now. 

I believe this fixes #38008


## 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).
- [x] Did you write any new necessary tests?


## Who can review?
@zach-huggingface, @SunMarc and @qgallouedec


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

Should we rerun checks again? Trying to merge so we can get this in the next release.
Avoid assumption that model has config attribute in deepspeed
Author: tomaarsen
Pull request: yes

# What does this PR do?

This avoids the assumption that a model has a `config` attribute, which results in a cleaner error message for downstream usage of the Trainer, e.g. with Sentence Transformers. Specifically, it would help with https://github.com/UKPLab/sentence-transformers/issues/3531

## 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?

cc @SunMarc

- Tom Aarsen

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_41207). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Trainer: Pass `num_items_in_batch` to `compute_loss` in `prediction_step`
Author: pramodith
Pull request: yes

# What does this PR do?

Ensures that `num_items_in_batch` is passed to the `compute_loss` function in the `prediction_step` to ensure that loss is calculated the same way both at train and eval time.

<!-- Remove if not applicable -->

Fixes #41108 


## 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).
- [X] 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.

@SunMarc 

Comments:
I think the test case could be better open to hearing suggestions to add tests (for multi-gpu?) or modify the test.

Hmmm these tests are still failing: tests/trainer/test_trainer.py::TrainerIntegrationTest::test_evaluate_with_jit, can you quickly check why ? I guess the simplest solution would be to check if we have `self.args.jit_mode_eval` or not. Btw, we should probably deprecate this arg also

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

> Hmmm these tests are still failing: tests/trainer/test_trainer.py::TrainerIntegrationTest::test_evaluate_with_jit, can you quickly check why ? I guess the simplest solution would be to check if we have `self.args.jit_mode_eval` or not. Btw, we should probably deprecate this arg also

Will take a look in a few hours once I'm off work.

The issue was with `RegressionPretrainedModel's` signature not having `num_items_in_batch` as an argument, which leads to `jit` compile mode not knowing about this argument. I've fixed those test cases in the least intrusive way possible by setting `trainer.model_accepts_loss_kwargs=False`. A bit hacky, let me know if that's not okay and if we should pass an argument to `get_regression_trainer` instead.

For now, let's do that !
`predict_step` in Trainer should pass `num_items_in_batch`
Author: pramodith
Pull request: no
Labels: name: trainer

name: Feature request

### Feature request

`predict_step` in `Trainer.py` doesn't currently pass the `num_items_in_batch` to the `compute_loss` function. 
https://github.com/huggingface/transformers/blob/869735d37d0f929311ac6611728c482a4414ba8c/src/transformers/trainer.py#L4900

This seems to be misaligned because the `training_step` function does https://github.com/huggingface/transformers/blob/869735d37d0f929311ac6611728c482a4414ba8c/src/transformers/trainer.py#L4019



### Motivation

The Trainer's `training_step` function uses `get_batch_samples` to calculate the `num_items_in_batch` which is used to scale the loss.

However with the `predict_step` not passing this value, a user with a custom loss function has to account for `num_items_in_batch` being null at eval time but not train time, which is a bit confusing. Ensuring that both train and predict steps calculate `num_items_in_batch` the same way ensures accurate logging and comparison of loss metrics.

### Your contribution

I'm happy to submit a PR for this unless there's a strong reason as to why the `predict_step` shouldn't be passing the `num_items_in_batch` to `compute_loss`

Comments:
Cc @SunMarc 

Hey @pramodith, thanks for this issue. Happy to add `num_item_per_batch` into `predict` method. I think this wasn't done in the past as we first added it for fixing the grad accumulation issue and this is not needed for the evaluation step. But then we updated it to also take into account DP (`average_tokens_across_devices`). Feel free to submit a PR to test that. Also, it would be nice to have a small test to check that we indeed get the same loss during training vs evals. 

Sounds good! I'll have something out in the next few days.
Make quantizers good citizens loading-wise
Author: Cyrilvallez
Pull request: yes

# What does this PR do?

The quantizers should NEVER need to access the `state_dict` during `param_needs_quantization` or `create_quantized_param`. This is mostly for the following reasons:

- Most models have a lot of different state dict (due to restricting the size of each file to 5GB), so relying on other params of the state dict for quantizing a given param is a very bad idea -> if the needed params happen to live on different saved state dicts (representing the same model), then the whole loading logic collapses without any possibility of fallback
- Without quantizers, we load parameters one by one without ever materializing the whole state dict at once, and it's quite annoying to have to change that logic only for a few quantizers, as it's a good thing in general

If the quantizers need several serialized keys to retrieve a given quantized param, then they should cache them individually, and then quantize back once all the needed keys are retrieved. Note that this DOES NOT incur a memory cost, as previously the whole state dict was loaded on memory, so the memory footprint was always higher before (-> a param will necessarily get quantized back, and its serialized tensors cleaned, before the whole state dict is materialized, assuming they all reside on the same state_dict as was done previously). So we actually have a gain of memory during loading with this PR.

Note that the current PR has less slow tests failing than main, especially for bitsandbytes, i.e. it actually solves a few issues in addition to simplifying the whole loading logic of the library.

A following PR will come to unify the `update_expected_keys`, `update_missing_keys`, and `update_unexpected_keys` methods, as some are currently redundant/not practical regarding the whole loading logic.

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

run-slow: bnb

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

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

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

run-slow: fbgemm_fp8, finegrained_fp8, higgs, hqq, mxfp4
feat: support cwm modeling
Author: xgal
Pull request: yes

# What does this PR do?

<!--
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.
-->

<!-- Remove if not applicable -->

Support CWM modeling https://huggingface.co/facebook/cwm/tree/main



## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker
- vision models: @amyeroberts, @qubvel
- speech models: @eustlb
- graph models: @clefourrier

Library:

- flax: @gante and @Rocketknight1
- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- pipelines: @Rocketknight1
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @zach-huggingface, @SunMarc and @qgallouedec
- chat templates: @Rocketknight1

Integrations:

- deepspeed: HF Trainer/Accelerate: @SunMarc @zach-huggingface
- 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: @Rocketknight1
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->


Comments:
cc @arthurzucker for text models

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

run-slow: auto, cwm

Superseded by https://github.com/huggingface/transformers/pull/41199
haggingface model unsupported torch backward hook
Author: rubbberrabbit
Pull request: no
Labels: name: bug

I am use huggingface transformer and pytoch, I am trying to add some hooks in the state of model backward computation, it works well in original torch module. But with huggingface model I got

UserWarning: For backward hooks to be called, module output should be a Tensor or a tuple of Tensors but received <class 'transformers.modeling_outputs.BaseModelOutputWithPoolingAndCrossAttentions'>

### System Info

UserWarning: For backward hooks to be called, module output should be a Tensor or a tuple of Tensors but received <class 'transformers.modeling_outputs.BaseModelOutputWithPoolingAndCrossAttentions'>



### Reproduction

```
    def pre_backward_time_hook(self, module, grad_output):
        # do something like print the grad_output
        print(grad_output)
        return None

    model_name = "bert-base-uncased"
    tokenizer = BertTokenizer.from_pretrained(model_name)
    bert_model = BertModel.from_pretrained(model_name)
    bert_model.register_full_backward_hook(hook_fn)
    input_text = "Hello, how are you?"
    inputs = tokenizer(input_text, return_tensors="pt") 
    outputs = bert_model(**inputs)  
    print(type(outputs.last_hidden_state))
    loss = outputs.last_hidden_state.mean()  
    loss.backward() 
```

### Expected behavior

hook run well, like geting output tensor and printing it

Comments:
hi @rubbberrabbit, unfortunately there's not much we can do to fix this! Our models always return structured output classes that inherit from `OrderedDict` rather than single tensors or tuples. Maybe you can wrap the HF model in your own model that returns a single tensor, and then add the backward hook to that?

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.

Can please suppress the `For backward hooks to be called, module output should be a Tensor or a tuple` warning?
[tests] `CausalLMTester` automatically infers other test classes from `base_model_class` 🐛 🔫 
Author: gante
Pull request: yes

# What does this PR do?

In this PR, I aim to reduce human error using `CausalLMTester`, fixing bugs in the process 🐛 🔫 

Issues tackled:
1. Tester attributes
    a. Before: tester class attributes where we set model classes were not validated -> we could set attributes with typos (e.g. `sequence_class = ...` instead of the expected `sequence_classification_class = ...`) -> silently skip tests
    b. Now: only need set `base_model_class`, others are inferred. Typos in the other attributes are also detected, when we explicitly set them.
    c. ⚠️ this change identified a LOT of places where we had typos ( = untested classes) ⚠️ 
2. `question_answering_class`
    a. Before: no corresponding test
    b. Now: generic test for this class
3. `all_model_classes`
    a. Before: redundant with the tester class attributes where we set model classes -> easy to have mismatched information in these two attributes that should hold the same classes
    b. Now: `all_model_classes` is only overwritten where it needs to be overwritten (i.e. in classes where `CausalLMTester`'s model tester attributes are not complete enough to contain all model classes). There is a single source of truth for most models. 
    c. ⚠️ in some models, this was hidding incomplete checks (e.g. missing pipeline map) because the model was relying on a custom `setUp` method ⚠️ 
4. Model-specific test issues (gemma family)

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

@Cyrilvallez re-requesting review due to major changes :)

I took your comment about better class checks, and went further in removing humans out of the way 🔪 

We now only need to set `base_model_class`. The other classes are directly imported from its module if they exist, follow standard naming conventions, and are unset by the tester. If we decide to manually set the other classes, that's also allowed. Our repo-wide checks still raise errors if public classes are untested.

(a few more tester errors were found in the process 🙄 )

________________________

I'll be off until next Tue, feel free to merge the PR if you approve it 🤗 

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

run-slow: apertus, arcee, blt, dbrx, deepseek_v2, dots1, ernie4_5, ernie4_5_moe, exaone4, falcon, flex_olmo, gemma, gemma2, gemma3, gemma3n, glm
[ESM] add accepts_loss_kwargs=False to EsmPreTrainedModel
Author: pstjohn
Pull request: yes

#40370 added a `**kwargs` to the ESM-2 model's forward pass, which means this model now triggers [these lines](https://github.com/huggingface/transformers/blob/6d2bb1e04db6c8d193549d4b0c99d2182837c0ad/src/transformers/trainer.py#L4127-L4132) in the HF trainer, making losses inconsistent between single and multi-device training.
```python
        if (
            self.args.average_tokens_across_devices
            and (self.model_accepts_loss_kwargs or self.compute_loss_func)
            and num_items_in_batch is not None
        ):
            loss *= self.accelerator.num_processes
```

We can avoid this by setting the `accepts_loss_kwargs` flag to False in the model definition.
CC @zucchini-nlp, @Rocketknight1 


Comments:
@pstjohn can you also run `make fix-copies` for CI checks?

@bot /style

maybe it runs fix copies 🤔 

Style fix runs successfully without any file modified.

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

run-slow: esm

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_41006). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Fix Gemma RMSNorm weight init
Author: albertz
Pull request: yes

# What does this PR do?

Gemma RMSNorm weight is used additively in `...*(1+weight)`,
thus it should be initialized with zero.

Fixes #40224


## Who can review?

@ArthurZucker


Comments:
Should this be here instead of in the Gemma modeling files? Leaving it to @ArthurZucker 

> Should this be here instead of in the Gemma modeling files? Leaving it to @ArthurZucker

I asked exactly about that in #40224 and @ArthurZucker suggested to do it that way. 


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

run-slow: gemma

I updated the code for Gemma, i.e. `GemmaModel` in `.../gemma/modular_gemma.py`.

I did not update the code in `.../gemma/modular_gemma.py` as this is automatically generated? How would I update this?

Similarly, I also did not update `Gemma2Model`, as this derives from `GemmaModel`, and should already be correct? Or not?

Similarly, I also did not update `Gemma3TextModel`.

As far as I understand the code, I don't need to set `module._is_hf_initialized = True`, as the `_initialize_weights` method already does this?


I just noticed that the changes I proposed have already been done in the meantime?

#40796 (https://github.com/huggingface/transformers/commit/b4ba4e1da0c77710833a2a9ba86f729b42098e46) by @vasqu.

I guess this PR here can be closed then.


Sorry about that @albertz, didnt know that there was already a different PR for this :/ got to it due to a different issue I noticed

Thanks nonetheless for the PR 🤗
Align pull request template to bug report template
Author: tomaarsen
Pull request: yes

# What does this PR do?

Align pull request template to bug report template: https://github.com/huggingface/transformers/blob/main/.github/ISSUE_TEMPLATE/bug-report.yml?plain=1
Follow-up of #40881 by @ArthurZucker 
The only difference is that I don't refer users to https://discuss.huggingface.co/ for hub issues, as the PR template isn't for issues.

## 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?

@ArthurZucker @LysandreJik 

- Tom Aarsen


Comments:
wow,  we are thinking the same thing

https://github.com/huggingface/transformers/pull/41221

at the same time

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