noamaanMulla-03/transformers-issues huggingface dataset:
Fix gradient checkpointing + fp16 autocast for most models
Author: younesbelkada
Pull request: yes

# What does this PR do?

This PR fixes a bug users can encounter when using gradient checkpointing under fp16 autocast context manager. Currently if a user trains a model using autocast and GC the last layer's weights will never get updated. 

<details><summary>Handy reproducible snippet</summary>

```python
import torch
from transformers import AutoModelForCausalLM

model_id = "facebook/opt-350m"

model = AutoModelForCausalLM.from_pretrained(model_id).to(0)
model.gradient_checkpointing_enable()
model.train()

assert model.training and model.is_gradient_checkpointing

optimizer = torch.optim.Adam(model.parameters(), lr=1e-3)
with torch.cuda.amp.autocast(True, dtype=torch.float16):
    dummy_input = torch.LongTensor([[0, 1, 0, 1]]).to(0)
    model.train()
    logits = model(dummy_input).logits
    loss = logits.sum()

    loss.backward()
    optimizer.step()

    for n, param in model.named_parameters():
        if param.grad is None:
            print(n)
```
</details>

As discussed internally, the fix seems to be to force-set `use_reentrant=False` when calling the gradient checkpointing. Putting that boolean to False lifts the restriction that the input tensors initially need to have if `use_reentrant=True` - according to PT team  `use_reentrant=True` led to some silent bugs and they are planning to remove that boolean in the next releases and use `False` by deafault.

This might be problematic for users that train adapters (using PEFT for example) where they will see some training performance downside. I propose a PoC to fix this for most common architectures until PyTorch remove that support for the next releases.

For more context, users that train models using PEFT end up using autocast inside the trainer as they use 4bit / 8bit base models


Related: #23990 

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Clean up dist import
Author: muellerzr
Pull request: yes

# What does this PR do?

Cleans up the `torch.distributed.X` imports in `training_args` to use the already imported `dist` module, which helps clean up our logic and cases quite a bit.

Fixes # (issue)


## 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.

@amyeroberts (cc @sgugger )


Comments:
_The documentation is not available anymore as the PR was closed or merged._
Check auto mappings could be imported via `from transformers`
Author: ydshieh
Pull request: yes

# What does this PR do?

As shown in #24364, we easily forget to add model mappings like `TF_MODEL_FOR_AUDIO_CLASSIFICATION_MAPPING` to some `__init__` files.

Let's add a check to avoid and could detect such issues as early as possible.

Along this new check, also add some missing mappings to `__init__` files.

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Remove redundant code from TrainingArgs
Author: muellerzr
Pull request: yes

# What does this PR do?

Removes some more redundant code that Accelerate can handle directly. Namely:

- World size
- Process index
- `main_process_first`

Fixes # (issue)


## 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 @pacman100 

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Explicit arguments in `from_pretrained`
Author: ydshieh
Pull request: yes

# What does this PR do?

[still incomplete]

Need to apply the same changes to other files containing `from_pretrained` (other framework, other classes like config, processor, auto, etc.)  but @sgugger let me know if I am not lost already in the early stage.

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

TODO:
 - for TF/Flax model `from_pretrained`
 - for tokenizer/processors
 - for auto

@sgugger Would be nice if you can take a quick look 🙏 . And do you want me to deal with all framework (TF/Flax), tokenizer/processor, and also `auto` in this PR, or I am allowed to separate them ..?

You have a lot of tests failing to fix 😅 , sure you want a review yet?

@sgugger No, I didn't request a new review since last time you have a look. But the changes pushed triggered you 😆
[Trainer] Fix optimizer step on PyTorch TPU
Author: cowanmeg
Pull request: yes

