noamaanMulla-03/transformers-issues huggingface dataset:
[several models] improve readability
Author: stas00
Pull request: yes

Honestly I had no idea what `torch.ones([]) * self.config.logit_scale_init_value` would return - it's not documented either.

~Proposing to change it to a very clear `torch.tensor(1.0)` which leaves no doubt to what it does.~

Proposing to change it to a very clear `torch.tensor(self.config.logit_scale_init_value)` which leaves no doubt to what it does.

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

> size ([int] – a sequence of integers **defining the shape** of the output tensor.

It's actually mentioned, but I agree it's a bit less explicit than `torch.tensor(1)`.

Gentlemen, sleeping more on it, actually it just came to me that the cleanest most readable solution is just:

```
self.logit_scale = nn.Parameter(torch.tensor(self.config.logit_scale_init_value))
```

`torch.ones` isn't even needed ;)

Do you agree?

and yes, I can then fix other places.

ok, I have used:

```
self.logit_scale = nn.Parameter(torch.tensor(self.config.logit_scale_init_value))
```
pattern everywhere I found `torch.ones([])`, plus in a few places I used `from_numpy` where the input was numpy, so I touched on other models as well.

Please have a look.


`from_numpy` didn't quite work in 2 models where I tried it, so going to use the normal `torch.tensor` constructor like everywhere else in this PR.

Nice finding :-)
Protobuf 4 support (again)
Author: dustyketchum
Pull request: no

### Feature request

Looking at https://github.com/huggingface/transformers/issues/21677#issuecomment-1435072007, I notice there are now new versions of tensorflow and tensorboard that may help with protobuf 4.x compatibility.  It would be awesome to get this upgraded, thanks!


### Motivation

easier upgrade path to python 3.10 and above


### Your contribution

nope, sorry.


Comments:
Hi @dustyketchum, 

Thanks for raising this issue. Managing the different dependencies in the library can be quite complex. As noted in the linked issue, the blocker for upgrading protobuf support was third-party libraries support of protobuf 4 rather than our own. 

If this is something you or someone else in the community believes is very important, please feel free to open a PR. Note that it's not just necessary for the CI to be green and protobuf 4 be supported, we must also remain backwards compatible with previous versions.

cc @ydshieh 

BTW, what's blocking when you try to use python 3.10 without protobuf 4.x ?

I am not blocked; I should have been more precise.


Fixed in #24599
PreTrainedTokenizerFast - whitespace merge skipped 
Author: cmp-nct
Pull request: no

### System Info

@ArthurZucker 

Most likely I'm wrong, been digging through tokenization for 10 hours in a row and quite new to the topic.
Vocab: https://huggingface.co/tiiuae/falcon-40b/blob/main/tokenizer.json
String: `"Hello  World"`
Two spaces are in the merge list right on top at line 19
" W" is in line 128

Running this through the tokenizer (`tokenizer = PreTrainedTokenizerFast(tokenizer_file='tokenizer.json')`)
Falcon IDs:
`         [9856, 204, 2889]`
Falcon tokens:
`         ['Hello', 'Ġ', 'ĠWorld']`

What I expected:
```
  9856 -> 'Hello'
   258 -> '  ' 
 12670 -> 'World'
```

From my understanding the two whitespaces form a rank 19 merge (the 2nd lowest one next to 'o r' at 12)
I most likely just misunderstand a special rule in BPE in relation to white space characters


### Who can help?

_No response_

### Information

- [ ] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

```
import sys
from transformers import PreTrainedTokenizerFast

# Load the tokenizer
tokenizer = PreTrainedTokenizerFast(tokenizer_file='tokenizer.json')

# Define the string to tokenize
text = "Hello  World"

# Check if a command line argument was provided
if len(sys.argv) > 1:
    text = sys.argv[1]

# Tokenize the string
output = tokenizer.encode(text)

# Print the token IDs
print("Falcon IDs:\n\t", output)
tokens = tokenizer.convert_ids_to_tokens(output)
print("Falcon tokens:\n\t", tokens)
```

