noamaanMulla-03/transformers-issues huggingface dataset:
FX tracing improvement
Author: michaelbenayoun
Pull request: yes

# What does this PR do?

This PR improves significantly the way transformers models are traced by the HFTracer (`torch.fx`).
This has 2 major consequences:

- More model architectures can be supported
- When a model can be traced, the resulting GraphModule can take any input shapes out of the box (compared to what was done before where a lot of work was needed to enable dynamic axes for a given model), this is both easier and less bug prone.

Because of these changes the `symbolic_trace` signature becomes easier: 
`symbolic_trace(model: PreTrainedModel, input_names: Optional[List[str]] = None) -> GraphModule`

There is no need to specify the batch size, the sequence length or the number of choices (for multiple-choice) anymore.
The same thing can be said about the `HFTracer`, which can be instantiated exactly the same way as the regular `torch.fx.Tracer` can.

Comments:
Hey, thanks for your PR @michaelbenayoun ! It seems there are a few failing tests (1096 :smile:), could you take a look at it?

Currently looking into it!
Sorry about that.

Fixed!

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.

Unstale comment

I am planning to try another approach to make both the code easier, and the tracing process cleaner, this will allow to add other models as well as to limit the number of bugs.
In the mean time, I think this can be merged because a few issues were posted to have `symbolic_trace` working for Pytorch 1.10, which this PR enables.
Fix typo
Author: Dobatymo
Pull request: yes

# What does this PR do?

Only a typo, but it can lead to confusion

## Before submitting
- [x] 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/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.


Comments:
_The documentation is not available anymore as the PR was closed or merged._
XGLM onnx support
Author: FrankHeijden
Pull request: no

### Feature request

I am trying to convert an XGLM based model using the following command, but am receiving an error that onnx does not support an XGLM based model:
```
./venv/bin/python -m transformers.onnx --model=facebook/incoder-1B onnx/
```

Error:
```
KeyError: "xglm is not supported yet. Only ['albert', 'bart', 'beit', 'bert', 'big-bird', 'bigbird-pegasus', 'blenderbot', 'blenderbot-small', 'camembert', 'convbert', 'data2vec-text', 'deit', 'distilbert', 'electra', 'flaubert', 'gpt2', 'gptj', 'gpt-neo', 'ibert', 'layoutlm', 'marian', 'mbart', 'mobilebert', 'm2m-100', 'roberta', 'roformer', 't5', 'vit', 'xlm-roberta'] are supported. If you want to support xglm please propose a PR or open up an issue."
```

Comments:
cc @lewtun

Hey @FrankHeijden, indeed this architecture is not yet supported in the ONNX exporter. If you'd like to have a go at it yourself, you can follow [this guide](https://huggingface.co/docs/transformers/v4.19.2/en/serialization#exporting-a-model-for-an-unsupported-architecture) and use the `BartOnxxConfig` as a template to work from (I think this model should be similar)

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.
layoutlm TokenClassificationPipeline
Author: uwong
Pull request: no

Hi, I was looking at using transformers.pipeline for TokenClassification with an instance of  microsoft/layoutlm-base-uncased that I have fine tuned.  I would like to use pipeline to take advantage of the entity aggregation_strategy feature for extracted entities.

However it is unclear to me how/whether TokenClassificationPipeline works with layoutlm for inference because layoutlm expects both a input text and input bounding boxes, unlike other text only models.  

Do you know if TokenClassificationPipeline is supposed to work with layoutlm and are there any examples?

Comments:
cc @NielsRogge 

I'm afraid the `TokenClassificationPipeline` will not work with LayoutLM, the reason being that, as you mention, the model expects an additional input besides text, namely bounding boxes.

We are currently discussing the design of pipelines for models like LayoutLM. 



Ok thanks for the information!  I'll just work around it for now.

@NielsRogge are you aware of any developments in terms of pipeline integration for LayoutLM-like models? Thanks :) 

@mishig25 worked on supporting LayoutLM for the object detection pipeline, but that wasn't added in the end. Not sure if we can add it to the existing pipeline, cause the model requires a few additional inputs (`bbox`, and `pixel_values`), cc @Narsil 

Hi, it probably won't be implemented directly in `transformers` because arguments are different and so on.

However you should be able to override the pipeline yourself doing something like

```python

pipe = pipeline(model="..." , pipeline_class=MyPipeline)

class MyPipeline(Pipeline):
    def preprocess(self, inputs):
       # Just return the inputs that will be sent to the model
       return model_inputs
       
    def _forward(self, model_inputs):
        model_outputs = self.model(**model_inputs)
        return model_outputs
        
     def postprocess(self, model_outputs):
         # Finalize the objects
         return final_object
```

If you inherit `TokenClassificationPipeline you could definitely reuse stuff being done with aggregation_strategies
Fix loss computation in TFWav2Vec2ForCTC
Author: Sreyan88
Pull request: yes

What does this PR do?

`TFWav2Vec2ForCTC` implementation was incorrect. The CTC loss calculation wasn't proper. The root of the problem was that the CTC target labels weren't reaching the loss calculation and it was None. So adding `@unpack_inputs` now unpacks the input properly and loss calculation is properly done.

Additionally, the loss needed to be reshaped for backpropagation.

Fixes #18009 

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

This looks good to me! We just made the same change to several other losses (reshaping the output from a scalar to a tensor with shape `(1,)`). 

My only concern is that I have no idea how the lack of `@unpack_inputs` was missed by tests, but I'm happy to merge this for now and think about how to expand test coverage afterwards!

@Sreyan88 I'm happy with this and I think it's ready to merge now - if you want to make any other changes, now's the time. If not, ping me and I'll merge it!

> @Sreyan88 I'm happy with this and I think it's ready to merge now - if you want to make any other changes, now's the time. If not, ping me and I'll merge it!

@Rocketknight1 I'm happy you can merge!
TF: T5 can now handle a padded past (i.e. XLA generation)
Author: gante
Pull request: yes

# What does this PR do?

In TF T5, we now fetch the correct slice of `position_bias` -- [the same way we do it in FLAX](https://github.com/huggingface/transformers/blob/main/src/transformers/models/t5/modeling_flax_t5.py#L339). The key difference is that FLAX relies on an [external variable](https://github.com/huggingface/transformers/blob/main/src/transformers/models/t5/modeling_flax_t5.py#L312) for the generated length that gets incremented every time past gets updated, and here the same value is obtained dynamically from the past array (latest filled past index = generated length - 1, where latest filled past index corresponds to the maximum index with non-0 values).

All slow tests are passing and we no longer have length restrictions on the XLA beam search test, which means that:
1. Although the code for eager execution was changed, all outputs remain the same;
2. XLA generation matches non-XLA generation.

Comments:
related issue: https://github.com/huggingface/transformers/issues/17935

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

Great job on finding and fixing the bug here @gante - cool that T5 works now :-)
[Flax] Bump to v0.4.1
Author: sanchit-gandhi
Pull request: yes

# What does this PR do?

The `flatten_dict` operator with the kwarg argument `sep` was added to `modeling_flax_utils` in https://github.com/huggingface/transformers/pull/17760:
https://github.com/huggingface/transformers/blob/f25457b273348733bfeb19a51ab0d21bd30a08b8/src/transformers/modeling_flax_utils.py#L127

This kwarg was only added to Flax in v0.4.1: https://github.com/google/flax/releases/tag/v0.4.1

This PR bumps the required Flax version in Transformers from v0.3.5 to v0.4.1.

## 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.

cc @ArthurZucker 


Comments:
Nice finding! I am not competent regarding which versions we want to always support, but LGTM

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

Thanks @patil-suraj! The latest version would be v0.5.2 (see https://github.com/google/flax/releases). Is the best way to test this by setting `flax>=0.5.2` and observe if the tests past?

IMO we should also try to stay compatible with older Flax versions, so let's not go as high as we can but only as high as we have to! PR looks good to me - but let's try to not always use the most recent Flax features in order to stay compatible with older versions as well 

Thanks for the details @patrickvonplaten. I'm in agreement that we should avoid potentially breaking use cases purely for the sake of being on the latest version, but try and integrate the latest features where applicable.

Interestingly, I actually went away and had a play with three different versions of Flax on my personal research project https://github.com/sanchit-gandhi/seq2seq-speech:

1. **v0.3.5:** issues regarding the `sep` arg of `flatten_dict` in `modeling_flax_utils` (described above)
2. **v0.4.2:** no apparent issues
3. **v0.5.2:** (latest) had issues with Flax's `scan` not working depending on JAX version

Seems like v0.4.2 sits in the sweet spot for newer Flax version whilst providing backwards compatibility!

Yes, but it's often worth to also not directly implement all the new features of Flax since:
- a) they might not work very well because they are new
- b) it breaks backwards comp
Return scalar losses instead of per-sample means
Author: Rocketknight1
Pull request: yes

This updates the TF XLA-compatible losses to return scalars instead of per-sample means. As @ydshieh pointed out, per-sample means give too much weight to samples with fewer masked positions. The new approach should match PyTorch losses exactly (up to floating-point error).

TODO:
- [x] Update expected sizes in tests

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

@ydshieh will investigate RAG and make sure tests pass before merging!

@ydshieh I reverted the RAG loss function to the pre-XLA version, so hopefully those tests pass now. All other tests are passing! Do you think there's anything else I'm missing before I merge?

@gante We do, but it [takes the mean of the TF loss in order to make the two losses comparable](https://github.com/huggingface/transformers/blob/main/tests/test_modeling_tf_common.py#L560-L567).

> Do you think there's anything else I'm missing before I merge?

Nothing I can think of as the tests pass now 🎉 . Thank you!
Evaluation results of `run_qa.py` are anormally low
Author: fxmarty
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.21.0.dev0
- Platform: Linux-5.15.0-39-generic-x86_64-with-glibc2.35
- Python version: 3.9.12
- Huggingface_hub version: 0.8.1
- PyTorch version (GPU?): 1.11.0+cu102 (False)
- Tensorflow version (GPU?): not installed (NA)
- 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?: no

### Who can help?

_No response_

### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [X] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

Do 
* `conda create -n tempoenv python=3.9`
* `git clone https://github.com/huggingface/transformers.git`
* `cd transformers`
* `pip install .[torch]`
* `pip install datasets`