# What does this PR do?
Update optimizer step for TPUs to user `self.optimizer.step()` instead of `xm.optimizer_step(self.optimizer)`. 
AcceleratedOptimizer properly calls `xm.optimizer_step` on the optimizer (https://github.com/huggingface/accelerate/blob/main/src/accelerate/optimizer.py#L129). 

This fixes a bug in transformers/trainer.py when using Pytorch on TPUs:
File "/usr/local/lib/python3.8/dist-packages/torch_xla/core/xla_model.py", line 471, in _fetch_gradients for param_group in optimizer.getstate()['param_groups']: KeyError: 'param_groups'


## 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 



Comments:
cc @muellerzr and @pacman100 

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

Thank you @cowanmeg for the fix!
Fix accumulation by epoch with Accelerate
Author: muellerzr
Pull request: yes

# What does this PR do?

Should solve accumulating via epoch when using Accelerate (seen in https://github.com/huggingface/transformers/issues/23935#issuecomment-1588134562). Requires https://github.com/huggingface/accelerate/pull/1624

Fixes # (issue)


## 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.

@pacman100 

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

Solved the problem for me with small naming fix in trainer.py:
```python
        accumulation_plugin = GradientAccumulationPlugin(
            num_steps=self.args.gradient_accumulation_steps, sync_with_dataloader=False
        )
```
(field name is num_steps not gradient_accumulation_steps)

We can close this one, as I have added a missing edge case to #24415
Support whisper-timestamped
Author: jozefchutka
Pull request: no

### Feature request

It would be great if [whisper-timestamped](https://github.com/linto-ai/whisper-timestamped) could be added into Transformers.

### Motivation

whisper-timestamped is an extension of [openai-whisper](https://pypi.org/project/whisper-openai/) python package and is meant to be compatible with any version of openai-whisper. On top of openai-whisper it provides word timestamps and give more accurate estimation of speech segments when transcribing.

This is suitable for karakoe style subtitles etc.

### Your contribution

Probably unable to help with this at the moment.

Comments:
We've been thinking about how to add word-level timestamps to Whisper. I still have to look at whisper-timestamped to see exactly what they're doing, but for now I'll reference https://github.com/huggingface/transformers/issues/21412 as the main issue for tracking this.

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.

Probably this one can be closed as work on https://github.com/huggingface/transformers/pull/23205 will deliver the feature

Closed by https://github.com/huggingface/transformers/pull/23205
Generate: PT's `top_p` enforces `min_tokens_to_keep` when it is `1`
Author: gante
Pull request: yes

# What does this PR do?

Fixes #23688 

Contrary to our description in the docstring, PT's `top_p` was not enforcing `min_tokens_to_keep` when it was 1 (the default). TF and FLAX were fine. This PR corrects it, and adds a check on `min_tokens_to_keep` (must be a non-negative integer)

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

> top_p was not enforcing min_tokens_to_keep when it was 1 

From the diff - I don't see how this is resolved. The checks ensure the value of `min_tokens_to_keeps` but doesn't seem to be conditional on `top_p`. Am I missing something? 

@gante I hit an issue related to this in the prior version of transformers, glad to see that it's fixed thanks! However why don't we enforce `min_tokens_to_keep >= 1`. 0 makes no sense right?

@njhill true, the initial check should be against `>=1`, patching it
TF safetensors reduced mem usage
Author: Rocketknight1
Pull request: yes

When we load safetensors files in TF, the entire safetensors state dict is materialized on GPU alongside the randomly initialized weights. This inflates our memory usage during loading a lot, up to about 2X - 2.5X the amount we actually need.

This PR grabs tensors iteratively from the underlying safetensors archives and assigns them. ~It's working for TF-formatted safetensors archives right now, and I'll add torch-format support next.~ Now supports PT and TF formatted archives! Load times still seem very quick for me locally, so I don't think this negatively impacts anything!

Fixes #24393 

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

This looks good in my testing! cc @Narsil for review and @amyeroberts for core maintainer review to save Sylvain from having to read code while he's distracted by the hallucinatory dental anaesthetic goblins 

To explain one detail: In general, all TF ops can take NumPy inputs. Often the "idiomatic" way to write TF code is to do preprocessing/once-off stuff in NumPy and just pass the result to TF. Since Safetensors have very efficient Numpy loading, I generally open them in `"np"` format and let TF handle any necessary conversions.

The one exception is when loading a PyTorch state dict archive. The reason here is that PyTorch weights often need to be transposed to load them into TF, and NumPy transposes on CPU are much slower than TF transposes on GPU. Model loading was several seconds slower when I passed `"np"` format archives to the PyTorch state dict crossloading function, but with GPU transposes this PR has almost no impact on performance, while hugely reducing peak memory usage.

Yeah, actually, I thought about it a little more - using `np` is idiomatic for TF, but it's a bit messy if the PyTorch crossloading uses `tf` for fast GPU transposes and the others use `np`, especially when it doesn't really matter. I'll use `tf` globally for consistency!

Just ran the slow tests for a few models that load safetensor checkpoints and this still seems good

Finished slow testing - performance and memory usage look good!

Everything looks good at this point - any objections to merging?
[`bnb`] Fix bnb serialization issue with new release
Author: younesbelkada
Pull request: yes

# What does this PR do?

Fixes the failing bnb tests in: https://github.com/huggingface/transformers/actions/runs/5329457026/jobs/9655258851

The recent release of bitsandbytes slightly broke the serialization mechanism for int8 models. In the new release, bitsandbytes has introduced a new way of serializing int8 weights that are more memory efficient and avoid OOM issues when saving for instance PEFT models.

https://github.com/TimDettmers/bitsandbytes/pull/503 

That PR introduced a new paradigm, when saving int8 state dict, [that state dict will contain some string values](https://github.com/TimDettmers/bitsandbytes/pull/503/files#diff-4d235c7e595546c6656c229dfa139298ce6602b356c2d0bafcb2352eb2cfae79R360-R363) to store some metadata information related to the quantized format.

Therefore the fix should be to slightly adapt the `shard_checkpoint` method (that is called regardless of if the model is sharded or not) by adding a new argument `state_dict_contains_metadata` that will skip manipulating the `weight` variable that is no longer a tensor but a string. We constraint `state_dict_contains_metadata` to be applicable only in the int8 case to ensure we don't break anything else 

cc @amyeroberts 

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

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_24416). All of your documentation changes will be reflected on that endpoint.
Enable non-causal mask (to enable MLM) for VisionEncoderDecoder models
Author: metemadi
Pull request: no
Labels: name: Feature request

### Feature request

Hello! The current (amazing!) VisionEncoderDecoder library supports text generation via a standard causal LM. Some recent work (linked [here](https://arxiv.org/abs/2306.07915)) has shown promise in having the text decoder be a MLM instead of a causal LM. I believe this is doable with the current VisionEncoderDecoder library by passing in [MASK] tokens for the decoder_input_ids and passing in the labels as usual, but this would still result in a causal mask. The code comment is as follows which makes me think this:
```
decoder_attention_mask (`torch.BoolTensor` of shape `(batch_size, target_sequence_length)`, *optional*):
            Default behavior: generate a tensor that ignores pad tokens in `decoder_input_ids`. Causal mask will also
            be used by default.
```
Is there a way to turn off causal masking to predict multiple text tokens at once using a VisionEncoderDecoder model?

### Motivation

Masked language modeling on top of a Vision encoder appears to be a promising new approach for image captioning and pre-training of vision models according to [this recent work](https://arxiv.org/abs/2306.07915).

### Your contribution

Thank you!

Comments:
Hi @metemadi, thanks for opening this issue! 

This sounds like an interesting project! I believe there's a few places that would need to be adapted in order to enable this properly, such as not forcing `add_cross_attention` to the decoder config and not shifting tokens (cc @ydshieh). The VisionEncoderDecoder model is not intended to be compatible with all encoder-decoder pairs or use cases. This isn't something we'll add to the library at the moment, but feel free to share a fork branch with an implementation here if you'd like!

Thank you for the insanely fast reply - HuggingFace is amazing as always! This all makes sense. Thanks again.

Sorry, I forgot to reply:

There is however `class VisionTextDualEncoderModel`. One checkpoint on the Hub is [clip-italian](https://huggingface.co/clip-italian/clip-italian). If you look the config file, it uses `BertForMaskedLM` and `clip_vision_model`.

It might be helpful, but some slight modification might be necessary if the goal is to do what have been done in the paper you mentioned.
Fix `save_cache` version in `config.yml`
Author: ydshieh
Pull request: yes

# What does this PR do?

In #22204, I changed the `restore_cache` to `v0.6` but forgot to change `save_cache`. In consequence, no cache is saved/loaded, and the 2 jobs spend 5 minutes to install things:

<img width="716" alt="Screenshot 2023-06-22 101905" src="https://github.com/huggingface/transformers/assets/2521628/c796d958-ef9c-4066-bb46-3009be7a8fcc">


This PR fixes this and save money/credit we spend on CircleCI .... Please don't punish me 😭 

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Create Pretrained module
Author: Ntrystan
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 # (issue)


## 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.
- [x] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker 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:
Invalid hyperlink error 404 in Transformer Doc for RayTune
Author: JoshuaEPSamuel
Pull request: no

Greetings,

Small issue here: following the HuggingFace Transformer Docs on Hyperparameter Search using Trainer API, for raytune the hyperlink for 'object_parameter' is now invalid and should be updated.

The other backends (sigopt, optuna, wandb) have correctly working hyperlinks for the 'object_parameter'.

Link to doc page:
https://huggingface.co/docs/transformers/hpo_train#how-to-enable-hyperparameter-search-in-example

Link to git markdown file:
https://github.com/huggingface/transformers/blob/main/docs/source/en/hpo_train.mdx

Comments:
feel free to open a PR to fix if you can:)
TF CI fix for Segformer
Author: Rocketknight1
Pull request: yes

This very small PR rewrites a couple of reshapes so the TF compiler can figure out the channels dim for Segformer even when some input dimensions are undefined. Should fix any CI issues the model has been having.

Comments:
Ah, I know, I just thought you might still be unconscious!

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