noamaanMulla-03/transformers-issues huggingface dataset:
[DOCS] add docstrings  to TypicalLogitsWarper
Author: akshayamadhuri
Pull request: yes

# What does this PR do?
Added some doc string to TypicalLogitsWarper with some examples as well.
@gante let me know if there's anything else  should be add or remove from the docs.


Fixes #24783 


## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).





Comments:
@akshayamadhuri you probably need to run `make fixup` on your terminal and then commit the changes to make our CI happy :D
Delete all checkpoints when set save_total_limit=1
Author: Pbihao
Pull request: no

### System Info

- `transformers` version: 4.31.0
- Platform: Linux-3.10.0-1160.el7.x86_64-x86_64-with-glibc2.27
- Python version: 3.11.4
- Huggingface_hub version: 0.16.4
- Safetensors version: 0.3.1
- Accelerate version: 0.21.0
- Accelerate config:    not found
- PyTorch version (GPU?): 2.0.1 (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?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>

### Who can help?

@sgugger

### 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

1. Set the `save_total_limit` in `TrainingArguments` as 1 
2. Set `output_dir` in `TrainingArguments`
3. Run `trainer.train()`
4. No checkpoint folder was saved (all are deleted)

I have found the reason and fix the bug, just report this bug.
The reason is that in [trainer](https://github.com/huggingface/transformers/blob/main/src/transformers/trainer.py#L1963):
```
if self.args.should_save and self.state.best_model_checkpoint is not None and self.args.save_total_limit == 1:
            for checkpoint in checkpoints_sorted:
                if checkpoint != self.state.best_model_checkpoint:
                    logger.info(f"Deleting older checkpoint [{checkpoint}] due to args.save_total_limit")
                    shutil.rmtree(checkpoint)
```
This line:
```
if checkpoint != self.state.best_model_checkpoint:
```
Directly compare two paths as strings and ignore the format of the path.

For example, in my case, when:
* checkpoint == 'outputs/' 
and
* self.state.best_model_checkpoint == './outputs',

this comparison returns `False` and all checkpoints are deleted even including the best model.

I fix this bug by changing the above line to:
```
if str(Path(checkpoint)) != str(Path(self.state.best_model_checkpoint)):
```


### Expected behavior

The best model is saved and other checkpoints are deleted.

Comments:
Hi @Pbihao ! 

Thank you a lot of reporting this issue. I can confirm it.

Would you like to open a PR to help us fixing this 🤗 ? 

Yeah, I have submitted the PR #25136 .

Many thanks.
fix delete all checkpoints when save_total_limit is set to 1
Author: Pbihao
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 -->

Fixes #25129 (issue)

Fix the bug that all checkpoints are deleted when set `save_total_limit ` in `TrainingArguments ` as 1.

More details can refer to  #25129 .

## Who can review?

@ydshieh @sgugger

<!-- 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 and @younesbelkada
- vision models: @amyeroberts
- speech models: @sanchit-gandhi
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @gante
- pipelines: @Narsil
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @sgugger

Integrations:

- deepspeed: HF Trainer/Accelerate: @pacman100
- ray/raytune: @richardliaw, @amogkam

Documentation: @sgugger, @stevhliu and @MKhalusova

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: @sgugger
- TensorFlow: @Rocketknight1

 -->


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_25136). All of your documentation changes will be reflected on that endpoint.
Clarify 4/8 bit loading log message
Author: BramVanroy
Pull request: yes

If you enable 4-bit loading, you will get a message that the model is being loaded in 8-bit. This can be a tad confusing. This tiny PR simply distinguishes in the logging between 4-bit and 8-bit loading.

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

cc @younesbelkada and @SunMarc

Thanks!
🚨🚨🚨Change default from `adamw_hf` to `adamw_torch` 🚨🚨🚨
Author: muellerzr
Pull request: yes

# What does this PR do?

This PR changes the default from `adamw_hf` to `adamw_torch`, as noted in https://github.com/huggingface/transformers/issues/25006 which fixes some breaking issues. Note that https://github.com/huggingface/transformers/issues/22141 still needs to be fulfilled once torch 2.1.0 is released (sometime in the next few months I imagine, as we're on 2.0.1) and swap it to be `ADAMW_TORCH_FUSED`. 

Fixes # (issue)

Solves #25006 

## Maintaining old behavior

To keep the old behavior prior to this change, ensure that you pass `"adamw_hf"` as the `optim` in your `TrainingArguments`


## 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#start-contributing-pull-requests),
      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.

@sgugger @stas00 


Comments:
_The documentation is not available anymore as the PR was closed or merged._
[`T5/LlamaTokenizer`] default legacy to `None` to not always warn
Author: ArthurZucker
Pull request: yes

# What does this PR do?
In a follow up to the patch that introduce the `legacy` argument #24622, this makes sure people are warn if the `legacy` is not set. Since online models were not changed, this does not really change much! 

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

Excellent - thank you for improving this feature, Arthur!
Using Trainer with custom model caused dimension error
Author: zhangyilun
Pull request: no

### System Info

```
- `transformers` version: 4.31.0
- Platform: Windows-10-10.0.19045-SP0
- Python version: 3.11.4
- Huggingface_hub version: 0.16.4
- Safetensors version: 0.3.1
- Accelerate version: 0.21.0
- Accelerate config:    not found
- PyTorch version (GPU?): 2.0.1+cu118 (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?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>
```

Not sure if it's the intended way of using the Trainer clas, but what I did was:
- Created a custom image+text classifier where the image encoder and text encoders are huggingface models (i.e., Bert, ViT) and I extracted the last hidden state from each embedding, concatenated them, and added a linear layer for binary classification.
- I modified the `compute_loss` function by subclassing the Trainer class but didn't do anything else.

The issue was, since the model class only outputs logits but nothing else (not dict, not tuple), this part of the code: https://github.com/huggingface/transformers/blob/main/src/transformers/trainer.py#L3344-L3347 where since the output from model forward isn't a dict, it assumed it's a tuple, and trimmed the first sample's logits in the batch causing dimension error when computing the loss since labels and logits dimensions don't match (off by # of batches).

This is probably solvable by modifying the model class to return what the trainer is expecting, but it's not communicated clearly either, maybe it's because the Trainer isn't fully suitable for custom model training?

I have created a commit in my branch to fix this on my side so that the training can continue: https://github.com/huggingface/transformers/compare/main...zhangyilun:transformers:allow-logits-only-outputs. Not sure if it's worth merging into the repo. I think the change shouldn't break anything else.

If you think I'm doing things wrong or I missed anything, please correct me!

### 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

Create a custom binary classification model where the forward method only returns the logits.
Use the Trainer class for training.

### Expected behavior

Dimension mismatch in compute_loss method.

Comments:
cc our trainer master @sgugger , but I think the best is to follow the standard output format (i.e. either tuple or dict)

This is communicated clearly on the [Trainer doc page](https://huggingface.co/docs/transformers/main_classes/trainer), scroll a bit to the big warning.

Thank you for pointing me to the doc!
[`MptConfig`] support from pretrained args
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Adds a setter for `attn_config` to allow passing dict when initializing for backward compatibility 
Fixes https://github.com/huggingface/transformers/issues/25114 

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

cc @sgugger
Add new model in doc table of content
Author: sgugger
Pull request: yes

# What does this PR do?

As requested by @stas00 , this PR makes sure that the `add-new-model-like` command adds the model in the doc table of content. Since we are using another model as a reference, we can simply add it to the same section as that base model.

Comments:
_The documentation is not available anymore as the PR was closed or merged._
[`IDEFICS`] Fix idefics config refactor
Author: younesbelkada
Pull request: yes

# What does this PR do?

Refactors the `IdeficsConfig` to match the configuration composition patterns of multimodal models on transformers

original PR: https://github.com/huggingface/transformers/pull/24796 

Summary of the changes

- Removed the copy of `CLIPTextConfig`, `CLIPConfig` in `clip.py` as they were used for type hints only
- Retrieve the correct attributes on `modeling_idefics.py` (i.e. attributes from `perceiver_config` & `vision_config`
- Adapted CI tests accordingly
- Make the `utils/check_config_attributes.py` pass - since there is a duplicated CLIPVisionConfig (1 in the clip itself and the other in `configuration_idefics.py`), that script checks the unused attributes of that config for some reason (didn't 
investigated further)

For compatiblity with weights on the Hub, changes similar than: https://huggingface.co/HuggingFaceM4/tiny-random-idefics/discussions/3 needs to be applied

The docstring of the new config objects needs to be cleaned up, but can be done on the main PR.

cc @stas00

Comments:
_The documentation is not available anymore as the PR was closed or merged._
[WIP] Add state in segments id calculation
Author: amyeroberts
Pull request: yes
Labels: name: WIP

# What does this PR do?

At the moment, when segments are calculated it's calculated on an image-per-image basis. This means  when predicting with certain models e.g. DETR, the segment id that each class corresponds can be different across each image in a batch and across batches. 

This PR adds a private attribute to the image processor class to store the class: to segment_id mapping as state. 

/!\ There is a possible breaking change, as `compute_segments` now returns 3 rather than two objects. 

Fixes #23461

## 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#start-contributing-pull-requests),
      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:
_The documentation is not available anymore as the PR was closed or merged._
`Pipeline.forward()` possibility to place `model_outputs` on GPU
Author: gugarosa
Pull request: no

### Feature request

In `transformers.pipelines.base.py` (line 1035):

`model_outputs = self._ensure_tensor_on_device(model_outputs, device=torch.device("cpu"))`

Is it possible to add a new argument that decides whether `model_outputs` should be placed on `self.device` instead of `torch.device("cpu")`?

### Motivation

The variable `model_outputs` is always placed on CPU, which can cause a slowdown if I perform additional operations on the `Pipeline.postprocess()` function.

For example, if I were to pass `logits` to `model_outputs`, the whole tensor would be transferred from GPU to CPU. If I do this extensively, I will face a severe slowdown in my pipeline.

### Your contribution

Right now, I have to override the method to remove that particular line, but I can submit a PR.

Comments:
`pipeline` is only a wrapper around the model and preprocessing class for quick demos. To customize things more to your needs, you should use those classes independently as you need :-)

Thanks for the feedback @sgugger! I will keep customizing it then 😄
Unexpected GPU requests during training
Author: rangehow
Pull request: no

### System Info

- `transformers` version: 4.29.2
- Platform: Linux-5.19.0-46-generic-x86_64-with-glibc2.31
- Python version: 3.11.4
- Huggingface_hub version: 0.15.1
- Safetensors version: not installed
- PyTorch version (GPU?): 2.0.1 (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?: No


**When CUDA_VISIBLE_DEVICES=1 is specified, the program suddenly applies for calling GPU:0 memory after executing thousands of steps.**

I executed the command below on a two-card 3090 server, and when I ran the program, GPU:0 card was already occupied.
`CUDA_VISIBLE_DEVICES=1 nohup python train.py &`
In the beginning, everything seemed to be normal, because I have done more than 10,000 steps
![image](https://github.com/huggingface/transformers/assets/88258534/90fa40f3-d34d-4f14-8a33-94614cbce155)
However, very suddenly, the program starts to apply for the memory of GPU0 during eval stage（although it has go through a lot of eval stage before）
![image](https://github.com/huggingface/transformers/assets/88258534/79d16b30-92d7-4aac-bc22-54e9c450f65b)
The error entry of the program is displayed as model.generate from the error report
![image](https://github.com/huggingface/transformers/assets/88258534/901567df-5672-493b-9486-0b3077049e60)


this is a related document retirval code run on t5-large,not sure the rest of code will grant help or not.
![image](https://github.com/huggingface/transformers/assets/88258534/2133aae7-d92a-474d-83ec-265845f503ca)
![image](https://github.com/huggingface/transformers/assets/88258534/4dbec245-d923-449e-b5e4-7ade02f54f11)
![image](https://github.com/huggingface/transformers/assets/88258534/7cf39de4-dd3d-4699-9e7b-0b57cea7c17a)


### Who can help?

@gan @ArthurZucker @younesbelkada  Since it's a nlp project and happened in generation stage, so maybe I need help from you sincerely.

### 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

It's a public project on github [DSI-transformers](https://github.com/ArvinZhuang/DSI-transformers), just run``CUDA_VISIBLE_DEVICES=1 nohup python train.py &`` to reproduce this problem.

### Expected behavior

The program can correctly realize that it can only use GPU specified in command.

Comments:
solved
extending metric_for_best_model to a list of strings 
Author: dorooddorood606
Pull request: no

# 🚀 Feature request

Hi
Currently ` metric_for_best_model(:obj:`str`, `optional`) ` only covers one metric, for some datasets like MRPC, one have several metrics, like accuracy/F1 or like in STSB both pearson/spearman, and user might need to choose the best based on average on all metrics, it could be helpful to have this option.

This is related to the trainer @sgugger 

thanks. 

Comments:
This could be added indeed. Note that there is already a workaround: when defining your `compute_metrics` function, you can add a new field with this average:
```
def compute_metrics(eval_preds):
    # Your previous metric computation
    metrics["combined"] = (metrics["accuracy"] + metrics["f1"]) / 2
    return metrics
```
and then you can pass `--metric_for_best_model combined`. This approach is also more flexible as you can completely define the way the combination is done (so you can pick weights for your mean or do something else than the mean).

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/master/CONTRIBUTING.md) are likely to be ignored.
Fix `.push_to_hub` and cleanup `get_full_repo_name` usage
Author: Wauplin
Pull request: yes

A few (vaguely) related changes in this PR. The main goal was to fix a bug when `.push_to_hub` is used with a repo_id in the form `"organization/repo_name"` that is also a local working directory. The organization gets removed which pushes the model to `"username/repo_name"` instead of under the organization. This bug has been reported [on slack](https://huggingface.slack.com/archives/C02EMARJ65P/p1690366443112179) (private link) by @NathanHB. In addition to this fix, I also made some changes to get rid of `get_full_repo_name` in most cases.

**List of changes:**
- fix `src/transformers/utils/hub.py` to work with organization (bug above)
- added some ValueError when using deprecated args in addition to existing args
- get rid of `get_full_repo_name` in training scripts (no need for it when using `create_repo`),  which saves 1 whoami call
- removed `get_full_repo_name` from keras_callback.py, modeling_tf_utils.py and trainer.py
- import `get_full_repo_name` from `huggingface_hub` instead of re-defining it.

I expect nothing to be broken by those changes.

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

Thanks for the review @ydshieh. I added some comments and fix a bug where token was not used (well done finding this one! :wink:). So I think we're good to go now. I'll merge the PR once CI is green :) 

@Wauplin Thanks for the update. Good to merge (you can re-run the failed tests and it should be fine).

Regarding the comment, thanks a lot. (What I originally mean is that adding some comments on the PR pages, but it could be on the code too.)



~@ydshieh I don't think I have the permissions to rerun a failed test. Could you trigger it for me please :pray:~

**EDIT:** I was logged out :smile: I just triggered it.
Incorrect backward pass in the four bits LLaMA 2 70B
Author: noamwies
Pull request: no

### System Info

This issue needs to be treated as a code review comment.

### Who can help?

@TimDettmers @ArthurZucker @younesbelkada
 
 It seems that when `config.pretraining_tp` is greater than one, then the projections of the keys, queries, and values in the `LlamaAttention` implemented using `torch.nn.functional.linear`.  Hence, in such cases the implementation bypasses `torch.nn.Linear.forward ` which if i understand correctly is problematic when the underlined linear module is replaced by `bitsandbytes`.

### Information

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

This issue needs to be treated as a code review comment. So, I didn't implement a concrete code for demonstrating the problem.

### Expected behavior

A seamless integration with `bitsandbytes`.

Comments:
Hi @noamwies 
For fine-tuning llama2 models that have `config.pretraining_tp>1` consider calling

```python
model.config.pretraining_tp = 1
```

Before training, make sure to use the main branch of `transformers` to include: https://github.com/huggingface/transformers/pull/24906 

```bash
pip uninstall transformers
pip install git+https://github.com/huggingface/transformers
```

This is a duplicate of #24961, as well https://github.com/facebookresearch/llama/issues/423 , and https://github.com/TimDettmers/bitsandbytes/issues/610. This is not something that will be fixed in `transformers` and not sure you need to fix it, since the pretraining tp should stay at 1 for most use cases