noamaanMulla-03/transformers-issues huggingface dataset:
`FlaxBartForConditionalGeneration` has a `.encode` method but `BartForConditionalGeneration` does not
Author: ayaka14732
Pull request: no

## Environment info

- `transformers` version: 4.18.0
- Platform: Linux-5.11.0-1018-gcp-x86_64-with-glibc2.31
- Python version: 3.10.4
- Huggingface_hub version: 0.5.1
- PyTorch version (GPU?): 1.11.0+cu102 (False)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): 0.4.1 (tpu)
- Jax version: 0.3.5
- JaxLib version: 0.3.5
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: No

### Who can help

@patil-suraj

## Information

Model I am using: BART

## To reproduce

Flax version (working):

```python
from transformers import BartTokenizer, FlaxBartForConditionalGeneration

tokenizer = BartTokenizer.from_pretrained('facebook/bart-base')
model = FlaxBartForConditionalGeneration.from_pretrained('facebook/bart-base')
inputs = tokenizer('Travelers wait about an hour and a half to cross the Tower.', return_tensors='jax')
outputs = model.encode(**inputs)
print(outputs)  # OK
```

PyTorch version (not working):

```python
from transformers import BartTokenizer, BartForConditionalGeneration

tokenizer = BartTokenizer.from_pretrained('facebook/bart-base')
model = BartForConditionalGeneration.from_pretrained('facebook/bart-base')
inputs = tokenizer('Travelers wait about an hour and a half to cross the Tower.', return_tensors='pt')
outputs = model.encode(**inputs)  # AttributeError: 'BartForConditionalGeneration' object has no attribute 'encode'
```

## Expected behavior

The PyTorch version should also have a `.encode` method to generate the encoder output, as the Flax version does.

## Actual behavior

The PyTorch version does not a `.encode` method.

Comments:
Hi! The `encode` method does not exist in PT model because it's possible to access the encoder using `model.get_encoder` and then call it to run the encoder. Flax does not allow accessing modules like this so we need to expose explicit methods like `encode`. This is not required for PT.

Thanks for the explanation!
Fix t5 shard on TPU Pods
Author: agemagician
Pull request: yes

The current script doesn't work properly on a TPU pod because the global batch is not divided correctly per host.
This pull request fixes this issue by dividing the global batch to each host before it is shared on each host.

Fixes # (issue)
https://github.com/huggingface/transformers/issues/16470

