noamaanMulla-03/transformers-issues huggingface dataset:
Fix type annotations for `distributed_concat()`
Author: Renovamen
Pull request: yes

# What does this PR do?

Fix the type annotations for [`trainer_pt_utils.distributed_concat()`](https://github.com/huggingface/transformers/blob/91df45516c9c21283df21d564bc352067d8c5f62/src/transformers/trainer_pt_utils.py#L160).

The input of this function could be a `torch.Tensor` or `list` / `tuple`, so I guess the type annotations:

https://github.com/huggingface/transformers/blob/91df45516c9c21283df21d564bc352067d8c5f62/src/transformers/trainer_pt_utils.py#L160

should be:

```python
def distributed_concat(
    tensor: Union[torch.Tensor, Any], num_total_examples: Optional[int] = None
) -> Union[torch.Tensor, Any]:
```


## Before submitting

- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests), Pull Request section?


## Who can review?

@sgugger

Comments:
Done.
incorrect loss calculation
Author: quantitative-technologies
Pull request: no

## Environment info
<!-- You can run the command `transformers-cli env` and copy-and-paste its output below.
     Don't forget to fill out the missing fields in that output! -->

- `transformers` version: 4.11.0.dev0
- Platform: Linux-5.4.104+-x86_64-with-Ubuntu-18.04-bionic
- Python version: 3.7.12
- PyTorch version (GPU?): 1.9.0+cu102 (False)
- Tensorflow version (GPU?): 2.6.0 (False)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: Yes

### Who can help
<!-- Your issue 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:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @Rocketknight1

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

Model hub:

- for issues with a model report at https://discuss.huggingface.co/ and tag the model's creator.

HF projects:

