noamaanMulla-03/transformers-issues huggingface dataset:
Fix donut token2json multiline
Author: qubvel
Pull request: yes

# What does this PR do?

Close the issue with token2json processing method for Donut

> There is a bug in the implementation of the function token2json in the DonutProcessor class. It doesn't work when there are multilines in a value as shown in the example below. It is related the regex code.

Fixes # (issue)
https://github.com/huggingface/transformers/issues/30272

## 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).
- [x] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.


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

 @NielsRogge could you please review, as you added this model
Create CodeAgent and ReactAgent
Author: aymeric-roucher
Pull request: yes
Labels: name: Agents

# What does this PR do?

Add the building blocks for the new agents: the base `Agent`, `CodeAgent`, `ReactAgent`, `Toolbox`.

@LysandreJik this is a squashed version of the [previous PR](https://github.com/huggingface/transformers/pull/29396), to make it cleaner. (btw please tell me if it's not a good practice to squash commits before opening a PR).

Comments:
There is no need to squash, the merge with main will have swatched the commits 

I think it's good to squash them even here so that commits to the `agents` branch keep a a purpose per commit.

However, you can squash on a branch and force push instead of creating a new branch/a new PR :)
Fix all torch pipeline failures except one
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix most torch pipeline failures. See comments in the changes

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30290). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Add jamba
Author: tomeras91
Pull request: yes
Labels: name: New model

# What does this PR do?

Add support for the Jamba architecture by AI21 Labs



## Who can review?

@ArthurZucker @younesbelkada



Comments:
Reviewing ! 

```
ValueError: The deployment of the documentation will fail because of the following errors:
The docstring of JambaForCausalLM.forward comports the following issue(s) and needs fixing:
- A code block has been opened but is not closed.
```
is the doc failure! 

All the rest you added LGTM

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_29943). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Add atol for sliding window test
Author: fxmarty
Pull request: yes

As reported (https://huggingface.slack.com/archives/C01NE71C4F7/p1713360898344479), the `test_sdpa_matches_eager_sliding_window` test added in https://github.com/huggingface/transformers/pull/30127 was passing on A100, but not on CPU due to a missing `atol`. Now passing on both.

https://app.circleci.com/pipelines/github/huggingface/transformers/89888/workflows/494537ba-1614-44a1-b8b8-28c01603d3be/jobs/1171908

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

# What does this PR do ?
This PR makes gemma compatible with multi-gpu device_map. To try out: 
```python 
from transformers import AutoTokenizer, AutoModelForCausalLM

tokenizer = AutoTokenizer.from_pretrained("google/recurrentgemma-2b-it")
model = AutoModelForCausalLM.from_pretrained(
    "google/recurrentgemma-2b-it", device_map="auto"
)
input_text = "Write me a poem about Machine Learning."
input_ids = tokenizer(input_text, return_tensors="pt").to("cuda")

outputs = model.generate(**input_ids,use_cache=True)
print(tokenizer.decode(outputs[0]))
```
I get the same output in the single gpu or multi gpu setup. 

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

> Thanks, the device thing could be fixed by placing them on the same device as self.key_states? rather than the device passed? Also tad bit scared of the slow down of doing it there? But LGTM otherwise

I think it will slow down if why place them on the same device as self.key_states for example. Let's say `self.key_states` is initialized on cuda:0 and we have 2 gpus. The problem is that the computed key_states can be on cuda:0 or cuda:1 depending on where the layer is. Hence, it is better to move` self.key_states` to the device of `key_states` to limit data transfert between gpus. Otherwise, we will need to move the data each time we have a layer in cuda:1.
Revert "Fix quality Olmo + SDPA (#30302)"
Author: ArthurZucker
Pull request: yes

This reverts commit ec92f983af5295fc92414a37b988d8384785988a.


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30315). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Revert "Re-enable SDPA's FA2 path (#30070)"
Author: ArthurZucker
Pull request: yes

This reverts commit 05bdef16b611df0946a6a602503f1ace604b6c80.


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30314). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Do not drop mask with SDPA for more cases
Author: fxmarty
Pull request: yes

As per title. Overlooked in https://github.com/huggingface/transformers/pull/30127 (issue with speculative decoding, where no attn_mask is passed to the main model).

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

