noamaanMulla-03/transformers-issues huggingface dataset:
GaLore implementation assigns same optimizer to galore_params and non_galore_params
Author: geronimi73
Pull request: no
Labels: name: optimization

### System Info

Hey everyone, 

I noticed something in the current implementation of GaLore (layerwise): the same optimizer is hooked onto `galore_params` and `non_galore_params`. 

Is this on purpose? 

https://github.com/huggingface/transformers/blob/76a33a10923ccc1074917f6b6a1e719e626b7dc9/src/transformers/trainer.py#L1296-L1301

The [official implementation](https://github.com/jiaweizzhao/GaLore) hooks `GaLoreAdamW8bit` to galore_params and `bnb.optim.Adam8bit` to all others:

https://github.com/jiaweizzhao/GaLore/blob/864eeb361dc96c1932c3fa429ad0119aaed8e617/torchrun_main.py#L339-L342



### Who can help?

@younesbelkada

### Information

- [X] 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

any script that uses `galore_*_layerwise`

### Expected behavior

not sure

Comments:
Hi @geronimi73 
Thanks for the issue - yes this is expected as Galore optimizers under the hood will fall back to the native optimizer if you don't pass any galore kwargs

yes, @jiaweizzhao told me the same that's why I closed the issue. would have noticed this myself if I looked at the code more carefully. thank you for your answer 

Thanks @geronimi73 !
updated examples/pytorch/language-modeling scripts and requirements.txt to require datasets>=2.14.0
Author: Patchwork53
Pull request: yes

# What does this PR do?
All `examples/pytorch/language-modeling` scripts pass `token` argument to `load_datasets()`. The `token` argument was introduced in `datasets==2.14.0`, replacing `use_auth_tokens`. I updated the scripts and the requirements.txt to require `datasets>=2.14.0`

In response to my issue https://github.com/huggingface/transformers/issues/30097
## Who can review?
@ArthurZucker @amyeroberts 


Comments:
fixing issue 30034 - adding data format for run_ner.py
Author: JINO-ROHIT
Pull request: yes

# What does this PR do?

This PR adds the data format in the README for explaining data format for run_ner.py script.

Fixes #30034 

## Before submitting
- [x] 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?

@NielsRogge @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:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30088). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
What is the data file format of `run_ner.py`?
Author: sahil3773mehta
Pull request: no
Labels: name: Good First Issue

### Feature request

What is the correct format for custom dataset in run_ner.py? Would it be possible to include a few lines on this with a helpful example? 

### Motivation

I am using the example script run_ner.py from [huggingface](https://github.com/huggingface)/transformers It is not possible to use standard conll format for the model fine-tuning of run_ner.

### Your contribution

We could include this in the corresponding readme.

Comments:
Answered here: https://discuss.huggingface.co/t/what-is-the-data-file-format-of-run-ner-py/41873/3?u=nielsr, but indeed would be nice to open a PR for this to make it more clear

@NielsRogge i would like to take this up
Add common processor tests
Author: NielsRogge
Pull request: yes
Labels: name: WIP

# What does this PR do?

Multimodal processors currently don't have common tests. This PR aims to work towards having a common API for our multimodal processors, making sure they all have the same inputs and outputs (e.g. making sure text+vision processors accept `text` as first kwarg, then `images`, etc.).

As a first work, I refactor the CLIP and BLIP-2 processor tests to leverage the common ones.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_27720). All of your documentation changes will be reflected on that endpoint.

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.

@NielsRogge  If you are OK with that, let's put the tests in another PR #27761 and close this one?

Isn't the other PR for saving processors? Or will it also add common tests?

Yes, that is a PR about saving processors. But we want to have tests to make sure it works as we want it. (to avoid later changes of files on the Hub - which is always almost impossible). 

>  add common tests?

Yes, probably with a different version than this PR
Patch fix - don't use safetensors for TF models
Author: amyeroberts
Pull request: yes

# What does this PR do?

There was an upstream change to the model weights used for testing: https://huggingface.co/hf-internal-testing/tiny-random-gpt2/tree/main

This resulted in TF generation tests using this checkpoint to fail. 