- datasets: [different repo](https://github.com/huggingface/datasets)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->
@sgugger

## Information

This bug report is based on reading the source code.

The training loss appears to be calculated incorrectly when the batch loss is either nan or infinity:

In trainer.py lines 1314--1318: 
```
if args.logging_nan_inf_filter and (torch.isnan(tr_loss_step) or torch.isinf(tr_loss_step)):
    # if loss is nan or inf simply add the average of previous logged losses
    tr_loss += tr_loss / 1 + (self.state.global_step - self._globalstep_last_logged)
else:
    tr_loss += tr_loss_step
```

I believe a pair of parenthesis was forgotten, and it should be:
```
if args.logging_nan_inf_filter and (torch.isnan(tr_loss_step) or torch.isinf(tr_loss_step)):
    # if loss is nan or inf simply add the average of previous logged losses
    tr_loss += tr_loss / (1 + (self.state.global_step - self._globalstep_last_logged))
else:
    tr_loss += tr_loss_step
```

Comments:
Indeed, thanks a lot for reporting. I would normally suggest to let you write a PR to fix this but we have a release planned today and the fix will need to be inside, so I'll fix it and make you co-author of the commit.

Fixed by #13760
Silence warning in gradient checkpointing when it's False
Author: sgugger
Pull request: yes

# What does this PR do?

Currently, when loading a `bert-base-cased` model, one gets the warning 
```
UserWarning: Passing `gradient_checkpointing` to a config initialization is deprecated and will be removed in v5 Transformers. Using `model.gradient_checkpointing_enable()` instead, or if you are using the `Trainer` API, pass `gradient_checkpointing=True` in your `TrainingArguments`.
```
because it's config has the line `"gradient_checkpointing": false`.

This PR only issues the warning only when the value passed for `gradient_checkpointing` is `True`.

Comments:
-100 when calculating perplexity of a model..
Author: HongyuanLuke
Pull request: no

Hi there,

I see in : https://huggingface.co/transformers/perplexity.html
there is a code block saying:


```
max_length = model.config.n_positions
stride = 512

lls = []
for i in tqdm(range(0, encodings.input_ids.size(1), stride)):
    begin_loc = max(i + stride - max_length, 0)
    end_loc = min(i + stride, encodings.input_ids.size(1))
    trg_len = end_loc - i    # may be different from stride on last loop
    input_ids = encodings.input_ids[:,begin_loc:end_loc].to(device)
    target_ids = input_ids.clone()
    target_ids[:,:-trg_len] = -100

    with torch.no_grad():
        outputs = model(input_ids, labels=target_ids)
        log_likelihood = outputs[0] * trg_len

    lls.append(log_likelihood)

ppl = torch.exp(torch.stack(lls).sum() / end_loc)
```


I am wondering why we are setting the tokens to -100. Is it a hard-coded number?

Comments:
-100 is the `ignore_index` of PyTorch's `CrossEntropyLoss`, as explained in their [docs](https://pytorch.org/docs/stable/generated/torch.nn.CrossEntropyLoss.html). It means that labels that are set to -100 to not contribute to the loss.

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.
Why repeat initializing loss modules in every forward?
Author: zhiqiangdon
Pull request: no

Hello,

I find that your implementations usually initialize loss modules, e.g., nn.CrossEntropyLoss, inside models' forward functions. I am curious about the reason of doing this. Generally, in Pytorch, a module should be initialized in __init__ and used in forward. Does the frequent initialization cause overhead and memory issues? 

Thanks,

Comments:
The module `nn.CrossEntropyLoss` does not contain any weights so here we don't allocate any memory really when initializing the module at every forward. However if we would do this with `nn.Linear(...)` at every forward step it should be considered bad practice IMO since in this case we would allocate a big tensor (the weights of the linear layer) at every forward step.

@patrickvonplaten , thanks for the answer. Pytorch has `functional.cross_entropy()`, which should be more suitable to use in forward. Although `nn.CrossEntropyLoss ` doesn't cause overhead, it doesn't follow Pytorch's convention that initializes a module in init and use it in forward. It confused me a little lit when reading the code. I was wondering any specific reason of using a `nn` module instead of a `functional` method in forward.

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.
[model loading] framework-agnostic dtype parameter
Author: stas00
Pull request: no
Labels: name: WIP

This is a split off from one of the discussions at https://github.com/huggingface/transformers/pull/13209:

1. It all started with trying to load torch models under either the desired dtype or the the dtype of the pretrained model - and thus avoid 2x memory usage needs e.g. if the model needs to be just fp16. So we added `torch_dtype` to `from_pretrained` and `from_config`.
2. Then we started storing `torch_dtype` in the config file for future possibly automatic loading model in the optimal "regime".
3. This resulted in a discrepancy where the same symbol sometimes means `torch.dtype` at other times a string like "float32" as we can't store `torch.dtype` in json.
4. then in https://github.com/huggingface/transformers/pull/13209#discussion_r693292542 we started discussing how `dtype` is really the same across pt/tf/flux and perhaps we should just use `dtype` in the config and variables and have it consistently to be a string ("float32") and convert it to the right dtype object of the desired framework at the point of use, e.g. `getattr(torch, "float32")`

A possible solution is to deprecate `torch_dtype` and replace it with `dtype` string both in config and in the function argument.

Possible conflicts with the naming:

1. we already have the `dtype` attribute in modeling_utils, which returns `torch.dtype` based on the first param's dtype.

   https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_utils.py#L205

   The context is different, but still this is something to consider to avoid ambiguity.

I may have missed some other areas. So please share if something else needs to be added.

Additional notes:
- wrt flux: https://github.com/huggingface/transformers/pull/13209#discussion_r694511759
>  #13098 - the idea of the PR is exactly to disentangle parameter dtype from matmul/computation dtype. In Flax, it's common practice that the dtype parameter defines the matmul/computation dtype, see: https://flax.readthedocs.io/en/latest/_autosummary/flax.linen.Dense.html#flax.linen.Dense.dtype instead of the parameter dtype and not the parameter dtype.
> So for Flax, I don't really think it would make sense to use a config.dtype to define weights dtype as it would be quite confusing with Flax's computation dtype parameter.


@LysandreJik, @sgugger, @patrickvonplaten 

Comments:
Would like to ping @Rocketknight1 regarding the TensorFlow management of types, and @patil-suraj for flax

This should work in Tensorflow too - you can use `tf.dtypes.as_dtype(dtype_string)` to turn strings into TF dtype objects.

@Rocketknight1 Sorry, but can you please elaborate on how to load the model in Tensorflow or point me in the right direction? I am new to hugging face and I have been looking all over for instructions on how to do it. Thank you.
Link is broken in Huggingface model zoo
Author: Embedding
Pull request: no

https://huggingface.co/google/bert_uncased_L-2_H-128_A-2

![image](https://user-images.githubusercontent.com/31317254/135032197-876f5513-bbae-4e14-9e9c-8d1f8ad9515c.png)


The link is broken for a long time which is very inconvenient for users. It would be very helpful if you can fix this bug.

Comments:
for @elishowk :)

Hi, thanks for reporting, it's a duplicate of https://github.com/huggingface/transformers/issues/13558
Do you mind if I close it and tag you on the other issue ?
Implement len in IterableDatasetShard
Author: sgugger
Pull request: yes

# What does this PR do?

Currently code using a size iterable dataset in distributed mode will fail because:
- the Trainer will detect the dataset is sized and try to get the length of the DataLoader
- the DataLoader will call the length of its dataset which is an IterableDatasetShard
- the IterableDatasetShard has no length

This PR adds the `__len__` method to `IterableDatasetShard` to solve that issue. The one thing that is midly annoying is that it will make all instance of `IterableDatasetShard` be recognized as `collections.abc.Sized` instances since they do implement the len method, even if that method will return an error. But that check is only used on the dataset passed along to the `Trainer`, not that wrapper, so I think we should be fine.

Comments:
[WIP] Make import simple
Author: shirayu
Pull request: yes

# What does this PR do?

Fix  #13390.
My be related to https://github.com/python/mypy/issues/7045.


## Before submitting
- [ ] 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/master/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/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/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:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @LysandreJik

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @n1t0, @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

HF projects:

- datasets: [different repo](https://github.com/huggingface/datasets)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->


Comments:
Fix LayoutLM ONNX test error
Author: nishprabhu
Pull request: yes

# What does this PR do?

Fixes the batch_size and seq_len computation during ONNX export in configuration_layoutlm.py.

PRs: #13702, #13562
Issue: #13300

## 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/master/CONTRIBUTING.md#start-contributing-pull-requests),
      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.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/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.

@michaelbenayoun @LysandreJik @NielsRogge @mfuntowicz 

Comments:
Hi,

Can you also add LayoutLM to the list of supported models? https://huggingface.co/transformers/serialization.html

Thanks!

Seems good to me!

@NielsRogge I think [he has already done it](https://github.com/huggingface/transformers/pull/13562/files#diff-f4bad33d844c5d91b09dd0af27395ade640e907920b357daf125dd19458bdd60R82), or you're talking about something different?

Yeah I'm talking about the documentation (.rst file): https://github.com/huggingface/transformers/blob/master/docs/source/serialization.rst

It also seems like other models (like mBART) are supported to be exported to ONNX, but they are not mentioned in the docs. I asked @sgugger if we can create an automagically updated table for this.

@NielsRogge I have added LayoutLM to the list of supported models. :)
How to run GLUE tasks on my model?
Author: orenpapers
Pull request: no

I trained a BERT model on my dataset.
Now , I want to run it on GLUE tasks, just to get eval score (no finetuning on GLUE).
Is this possible?

I found this proposed example: 
https://pypi.org/project/pytorch-transformers/#quick-tour-of-the-fine-tuningusage-scripts
but it doesn't explain where I can find the `run_glue.py `script.
I found this link:
https://github.com/huggingface/transformers/blob/master/examples/run_glue.py
But it is broken


Comments:
You can run the `run_glue.py` script, only specifying `--do_eval` (and not `--do_train`).

It's located here: https://github.com/huggingface/transformers/tree/master/examples/pytorch/text-classification

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.
CausalLM vs HeadModel
Author: StellaAthena
Pull request: no

@patrickvonplaten, @LysandreJik @sgugger

GPT-Neo implements the class `GPTNeoForCausalLM` and GPT-2 implements the class `GPT2LMHeadModel`. These look like they're supposed to do roughly the same thing. What is the reasoning behind having different names? Do they have any functional differences (other than using different models obviously)?

Comments:
They are exactly the same `LMHeadModel` was a badly chosen name in the beginning of the library - we are trying to have all causal language models called `...ForCausalLM` now

> They are exactly the same `LMHeadModel` was a badly chosen name in the beginning of the library - we are trying to have all causal language models called `...ForCausalLM` now

I see. FYI, there are already downstream users who are basing their codebases on `LMHeadModel` such as [Google's BIG-Bench](https://github.com/google/BIG-bench/blob/main/bigbench/models/huggingface_models.py). I worry that this disconnect will build significant technical debt if it is not resolved promptly.

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.
fix: keep separate hypothesis for different beam group
Author: ayubSubhaniya
Pull request: yes

# What does this PR do?
It fixes the issue in diverse beam search implementation. <be>

# What was the issue?
By definition, diverse beam search is a variant of beam search which tries to group beams and introduce variance between the groups.

After successful decoding, we select beams from each group thus giving diverse solutions.

For example with beam size 3 and group size 3, we will have 1 beam in each group. And finally, if we want the top 3 suggestions, we should select the top suggestion from each group.

This condition was violated in the current implementation thus giving similar generated sequences in some cases.

Paper for reference [Diverse Beam Search](https://arxiv.org/pdf/1610.02424.pdf)


## Who can review?
@patrickvonplaten Please review changes

Comments:
@ayushtiku5 could you maybe take a look here? :-)

@patrickvonplaten @ayushtiku5 doesn't seem to be available, could you please take a look?

Regarding test failure, It is related to the change itself, there are some assertions on structure of beam hypothesis array, which needs to be changed. Once change seems fine to you I can make test work.

@patrickvonplaten If you need some examples I can try to find them.
I tried to debug this issue on my custom trained weights, it would be hard to find an example on public models.

Thanks for pinging me again on this @ayubSubhaniya! I finally took some time to look a bit deeper into the PR - IMO it's not really a bug, but just a matter of how to define "beam_per_group". If `num_beams` is thought of as the total number of beams for generation then, `num_beams // num_beam_groups` is the number of beams per group. 

It is questionable whether `num_beams` in this case should represent the overall number of beams across all groups or *per* group - if I understand correctly in your opinion in should be per group. I understand this point, but the problem is that making this change now is a big backwards compatibility breaking change: Image all the users using group beam search in their pipelines now that all of a sudden get different results because the meaning of `num_beams` change. 

So, I'd prefer to not merge this PR as IMO it doesn't really fix a bug but just re-interprets the meaning of `num_beams` which is a very public API

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.
Bart: check if decoder_inputs_embeds is set
Author: silviu-oprea
Pull request: yes

In BartForConditionalGeneration.forward, if labels are provided,
   decoder_input_ids are set to the labels shifted to the right.
   This is problematic: if decoder_inputs_embeds is also set,
   the call to self.model, which eventually gets to BartDecoder.forward,
   will raise an error.
   The fix is quite simple, similar to what is there already in
   BartModel.forward. Mainly, we should not
   compute decoder_input_ids if decoder_inputs_embeds is provided.

# What does this PR do?

Fixes #12475

## Who can review?

@patrickvonplaten

Comments: