noamaanMulla-03/transformers-issues huggingface dataset:
unclear `prepare_seq2seq_batch` deprecation
Author: stas00
Pull request: no

When using `prepare_seq2seq_batch` the user now gets:

> transformers-master/src/transformers/tokenization_utils_base.py:3277: FutureWarning: `prepare_seq2seq_batch` is deprecated and will be removed in version 5 of 🤗 Transformers. Use the regular `__call__` method to prepare your inputs and the tokenizer under the `with_target_tokenizer` context manager to prepare your targets. See the documentation of your specific tokenizer for more details.

It's very hard to act on as, I'm not sure what "regular `__call__` method" refers to and I could find any tokenizer documentation that ever mentions `with_target_tokenizer`.

Perhaps this is an unintended typo? was it meant to be `with target_tokenizer`? `with FooTokenizer`?

Please kindly suggest a more user-friendly deprecation and at least one example or a link to such.

Thank you.

@sgugger, @LysandreJik 

Comments:
Why is `__call__` hard to understand? It's the regular Python method for when the tokenizer is called directly on inputs. How would you formulate that better?

For the `with_target_tokenizer` it's a typo indeed, it should be `as_target_tokenizer`.

As for an example, this is what is used in every example script, see for instance the [run_translation](https://github.com/huggingface/transformers/blob/9adff7a0f49f88a6cc718a1d30088988dc78bb6a/examples/pytorch/translation/run_translation.py#L414) script. 

I'm curious, where did you still find a reference to this method? It's been removed from all examples and documentation normally (and has been deprecated five months ago).

It's totally obvious once I see an example that I was now able to find since you gave the correct context manager name, it is so not obvious from the warning message. Moreover, none of the tokenizers document that as suggested by the warning's message. They do document the specifics of its usage.

I made an attempt at another version here: https://github.com/huggingface/transformers/pull/12669

> I'm curious, where did you still find a reference to this method? It's been removed from all examples and documentation normally (and has been deprecated five months ago).

In several of the scripts I used in the past to make tiny models.

I'm curious in turn why was this wrapper deprecated? To make things more explicit? Looks like a lot more code to write instead of the wrapper.


I stumbled upon this issue when googling the warning. For the translation task this
`tokenized_text = tokenizer.prepare_seq2seq_batch([text], return_tensors='pt')`
has to be replaced by this:
```
with tokenizer.as_target_tokenizer():
    tokenized_text = tokenizer(text, return_tensors='pt')
```
Which is much clearer than using `prepare_seq2seq_batch`, but for anyone coming from other languages but python, the concept of `__call__`  might not be transparent in first place :)

I'm getting the same text, not the translated one when I change from `prepare_seq2seq_batch` to `as_target_tokenizer`
[test] add test for --config_overrides
Author: stas00
Pull request: yes

https://github.com/huggingface/transformers/issues/14389 suggested that `--config_overrides` doesn't work.

The feature works just fine, it's just the multiple logging of the config done by the framework is confusing. I already flagged this issue here: https://github.com/huggingface/transformers/issues/11104 I have no idea why loading a tokenizer triggers dumping of model config - as its contents are mostly irrelevant to the tokenizer and surely doesn't contribute anything useful to the user, other than avoiding looking at the log completely.

So there will be no confusion I added an additional dump with the updated config (terrible!) and a test so that we don't accidentally break this feature.

I'm not sure how else to improve that other than revisiting the design of when the model config is dumped. IMHO:
1. it's dumped too soon for the model (before it can be updated)
2. it shouldn't be dumped at all for the tokenizer

Fixes: https://github.com/huggingface/transformers/issues/14389

@sgugger, @LysandreJik 

Comments:
[deepspeed integration] HF Trainer takes over GPUs for DP
Author: stas00
Pull request: no
Labels: name: DeepSpeed

on multi-gpu machine when `deepspeed` launcher is used w/o the `--num_gpus` arguments to run HF/DS integration, HF Trainer takes over and does DP over all available GPUs, which defeats the purpose of using deepspeed to fit a larger than GPU model.