This resolves test which are currently failing on main and open PRs e.g. [here](https://app.circleci.com/pipelines/github/huggingface/transformers/88901/workflows/0cad538f-e551-478f-873c-1e9e0df45b5b/jobs/1158132)

```
FAILED tests/generation/test_tf_utils.py::TFGenerationIntegrationTests::test_eos_token_id_int_and_list_beam_search - AssertionError: False is not true
FAILED tests/generation/test_tf_utils.py::TFGenerationIntegrationTests::test_eos_token_id_int_and_list_contrastive_search - AssertionError: False is not true
FAILED tests/generation/test_tf_utils.py::TFGenerationIntegrationTests::test_eos_token_id_int_and_list_greedy_search - AssertionError: False is not true
FAILED tests/generation/test_tf_utils.py::TFGenerationIntegrationTests::test_eos_token_id_int_and_list_top_k_top_sampling - AssertionError: False is not true
```

The tests did not start failing for the PT models, indicating there is an mis-match between safetensors loading behaviour for PT and TF. 

An issue has been opened here: #30117


Comments:
cc @Rocketknight1, @gante 

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

Merging as the failing tests are unrelated to this PR
examples/pytorch/language_modeling/requirements.txt needs to be updated
Author: Patchwork53
Pull request: no
Labels: name: Examples

### System Info

- `transformers` version: 4.40.0.dev0
- Platform: Linux-5.15.133+-x86_64-with-glibc2.31
- Python version: 3.10.13
- Huggingface_hub version: 0.21.4
- Safetensors version: 0.4.2
- Accelerate version: 0.28.0
- Accelerate config: 	not found
- PyTorch version (GPU?): 2.1.2 (True)
- Tensorflow version (GPU?): 2.15.0 (True)
- Flax version (CPU?/GPU?/TPU?): 0.8.2 (gpu)
- Jax version: 0.4.23
- JaxLib version: 0.4.23.dev20240116
- Using GPU in script?: yes
- Using distributed or parallel set-up in script?: yes

### Who can help?

@ArthurZucker 

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

# Observation
The requirement specifies datasets>=1.8.0 however, all scripts (such as `run_mlm.py`) pass the `token` argument to `load_dataset()`.  `token` was added in datasets version 2.14.0 (replacing `use_auth_token`). Tried simply replacing with  `datasets==2.14.0` or  `datasets>=2.14.0` but had dependency issues.

# Reproduction

1. Use an env with datasets<2.14.0
2. Pip install examples/pytorch/language-modeling/requirements.txt
3. Run script (from README.md)
```
python run_mlm.py \
    --model_name_or_path FacebookAI/roberta-base \
    --train_file path_to_train_file \
    --validation_file path_to_validation_file \
    --per_device_train_batch_size 8 \
    --per_device_eval_batch_size 8 \
    --do_train \
    --do_eval \
    --output_dir /tmp/test-mlm
```

# Traceback
```python
Traceback (most recent call last):
  File "/kaggle/working/run_mlm.py", line 704, in <module>
    main()
  File "/kaggle/working/run_mlm.py", line 371, in main
    raw_datasets = load_dataset(
  File "/opt/conda/lib/python3.10/site-packages/datasets/load.py", line 1664, in load_dataset
    builder_instance = load_dataset_builder(
  File "/opt/conda/lib/python3.10/site-packages/datasets/load.py", line 1516, in load_dataset_builder
    builder_instance: DatasetBuilder = builder_cls(
  File "/opt/conda/lib/python3.10/site-packages/datasets/builder.py", line 265, in __init__
    self.config, self.config_id = self._create_builder_config(
  File "/opt/conda/lib/python3.10/site-packages/datasets/builder.py", line 367, in _create_builder_config
    builder_config = self.BUILDER_CONFIG_CLASS(**config_kwargs)
TypeError: TextConfig.__init__() got an unexpected keyword argument 'token'
```


### Expected behavior

Train model normally.

Comments:
Hi @Patchwork53, thanks for raising this! Would you like to open a PR with this fix? This way you get the github contribution for spotting and recommending the fix
Fix `torch.fx` symbolic tracing for LLama
Author: michaelbenayoun
Pull request: yes

# What does this PR do?

As per title.


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30047). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Fix falcon with SDPA, alibi but no passed mask
Author: fxmarty
Pull request: yes

As per title, fixes https://github.com/huggingface/transformers/issues/29946

There was a (minor) bug in https://github.com/huggingface/transformers/pull/26572, where if an user uses a Falcon model with alibi and SDPA, but not passing `attention_mask` input, the `attn_mask` argument passed to SDPA would not be causal (as it use to be (see [here](https://github.com/huggingface/transformers/blob/v4.35.2/src/transformers/models/falcon/modeling_falcon.py#L1101-L1103) and [here](https://github.com/huggingface/transformers/blob/v4.35.2/src/transformers/models/falcon/modeling_falcon.py#L506-L508)). I likely misunderstood something at the time.

I think this bug was not catched in the CI because Falcon tests do not test Alibi thoroughly. 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30123). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Trainer / Core : Do not change init signature order
Author: younesbelkada
Pull request: yes

# What does this PR do?

Currently the TRL CI is broken on transformers main due to the addition of `image_processors` in the middle of trainer's init signature 🤯 

On TRL (and probably other libs such as axolotl), we do subclass the trainer and call:
```python
        super().__init__(
            model,
            args,
            data_collator,
            train_dataset,
            eval_dataset,
            tokenizer,
            model_init,
            compute_metrics,
            callbacks,
            optimizers,
            preprocess_logits_for_metrics,
        )
```

