noamaanMulla-03/transformers-issues huggingface dataset:
Fix doc examples: modify config before super().__init__
Author: ydshieh
Pull request: yes

# What does this PR do?

Some PT Causal LM models modify config after `super().__init__(config)`. This causes some doc examples failed.
See #14672 for the details.

This PR fixes this issue: It does the same as what has been done in 
```
class ProphetNetForCausalLM(ProphetNetPreTrainedModel):
    def __init__(self, config):
        # set config for CLM
        config = copy.deepcopy(config)
        config.is_decoder = True
        config.is_encoder_decoder = False
        super().__init__(config)
        self.prophetnet = ProphetNetDecoderWrapper(config)
```

Fixes #14672

## Who can review?
@patrickvonplaten 


Comments:
 This makes sense to me - thanks for outlining the problem in the issue! @sgugger @LysandreJik - can you take a quick look as well?
Fix doc examples: KeyError
Author: ydshieh
Pull request: yes

# What does this PR do?

The doc examples in `BlenderbotSmall` have

```
        >>> inputs = tokenizer([UTTERANCE], return_tensors='pt')
        >>> inputs.pop("token_type_ids")
```
However, `BlenderbotSmallTokenizer` doesn't give `token_type_ids`, and cause `KeyError` here.

This PR fix this.

## Who can review?


@patrickvonplaten @patil-suraj

Comments:
Fix doc examples: cannot import name
Author: ydshieh
Pull request: yes

# What does this PR do?

In doc examples in some PT model files, there are `cannot import name` error. This PR fixes this issue.

## Who can review?

@patrickvonplaten 

## Question:

@patrickvonplaten , in `modeling_unispeech.py`, there are examples using

```
from transformers import UniSpeechSatFeatureExtractor
```
And in some docstring, there are 
```
:class:`~transformers.UniSpeechSatProcessor`
```

However, these can't be imported

```
ImportError: cannot import name 'UniSpeechSatFeatureExtractor' from 'transformers' (/home/ydshieh/Desktop/ydshieh/transformers/src/transformers/__init__.py)
```
Is there any reason to make these objects invisible to `transformers`?

Comments:
Great catch - thanks a lot for fixing it! I think we should do the exact same thing for the speech models (UniSpeech, SEW, UniSpeechSAT) actually. Feel free to open a PR if you want - I'll put it on my ToDo list as well though

@patrickvonplaten 

So `UniSpeechSatFeatureExtractor` shouldn't be used by `UniSpeechSatModel` , and it is supposed to use `Wav2Vec2FeatureExtractor`?

I have seen 

```
_PROCESSOR_FOR_DOC = "Wav2Vec2Processor"
_SEQ_CLASS_PROCESSOR_FOR_DOC = "Wav2Vec2FeatureExtractor"
```
and `UniSpeechSatFeatureExtractor` is not visible to `transformers`. But I don't have the full context.

> @patrickvonplaten
> 
> So `UniSpeechSatFeatureExtractor` shouldn't be used by `UniSpeechSatModel` , and it is supposed to use `Wav2Vec2FeatureExtractor`?
> 
> I have seen
> 
> ```
> _PROCESSOR_FOR_DOC = "Wav2Vec2Processor"
> _SEQ_CLASS_PROCESSOR_FOR_DOC = "Wav2Vec2FeatureExtractor"
> ```
> 
> and `UniSpeechSatFeatureExtractor` is not visible to `transformers`. But I don't have the full context.

Yeah there is no `UniSpeechFeatureExtractor` since it's the same as `Wav2Vec2FeatureExtractor` so we should instead just use `Wav2Vec2FeatureExtractor`. Happy to resolve this is another PR though!

@ydshieh - if ok for you, I would like to merge this one now and then if you want we could open a new one for `UniSpeechFeatureExtractor`

> @ydshieh - if ok for you, I would like to merge this one now and then if you want we could open a new one for `UniSpeechFeatureExtractor`

Sure, ok for me.
Fix: change tooslow to slow
Author: ydshieh
Pull request: yes