@ArthurZucker will be cleaner once https://github.com/huggingface/transformers/pull/30070 is brought back

I 'll be harsher to make it clearer!
FIX: Fixes unexpected behaviour for Llava / LLama & AWQ Fused modules + revert #30070 at the same time
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fixes a silent behaviour introduced by a recent PR, passing a `None` attention mask results in unexpected behaviour for awq fused modules. The fix is simply to force set a dummy `_attn_implementation` on the config objects of the modules that contain fused modules

I can confirm the failing slow tests now pass with these changes

cc @ArthurZucker @fxmarty 

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

Thanks @fxmarty @ArthurZucker ! just tested everything seems to all pass !
Fix missing `prev_ci_results`
Author: ydshieh
Pull request: yes

# What does this PR do?

#29914 added a new step in slack report workflow, but it used `actions/checkout@v4` after the first part (about model CI job). This indeed removes all files in the same directory, so the `actions/checkout@v4` created in the first part is moved, and the last step of uploading artifact can't find it. This makes the `new mdoel failures` missing in the slack report channel.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30313). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Fix: remove `pad token id` in pipeline forward arguments 
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

Fixes #30181. Moves `pad_token_id` as kwarg only in generation pipeline.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30285). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
fix Parameter dtype in audio models
Author: ylacombe
Pull request: yes

# What does this PR do?

`tests/pipelines/test_pipelines_automatic_speech_recognition.py::AutomaticSpeechRecognitionPipelineTests::test_wav2vec2_conformer_float16` was failing because `masked_spec_embed` is initialized as a `fp32` tensor.

https://github.com/huggingface/transformers/blob/28a22834bf609b27d0178b5ba99224f69577a3b6/src/transformers/models/wav2vec2_conformer/modeling_wav2vec2_conformer.py#L1238


I thus changed the tensor from `FloatTensor` to `Tensor`. Ideally, I would have moved the `.uniform_`, to the `_init_weights`, but turns that `model.apply` doesn't iterate on `Parameter`.

Slow tests on every modeling tests of the files I've changed all passed, but I might have missed some other tests to run, any ideas @ydshieh ? 


cc @ydshieh because of the failing test and @ArthurZucker because we've talked about it 

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

Let me check CI

The original failed pipeline test also passes. I will run a whole pipeline test and let's see. For model testing, since you checked every model modified, I also think it's fine and don't see what else should be checked manually.

all good! merge now, thanks a lot.
Idefics2 raises shape error in the backward pass with gradient_checkpointing
Author: chenzizhao
Pull request: no

I get a shape error if I PEFT the latest Idefics2 model at head with gradient checkpointing. The symptom is identical to [this](https://stackoverflow.com/questions/77656467/attention-mask-error-when-fine-tuning-mistral-7b-using-transformers-trainer). Seems like the following block isn't compatible with gradient checkpointing.
https://github.com/huggingface/transformers/blob/e4ea19b958c89d61e42461fac6ac8441787121f8/src/transformers/models/idefics2/modeling_idefics2.py#L1593-L1596

In my local workspace a fix similar to https://github.com/huggingface/transformers/pull/28061/commits/91f4b7e2b8c305dfa9044463833ecbb0ae6ba560 removed the shape error but a quick review of the logic from a maintainer would be lovely. Happy to submit PR after. Thanks!

Comments:
Patch: https://github.com/huggingface/transformers/commit/a317323414387a25b8fcf4398494c4cf5f12afdc

Hi @chenzizhao, thanks for raising this and providing links to the patch and relevant commits! 

Would you like to open a PR with these changes? This way you get the github contribution for the solution. 

cc @younesbelkada @gante for reference 

Nice catch @chenzizhao ! 
This is not catched by our CI sadly, we need to work on that ! adding it on my TODOs !
🚨🚨🚨Deprecate `evaluation_strategy` to `eval_strategy`🚨🚨🚨
Author: muellerzr
Pull request: yes

# What does this PR do?

Something I've found rather discouraging when working with the `TrainingArguments` is we have made it known that `eval_X` is a thing, and yet there are some cases of using `evaluation_X` instead.

Case in point:
Eval:
* `eval_steps`
* `per_device_eval_batch_size`
* `eval_accumulation_steps`
* `eval_delay`

Problematic eval:
* `evaluation_strategy`

This PR creates an alias, `eval_strategy`, which aligns to the prior design decisions. I'm not sure a full deprecation is in order, but we are leading users to X and then presenting them Y. We should allow them to do X still for Y, but I don't think a full overhaul is needed (e.g. adding `evaluation_accumulation_steps` etc etc) for one simple flag.

I'll write some tests after we're okay with this being an option.

## 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.

@amyeroberts 

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

This even goes further: in many of the example scripts the dataset is checked for "train", "test", "validation" splits (and not "eval" or "evaluation"), which may be a tad confusing. 

Sounds good @amyeroberts, mostly wanted this PR to make sure if we wanted a blanket-wide `eval` or `evaluation`. (aka, shorthand for everything)

@muellerzr Sounds good. It think accepting both is fine, but we ideally want to prioritise one. As you mentioned, `eval_xxx` for consistency seems like a good choice 

@amyeroberts (still working on getting tests to pass but) what version should we deprecate to? v4.44? Or v5 (Current pypi is v4.39)

6 months sounds great to me!
Fix `AssertionError` in clip conversion script
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix #30037(`AssertionError`) part. The failure comes from the fact that the prepared inputs in not really meaningful (not processed by clip tokenizer).

The conversion is still failing with `saftensors` error

```bash
ValueError: You are trying to save a non contiguous tensor
```
I will ping people once this PR merged.


Full log

```bash
Traceback (most recent call last):
  File "convert_clip_original_pytorch_to_hf.py", line 146, in <module>
    convert_clip_checkpoint(args.checkpoint_path, args.pytorch_dump_folder_path, args.config_path)
  File "/usr/local/lib/python3.8/dist-packages/torch/utils/_contextlib.py", line 115, in decorate_context
    return func(*args, **kwargs)
  File "convert_clip_original_pytorch_to_hf.py", line 136, in convert_clip_checkpoint
    hf_model.save_pretrained(pytorch_dump_folder_path)
  File "/transformers/src/transformers/modeling_utils.py", line 2584, in save_pretrained
    safe_save_file(shard, os.path.join(save_directory, shard_file), metadata={"format": "pt"})
  File "/usr/local/lib/python3.8/dist-packages/safetensors/torch.py", line 284, in save_file
    serialize_file(_flatten(tensors), filename, metadata=metadata)
  File "/usr/local/lib/python3.8/dist-packages/safetensors/torch.py", line 488, in _flatten
    return {
  File "/usr/local/lib/python3.8/dist-packages/safetensors/torch.py", line 492, in <dictcomp>
    "data": _tobytes(v, k),
  File "/usr/local/lib/python3.8/dist-packages/safetensors/torch.py", line 406, in _tobytes
    raise ValueError(
ValueError: You are trying to save a non contiguous tensor: `visual_projection.weight` which is not allowed. It either means you are trying to save tensors which are reference of each other in which case it's recommended to save only the full tensors, and reslice at load time, or simply call `.contiguous()` on your tensor to pack it before saving.
```


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30321). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Avoid `jnp` import in `utils/generic.py` 
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix #30226 to avoid jax being imported when it is unnecessary

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

I take it as a ✅  and merge :-)

@ydshieh Yep, sorry, it was meant to be an approve!
Loss calculation bug in BartForCausalLM
Author: willxxy
Pull request: no

### System Info

transformers==4.39.1
python==3.8.18

### Who can help?

@ArthurZucker and @younesbelkada

### 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