Normally when no `--num_gpus` argument is passed, deepspeed automatically uses all gpus, so here we have a conflict for those users who are used to that automatic behavior.

So currently we need to always use something like `deepspeed --num_gpus 2` explicitly which tells HF Trainer not use DP over those GPUs.

The proper solution is to code that when `--deepspeed` is passed HF Trainer won't launch DP.

Thanks to Lintang Sutawika for reporting this.

Comments:
Odd, I can't reproduce this. 

on 2 gpus with `deepspeed --num_gpus 2` or just `deepspeed` I get the same world_size=2 inside Trainer, so it shouldn't make any difference.

So I'm going to close this for now. we can re-open this if someone reproduces the issue.
fixes some key names for in LayoutLMv2 / LayoutXLM tokenizers
Author: valentinkoe
Pull request: yes



# What does this PR do?

in case of left padding_side there was a (probably copy/paste) error assigning the bbox data to the labels.
I consider this to be a typo fix.


## 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?

I guess @sgugger as you reviewed https://github.com/huggingface/transformers/pull/12604
cc @NielsRogge 


Comments:
Support saving (and loading) models to a remote bucket
Author: oriram
Pull request: no

# 🚀 Feature request

Support saving and loading models to and from remote buckets, e.g. Google Cloud Storage and Amazon S3.

## Motivation

Currently this is not supported, making it more difficult to train in AWS/GCP.