# What does this PR do?

`test_saved_model_creation_extended` in `TFViT` test is currently `tooslow`. It was copied from TF common test.
#14415 move it from common test to TF core test + changed it to `slow`.

This PR fix this inconsistency.

## Who can review?

@Rocketknight1 

Comments:
LGTM!
Avoid using tf.tile in embeddings for TF models
Author: ydshieh
Pull request: yes

# What does this PR do?

Some TF models use

```
position_embeds = tf.gather(params=self.position_embeddings, indices=position_ids)
position_embeds = tf.tile(input=position_embeds, multiples=(input_shape[0], 1, 1))
```
which assume that `position_ids` has size 1 along batch dimension. If users don't specify `position_ids`, we create it
(before using it)
```
if position_ids is None:
    position_ids = tf.expand_dims(
        tf.range(start=past_key_values_length, limit=input_shape[1] + past_key_values_length), axis=0
    )
```
which will have batch size 1. However, in `INPUTS_DOCSTRING`, it specifies the shape to be `(batch_size, seq_len)`.
If a user provides a full batch for `position_ids` (although this is very unlikely), `tf.tile` shouldn't be used here.

This PR fixes this issue.

## Who can review?

@Rocketknight1 

Comments:
I love it, thank you for doing this! I wonder if there's a reason for using the `Add()` layers like that originally? It feels very odd.

Either way, it's a straightforward change and I'm happy to merge as-is, so let me know once you're ready.

> I love it, thank you for doing this! I wonder if there's a reason for using the `Add()` layers like that originally? It feels very odd.

I also feel the same, and don't know why `Add()` is used. I removed it here also because `Add()` requires the shape to be  the same, including batch dim (and won't work after I removed tf.tile).

The PR is ready. I can rebase on master to see if I can make the tests green.

Failed tests are irrelevant to this PR. Let me know if you prefer to wait and rebase later.

No, we're seeing those tests on every PR. I'm happy to merge now - let me know whenever the PR is done!

> No, we're seeing those tests on every PR. I'm happy to merge now - let me know whenever the PR is done!

It's is done. You can merge. Thanks!

Done!
Fix preprocess_function in run_summarization_flax.py
Author: ydshieh
Pull request: yes

# What does this PR do?

`run_summarization_flax.py` has 

https://github.com/huggingface/transformers/blob/e7ed7ffdcb66c78d3437ed4c3a63c3640f50f436/examples/flax/summarization/run_summarization_flax.py#L535-L537

Using `jnp.array` here will cause `preprocess_function` to hang forever when it is used by `datasets.Dataset.map()` with `num_proc > 1`, when this script is running on a TPU VM.

I think it is related to #12719 and #12720

## Who can review?

@patil-suraj  @patrickvonplaten 

 


Comments:
@patil-suraj - could you take this one? :-)
Fix doc examples: ... takes no keyword arguments
Author: ydshieh
Pull request: yes

# What does this PR do?

In some doc examples, there are

```
          >>> y = XSoftmax.apply(x, mask, dim=-1)
```

which gives `TypeError: apply() takes no keyword arguments`.

This PR fixes it by

```
          >>> # Specify the dimension to apply softmax
          >>> dim = -1

          >>> y = XSoftmax.apply(x, mask, dim)
```

## Who can review?

@LysandreJik 

Comments:
I tried to solve the merge conflicts but I ended up with code quality issues - do you mind taking a look when you have a minute? Sorry about that!

> I tried to solve the merge conflicts but I ended up with code quality issues - do you mind taking a look when you have a minute? Sorry about that!

Looks fine now :)

This is perfect, thanks @ydshieh!
Add TFCLIPModel
Author: ydshieh
Pull request: yes

# What does this PR do?


Add `TFCLIPModel`, along with `TFCLIPTextModel` and `TFCLIPVisionModel`.

(and fixed doc examples in `modeling_clip.py`)

## Who can review?
@Rocketknight1 @patrickvonplaten @NielsRogge @sgugger @LysandreJik 