Without explicit pos arguments

https://github.com/huggingface/transformers/pull/29896

To be on the safe zone I propose to simply put `image_processors` at the very end of trainer's init

cc @amyeroberts @NielsRogge 


Comments:
We can add this quick patch - which will be eventually be subsumed by #30102

So that we can make sure things don't break in the future - is there a reason the init is called with positional arguments rather than kwargs? 

good point, I will also update that in TRL ! actually there is no strong reason to not use kwargs :/ 
I think other libs might possibly do the same thing as what we did in TRL 

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30126). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Make vitdet jit trace complient
Author: fxmarty
Pull request: yes

Fixes https://github.com/huggingface/optimum/issues/1795 & https://github.com/huggingface/optimum/pull/1582#issuecomment-2028582921

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30065). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Add `mps` as supported device for apple silicon to the `Pipeline` class
Author: fnhirwa
Pull request: no
Labels: name: Good Second Issue

name: Feature request

### Feature request

As in the latest `torch` version the `mps` accelerator is supported. 
I suggest that it could be added to the supported devices here https://github.com/huggingface/transformers/blob/fce52cefa744a5900fc065aafb2f55d846d1202c/src/transformers/pipelines/base.py#L849
This will enable to train using Apple silicon accelerators.

### Motivation
```python
from transformers import pipeline

generator = pipeline("text-generation", model="gpt2", device=0)
prompt = "In a hole in the ground there lived a hobbit."
output = generator(prompt, max_length=50)

print(output[0]["generated_text"])

Fails for `mps` accelerator using `torch version 2.2.2`
```
### Your contribution

Yeah I would love to open a complete PR if this change is accepted and ensure the check of supported `torch` version is followed.

Comments:
No idea why it's not already there, would you like to open a PR? something like `if is_mps_available()` with ` torch.backends.mps.is_available() ` wrapped in it. 

> No idea why it's not already there, would you like to open a PR? something like `if is_mps_available()` with `torch.backends.mps.is_available()` wrapped in it.

Sure will get it added 👍
Fix typo at ImportError
Author: DrAnaximandre
Pull request: yes

# What does this PR do?

When instantiating a `QuantoHfQuantizer`, the `ImportError` message that is risen for `accelerate` has a typo in it.


## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. 



Comments:
Merging as the fix was merged into main, and the change here is unrelated, so no need to run CI again
Fix: handle logging of scalars in Weights & Biases summary
Author: parambharat
Pull request: yes

# What does this PR do?

This PR handles logging scalars to wandb correctly by adding the scalars to the wandb summary rather than the workspace run logs.

Here's an example workspace [run](https://wandb.ai/parambharat/test-transformers/runs/9p7kwbfl) with the fix