The gist of the issue is found in this [issue](https://github.com/huggingface/transformers/issues/27517), where the primary problem is the way that the loss is being calculated in the BartForCausalLM module in modeling_bart.py file.

The current implementation of the loss is like so 

```
loss = None
if labels is not None:
     labels = labels.to(logits.device)
     loss_fct = CrossEntropyLoss()
     loss = loss_fct(logits.view(-1, self.config.vocab_size), labels.view(-1))
```

This I believe is the mlm loss calculation and not aligned with the causallm loss calculation.


### Expected behavior

I am not entirely sure but I wanted confirmation from the quick fix that was shown in [issue](https://github.com/huggingface/transformers/issues/27517).

It seems like in the forward function of BartForCausalLM in modeling_bart.py instead of calculating the loss like this 

```
loss = None
if labels is not None:
     labels = labels.to(logits.device)
     loss_fct = CrossEntropyLoss()
     loss = loss_fct(logits.view(-1, self.config.vocab_size), labels.view(-1))
```

they switched it out with this

```
loss = None
if labels is not None:
    labels = labels.to(logits.device)

    # Shift so that tokens < n predict n
    shift_logits = logits[..., :-1, :].contiguous()
    shift_labels = labels[..., 1:].contiguous()

    loss_fct = CrossEntropyLoss()
    loss = loss_fct(shift_logits.view(-1, self.config.vocab_size), shift_labels.view(-1))

```

Additionally, the labels in the modified causal loss case would be similar to that of Llama where the labels would be the same as the inputs since we are shifting them in the loss calculation.
I wanted to confirm whether this is the correct way to do this. 

Thank you!

Comments:
Llama is probably up to date, but changing this today would be breaking 🤗 

Ok got it. Thank you!
[UDOP] Add special tokens to tokenizer
Author: NielsRogge
Pull request: yes

# What does this PR do?

This PR makes sure the 1201 additional special tokens can be encoded/decoded properly.

Fixes #29591  

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

Thanks, I've added tests. Should we add `spaces_between_special_tokens` as argument to `UdopTokenizer`, to make it decode the same as the fast tokenizer?

You can add it and default it to `False` in the `decode` maybe? Cuz I don't think `decode` takes `self. spaces_between_special_tokens` into account no? 

Yeah indeed, shouldn't the `_decode` method [here](https://github.com/huggingface/transformers/blob/caa5c65db1f4db617cdac2ad667ba62edf94dd98/src/transformers/tokenization_utils.py#L1026) take into account `self. spaces_between_special_tokens` in case it exists?



I don't think it's a breaking change, since adding `spaces_between_special_tokens` as an attribute doesn't do anything. The `_decode` [method](https://github.com/huggingface/transformers/blob/e4ea19b958c89d61e42461fac6ac8441787121f8/src/transformers/tokenization_utils.py#L991-L997) doesn't use it (whereas I think it should).

If it does not do anything why are we adding it? 

Haha yes I'll remove it, and I'll remove it for Gemma too
Test datasets@main for datasets-2.19.0 release
Author: albertvillanova
Pull request: yes

This PR tests that the transformers CI is green with datasets@main before we make the datasets-2.19.0 release.

I had to open a PR because I do not have right to push the branch to upstream:
```shell
$ git push -u upstream test-datasets-main-for-2.19.0-release
ERROR: Permission to huggingface/transformers.git denied to albertvillanova.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
```

TODO for the next release:
- Ask @huggingface/transformers team for rights to push to this repository

TODO: Rerun CI once these merged
- [x] https://github.com/huggingface/datasets/pull/6822
- [x] https://github.com/huggingface/transformers/pull/30319

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

The CI is red. 

@huggingface/datasets, any ideas why? I am not able to reproduce the issues locally.

Hmmm let me investigate a bit

(btw using PR is maybe fine to run tests actually)

Ok I found the issue - opening a PR in `datasets`

So the issue came from a new feature in `datasets` that reuses the exported `Features` from the Viewer.

This test dataset had erroneous exported `Features` in the Viewer and re-running the job fixed it.
I still opened https://github.com/huggingface/datasets/pull/6822 in `datasets` to avoid future issues when re-running jobs.

Feel free to re-run the tests once this PR is merged

There is still a test that does not pass (https://app.circleci.com/pipelines/github/huggingface/transformers/89988/workflows/70199005-4b9b-4b1d-a0cc-02dbe72f2845/jobs/1173541) due to the breaking change introduced by this PR:
- https://github.com/huggingface/datasets/pull/6739

I think we should have started a deprecation cycle instead, to give users time to adapt their codes. What do you think @huggingface/datasets?

I have opened a PR to fix the test:
- #30319

It's more a bug fix that something breaking @mariosasko no ?

Yes, I think we can consider this a bug fix (the EXIF orientation tag should not be ignored in ML workflows), which means the deprecation cycle is not needed.

I am merging the main branch once #30319 is merged.

The CI is green. I am making the release of datasets.