Comments:
Finally finalized this PR after working on something else. It is ready for review :)

@patil-suraj You know CLIP much better than me, but feel free to assign me as a reviewer too if you want me to double-check the TF code!

Thanks, @Rocketknight1 . I will re-work the initialization part (forgot that PT model codes have `_init_weights`).
Could you check my comment about your question on `tf.tile/tf.broadcast_to`. I have a question there.

I addressed all the reviews, in particular the weight initialization.

In PT version, there is 

```
if isinstance(module, CLIPTextEmbeddings):
    module.token_embedding.weight.data.normal_(mean=0.0, std=factor * 0.02)
    module.position_embedding.weight.data.normal_(mean=0.0, std=factor * 0.02)
```

I am not sure why `0.02` is used instead of `config.initializer_range`. cc @patil-suraj 

Should this be merged @Rocketknight1 @patil-suraj? :smiley: 

> Should this be merged @Rocketknight1 @patil-suraj? 😃

There is (new) conflicts I just saw. I will resolve later today or tomorrow.

okay for merge on my side (once the conflicts are resolved) !

Merging will require:
- a rebase on master
- converting all docstrings to Markdown.

You can install our `doc-builder` utility to do this:
```
pip install git+https://github.com/huggingface/doc-builder
```

Then run from your branch in the Transformers repo
```
doc-builder convert src/transformers/models/clip/modeling_tf_clip.py
```
to do this in the newly added modeling_tf_clip file.

Note that clip.rst has disappeared and you will need to add your model to clip.mdx instead.

Let us know if you need any help and please ping for review when you're done!

@patil-suraj @Rocketknight1 @sgugger I rebased on master and fixed errors. The last failed test is irrelevant I think.
Ready to be merged when you have some time :)

Thank you for reviewing this PR!

@sgugger I'm happy at this point!
[VisionTextDualEncoder] Fix doc example
Author: ydshieh
Pull request: yes

# What does this PR do?

In `modeling_vision_text_dual_encoder.py`, this line (doc example) will fail
```
>>> loss, logits_per_image = outputs.loss, outputs.logits_per_imag
```

It should be `outputs.logits_per_image` (`CLIPOutput`)

## Who can review?

@patil-suraj 

Comments:
Fix doc example: mask_time_indices (numpy) has no attribute 'to'
Author: ydshieh
Pull request: yes

# What does this PR do?

In speech to text models, there are doc examples

```
    >>> mask_time_indices = _compute_mask_indices((batch_size, sequence_length), mask_prob=0.2, mask_length=2)
    >>> with torch.no_grad():
    ...     outputs = model(input_values, mask_time_indices=mask_time_indices)
```
which gives `numpy.ndarray`, and the following line fails

https://github.com/huggingface/transformers/blob/19d37c2dd36d73537a6855c56808e524fb584459/src/transformers/models/wav2vec2/modeling_wav2vec2.py#L1481

with `AttributeError: 'numpy.ndarray' object has no attribute 'to'`.

This PR add 

```
mask_time_indices = torch.tensor(mask_time_indices, device=input_values.device, dtype=torch.bool)
```
in the examples to make them work.


## Who can review?

@patrickvonplaten 

Comments:
Thank you!
[Fix doc example] UniSpeechSatForPreTraining
Author: ydshieh
Pull request: yes

# What does this PR do?

In `UniSpeechSatForPreTraining`, this doc example fails