## 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?
- [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/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?

Models:

-   t5: @patrickvonplaten, @patil-suraj


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

This looks good to me!

@patil-suraj @borisdayma - could you take a look here?

Yes this approach works!

Thinking about it I think there could be some issues with last batch so we probably need to ensure they all have same number of items and that they are multiple of the number of local devices.

@borisdayma 
This line already makes sure that all batches are of same length.

https://github.com/huggingface/transformers/blob/2831826bc60ee11b86179252ffc8401cb03c5904/examples/flax/language-modeling/run_t5_mlm_flax.py#L857
update decoder_vocab_size when resizing embeds
Author: patil-suraj
Pull request: yes

# What does this PR do?

- Update the `config.decoder_vocab_size` when resizing embeds if the enc, dec embeds are shared.
- Use `config.decoder_vocab_size` to reshape the `lm_logits`

Fixes #16670

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Fix TF_MASKED_LM_SAMPLE
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix `TF_MASKED_LM_SAMPLE`: there is currently a dimension issue regarding `mask_token_index` and `predicted_token_id`, which gives different results between PT/TF masked LM code samples

PT: `paris`
TF: `p a r i s`

See below for details.

(This is related to #16523)

##

### PT_MASKED_LM_SAMPLE
```python
from transformers import BertTokenizer, BertForMaskedLM
import torch

mask = "[MASK]",
checkpoint = "bert-base-uncased"

tokenizer = BertTokenizer.from_pretrained(f"{checkpoint}")
model = BertForMaskedLM.from_pretrained(f"{checkpoint}")

inputs = tokenizer(f"The capital of France is {mask}.", return_tensors="pt")

with torch.no_grad():
    logits = model(**inputs).logits

# retrieve index of {mask}
mask_token_index = (inputs.input_ids == tokenizer.mask_token_id)[0].nonzero(as_tuple=True)[0]
predicted_token_id = logits[0, mask_token_index].argmax(axis=-1)
expected_output = tokenizer.decode(predicted_token_id)


print(mask_token_index)  # tensor([8]): row dimension from `nonzero()`
print(predicted_token_id)  # tensor([3000])
print(expected_output)  # paris
```

### TF_MASKED_LM_SAMPLE (on `main`)
```python
from transformers import BertTokenizer, TFBertForMaskedLM
import tensorflow as tf

tokenizer = BertTokenizer.from_pretrained(f"{checkpoint}")
model = TFBertForMaskedLM.from_pretrained(f"{checkpoint}")

inputs = tokenizer(f"The capital of France is {mask}.", return_tensors="tf")
logits = model(**inputs).logits

# retrieve index of {mask}
mask_token_index = tf.where(inputs.input_ids == tokenizer.mask_token_id)[0][1]
predicted_token_id = tf.math.argmax(logits[0, mask_token_index], axis=-1)
expected_output = tokenizer.decode(predicted_token_id)

print(mask_token_index)  # tf.Tensor(8, shape=(), dtype=int64): no row dimension
print(predicted_token_id)  # tf.Tensor(3000, shape=(), dtype=int64)
print(tokenizer.decode(predicted_token_id))  # p a r i s (not good)
```

### TF_MASKED_LM_SAMPLE (this PR)
```python
# retrieve index of {mask}
mask_token_index = tf.where((inputs.input_ids == tokenizer.mask_token_id)[0])
selected_logits = tf.gather_nd(logits[0], indices=mask_token_index)
predicted_token_id = tf.math.argmax(selected_logits, axis=-1)
expected_output = tokenizer.decode(predicted_token_id)

print(mask_token_index)  # tf.Tensor([[8]], shape=(1, 1), dtype=int64): with row dimension
print(predicted_token_id)  # tf.Tensor([3000], shape=(1,), dtype=int64)
print(tokenizer.decode(predicted_token_id))  # paris
```

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Enable more test_torchscript
Author: ydshieh
Pull request: yes

# What does this PR do?

Enable more `test_torchscript` (in 30 files) by updating `_create_and_check_torchscript`.

(There are still 21 files with 23 places being `False` at this moment - they still give errors.)

The main place to review is in `test_modeling_common.py`.
The changes in model specific test files are removing lines regarding `test_torchscript = True/False`.


Comments:
_The documentation is not available anymore as the PR was closed or merged._
Rename the method test_torchscript 
Author: ydshieh
Pull request: yes

# What does this PR do?

`class ModelTesterMixin` has attribute `test_torchscript` as well as method named `def test_torchscript`.

In a few model specific test files, we have `test_torchscript = True` defined, for example, 

`T5ModelTest`:
https://github.com/huggingface/transformers/blob/7c5d79912a21880ce13d77881940458e90d98917/tests/t5/test_modeling_t5.py#L515

`DistilBertModelTest`:
https://github.com/huggingface/transformers/blob/7c5d79912a21880ce13d77881940458e90d98917/tests/distilbert/test_modeling_distilbert.py#L214

This actually makes the `test_torchscript` **being a boolean value** instead of a method, therefore `test_torchscript` is **not run** for these places. See the last section for a dummy example.

## Fix

Although this could be fixed just by removing `test_torchscript = True`, it would be a better idea to rename the method `def test_torchscript` to something else like `def test_torchscript_simple`.

## Dummy example (to reproduce the issue)

```python
class DummyCommonTest:

    test_me = True

    def test_me(self):
        a = 3
        print(a)

class DummyModelTest1(DummyCommonTest):

    pass

class DummyModelTest2(DummyCommonTest):

    test_me = True

dummy_test = DummyCommonTest()
print(dummy_test.test_me)
dummy_test.test_me()

dummy_model_test_1 = DummyModelTest1()
# A method
print(dummy_model_test_1.test_me)
# can be called
dummy_model_test_1.test_me()

dummy_model_test_2 = DummyModelTest2()
# A boolean
print(dummy_model_test_2.test_me)
# can't be called: (TypeError: 'bool' object is not callable)
dummy_model_test_2.test_me()
```



Comments:
_The documentation is not available anymore as the PR was closed or merged._
Reduce the memory leak caused by `torch.jit.trace`
Author: ydshieh
Pull request: yes

# What does this PR do?

Reduce the memory leak caused by `torch.jit.trace`.

Without this fix, each call to `_create_and_check_torchscript` increases RAM usage by ~20MB.
(Even with this call, there are still memory leak by ~0.04MB)

## Remark

Since our torchscript tests are slow tests, they are run only in scheduled CI (where the test jobs are organized by models), this memory leak issue is not important (memory is released after each job process is done/exit).

However, for PR like #16679, I need to make sure the modified tests will pass. When I run in a GCP VM, I got OOM issue.
So the change in this PR still have its value, I think.

## More information

The method is copied from `torch`

https://github.com/pytorch/pytorch/blob/bcf6974c207ac0339bfb8bdfdb0b0ec348f7a22f/torch/testing/_internal/jit_utils.py#L59

This is called in torch/test, including

https://github.com/pytorch/pytorch/blob/bcf6974c207ac0339bfb8bdfdb0b0ec348f7a22f/test/test_jit.py#L12962
```python
    # clear the class registry as we will be defining foo multiple times
    jit_utils.clear_class_registry()
```

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Don't push checkpoints to hub in `no_trainer` scripts
Author: muellerzr
Pull request: yes
Labels: name: PyTorch

name: External

name: Examples

# Don't push checkpoints to the Hub in `no_trainer` scripts

## What does this add?

- Creates a `gitignore` file in the base folder if `push_to_hub` was passed and one does not exist
- During each call to `save_state`, if `push_to_hub` was passed then then directory is added to the .gitignore

## Why is it needed?

Users shouldn't expect to have all of their checkpoints sent to both the Hub and saved locally, it should just be locally until they are ready for the final save

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Private repo TrainingArgument
Author: nbroad1881
Pull request: yes

# What does this PR do?

Creates a new argument for `TrainingArguments` called `hub_private_repo`. If True, the hub repo created by `Trainer` will be set to private. Defaults to False (public).


## 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?

@sgugger 


Comments:
_The documentation is not available anymore as the PR was closed or merged._
fixed crash when deleting older checkpoint and files with name f"{checkpoint_prefix}-*" exist
Author: sadransh
Pull request: yes

What does this PR do?


I create an archive of older checkpoints during training the checkpoint has a  name with `f"{checkpoint_prefix}-*.zip/.tar ` 
previously `glob(f"{checkpoint_prefix}-*")` takes all files/folders starting with the name checkpoint, and later `shutil.rmtree(checkpoint)` takes a folder name; since at some point it my get a zip file; it crashes training; adding this `if os.path.isdir(x)` allows only folders on `glob_checkpoints`. 

let's say output folder structure is like: (with `save_limit=5`)
```
checkpoint-36000
checkpoint-35000
checkpoint-34000
checkpoint-33000
checkpoint-33000.zip
```
then code attempts to remove oldest checkpoint 
since  we have a file (checkpoint-33000.zip) and pass the file to `shutil.rmtree(checkpoint)` to delete it will fail.

by avoiding storing files on `glob_checkpoints` this will get fixed! ( checking everything is folder as checkpoints are folders not single files.) 

**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?

@sgugger 




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

@sgugger
Handle image_embeds in ViltModel
Author: ydshieh
Pull request: yes

# What does this PR do?

Handle `image_embeds` in `ViltModel` / `ViltForImagesAndTextClassification`.

(looks like `Vilt` is the first model introducing `image_embeds` argument.)

## More Info

The `image_embeds` in `ViltForImagesAndTextClassification` should have `num_images` dimension as `pixel_values` has, as far as I understand.

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

> Thanks for improving this!
> 
> Out of interest: were you experimenting with ViLT?

Not for this PR. I tried to fix a CI (vit-mae), which was about `test_torchscript`.
It turns out to be related to model main input -> I worked/improved on it -> more models involved including ViLT -> I just took this chance to work on this PR (otherwise I would forget it very quickly)
Improve PT/TF equivalence test
Author: ydshieh
Pull request: yes

# What does this PR do?

Improve PT/TF equivalence test.

To make the review a bit easier for you, I made some comments. And here are a summary of changes:

- `test_pt_tf_model_equivalence` in TensorFlow `LED` and `CLIP` are removed: the common one can handle it.
- `test_pt_tf_model_equivalence` in TensorFlow `LXMERT` and `ViTMAE` are removed: we only need to overwrite
  -  `prepare_pt_inputs_from_tf_inputs` for `LXMERT`
  -  `check_pt_tf_models` for `ViTMAE`
- Main changes in `TFModelTesterMixin.test_pt_tf_model_equivalence`
  - restructure the code into components, so they could be overwritten separately instead of the whole big block
  - move some ugly (temporary) logic blocks outside:
    - `_make_attention_mask_non_null`
    - `_postprocessing_to_ignore_test_cases`
  - About `check_pt_tf_outputs`:
    - it now can handle instances of `ModelOutput` (for CLIP model)
    - better failure message: print the tensor name where the large diff between PT/TF occurs, like `output.hidden_states` or `output.text_model_output.attentions_1`
  -  A better way to handle the cases where PT/TF outputs have different keys: we try to test the output values for the common keys in both outputs.
  
  
Once this PR is approved/merged:
 - To work on the same PT/TF equivalence test on PT side (should be very quick)
 - To apply the same logic to PT/Flax equivalence test, both on Flax and PT sides.

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

(just rebase on main - no real change since your last review)

Merge now. Don't hesitate to leave comments in any :-)
Fix example logs repeating themselves
Author: muellerzr
Pull request: yes
Labels: name: Examples

name: bug

# Fix Loggers repeating themselves during the Examples tests

## What does this add?

This PR refactores slighly how the `stream_handler` is made and passed to `logger` during all of the examples tests

## Why is it needed?

When running the `no_trainer` tests, I was noticing that after each test was called the data would repeat itself. So after test 1 it was only logged once, test 2 twice, etc. 

For example something like the following was printed:
```
***** Running training *****
***** Running training *****
***** Running training *****
  Num examples = 10
  Num examples = 10
  Num examples = 10
```
After further investigation, I found that this was due to the `stream_handler` being added at the start of each test. As discussed in this [SO post I found](https://stackoverflow.com/questions/6729268/log-messages-appearing-twice-with-python-logging), since the loggers are global we keep adding more handlers without actually removing them. 

As a result we kept getting multitudes of sys.stdout loggers being added, leading to these multiple print statements each time. 

The fix is before each test case is written, add in our handler (e.g.):
```python
stream_handler = logging.StreamHandler(sys.stdout)
logger.addHandler(stream_handler)

class SomeTestCase(TestCasePlus):
  ...
```

Also unsure why this didn't quite happen during CircleCI, but this was what occurred locally for me

Comments:
_The documentation is not available anymore as the PR was closed or merged._
TF: Finalize `unpack_inputs`-related changes
Author: gante
Pull request: yes

# What does this PR do?

Closes https://github.com/huggingface/transformers/issues/16051

Please read this before diving into the changes :) This PR finalizes the changes related to the `unpack_inputs` and is slightly more complex than the other PRs.

Changes:
1. Removes `**kwargs` from most `call` methods in our TF models: 
    1. This argument was not documented and, after adding the decorator, not being used; 
    2. It was used before exclusively as an input to `input_processing`, to handle some special cases (which are now handled inside the decorator); 
    3. The exception is the `encoder_decoder` models (see below);
    4. **Removing it implies that there are no more hidden parameters being passed to the models**. Somewhat expected, a few tests were passing unused parameters, and had to be corrected. I've added comments throughout the PR to elaborate here.
2. Replaces the use of `input_processing` by the decorator in the `encoder_decoder` models: 
    1. This was not a 1:1 change, like in the other models -- `input_processing` was being used before the `encoder` and the `decoder` were called, which was redundant (the `encoder`/`decoder` now have the decorator, which also calls the function); 
    2. However, it was also being used for its side effects, i.e. to set some variables (like `use_cache`), which is equivalent to adding the decorator on the `encoder_decoder` model; 
    3. Because these `encoder_decoder` models *must* use kwags, as the `encoder`/`decoder` might have a myriad of arguments, the decorator was updated so as to allow random kwargs on models that expect them. This brings us back to 1. -- no other models have kwags now.
3. Icing on the cake -- `input_processing` is now only used in the decorator, so I made the function protected :) This means we can start modernizing it without the fear of it being used in other places. 


Comments:
_The documentation is not available anymore as the PR was closed or merged._
TF generate refactor - Beam Search
Author: gante
Pull request: yes

# What does this PR do?

As discussed in the original TF generate refactor plan (https://github.com/huggingface/transformers/pull/15562), adds `beam_search`. 

This Beam Search implementation was inspired by our FLAX implementation, which is XLA-friendly. However, this PR is not yet XLA-ready (😭). To pass existing tests, a few tweaks were added on top of the FLAX adaptation -- I added some comments in the PR to explain the differences (and why they were needed), hopefully making the review process easier. 

Tests ran (and passing):
- [x] GPT-2
- [x] T5
- [x] BART
- [x] Vision Encoder Decoder
- [x] Encoder Decoder
- [x] Speech to Text
- [x] RAG

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

![image](https://user-images.githubusercontent.com/12240844/161332833-13b7c816-caa9-42c1-b9ed-5343354d2831.png)