### Example workspace before fix:
![image](https://github.com/huggingface/transformers/assets/12809212/73159045-dd9e-4aad-a357-8148ddd50a5e)


### Example workspace after fix:

![image](https://github.com/huggingface/transformers/assets/12809212/6bb204f9-a8b3-42d8-90ec-166c7de4cb1a)

### Example Summary after fix:

![image](https://github.com/huggingface/transformers/assets/12809212/971e6120-dc27-471d-bad4-f37da9fb8f7b)


Fixes #29430


## Before submitting
- [X] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [X] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [X] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.

## Who can review?

@muellerzr
@nmd2k
@ArthurZucker 



Comments:
cc @amyeroberts for final review :) 

Thanks, it fix the problem

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_29612). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Fix failing DeepSpeed model zoo tests
Author: pacman100
Pull request: yes

# What does this PR do?
1. Fixing DeepSpeed model zoo tests
    a. `tests/deepspeed/test_model_zoo.py::TestDeepSpeedModelZoo::test_zero_to_fp32_zero3_img_clas_vit` was failing earlier due to `ValueError: --label_column_name label not found in dataset 'hf-internal-testing/cats_vs_dogs_sample'. Make sure to set `--label_column_name` to the correct text column - one of image, labels.`. Fixed this.
    b. `tests/deepspeed/test_model_zoo.py::TestDeepSpeedModelZoo::test_zero_to_fp32_zero3_trans_fsmt` was failing earlier due to `TypeError: Old language model head is of type <class 'torch.nn.modules.sparse.Embedding'>, which is not an instance of <class 'torch.nn.modules.linear.Linear'>. You should either use a different resize function or make sure that `old_lm_head` are an instance of <class 'torch.nn.modules.linear.Linear'>.`. This was because the `resize_token_embeddings` didn't account for the case when lm_head is an instance of `torch.nn.Embedding` and it was forcing the new lm_head to be an instance of `torch.nn.Linear`.
    c. `tests/deepspeed/test_model_zoo.py::TestDeepSpeedModelZoo::test_zero_to_fp32_zero2_trans_m2m_100` was failing earlier due to `ValueError: `--max_source_length` is set to 1024, but the model only has 512 position encodings. Consider either reducing `--max_source_length` to 512 or using a model with larger position embeddings`. Fixed the test to use appropriate source and target sequence lengths.


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

> Would be nice to add a test, but alright otherwise

Could you please clarify what you mean?
Tokenizer `use_fast=True` encode has fatal bug
Author: theodoruszq
Pull request: no

### System Info

Transformers=4.37
Pytorch=2.1.1

- [x] My own modified scripts
- [x] My own task or dataset (give details below)

### Reproduction

For yi34b model, after SFT, we save the model without `tokenizer.json` but with anything else about tokenizer, then we run `tokenizer.convert_ids_to_tokens([64001])` (the tokenizer use_fast being set to default value), it gets `
`. But the vocab has only 64000, therefore it triggers CUDA device assertion error.


This bug is prone to be reproduced. Please fix it.

### Expected behavior

Fix the bug ASAP

Comments:
Hey! The token was probably added 🤗 
> vocab has only 64000 

what happends when you print the tokenizer


> tokenizer.convert_ids_to_tokens([64001])

@ArthurZucker 

Here is the output:
```
>>> tokenizer.convert_ids_to_tokens([64001])
['
']
>>> tokenizer.convert_ids_to_tokens([64001])
['
']
>>> tokenizer.convert_ids_to_tokens([64002])
[None]
>>> tokenizer.convert_ids_to_tokens([1])
['
']
>>> tokenizer.convert_ids_to_tokens([0])
['<unk>']
>>> tokenizer.vocab_size
64000
```

The results seem harmless, however, when use `use_fast=False`, it outputs:

```
>>> tokenizer = AutoTokenizer.from_pretrained(model_path, use_fast=False)
>>> tokenizer.convert_ids_to_tokens([64001])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/tiger/anaconda3/lib/python3.9/site-packages/transformers/tokenization_utils.py", line 982, in convert_ids_to_tokens
    tokens.append(self._convert_id_to_token(index))
  File "/home/tiger/anaconda3/lib/python3.9/site-packages/transformers/models/llama/tokenization_llama.py", line 280, in _convert_id_to_token
    token = self.sp_model.IdToPiece(index)
  File "/home/tiger/anaconda3/lib/python3.9/site-packages/sentencepiece/__init__.py", line 1179, in _batched_func
    return _func(self, arg)
  File "/home/tiger/anaconda3/lib/python3.9/site-packages/sentencepiece/__init__.py", line 1172, in _func
    raise IndexError('piece id is out of range.')
IndexError: piece id is out of range.
```

Alright I think `yi` is a special case, #29797 will be fixing this! 

> Alright I think `yi` is a special case, #29797 will be fixing this!

Thanks, closing the issue~
[`core`] Refactor of `gradient_checkpointing`
Author: younesbelkada
Pull request: yes

# What does this PR do?

Alternative to https://github.com/huggingface/transformers/pull/26917 

This way we make `set_gradient_checkpointing` more modulable, as requested by some users - e.g. https://github.com/huggingface/transformers/issues/21381#issuecomment-1757690624 

Fixes some issues with DDP such as: https://github.com/huggingface/trl/issues/835 

Also removed GC support from `TFSwin` as in theory `gradient_checkpointing` is used only for PT models.

Added also a CI tests for that

For users that want to use `gradient_checkpointing` with `use_reentrant=False`:

```python
...
model.enable_gradient_checkpointing(gradient_checkpointing_kwargs={"use_reentrant": False})
```



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

Can you add a test to make sure setting and unsetting both work as expected (specifically for the fix we are implementing in TRL)

+1

Ran some training tests with PEFT + GC using this branch and everything seem to pass! Merging once the CI is green

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_27020). All of your documentation changes will be reflected on that endpoint.

Whatis the difference of enable_gradient_checkpointing and gradient_checkpointing_enable??

@lucasjinreal I can only see `gradient_checkpointing_enable` here: https://github.com/huggingface/transformers/blob/main/src/transformers/modeling_utils.py

I using model.enable_gradient_checkpointing no errors appear...

![image](https://github.com/huggingface/transformers/assets/21303438/bacd3acb-50a8-4405-9c61-d8340ed1e268)


How to explain this

🤯 
I think this was a typo, it is weird that you don't get any error, you should use `gradient_checkpointing_enable` https://github.com/huggingface/transformers/blob/af4c02622bfb4521367c459c6743014ef9be788d/src/transformers/modeling_utils.py#L2195
Fix docs Pop2Piano
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

As the title says, this PR fixes docs of pop2piano. This fix is the result of 'make fix-copies' from another PR, but I think better to make it its own PR :)

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