noamaanMulla-03/transformers-issues huggingface dataset:
Mismatch between `_convert_token_to_id_with_added_voc` and `encode` for Llama-3.2 tokenizer
Author: g-benton
Pull request: no
Labels: name: Usage

name: bug

### System Info

- `transformers` version: 4.47.1
- Platform: macOS-13.6.3-arm64-arm-64bit
- Python version: 3.12.0
- Huggingface_hub version: 0.25.2
- Safetensors version: 0.5.1
- Accelerate version: 1.2.1
- Accelerate config: 	not found
- PyTorch version (GPU?): 2.5.1 (False)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using distributed or parallel set-up in script?: N/A

### Who can help?

@ArthurZucker and @itazap - thanks!

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

Simple reproducible example:
```{python}
from transformers import AutoTokenizer

tokenizer = AutoTokenizer.from_pretrained("meta-llama/Llama-3.2-1B")

print(tokenizer._convert_token_to_id_with_added_voc("\n") is None)
print(tokenizer.encode("\n"))
print(repr(tokenizer.decode(198)))
```
should give
```{python}
True
[128000, 198] # 12800 is just the added beginning of text token
'\n'
```
with `encode` and `decode` we get `\n` has id 198 and both directions work, but `tokenizer._convert_token_to_id_with_added_voc("\n")` returns `None`.

Thanks in advance!

### Expected behavior

`_convert_token_to_id_with_added_voc` should ideally process tokens the same way as `encode` and `decode`

Comments:
Hey! not sure again why you are having this issue, documentation might be wrong but it's again expected. `\n` is not a token, the associated token is the bytelevel representation:
```python
tokenizer.convert_ids_to_tokens([198])
['Ċ']
```


Thanks for the reply! Can you help me understand why these methods are different? If indeed `\n` is not a token, why do the encode and decode methods support it? 

```
print("encode \\n= ", tokenizer.encode("\n", add_special_tokens=False))
print("convert \\n= ", tokenizer.convert_tokens_to_ids("\n"))
encode \n=  [198]
convert \n=  None

print(repr(f"decode 198= {tokenizer.decode(198)}"))
print("convert 198= ", tokenizer.convert_ids_to_tokens(198))
'decode 198= \n'
convert 198=  Ċ

print("encode Ċ= ", tokenizer.encode("Ċ", add_special_tokens=False))
print("convert Ċ= ", tokenizer.convert_tokens_to_ids("Ċ"))
encode Ċ=  [128, 232]
convert Ċ=  198

print(repr(f"decode 128 232 = {tokenizer.decode([128, 232])}"))
print("convert 128 232= ", tokenizer.convert_ids_to_tokens([128, 232]))
'decode 128 232 = Ċ'
convert 128 232=  ['Ä', 'Ĭ']
```

It doesn't seem like desirable behavior that I can encode something that is ostensibly a token (given it gets encoded to an ID and that ID can be decoded to recover the token) but if I use `convert_ids_to_token` I end up with a different string.

Hey! Inviting you to read this https://huggingface.co/docs/tokenizers/api/pre-tokenizers#tokenizers.pre_tokenizers.ByteLevel about bytlevel! I'll close this as it's expected 🤗
Make `output_dir` optional in TrainingArguments
Author: ChanderG
Pull request: no
Labels: name: Good Second Issue

name: Feature request

### Feature request

Currently, there is only 1 required param in creating a TrainingArguments object - `output_dir`. HFTrainer manually creates an object with a default value "tmp_trainer" if no Args object is passed to it.

Instead, we should make even this one param optional in the TrainingArguments class (and use a default inside the class implementation).

### Motivation

This is useful when creating and passing TrainingArguments in other runners - for eg, trl/SFTTrainer. I would like sensible defaults for all params, so that I only specify the particular arguments I am interested in.

### Your contribution

I can open a PR, if this is of interest.

Comments:
cc @muellerzr sounds good to me wdyt? 

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.

I've found a workaround using inheritance:

```
@dataclass
class MyTrainingArguments(TrainingArguments):
    output_dir: str = field(
        default="models",
    )
```

not everyone want to save the model

Let's leave it to the community!

i'll take this up . 

raised a PR #35735 to fix this issue . maintainers kindly give this a look .
[docs] minor doc fix
Author: faaany
Pull request: yes

## What does this PR do?
minor fix in README.

Documentation: @stevhliu

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