### Expected behavior

The two spaces form a higher rank than space + W so I'd expect this outcome
  9856 -> 'Hello'
   258 -> '  '
 12670 -> 'World'

Comments:
The behavior switches to what I expected if you disable pre_tokenizer->use_regex which ignores the rank and contains quite a bit of english grammar rules.
Not sure if that regex snake should really be used by default, given the international use of tokenizers. (ironically TII has chosen it despite being in an arabic speaking country)
Is it possible to use the transformers library with models, e.g. t5-small, commercially?
Author: ozgesevgili
Pull request: no

Hi!

I would like to use the transformers library with models, e.g. t5-small, commercially. I checked the license of the transformers library is Apache 2.0. And the license of the model, e.g. t5-small (https://huggingface.co/t5-small), is also Apache 2.0. So, can I use the library with such models, commercially?

Comments:
Questions like this are better suited on the [forums](https://discuss.huggingface.co/) as we keep issues for feature requests and bugs only. The Transformers library is Apache 2.0 so there is no problem using it for commercial use. Then it's up to the model you are using. As you noted `t5-small` should be fine.

Thanks for the quick reply.

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.
High memory usage for BigBirdForPreTraining
Author: kuben-joz
Pull request: no

### System Info

DGX-A100 
transformers 4.29.2

### Who can help?

@ArthurZucker
@ydshieh 

### Information

- [ ] The official example scripts
- [X] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [X] My own task or dataset (give details below)

### Reproduction

Run BigBirdForPreTraining with the BigBirdConfig left at default (except vocab size set to 32k). Essentially I recreated pretraining from section F.1 of the original publication https://arxiv.org/pdf/2007.14062.pdf

### Expected behavior

Should be able to fit a batch size of 4 in ~16 GB according to the paper but in reality a batch size of 4 exceeds 40GB on the forward call.

Comments:
Sorry, but this is lacking a lot of information, especially a reproducing script. Lots of things can impact the RAM taken by the model, this should be asked on the [forum](https://discuss.huggingface.co/top?period=weekly), not here. 

I added a reproducing script here:
https://github.com/kuben-joz/bigbird-example/tree/master

I thought it would be better to make  a submission here rather than the forums as it concerns the implementation details of the model. If nevertheless you prefer for this to be moved to the forum I can do so.

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.
Can not using LION optimizer
Author: lucasjinreal
Pull request: no

### Feature request

Error:

```
RuntimeError: Expected to mark a variable ready only once. This error is caused by one of the following reasons: 1) Use of a module parameter outside the `forward` function. Please make sure model parameters are not shared across 
multiple concurrent forward-backward passes. or try to use _set_static_graph() as a workaround if this module graph does not change during training loop.2) Reused parameters in multiple reentrant backward passes. For example, if 
you use multiple `checkpoint` functions to wrap the same part of your model, it would result in the same set of parameters been used by different reentrant backward passes multiple times, and hence marking a variable ready 
multiple times. DDP does not support such use cases in default. You can try to use _set_static_graph() as a workaround if your module graph does not change over iterations.
Parameter at index 129 has been marked as ready twice. This means that multiple autograd engine  hooks have fired for this particular parameter during this iteration. You can set the environment variable TORCH_DISTRIBUTED_DEBUG to
either INFO or DETAIL to print parameter names for further debugging.
  0%|    
```

Does there any example code of using LION optimzier?

I using code from here https://github.com/lucidrains/lion-pytorch/blob/main/lion_pytorch/lion_pytorch.py but got error above.

### Motivation

LION need to be support since it converges more faster.

### Your contribution

no

Comments:
We can't do anything to help without knowing the code that triggered the error.

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.
ImportError: cannot import name 'PartialState' from 'transformers.trainer_pt_utils'
Author: hoangledoan
Pull request: no

### System Info

`transformers` version: 4.28.0
`accelerate` version: 0.19.0


### Who can help?

_No response_

### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

`from transformers.trainer_pt_utils import PartialState`

### Expected behavior

it cannot import the class, eventhough I tried to downgrade transformers and install accelerate

Comments:
That class is defined in Accelerate, not in `trainer_utils`.

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.
BertTokenizer.save_vocabulary does not save the full vocab
Author: dennymarcels
Pull request: no

### System Info

The current version still has the issue.

The [documentation](https://huggingface.co/transformers/v4.8.2/model_doc/bert.html?highlight=berttokenizer#transformers.BertTokenizer.save_vocabulary) states that `save_vocabulary` "save[s] only the vocabulary of the tokenizer **(vocabulary + added tokens)**" but the [code](https://github.com/huggingface/transformers/blob/e45e756d22206ca8fa9fb057c8c3d8fa79bf81c6/src/transformers/models/bert/tokenization_bert.py#L358) only deals with the vocab. I believe changing this line to:
```
for token, token_index in sorted(dict(self.vocab, **self.added_tokens_encoder).items(), key=lambda kv: kv[1]):
```
would solve it.

### Who can help?

@ArthurZucker @younesbelkada 

### Information

- [ ] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

Running `save_vocabulary` as is reproduces the issue.

### Expected behavior

The full (new) vocabulary should be saved to file.

Comments:
Hey! Not sure which part of the documentation says that, but it is in the wrong! The additional special tokens are saved somewhere else, and properly handled. `Bert` is also one of our core model, but also an old one, which is why the doc might  not be up to date. When loading the vocab, only the actual vocabulary is expected. 
Tell me if this does not solve your confusion! 

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.
Remove the multi step tokenization warning when using HF Data Collators
Author: JulesGM
Pull request: yes

We create a new function that pads without warning the user to instead calling `tokenizer.forward` because it's faster.

We use it for Transformers' own DataCollator calls.

It doesn't make much sense that a DataCollator would change the state of a tokenizer imho, so every time: 
    - we save the state of the tokenizer with regards to the warning
    - disable the warning
    - pad
    - restore the state of whether we want to warn or not.

See https://github.com/huggingface/transformers/issues/22638

Comments:
@sgugger

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_23742). All of your documentation changes will be reflected on that endpoint.

Doing things this way could lead to weird (though not dangerous) race behaviour in threaded code.

The only way I see to not get that race behavior is to let `pad` take an extra argument to disable the warning, though I understand that API providers are very hesitant to alter APIs in any way
 

The race behavior would just be that the warning would not be displayed ofc

The data collator cannot really be in threaded code. Different processes? Definitely for distributed training but then they will each have their own tokenizer. So I think it's a risk we can take.

I'm sorry @sgugger, you mean the branch name? Or the pull request name? Or the function name? I changed the name of the pull request.

Also, am I supposed to run `make quality; make style` first? & am I supposed to run some tests?

I synced the branch

The PR now deletes 26 doc files, so it's not really something we can merge :grimacing: 

OK I fixed the weird document files deletion. Really not sure how that happened. Sorry about that.

Could you just run `make style` to fix the formatting issues? Thanks!

done

There is still an error. Can you make sure to do `pip install transformers["quality"] --upgrade` (to make sure you have a proper version of all the necessary libs)?

Mmm now we're back to 43 files changed :cry: 

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.
Trainer的使用问题
Author: fengxb2077
Pull request: no



Trainer在实例化的时候不是已经传入加载的模型了吗，为什么 trainer.train(resume_from_checkpoint=checkpoint)还可以从保存的检查点加载模型？？？？

Comments:
Hi @fxb392 

It's mostly for loading the  optimizer's scheduler and other states. But it's also convient if you load a canonical model (say from the Hub) while instantiating a trainer but want to use other checkpoints.

You don'

OK，thanks for you guidence.Does this  only means the trainer can conveniently load any checkpoints for train?

Yes, but you have to be careful to load the checkpoint which is saved by a trainer that loaded the same model type and the same model configuration.

Okay, I understand, thank you again.
Type hinting Inconsistency in beam_search.py 
Author: mert-kurttutan
Pull request: no

### System Info

Main branch

### Who can help?

@gante

### Information

- [ ] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

Hi,

In `beam_search.py`file, the process function (in actual classes), have the following signature 

```

    def process(
        self,
        input_ids: torch.LongTensor,
        next_scores: torch.FloatTensor,
        next_tokens: torch.LongTensor,
        next_indices: torch.LongTensor,
        pad_token_id: Optional[int] = None,
        eos_token_id: Optional[Union[int, List[int]]] = None,
        beam_indices: Optional[torch.LongTensor] = None,
    ) -> Tuple[torch.Tensor]:
```
even though it returns `UserDict`. Any reason why not annotate with Dict, Mapping rather than Tuple?

This type of mismatching might exist in other places, not sure

### Expected behavior

IMO, it should return Dict, or Mapping

Comments:
Hey @mert-kurttutan 👋 

That is absolutely correct. Would you like to open a PR to fix it? :)

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.

Hello, is anyone working on this issue? If not I can take it on @gante.

Hey @jprivera44 -- AFAIK no one is working on it, feel free to take it 🙌 

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.
Pin `Pillow` for now
Author: ydshieh
Pull request: yes

# What does this PR do?

`Pillow 10.0.0` is out 2 days ago.

Our CI get errors (via the usage of `detectron2`):

```bash
...
/usr/local/lib/python3.8/dist-packages/detectron2/data/transforms/transform.py:36: in <module>
...
>       def __init__(self, src_rect, output_size, interp=Image.LINEAR, fill=0):
E       AttributeError: module 'PIL.Image' has no attribute 'LINEAR'
``` 

This is due to the previous deprecation and the removal now
```bash
<stdin>:1: DeprecationWarning: LINEAR is deprecated and will be removed in Pillow 10 (2023-07-01). Use BILINEAR or Resampling.BILINEAR instead.
```

This PR pins `Pillow` for now until `detectron2` fixes it.


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

Merge as the 2 failed tests seems flaky.
Convert Pre-LN Transformers into equivalent Pre-RMSNorm Transformers to accelerate inference and training
Author: ZixuanJiang
Pull request: no

### Feature request

LayerNorm and RMSNorm are the top two normalization methods in Transformers. We unify them in Pre-Normalization Transformers in our paper https://arxiv.org/abs/2305.14858. The arithmetic equivalence allows us to convert Pre-LN Transformers into Pre-RMSNorm models without impact on the model functionality. Since RMSNorm offers superior efficiency compared to LayerNorm, our method enables faster equivalent inference and training for any Pre-LN Transformers, e.g., GPT, ViT.

Our implementation is at https://github.com/ZixuanJiang/pre-rmsnorm-transformer. As the first step, we can start by accelerating the deployment of the existing Pre-LN Transformers.

### Motivation

Related GitHub issue: https://github.com/pytorch/pytorch/issues/72643#issue

### Your contribution

We have provided our reference implementation at https://github.com/ZixuanJiang/pre-rmsnorm-transformer. We are open to submitting a related PR in the future.

Comments:
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.
My QUESTION is how run a very big model like bloom on a cluster of machines ?
Author: patnelt
Pull request: no

### System Info

bloom, pytorch, ubuntu

### Who can help?

_No response_

### Information

- [ ] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

..

### Expected behavior

Hello i can run opt 66b on one server with 6 gpu 24 Gb by using you page on huggingface on how load big models : I give device_map. I can also run bloom on one server with 8 GPUs 24 GB by giving device_map but it uses offload on CPU and it takes time to answer. My QUESTION is how run a very big model like bloom on a cluster of machines indeed bloom would need 20 GPus 24 Gb and it needs a cluster of 3 machines with 8 gpus to deploy, with accelerate it is not possible as we are limited to only one machine. I have tried everything like to use RPC Framework but it seems it is only for CPU. Thanks for your help. Regards Pat

Comments:
Please use the [forums](https://discuss.huggingface.co/) for such questions.

yes thanks for your answer. i wrote it also on forum but it not an easy question and only specialists like you can answer or give me a give me some help so i can continue ... Regards pat

so could you give some technical help, regards, pat

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.