and 
```
python examples/pytorch/question-answering/run_qa.py --model_name_or_path distilbert-base-uncased-distilled-squad \
    --dataset_name squad --do_eval --max_seq_length 384 \
    --doc_stride 128 --output_dir /tmp/debug_squad/
```

giving 

```
***** eval metrics *****
  eval_exact_match =  0.369
  eval_f1          = 0.4673
  eval_samples     =    100
```

which is much lower than what we used to see (about 0.85 f1), see e.g. https://huggingface.co/distilbert-base-uncased-distilled-squad or https://huggingface.co/spaces/autoevaluate/leaderboards?dataset=squad . It's not clear why the eval is only on 100 samples.

### Expected behavior

Higher f1 score

Comments:
Removing the cached arrow processed dataset solved the issue.
Fix T5/mT5 tests for TensorFlow
Author: Rocketknight1
Pull request: yes

Some tests used `tf.reduce_sum()` on a per-token loss. Since we now return a scalar loss to enable XLA compilation and match PyTorch, the expected loss in these tests is wrong. If we change the tests to use `tf.reduce_mean()` and divide the expected value by the number of tokens, then the tests work with both the modern and legacy losses.

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

@ydshieh I ran them all locally and they passed!
Support MobileBert model in transformer.onnx package
Author: YUNQIUGUO
Pull request: no

### Feature request

Just wondering would it be possible to support mobilebert in transformer.onnx package? or is there any quick hack that we can try to export the mobilebert model from huggingface to onnx?

Thanks. 

The mobile I am trying is : `google/mobilebert-uncased`

And the command : `python -m transformers.onnx --model=google/mobilebert-uncased onnx/`

`raise KeyError(
KeyError: "mobilebert is not supported yet. Only ['albert', 'bart', 'mbart', 'bert', 'ibert', 'camembert', 'distilbert', 'flaubert', 'marian', 'm2m-100', 'roberta', 't5', 'xlm-roberta', 'gpt2', 'gpt-j', 'gpt-neo', 'layoutlm', 'electra', 'vit', 'beit', 'blenderbot', 'blenderbot-small'] are supported. If you want to support mobilebert please propose a PR or open up an issue."`

### Motivation

Trying to get mobileBert model exported to onnx format to further investigate and for the usage of some ORT mobile scenarios.

### Your contribution

A PR is not available for now.

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.

It seems it is supported now: https://github.com/huggingface/transformers/pull/17029

Thanks for the pointer to this pr!
TF: Correct XLA generation with GPT-2 and T5
Author: gante
Pull request: yes

# What does this PR do?

(WIP)

Comments:
For future reference: the most recent commit, where T5 was adapted to discard padded past values (similarly to the FLAX implementation), works correctly numerically. However, when compiling to XLA, we get the following `NotImplementedError`:

<img width="1511" alt="Screenshot 2022-05-21 at 14 17 40" src="https://user-images.githubusercontent.com/12240844/169653445-25892c4b-3380-475b-ae24-861e840a9f05.png">


Because of that, I'll try a new strategy: the model receives as input the sliced past, without padding.

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

The previous commit had a different approach (feed to the model the unpadded past), but resulted in the exact same exception. Both are related to a dynamic-size slicing of the past.

superceded by https://github.com/huggingface/transformers/pull/17426, which grabbed the good changes from this PR. T5 needs further 🔍
TF: XLA Logits Warpers
Author: gante
Pull request: yes