## What does this PR do?
This PR fixes some issues related to the awq docs:
1. autoawq would downgrade transformers version to 4.47.1 (see [link](https://github.com/casper-hansen/AutoAWQ/blob/main/setup.py#L39)) -> add a note
2. add xpu support 
3. other minor fixes


Documentation: @stevhliu


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

Thanks so much for the commit suggestions! Updated.

FYI, reinstalling transformers after installing AutoAWQ will only work for inference, not if you want to quantize a model. Still pending a fix after 4.48 which broke AutoAWQ.

Thanks for the clarification, let's also include that note in the warning

Thanks for the info! @casper-hansen Doc updated.
Make `output_dir` Optional in `TrainingArguments` #27866
Author: sambhavnoobcoder
Pull request: yes

## Description

This PR addresses an issue where specifying an `output_dir` was mandatory in `TrainingArguments`. This behavior could be confusing or cumbersome in scenarios where users do not necessarily need a dedicated output directory or prefer to let the library handle default paths. 

By making `output_dir` optional and defaulting to a temporary directory (named `"tmp_trainer"`) when not specified, we simplify the API usage. If a user later needs to track outputs in a dedicated directory, they can still provide a valid path.

Fixes : #27866 

## Proposed Solution

• Allow `output_dir` to be set to `None`.  
• Default to a built-in location (e.g., `"tmp_trainer"`) if `None` is provided.  

## Implementation

The changes introduce a check to automatically assign a default directory if none is specified. This preserves backward compatibility for users already specifying `output_dir` while improving the experience for those who prefer a simpler workflow.

## Testing

• Added a test covering the scenario when `output_dir` is not provided.  
• Verified existing tests using a custom `output_dir` are unaffected.  
• Extended tests to ensure the directory is only created when needed (depending on the save strategy).

## Screenshots

Below is a screenshot of all testcases passing : 
<img width="1295" alt="Screenshot 2025-01-16 at 10 57 20 PM" src="https://github.com/user-attachments/assets/dba60269-7076-4eba-bd28-bb938621414e" />

cc :  @ArthurZucker  @Rocketknight1 

Comments:
cc @sunmarc @muellerzr for Trainer!

Thank you ☺️
xpu: fix benchmarking scripts for xpu devices
Author: dvrogozh
Pull request: yes

Benchmarking scripts were previously enabled with xpu backend support. However, if cuda nvml library is not installed then scripts will fail on xpu with the following error:
```
py3nvml.py3nvml.NVMLError_LibraryNotFound: NVML Shared Library Not Found
```
This commit addresses the issue disabling memory collection in the benchmarking scripts for xpu.

CC: @ArthurZucker, @SunMarc @faaany 


Comments:
@ArthurZucker, @SunMarc : I saw deprecation warning for benchmark classes. However, considering that 1) xpu support is already enabled for them, 2) scripts seem to run fine on XPU and `tests/becnhmark/` pass (if NVML is installed), - I thought it will be better to just fix the issue I observe in the case when NVML is not installed. My goal is to fix `tests/` for xpu device. Please, let me know if you think we need to follow alternate path for this case.

Benchmark code pruned by 90b46e983f755335e12f8bb74b90da60b2726d98. No need in this PR anymore.

Yep thanks for it anyways 🤗
Whisper: remove redundant assisted generation tests
Author: gante
Pull request: yes

# What does this PR do?

Part of making CI green in preparation for https://github.com/huggingface/transformers/pull/34807 (test fetcher fetches generation tests)

1. Removed from the PR ~Reverts recent default `max_length`-related changes. The default max length is `20` and, due to recent changes, a different length was being returned in some cases. Whisper was one of the affected models, it was defaulting to a length of `21`.~
2. Deletes `test_assisted_decoding_encoder_decoder_shared_encoder` (integration test) -- the point of this test was to test assisted generation with DistilWhisper-like structure, which used to have a custom input (`assistant_encoder_outputs`). a) That input is no longer present in Whisper b) the DistilWhisper structure didn't take off, so it's overkill to have an abstract test with dummy classes c) we have other DistilWhisper tests.
3. Deletes `test_model_kwarg_assisted_decoding_encoder_decoder` (integration test) -- added when DistiWhisper was added. Meanwhile, assisted generation was modified to work with any encoder-decoder model, and we have mixin tests.

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

(failures look related to other failures on `main`, waiting for them to be fixed before rebasing and requesting a re-review)

@ArthurZucker @ydshieh ready for review :) (this PR now just removes deprecated features and tests -- see PR header)
Reload Transformers imports
Author: KareemMusleh
Pull request: no
Labels: name: Good Second Issue

name: Feature request

### Feature request

It would be nice to have a transformers.clear_import_cache() or any other way to reset the dynamic imports of transformers.

### Motivation