```
>>> feature_extractor = UniSpeechSatFeatureEncoder.from_pretrained("patrickvonplaten/unispeech_sat-base")
>>> model = UniSpeechSatForPreTraining.from_pretrained("patrickvonplaten/unispeech_sat-base")
```
(can't find this checkpoint on the Hub)

This PR changes it to

```
>>> feature_extractor = Wav2Vec2FeatureExtractor.from_pretrained("microsoft/unispeech-sat-base")
>>> model = UniSpeechSatForPreTraining.from_pretrained("microsoft/unispeech-sat-base")
```

## Who can review

@patrickvonplaten 

Comments:
Enhance TFTrainer.save_model()
Author: ydshieh
Pull request: yes

# What does this PR do?

Currently, `TFTrainer.save_model()` raises errors if the model is not `TFPreTrainedModel` . However `Trainer` works fine with `torch.nn.modules.Module`. 

This is a step to make TFTrainer work with usual `tf.keras.models.Model` models. The idea (from @sgugger) is that a user is building their own models that work like ours (e.g., return the loss as the first output) and can train them with Trainer.

Furthermore, a SavedModel is also saved using `tf.saved_model.save()`.

I tried to avoid duplicated code (check and create output directory before saving), and therefore there is a new method `save_tf_model()` in `modeling_tf_utils`, which is used in `trainer_tf.py`.

For @jplu and @sgugger .

Comments:
Closing in favor of #7619
fix md file to avoid evaluation crash
Author: ydshieh
Pull request: yes

# What does this PR do?

Fix the crash due to the memory usage in the instructions for model evaluation in `FINE_TUNE_XLSR_WAV2VEC2.md`.
The original version `test_dataset["speech"][:2]` load the whole speech array into memory which is too large.
Change it to `test_dataset[:2]["speech"]` runs smoothly and much faster.

## Before submitting
- [ ] This PR improves the docs

## Who can review?

@patrickvonplaten 


Comments:
@patrickvonplaten It seems the finetune notebook is on your own github repository, not on HuggingFace's transformers.

@ydshieh yes this is correct - if it's ok for you feel free to open a PR there :-)
Enhance TFTrainer.save_model()
Author: ydshieh
Pull request: yes

# What does this PR do?

@jplu , could you close PR #7597, please? This PR is a clean one, without the file `modeling_tf_utils` being changed at all.

Currently, TFTrainer.save_model() raises errors if the model is not TFPreTrainedModel . However Trainer works fine with torch.nn.modules.Module.

This is a step to make TFTrainer work with usual tf.keras.models.Model models. The idea (from @sgugger) is that a user is building their own models that work like ours (e.g., return the loss as the first output) and can train them with Trainer.

Furthermore, a SavedModel is also saved using tf.saved_model.save().

For @jplu and @sgugger .

Comments:
> 
> 
> Awesome! Did you try it with a usual training example to see if everything is ok?

Yes, with example/text-classification.
I didn't check yet with a usual `tf.keras.models.Model` (i.e. not TFPretrainedModel). But when I continue with `test_trainer_tf.py`, it will be tested.

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.

Hi, @sgugger & @LysandreJik ,

I didn't realized that this PR is not merged into master. Since it has been for some time, I rebased the branch. All the suggestions from @sgugger  are done in the latest version.

It would be great if you can merge  this PR. Thanks!

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.
Add type hints for BERTGeneration
Author: robsmith155
Pull request: yes

# What does this PR do?
I added type hints for the `BERTGenerationEncoder` and `BERTGenerationDecoder` classes as requested in [#16059](https://github.com/huggingface/transformers/issues/16059) and demonstrated in [#16074](https://github.com/huggingface/transformers/pull/16074). 

I wasn't completely sure on what to use for the `past_key_values` argument. I set it to `Optional[Tuple[Tuple[torch.FloatTensor]]]`, but let me know if this is wrong. Also, not sure if I should also add type hints for the `BertGenerationConfig` class?

@Rocketknight1 

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

> I wasn't completely sure on what to use for the `past_key_values` argument. I set it to `Optional[Tuple[Tuple[torch.FloatTensor]]]`, but let me know if this is wrong. Also, not sure if I should also add type hints for the `BertGenerationConfig` class?

What you put here is fine, and type hints for `BertGenerationConfig` are nice but optional - if you want to do them you can, but the main thing we're interested in is the core model classes. Let me know either way - if you don't want to do it, this is ready to merge now!

Okay great, you can go ahead and merge it then. I'll run your notebook to see what else needs to be done and work on some of those instead. Cheers

Got it. Thanks for the PR!
add type annotations for `gptj`
Author: manandey
Pull request: yes

# What does this PR do?

This PR adds type annotations for GPT-J (PyTorch) as described in #16059.

## Who can review?

@Rocketknight1

Anyone in the community is free to review the PR once the tests have passed.



Comments:
_The documentation is not available anymore as the PR was closed or merged._
MLflowCallback set experiment name
Author: orieg
Pull request: yes

# What does this PR do?

This PR includes the following:
- Resolves #12841: uses the MLFLOW_EXPERIMENT_NAME environment variable and the mlflow.set_experiment() method to ensure the experiment is created if it does not exist already.
- Fixes #17066: Checks properly for an active run using mlflow.active_run() (Bug introduced in #16131).


## 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.
- [x] 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.

@sgugger 

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

Oh actually, multiple checks in our CI are not running for some reason. Can you try sending empty commits to see if it triggers it?

@sgugger I tried an empty commit, a comment change on the py file, and a rebase. They all trigger only two checks. They seem to pass tho.

Yes, but for some reason, the whole battery of tests run by cicleCI is not launching (check any other PR to see there are actually 18 to 20 checks). I have no idea why they don't, and can't merge without being sure nothing is broken by the PR.

@sgugger no clue what's going on. I even tried a new PR in #17091 which also trigger only two CI jobs.

I have no idea what the problem is. Wrote to circleCI support to try to get some help.

Closing this PR in favor of #17091, which is running all the CI tests.
Incorrect check for MLFlow active run in MLflowCallback
Author: orieg
Pull request: no
Labels: name: bug

### System Info

```shell
- mlflow==1.25.1
- `transformers` version: 4.19.0.dev0
- Platform: Linux-5.10.76-linuxkit-aarch64-with-glibc2.31
- Python version: 3.9.7
- Huggingface_hub version: 0.2.1
- PyTorch version (GPU?): 1.10.2 (False)
```


### Who can help?

Should be fixed by #17067

### 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

Steps to reproduce:
1. Follow Training tutorial as per https://huggingface.co/docs/transformers/training
2. Change the training arguments to use `TrainingArguments(output_dir="test_trainer", report_to=['mlflow'], run_name="run0")`
3. On `trainer.train()` the MLFlow UI should report a run with a Run Name of `run0` which is not currently the case.

Cause of the Issue:
```
>> import mlflow
>> print(mlflow.active_run is None, mlflow.active_run() is None)
False True
```

In `src/transformers/integrations.py` the line `if self._ml_flow.active_run is None:` need to be replaced by `if self._ml_flow.active_run() is None:`

### Expected behavior

PR #14894 introduce support for run_name in the MLflowCallback. Though, this does not work as expected since the active run is checked using a method reference that always returns true. Bug introduced by #16131.


Comments:
Fix MLflowCallback and add support for MLFLOW_EXPERIMENT_NAME
Author: orieg
Pull request: yes

# What does this PR do?

This PR includes the following:
- Resolves #12841: uses the MLFLOW_EXPERIMENT_NAME environment variable and the mlflow.set_experiment() method to ensure the experiment is created if it does not exist already.
- Fixes #17066: Checks properly for an active run using mlflow.active_run() (Bug introduced in #16131).

Supersedes #17067 due to CI build errors.

## 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.
- [x] 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.

@sgugger 

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

Managed to activate the test by checking out your PR and pushing it to a new branch in the `transformers` namespace. I have no idea why it worked.

Anyhow, please run `make style` on your branch to fix the code quality issue and we should be good to merge! (you'll need to install the latest versions of our deps for styling with `pip install .[quality] -U` )

@sgugger All checks passed! 🤗 

Thanks a lot!
Fix missing "models" in pipeline test module
Author: ydshieh
Pull request: yes

# What does this PR do?

My PR #17034 forgot to update 
```
package=f"tests.{model_slug}"
```
to
```
package=f"tests.models.{model_slug}"
```
which lead to a lot of pipeline tests being skipped.

Fixed it here.

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