noamaanMulla-03/transformers-issues huggingface dataset:
Deprecate quanto and switch to optimum-quanto
Author: MekkCyber
Pull request: yes

# What does this PR do?
Deprecates the use of `is_quanto_available` after switching to `optimum-quanto` package instead

## Who can review ?
@SunMarc 

Comments:
To merge a bit before the release 

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

can you resolve the conflicts please?
BLIP: this is correct now
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

Follows the prev PR https://github.com/huggingface/transformers/pull/34876, as the prev one was appending image tokens after BOS. It should be the other way around. Sorry, the last commit to increase readability messed up concatenation order

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35081). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Mismatched keyword argument names of llama make GA fix invalid
Author: hiyouga
Pull request: no
Labels: name: trainer

name: bug

### System Info

- Transformers 4.47.0.dev0 (latest commit 33868a057c02f0368ba63bd1edb746be38fe3d90)

### Who can help?

@ArthurZucker @muellerzr 

### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [X] My own task or dataset (give details below)

### Reproduction

https://github.com/huggingface/transformers/pull/33932 may breaks the logic for the trainer's `model_accepts_loss_kwargs`. The llama model would not receive a `num_items_in_batch` argument, making the fix of https://github.com/huggingface/transformers/pull/34283 invalid again

https://github.com/huggingface/transformers/blob/33868a057c02f0368ba63bd1edb746be38fe3d90/src/transformers/trainer.py#L605

Moreover, the names of keyword arguments are different for llama and other models, we might expect the same keyword arguments for different models.

https://github.com/huggingface/transformers/blob/33868a057c02f0368ba63bd1edb746be38fe3d90/src/transformers/models/llama/modeling_llama.py#L1146-L1161

https://github.com/huggingface/transformers/blob/33868a057c02f0368ba63bd1edb746be38fe3d90/src/transformers/models/gemma/modeling_gemma.py#L1015-L1030


### Expected behavior

The models' forward functions should have a consistent keyword argument list.

Comments:
cc @muellerzr I think when reviewing I mentioned something about checking the type rather than names (like start kwarg type) would easily fix this!
Fix flaky Hub CI (`test_trainer.py`)
Author: ydshieh
Pull request: yes

# What does this PR do?

The goal is to avoid race condition in Hub repo. related tests. Based on #31973, this PR:

- remove `_try_delete_repo` and `try ... except ...` in many files
- create `TemporaryHubRepo` in `testing_utils.py` (similar to `tempfile.TemporaryDirectory`)
- apply the same fix to `test_trainer.py` (which is currently flaky)


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

Thank you @Wauplin Answering a few questions here:

> Oh, I think I misunderstood the use case here then 😕 What's the goal of TemporaryHubRepo then?

Yes, it's a bit confusing. Actually so far it just prepares a `repo_id` to be used in the test, for example, `push_to_hub(repo_id)`. Of course we can create the repo. in `__init__` or `__enter__` (which seems more nature and make more sense!).

> (nit) be careful that this is a repo_name and not a repo_id per-se since it doesn't contain the namespace. 