Some people are having bugs after hacking on transformers [1](https://github.com/unslothai/unsloth/issues/1410), [2](https://github.com/unslothai/unsloth/issues/1499). Hacking on transformers [seems to be supported](https://huggingface.co/docs/transformers/how_to_hack_models)

### Your contribution

given some amount of guidance, I am willing to write a PR.

Comments:
@ArthurZucker @SunMarc @Rocketknight1 This is a feature request/bug report

Happy to have this, do you want to open a PR? 🤗 

Hey @ArthurZucker I have raised a Pull request that fixes this issue . hope you can review this and merge if issue is resolved . I'll acknowledge any comments you have on it as well if required .
Also, @KareemMusleh thank you for raising this issue . This is really a useful feature .
VLM: enable skipped tests
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

Many tests for cached generation were being skipped for VLMs because it was testing for the config attributes, instead of text config. This PR enables the tests, fixes when needed or skips otherwise for specific models

It will unblock https://github.com/huggingface/transformers/pull/35591, after making sure CI is green for all models. 

- Ran all the tests changed for all model, green on my side


Comments:
Should be ready for review, CI was green on my side 

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

Let's merge, to make sure no multimodal model breaks without us spotting it. Core maintainer's review isn't needed, only tests modified
[commands] remove deprecated/inoperational commands
Author: gante
Pull request: yes

# What does this PR do?

While working on `chat` (https://github.com/huggingface/transformers/pull/35714), I've noticed some commands have deprecation warnings and/or are not operational.

Since we are gearing up for v5, I thought we could get rid of them. These commands were deprecated >3years ago.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35718). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Fix Gradient Checkpointing for Deberta & Deberta-V2 using PEFT / Adapters
Author: lenglaender
Pull request: yes

# What does this PR do?

This PR replaces in-place operations in the Deberta and Deberta-V2 implementations. This fixes gradient checkpointing for Deberta and Deberta-V2 when using the Adapters library or Hugging Face PEFT. 

Before this PR, when using `model.enable_input_require_grads()` on a PEFT / Adapters model, we get the following error: `RuntimeError: a leaf Variable that requires grad is being used in an in-place operation` (see https://github.com/adapter-hub/adapters/pull/759). To reproduce for PEFT, run the following script:

```python
from transformers import DebertaConfig, DebertaForSequenceClassification
from peft import get_peft_model, LoraConfig, TaskType
import torch

# Create a minimal DeBERTa config for testing
config = DebertaConfig(
    hidden_size=32,
    num_hidden_layers=5,
    num_attention_heads=4,
    intermediate_size=37,
    relative_attention=True,
)

# PEFT model
model = DebertaForSequenceClassification(config)
peft_config = LoraConfig(task_type=TaskType.SEQ_CLS, inference_mode=False, r=8, lora_alpha=32, lora_dropout=0.1)
model = get_peft_model(model, peft_config)
model.train()

# Enable input gradients
model.enable_input_require_grads()

# Move to GPU if available
device = torch.device("cuda" if torch.cuda.is_available() else "cpu")
model = model.to(device)

# Create a random input tensor for the forward pass
batch_size = 2
seq_length = 10
input_ids = torch.ones((batch_size, seq_length), dtype=torch.long).to(device)
attention_mask = torch.ones_like(input_ids).to(device)

# Without this PR, this throws a RuntimeError
outputs = model(input_ids=input_ids, attention_mask=attention_mask)
```

## 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.
- [ ] 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.

Maybe one of @Rocketknight1 @SunMarc  @ArthurZucker 

Comments:
🚨 Remove cache migration script
Author: Wauplin
Pull request: yes

This PR removes the "cache migration script" from `src/transformers/utils/hub.py`. No need to maintain it anymore IMO. 

This script has been introduced in `v4.22.0` (Sept. 2022) to move files from the old cache structure to the new one. Since it has now been 28 months, I think we can safely assume all caches have been moved by now. Worth case scenario, users will have to redownload their 2+ years old models if they update `transformers`. 

I still kept a dummy deprecated `move_cache` method to avoid breaking imports. I can remove it if you think it's not necessary.

Comments:
> Not sure about keeping the deprecated move_cache method - I think it can probably be deleted, but it's not a big problem to leave it.

I'll let the `transformers` decide! I also think we should remove it but didn't want to introduce the breaking change myself :smile: 

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_35810). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
make fix-copies breaks unrelated model
Author: ziereis
Pull request: no
Labels: name: bug

### System Info


- `transformers` version: 4.49.0.dev0
- Platform: Linux-6.8.0-51-generic-x86_64-with-glibc2.39
- Python version: 3.11.11
- Huggingface_hub version: 0.28.1
- Safetensors version: 0.5.2
- Accelerate version: not installed
- Accelerate config: not found
- PyTorch version (GPU?): 2.6.0+cu124 (True)
- Tensorflow version (GPU?): 2.13.1 (False)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using distributed or parallel set-up in script?: <fill in>
- Using GPU in script?: <fill in>
- GPU type: NVIDIA GeForce RTX 4090

### Who can help?

_No response_

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

i made a small change int the modeling_llama.py, but when i run make fix-copies it performs changes in files unrelated to my change and breaks them. 

commit after fix-copies:

https://github.com/huggingface/transformers/pull/36128/commits/92ab894cf31bfdf41c50c7d59372107f69777a26#diff-d97f019b26f1107134650e831eee0e3384df2605cf79e3a3927e18643ca14b4aL298

broken model:

https://app.circleci.com/pipelines/github/huggingface/transformers/118132/workflows/1a54d5b6-7636-4d3f-964d-b6376d97584d/jobs/1592077

### Expected behavior

dont break 

Comments:
![Image](https://github.com/user-attachments/assets/b8e990de-9b57-456d-9992-e7dd8a1a6bd8)

Closing this issue because I was unable to reproduce - I'll discuss in the PR at #36128
Wrong corners format of bboxes in function center_to_corners_format (image_transforms.py)
Author: majakolar
Pull request: no

In [image_transforms.py](https://github.com/huggingface/transformers/blob/main/src/transformers/image_transforms.py), function `center_to_corners_format` describes the corners format as "_corners format: contains the coordinates for the top-left and bottom-right corners of the box (top_left_x, top_left_y, bottom_right_x, bottom_right_y)_". All subsequent functions such as `_center_to_corners_format_torch` compute the center format in the following way:

```
def _corners_to_center_format_torch(bboxes_corners: "torch.Tensor") -> "torch.Tensor":
    top_left_x, top_left_y, bottom_right_x, bottom_right_y = bboxes_corners.unbind(-1)
    b = [
        (top_left_x + bottom_right_x) / 2,  # center x
        (top_left_y + bottom_right_y) / 2,  # center y
        (bottom_right_x - top_left_x),  # width
        (bottom_right_y - top_left_y),  # height
    ]
    return torch.stack(b, dim=-1)

```

The problem is that the height of the bounding box would have a negative value. This code computes the center format from the corners format (bottom_left_x, bottom_left_y, top_right_x, top_right_y). The expected format or the code should be changed.

Comments:
Hi @majakolar - looks like you closed this, did you change your mind about the bug?

@Rocketknight1 Yes, after some additional debugging I realized I had a bug in my own code.

No probs, and thanks for updating us!
Update doc re list of models supporting TP
Author: kwen2501
Pull request: yes

# What does this PR do?

Updated the list of models that support TP. 

## 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?

- text models: @ArthurZucker

Comments:
Can you run `make style` to pass the cis? 🤗 

@ArthurZucker thanks for the pointer! Super excited to see https://github.com/huggingface/transformers/pull/35870 adding more models. 

Since Gemma support has been added there, I am shrinking my PR to just update the doc. Would appreciate your review.

Long term we want a bot that just imports models and checks that they support and writes this to the file instead of manually doing it ourselves!
Mangled tokenization with Llama 3.1 for string sequences containing<space>'m
Author: tomjorquera
Pull request: no
Labels: name: bug

We observed that trying to tokenize/detokenize strings containing the sequence `<space>'m` would not give back the initial string, but would "eat" the leading whitespace.

For example, the string "for 'manual'" will be transformed into "for'manual'"

Investigating further, we also observed issue with strings containing `<space>'s`, making us think the issue may be related to trying to handle sequences such as "I'm".

### System Info

transformers==4.46.2

### Who can help?

I guess it's for @ArthurZucker and @itazap 

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

Running:

```python
from transformers import AutoTokenizer

prompt = """for 'manual'"""

tokenizer = AutoTokenizer.from_pretrained("meta-llama/Meta-Llama-3.1-8B-Instruct")
tokenizer.batch_decode(tokenizer([prompt])["input_ids"], skip_special_tokens=True)[0]
```

prints

```
"for'manual'"
```

(missing whitespace before the leading ')

### Expected behavior

It should output the following

```
"for'manual'"
```

Comments:
Note: we encountered this issue first from TGI, as reported https://github.com/huggingface/text-generation-inference/issues/2927

Hey! I think this is related to `cleanup_tokenization_space` that default to `True`in the version of `transformers` that you are using 

Thanks for the reply @ArthurZucker !

I can confirm that if I add `clean_up_tokenization_spaces=False` to the call to `batch_decode`, the issue disappears.

I'm not sure if this means this behavior is "normal" (or let's say expected at least), and what will be the consequences for setting this option to False.

I tested updating to v4.48.1 (actually I forgot to report I had already tested with v4.48.0 before, sorry), and the behavior is still the same. From the doc it seems indeed `True` is still the default in the latest version.

Is there something to improve here or shall will be left at that?

In #31938 we should have set it to `False` by default 

But for some tokenizers on the hub, it's explicitly set to `True` : https://huggingface.co/meta-llama/Llama-3.1-8B-Instruct/blob/main/tokenizer_config.json#L2054. We can't do much against this appart from raising awarness here!