# What does this PR do?

This PR enables XLA on the logits warpers... which actually needed no changes. In essence, it adds XLA tests to ensure we don't regress.

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

@patrickvonplaten Sorry, I know you've already reviewed this, but I'm going to re-request your review. I realized the tests were much easier to understand (and with fewer lines) if they were parametrized, instead of having two tests (one for XLA, another for non-XLA) with shared code 😅
TF: XLA Beam Search
Author: gante
Pull request: yes

# What does this PR do?

WIP

Dependencies:
1. #17426 
2. #17479 

Status log:
1. 2022/05/27: XLA compiles, but fails at runtime. The same code runs in eager execution. The XLA compiler seems like it is not able to pick up the right information during beam search, as it complains inside the forward pass on things that work fine with greedy search/sample. Maybe it's GPT-2 specific. Going to fix enable XLA on BART and retry there.
2. 2022/05/30: Now with BART. Same thing -- it compiles, but gets confused at run time. The exact same code path, with eager execution, runs fine. I suspect the cache creation inside the while loop doesn't help.

Ideas yet to explore:
1. Split past cache creation from past cache update, and create the cache before the loop. XLA expects variable creation at  trace time, and we can do it with the proper separation;

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

Closed in favor of #17857
`log_metrics` method of `Trainer` is using `print` instead of `logging`
Author: ImpleLee
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.20.1
- Platform: Linux-5.4.0-86-generic-x86_64-with-debian-bullseye-sid
- Python version: 3.7.13
- Huggingface_hub version: 0.1.2
- PyTorch version (GPU?): 1.10.0 (True)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: Yes
- Using distributed or parallel set-up in script?: No

### Who can help?

@sgugger @stas00 (related to #12276)

### Information

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

### Tasks

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

### Reproduction

run the following code snippet
``` python3
from transformers import BertConfig, Trainer, BertForSequenceClassification
trainer = Trainer(BertForSequenceClassification(BertConfig()))
trainer.log_metrics('eval', {'accuracy': 0.9})
```

### Expected behavior

I expect the output should be something like the following, which comes from `logging` library, and can be easily redirected to somewhere else with proper configuration.
```
[INFO|trainer_pt_utils.py:908] 2022-07-05 12:32:08,846 >> ***** eval metrics *****
[INFO|trainer_pt_utils.py:913] 2022-07-05 12:32:08,846 >>   accuracy = 0.9
```

The actual output comes from a `print()`, which is inconsistent with other part of this library, and cannot be easily redirected unless we just redirect `stdout` itself.
https://github.com/huggingface/transformers/blob/f0982682bd6fd0b438dda79ec45f3a8fac83a985/src/transformers/trainer_pt_utils.py#L948-L953

**BEFORE** #12276, the code looked like the following, which behaves as expected.
https://github.com/huggingface/transformers/blob/a4ed074d4b8c1ab0a1045bd86963ef209c3c467f/src/transformers/trainer_pt_utils.py#L908-L913

Comments:
In short, I want to redirect logging info (including metrics) with the `logging` library, but a bare `print` does not allow me to do that.

Why are you not using `save_metrics` then? All our examples use both the prints of `log_metrics` and `save_metrics` to have them accessible in a file.

After digging the code, I realized what I want is to log the metrics of the evaluation loop, which should be handled with callbacks, and is not related to `log_metrics` or `save_metrics`.
Sorry for wrong report.

No problem, glad you found a solution!
Add ONNX support for YOLOS
Author: NielsRogge
Pull request: yes

# What does this PR do?

Make it possible to export the YOLOS model to ONNX format. 

Linked to https://github.com/huggingface/transformers/issues/16308

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

Thanks, I've removed the file and squashed the commits.
TF: GPT-J compatible with XLA generation
Author: gante
Pull request: yes

# What does this PR do?

This PR modifies TF GPT-J so as to be compatible with XLA generation. It borrows the new code from FLAX -- in essence, instead of computing the embedded positions (`sincos`) at each call, given the size of the sequence (which could be obtained from the size of the past), now pre-computes the embedded positions and gathers them given the `position_ids`.

⚠️ The integration tests are disabled with `@tooslow`, due to the size of the model. I've reworked the tests BEFORE touching GPT-J code, to test all needed features correctly. All but the XLA test were passing before GPT-J was changed, and all tests pass after the changes. We still have two XLA tests being run in CI frequently (`test_xla_generate_fast` and `test_xla_generate_slow`), as well as a couple of generic generate tests -- they just don't use the trained model weights.

Comments:
Related issue: #17935 

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