so far both can be passed without problem (in the test, it's the actual repo_id with `/` passed). But yeah, you are right, it makes the API unnecessary more complicated.

> Also since self.random_id can be None, then the repo_name might not be unique.

In this case I will prepare a random id (just not implemented yet so far, sorry for confusion)

> What I would advice you to do is to remove both random_id and prefix from this helper to simplify things as much as possible for the users.
- For `repo_id` argument: I was trying to keep `f"{USER}/test-generation-config` stuff (already in `main` branch)
  - (kind have its value of keeping information)
- For `random_id` argument: to allow passing the id created from `tempfile.TemporaryDirectory`, so there is a correspondence between the local dir and hub repo.

But yes, none of these 2 are really necessary.

Updated as suggested. @Wauplin Could you take another look and let me know how you think? Especially regarding the arguments for __init__. Thanks
The document of generation seems to wrongly describe the default value of top_p, top_k and temperature
Author: Kamichanw
Pull request: no
Labels: name: bug

### System Info

The newest transformers

### Who can help?

@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

The [document](https://huggingface.co/docs/transformers/v4.46.3/en/main_classes/text_generation#transformers.GenerationConfig.top_p) of `generation` says that `top_p`, `top_k` and `temperature` will be set to a specific value.

However, as shown below, the settings of `top_p`, `top_k`, and `temperature` depend on the generation configuration of the model, which is not guaranteed to be consistent with the behavior described in the docstring.
https://github.com/huggingface/transformers/blob/052e652d6d53c2b26ffde87e039b723949a53493/src/transformers/generation/utils.py#L1035-L1044

### Expected behavior

I'd like to know if I'm missing something, and if not, please make the docstring of "generation" and its behavior consistent.

Comments:
Thanks for the feedback! I think we can clarify the docstrings of these parameters to reflect what you're saying. Something along the lines of:

```md
temperature (`float`, *optional*, defaults to 1.0) - The value used to module the next token probabilities. This value is set in a model's `generation_config.json` file. If it isn't set, the default value is 1.0.
```

Yes, that's make sense
Make it possible to save and evaluate checkpoint on CTRL+C / `KeyboardInterrupt` with Hugging Face Trainer
Author: umarbutler
Pull request: no
Labels: name: Feature request

### Feature request

I would like to request that one or more optional flags be added to the Hugging Face Trainer, perhaps named `save_on_exit`/`save_on_interrupt` and `eval_on_exit`/`eval_on_interrupt`, to ensure that a checkpoint is always saved upon CTRL+C or perhaps even a kill command sent via `wandb`.

### Motivation

Extremely often I will be training a model but then need to utilise the GPU on which I am conducting the training and so I need to pause my training. For example, over the past month, I have been training models on my PC but often I have needed to use my GPU and so I have exited my training and then resumed it at night.

Because, at present, it is not possible to have the Hugging Face Trainer save a checkpoint upon recieving a `KeyboardInterrupt`, I end up needing to save checkpoints at excessively short intervals to minimise lost progress if I quit training at any arbitrary point. This invariably still ends up with some amount of progress being lost and it also does a lot of write wear on my SSDs, which, like all hard drives, have a limited write lifetime. The wear can in fact add up to quite a lot of writing if you are saving multigigabyte models.

By allowing for progress to automatically save upon exit, I can be assured that, barring an unexpected system crash, or repeated CTRL+Cs being sent, my progress will always be saved and so I do not need to save and evaluate checkpoints so frequently.

### Your contribution

N/A

Comments:
cc @muellerzr @SunMarc 

Temporary solution:

You could try catching your keyboard interrupts or other program crashes and save your model manually with a call to save_pretrained() or any other method based on your needs before your program exits.
[`trainer`] fix the GA `model_accepts_loss_kwargs`
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Fixes #34577
`model_accepts_loss_kwargs` was wrongly looking at kwarg names, while you should only need kwargs (since the name can vary for FlashAttentionKwargs, LossKwargs etc)

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

I think this pr introduced a new bug that if user use user defined loss funcion and `model_accepts_loss_kwargs` is `False`, `compute_loss` function cannot get the `num_items_in_batch` argument. Finally, user defined compute_loss_func will not receive this argument either.

`model_accepts_loss_kwargs` is supposed to be determined at init time and only depends on the forward pass of the model

I understand now, I misinterpreted the if condition earlier. However, in this PR, when model_accepts_loss_kwargs is True, it won't pass the num_items_in_batch parameter, which would make the GA loss modification functionality ineffective. Why is that?

In newest code, run
```
export RUN_SLOW=True
pytest tests/trainer/test_trainer.py::TrainerIntegrationPrerunTest::test_gradient_accumulation_loss_alignment
```
will cause error:
```
======================================================= short test summary info =======================================================
FAILED tests/trainer/test_trainer.py::TrainerIntegrationPrerunTest::test_gradient_accumulation_loss_alignment - AssertionError: 0.9038000000000004 not less than 0.1 : Difference -0.9038000000000004 is not within 0.1
============================================== 1 failed, 2 warnings in 102.43s (0:01:42) ==============================================
```

Ah shit the if condition is reversed 

Opened a PR for a fix, thanks!
Fix private forked repo. CI
Author: ydshieh
Pull request: yes

# What does this PR do?

We need the token `CIRCLE_TOKEN` to be used in the generated CircleCI config file.

Comments:
cc @molbap FYI

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35114). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Past Keys Output now working with output router logits
Author: csking101
Pull request: yes

# What does this PR do?

This handles the case where output_router_logits are True, and past key values are being used for inference, in the Mistral model. 

Fixes #30731 


## 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?
- [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?

@ArthurZucker 


Comments:
Hi @ArthurZucker , could you give me a pointer as to how to go about it, as I think I confused myself with the approach. The main reason why I did this was, because I was unsure about what to do with the missing attention for the previous outputs, when running the model in inference mode.
[Query] Preprocessing Tutorial
Author: ariG23498
Pull request: no

https://github.com/huggingface/transformers/blob/98e8062df3cf82b367e8a243963e4afb0d9d3407/docs/source/en/preprocessing.md?plain=1#L394

I think there should be a `min()` between the height and the width here to get the shortest edge.


Comments:
I think image processors can specify either a `shortest edge` or separate `height` and `width` for the desired image size, so I think keeping the 2-tuple rather than the `min()` is correct
fix check for loss_kwargs/GA
Author: winglian
Pull request: yes

# What does this PR do?

https://github.com/huggingface/transformers/pull/34915 didn't quite do what was intended and seems to break packing. Since `loss_kwargs` was replaced with `num_items_in_batch`, we should probably be checking for that. This fixes tests on our end that runs a fine-tune with packing  and validates the loss. Without this fix, the loss is ~3x larger than it should be with packing.

Fixes # (issue)


## 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.

<!-- 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:
I effed this one up
logic was inverted for passing loss_kwargs to forward pass
Author: winglian
Pull request: yes

# What does this PR do?
Bug introduced in https://github.com/huggingface/transformers/pull/34915 inverted the loss kwargs logic.

Fixes # (issue)


## 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.

<!-- 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:
num_quantizer in EncodecConfig should accept variable codebook size
Author: mindmapper15
Pull request: no
Labels: name: bug

### System Info

transformers == 4.46.1

### Who can help?

_No response_

### Information

- [ ] The official example scripts
- [X] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [X] My own task or dataset (give details below)

### Reproduction

I pretrained custom EnCodec model of my own and trying to convert it to huggingface model format
I used a different codebook size in my custom EnCodec model such as 4096 or 8192 (the default is 1024)

Here is the problem :
The user can change the codebook size in EncodecConfig by passing the argument.
When the num_quantizer is calculated however, the codebook size is fixed to default value 1024.

https://github.com/huggingface/transformers/blob/bc598c00db37d1fbb1551723873d37e238c3ede7/src/transformers/models/encodec/configuration_encodec.py#L187-L189

Here, 10 multiplied with self.frame_rate, which stands for the number of bit consumed by the codebook size, is fixed.

### Expected behavior

num_quantizer should accept variable codebook size since we can already change the codebook size in argument.
Here is the modified code based on official implementation.

```
    @property
    def num_quantizers(self) -> int:
        return int(max(1, math.floor(self.target_bandwidths[-1] * 1000 / self.frame_rate *  math.log2(self.codebook_size))))
```

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.
DDP error with load_best_model_at_end enabled
Author: zhiyuanhhh
Pull request: no

### System Info


- `transformers` version: 4.40.1
- Platform: Linux-5.10.214-202.855.amzn2.x86_64-x86_64-with-glibc2.35
- Python version: 3.10.14
- Huggingface_hub version: 0.23.0
- Safetensors version: 0.4.3
- Accelerate version: 0.29.3
- Accelerate config: #011not found
- PyTorch version (GPU?): 2.3.0 (True)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: yes
- Using distributed or parallel set-up in script?: ddp



### Who can help?

@muellerzr and @pacman100 

### Information

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

### Tasks

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

### Reproduction

1. Use DDP to trigger the training script
`torchrun --standalone --nnodes=1 --nproc-per-node=$NUM_GPUS train.py --config /opt/ml/input/config/hyperparameters.json`
2. In trainer argument set load_best_model_at_end to true
3. At the end of the script, all GPUs except the rank 0 emmit the following error

```
RuntimeError: DDP expects same model across all ranks, but Rank 5 has 128 params, while rank 0 has inconsistent 1506656875 params.
    return dist._verify_params_across_processes(process_group, tensors, logger)
    return dist._verify_params_across_processes(process_group, tensors, logger)RuntimeError
``` 

### Expected behavior

No Error occur.

Comments:
cc @muellerzr @SunMarc 

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.

Hey @zhiyuanhhh, could you share the training script as well as the config that you are using. Otherwise, it is a bit hard for us to fix it ! Thanks and sorry for the delay 

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.
[WIP] Add flex attention for qwen2
Author: MekkCyber
Pull request: yes

# What does this PR do?
Adding `flex_attention` for Qwen2 models following https://github.com/huggingface/transformers/issues/34809

## Who can review?




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

we need to make sure  CIs are green (make fixup etc) 🤗 do you need help on that ?

I just didn't find time to finish it, I will finish it as fast as possible ! thanks 🤗