Comments:
For reference, pandas leverages `s3fs` and `gcsfs`. A generic implementation is available through `fsspec.open()` (see [docs](https://filesystem-spec.readthedocs.io/en/latest/usage.html#higher-level)). Would the maintainers be open to that? I would be happy to contribute.

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.

This seems to be a useful feature. any thoughts about implementation of this?

[Hugging Datasets](https://huggingface.co/docs/datasets/filesystems.html) supports this feature. It really looks like a useful feature that the PreTrainedModel doesn't support today
Doc fixes
Author: sgugger
Pull request: yes

# What does this PR do?

This PR adds multiple fixes for the new frontend, mainly:
- remove all the sphinx-specific files
- convert the old index.rst to index.mdx
- adapt the scripts that touched the index.rst (to update the list of models and the table of frameworks)
- fix the image links in the parallelism.md file
- convert the detr.rst file to have the table in MarkDown format.

Comments:
[deepspeed] zero inference
Author: stas00
Pull request: yes

This PR extends HF/DS integration to support Deepspeed Zero inference. Now we don't need to waste gpu memory on allocating the optimizer/scheduler and then dropping them. And in some cases enabling what was not possible before - in case the user doesn't have the extra gpu memory and was getting OOM on inference.

Blocking events:
- [x] merged https://github.com/microsoft/DeepSpeed/pull/1514 
- [x] new deepspeed release (want v0.5.7 to be out)
- [x] update dependency table once above is done

@jeffra, @sgugger 

The CI errors seem to be irrelevant to this PR

Comments:
Thanks a lot for the review and the suggestions, Sylvain. All addressed, please have another look at your convenience. Thank you.
Enabling Discussion on github
Author: sadakmed
Pull request: no

# 🚀 Feature request

Enabling Discussion on github 

## Motivation

reserve issues for only feature request, bugs (anything that would be an addition to the transformers library). While, discussion will be for guidance and questions.
I know that this is the use of [discuss huggingface](https://discuss.huggingface.co/c/transformers/9). However, I think large portion of users interact just from github, it makes sense to have issues not include general question and guidance (**false issues**) and you as transformers maintainers will relate to the amount of notifications from it.


Comments:
Good idea!!

Hello @sadakmed, thank you for your proposal! As you have mentioned, we have a forum that is already active. Rather than opening a new communication channel that would split interaction between the forum and the GitHub discussions, we'd rather stay focused on having only the forum.

Does that make sense?

cc @sgugger 

Yes, we do not want to duplicate the communication channels and I think it would just be too confusing for everyone to have the same discussions in two different places. The forums via Discourse were chosen as they are ultimately better integrated with the rest of the Hugging Face website and model Hub (same login, easy going from one to the other) whereas the Discussions on GitHub did not offer that feature.

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.
[CI] clear `~/.cache/torch_extensions` between builds
Author: stas00
Pull request: yes

This PR is trying to address CI failures with pt-nightly. https://github.com/huggingface/transformers/runs/4280926354?check_suite_focus=true

`~/.cache/torch_extensions/` currently uses a single hardcoded path to install all custom cuda extensions and so when it was built with pt-1.8 but then attempted to be used with pt-nightly (pt-1.11-to-be), the following happens:

```
ImportError: /github/home/.cache/torch_extensions/py38_cu111/cpu_adam/cpu_adam.so: undefined symbol: curandCreateGenerator
``` 

pt-1.10 has improved the situation by adding a prefix: `~/.cache/torch_extensions/py38_cu113` which makes the builds not shared between different cuda and python versions, but it missed the crucial pt-version in that prefix. I reported the issue here:
https://github.com/pytorch/pytorch/issues/68905

And of course ideally all the builds should be installed into the virtual python environment and not have a global shared dir.

This PR tries to address the issue by wiping out `~/.cache/torch_extensions/` completely when CI starts.

This of course means `deepspeed` will rebuild the extensions on every CI run, but this is actually a good thing, because then we really test the right version of it. It does it really fast so it shouldn't introduce a large overhead.

@LysandreJik 

Comments:
[DeepSpeed in notebooks] Jupyter + Colab
Author: stas00
Pull request: yes
Labels: name: DeepSpeed

This PR addresses issues raised in https://github.com/huggingface/transformers/issues/10011 when the user tried to use DeepSpeed in a notebook (most likely colab).

This PR:

* forces device and distributed setup init from TrainingArguments explicitly at the beginning of Trainer's `__init__`. This is needed since until now the init was happening as a side effect of someone calling `device` or `n_gpus`, which doesn't happen if someone runs their own version of `Trainer` w/ deepspeed - which is the case with notebooks - so we are missing out on DeepSpeed init and things weren't working. Let's do it explicitly, and not as a side-effect, so everything is loud and clear.
* sets up `self.local_rank` based on LOCAL_RANK env var under deepspeed to save users a hassle - deepspeed must have `local_rank>-1`. I guess the fake launcher env setup could be folded into the init as well, but then the user loses control over the port number and they may need to edit it, so for now leaving it outside - but will ask deepspeed to provide a wrapper function to make it easy for the user. perhaps once the wrapper is available it could be automated completely. Alternatively, if they make `mpi4py` a dependency then fake launcher env setup won't be needed at all.
* documents how to run DeepSpeed in the notebook env
* adds a test that mocks a notebook environment and runs deepspeed w/o a launcher

I may wait to hear about the follow up to https://github.com/microsoft/DeepSpeed/issues/748 to merge this, if it looks quick, but otherwise I will revise the setup doc in a future PR. The main changes of this PR besides the doc are required anyway.

@sgugger 

Comments:
[flax] unfreeze initial cache in gpt models
Author: patil-suraj
Pull request: yes

# What does this PR do?

Fix flax `generate` for GPT models when the initial `seq_len` is 1.

The issue is the init_cache method of flax GPT2 returns the cache as a `FrozenDict`, but the model’s forward returns cache as a `dict`.

It works with seq_len > 1 because, when seq_len > 1, we call the body fun outside of the while loop -> body calls forward -> which returns cache as a `dict`.

then we iterate over `body_fn`, using `lax.while_loop`, and it works as the type signature of `cache` is similar.


It breaks for seq_len = 1 because, when it’s one we directly call the `body_fn` with `lax.while_loop` , so here the initial type of cache is `FrozenDict` but the forward in `body_fn` returns `dict`, which raises this error

```body_fun output and input must have same type structure, got PyTreeDe...```

cc @Narsil 

Comments:
cannot import name 'DataCollatorForSeq2Seq' from 'transformers'
Author: QzzIsCoding
Pull request: no

## Environment info

- `transformers` version:  4.13.0.dev0
- Platform: Ubuntu
- Python version: 3.7
- PyTorch version (GPU?): 1.10.0


### Who can help
@sgugger

## Models:
- encoder-decoder models


## To reproduce

Steps to reproduce the behavior:
`git clone https://github.com/huggingface/transformers.git`
`cd transformers`
`pip install -e .`

![image](https://user-images.githubusercontent.com/56379157/143610360-fc75a023-6a03-4cc2-9532-2fc4a684e5a7.png)
![image](https://user-images.githubusercontent.com/56379157/143610551-03701cde-8527-4df8-a7cb-e4541eb59aa4.png)
The class  DataCollatorForSeq2Seq  is  in  data_collator.py.   I don't know why this error occurs.   Could you help me to solve it?




Comments:
After restarting jupyter, the problem is solved.
Unable to load DeBERTa-v3 tokenizer
Author: maiiabocharova
Pull request: no

```
from transformers import AutoTokenizer
  
tokenizer = AutoTokenizer.from_pretrained("microsoft/mdeberta-v3-base")
```
Gives me an error:
ValueError: This tokenizer cannot be instantiated. Please make sure you have `sentencepiece` installed in order to use this tokenizer.
Installing sentencepiece doesn't help.


Comments:
Hello! Could you please share your environment information? Thanks.

> Hello! Could you please share your environment information? Thanks.

Hello, sure! transformers==4.12.5, python 3.7. I am using Colab for experiments

If you're using colab, then have you restarted the runtime after installing `sentencepiece`?

> If you're using colab, then have you restarted the runtime after installing `sentencepiece`?

Thank you!!! It solved the issue. Thank you once again!

Glad to hear it!
Add TAPAS trained on NQ
Author: xhluca
Pull request: no
Labels: name: New model

The authors of the "Open Domain Question Answering over Tables via Dense Retrieval" papers released the weights for TAPAS retriever trained on NQ here: https://github.com/google-research/tapas/blob/master/DENSE_TABLE_RETRIEVER.md

It would be nice if those weights could be converted to Huggingface model (I would presume it's fairly similar the other finetuned model since they share the same architecture, and I'd be happy to do it myself if there's some scripts I can run)

Comments:
Hi,

Yes you can use the existing conversion script: https://github.com/huggingface/transformers/blob/master/src/transformers/models/tapas/convert_tapas_original_tf_checkpoint_to_pytorch.py

Note that there was already an attempt for this in #11907.

Thanks @NielsRogge! I tagged you in #11907 about some differences between the current script and the new retrieval weights.

In the mean time I ran the scripts and uploaded the weights to [huggingface](https://huggingface.co/xhlu/tapas-large-finetuned-nq-hn). If it turns out `bert_1` was something important to add, I'm happy to run an updated script and push the changes to the repo. 

Note I also uploaded a `Wq.pt` and `Wt.pt` as well so they can be loaded separately (otherwise, I think I would need to modify the underlying code of the TAPAS model; lmk if you know a simpler way)

Hi,

`bert_1` is important, yes. The retriever consists of 2 encoders, one to encode the question, one to encode the table. The authors released the retriever checkpoints as "dual encoders", meaning they contain the weights of both encoders.

The encoders each correspond to `TapasModel` in Huggingface, with an optional additional projection layer on top (see the "down project" column which is yes/no).

The reader on the other hand corresponds to `TapasForQuestionAnswering`, however it should produce span indexes and logits as output, which is not yet implemented.

Thank you for expanding on this! It's not clear whether `bert_1` corresponds to the table or the question encoder unfortunately. The model I uploaded only uses `bert`, I'm happy to update the name of the repo to the correct version (question or table encoder) once this is disambiguated.

Moreover, I'm not sure about the best way to add an extra layer to a huggingface model, hence I saved it as a supplementary `Wq.pt` file inside the repo (that can be easily downloaded by `requests` get)
fix bug for trainer_seq2seq, generation_inputs should be a dict befor…
Author: TranSirius
Pull request: yes

…e sending to model.generate (Arround Line 165 - Line 185)

# Fixing Bug

<!--
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)

In `trainer_seq2seq.py / Seq2SeqTrainer / prediction_step`, Line 174 - Line 177 read:

```python
generated_tokens = self.model.generate(
    **generation_inputs,
    **gen_kwargs,
)
```

which require the generated_tokens to be a `dict`. However, in the `else` branch in Line 171, the `generation_inputs` is created as a `Tensor` object, which will cause a problem.

Fix this by creating `generation_inputs` as a dict, and add a key called `input_ids`.

Comments:
`EncoderDecoderModel` `generate` for a `ViT` as encoder
Author: sachinruk
Pull request: no

I was wondering if there was a way to do `generate` for a ViT to GPT2 `EncoderDecoderModel`. I managed to figure out how to get the loss by using the outputs of the ViT and pushing in the `encoder_outputs` into the model as shown below. However, it seems that for `generate` it is explicitly expecting `inputs_ids`. I'm fairly certain that somewhere under the hood all you need is just the `encoder_outputs` (and `inputs_ids` is unnecessary in that case). Is there a way to do this?

Also I realise that there is a `VisionEncoderDecoderModel` but I am trying to do this as a learning exercise.

```python
vit2gpt2 = EncoderDecoderModel.from_encoder_decoder_pretrained(VIT_MODEL, DISTIL_GPT2)

tokenized_captions = gpt2_tokenizer_fn(captions)
labels = tokenized_captions["input_ids"].clone()
labels[tokenized_captions["attention_mask"]==0] = LABEL_MASK
encoder_outputs = vit2gpt2.encoder(pixel_values=images)
outputs = vit2gpt2(
    encoder_outputs=encoder_outputs,
    decoder_input_ids=tokenized_captions["input_ids"],
    decoder_attention_mask=tokenized_captions["attention_mask"],
    labels=labels,
    return_dict=True,
)
```
[Here](https://www.kaggle.com/sachin/vit-to-gpt2-encoder-decoder-model) is a kaggle kernel to a runnable version of above snippet.

## Update 1:
So I've managed to narrow down my necessary search down to `generation_utils.py` but I cannot see where in there it loops over the predicted values and feeds it back into the model. I'm hoping to replicate the process from there.

Comments:
Maybe of interest to @NielsRogge even if the question would be better asked on the forum :)

Sorry, I'll close this here, opened the issue [in the forum](https://discuss.huggingface.co/t/encoderdecodermodel-generate-text-for-a-vit-as-encoder/12332).
Cannot find 'blob' directory in your 'transformers' repository
Author: parkhaemi
Pull request: no

We are reading the paper 'EVALUATION OF NEURAL ARCHITECTURES TRAINED
WITH SQUARE LOSS VS CROSS-ENTROPY IN CLASSIFICATION TASKS' which is published as a conference paper at ICLR 2021.
In 13page, we found that 'https://github.com/huggingface/transformers/blob/master/examples/text-classification/run_glue.py'. is no longer accessed. 

Comments:
It is here, https://github.com/huggingface/transformers/blob/master/examples/pytorch/text-classification/run_glue.py @parkhaemi 

Thank you so much!!
Have a nice day :)
TAPAS tanh activation on the pooling layer
Author: xhluca
Pull request: no

I noticed the following in the TAPAS pooling layer:

https://github.com/huggingface/transformers/blob/d83b0e0c079f0826d186270a86622ff5f1efd9c1/src/transformers/models/tapas/modeling_tapas.py#L696-L709

I'm curious about the use of `nn.Tanh()`. I wasn't able to find more information about that activation in [the paper](https://arxiv.org/abs/2004.02349). Is it possible to know where it comes from? Thanks!

Comments:
Hi,

The TAPAS authors borrowed this from the original BERT paper, which decided to apply a tanh layer. 

The BERT author explains why he did that [here](https://github.com/google-research/bert/issues/43).

Ah thanks, you are right. They indeed use tanh in the code: https://github.com/google-research/tapas/blob/f3d9f068e6eedb252883049b582516a1294ff951/tapas/models/bert/modeling.py#L269-L277

Wish it was mentioned in the appendix of the TAPAS paper 🤷 Thanks for clarifying!