noamaanMulla-03/transformers-issues huggingface dataset:
NameError: name 'PartialState' is not defined.
Author: gli-mrunal
Pull request: no

I am using the following version of transformer, datasets and huggingface_hub. 

![image](https://user-images.githubusercontent.com/77198742/232941383-cc398bb4-88c0-4a12-9ff1-c59f8c5aa1a6.png)

I am running into the following error:

```sh
 NameError: name 'PartialState' is not defined.
```


How to resolve this issue  to work with my versions of the  transformer, datasets and huggingface_hub ?
My code was running just fine until yesterday.









Comments:
Closing, as this issue is a duplicate of a comment on #22816, where it it being followed up on.
Preprocess/transform logits before gathering them for computing metrics.
Author: ChenyangLiu
Pull request: no

### Feature request

In `trainer.evaluation_loop`, `preprocess_logits_for_metrics` should be executed before `_nested_gather` to avoid GPU OOM.

### Motivation

`preprocess_logits_for_metrics` processes logits after gathering them when do distributed learning. When training with large batch_size, token_length or vocab_size, gathering all logits to 1 node will cause to out of GPU memory. This preprocessing should be executed before `_nested_gather`.

### Your contribution

The main modification would be this in `trainer.evaluation_loop`:
```
           if logits is not None:
                logits = self._pad_across_processes(logits)
                if self.preprocess_logits_for_metrics is not None:
                    logits = self.preprocess_logits_for_metrics(logits, labels)
                logits = self._nested_gather(logits)
                preds_host = logits if preds_host is None else nested_concat(preds_host, logits, padding_index=-100)
```

Comments:
As replied on the PR, this is incorrect. You should probably use a custom training loop powered by Accelerate to be able to move the logits when you want.
Remove 'main' from doc links
Author: amyeroberts
Pull request: yes

# What does this PR do?

There's a bunch of models added to the readme which have `main` in their doc link. This shows up in a lot of contributor's PRs' diffs, which is a bit annoying. This resolves this. 

## 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?


Comments:
_The documentation is not available anymore as the PR was closed or merged._
feat(model parallelism): move labels to the same device as logits for M2M100
Author: elabongaatuo
Pull request: yes

# What does this PR do?

Moves labels to same device as logits for M2M100

Related to  #22561

@sgugger hello please review.

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

> Thanks a lot!

Thank you 😊
Make GPT2ForSequenceClassification computationllay efficient
Author: Kyeongpil
Pull request: no

### Feature request

Compute the logits using only the hidden state for the last position of the input sequence. 

### Motivation

The current GPT2ForSequenceClassification module computes logits using all hidden states, which computationally cost is proportional to the length of the input sequence.
https://github.com/huggingface/transformers/blob/v4.26.1/src/transformers/models/gpt2/modeling_gpt2.py#L1384
If we compute the logits using only the hidden state for the last position of the input sequence, the cost is not proportional to the length.

It can not be only applied to , but also to other XXXForSequenceClassification.

### Your contribution

The followings are the code changes that I suggest:
https://github.com/Kyeongpil/transformers/commit/f97f6e38f444522a55f236b37ca70b4e35096e12


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.
Show diff between 2 CI runs on Slack reports
Author: ydshieh
Pull request: yes

# What does this PR do?

It has now become more difficult to identify the **new** CI failures on Slack reports, as the number of failures is in the range of `[100, 200]` + the number of rows in the reported table is kept at around `40`.

This PR adds a **diff** of the **(model) failure tables** reported by the latest run against by the last previous run.

### The effect

<img width="720" alt="Screenshot 2023-04-17 060007" src="https://user-images.githubusercontent.com/2521628/232374887-66f9259b-9878-4ade-b337-553d3d34dc71.png">


Comments:
_The documentation is not available anymore as the PR was closed or merged._
Remove some pipeline skip cases
Author: ydshieh
Pull request: yes

# What does this PR do?

Remove some pipeline skip cases after #22428: the real tokenizers avoid a lot of failing cases - except for QA with slow tokenizers.

P.S. As discussed once on Slack: the QA pipeline with slow tokenizer uses some methods in `src/transformers/data/processors/squad.py`, and we plan not to make any change to this file.

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Fixup multigpu local_rank
Author: muellerzr
Pull request: yes
Labels: name: Distributed Training / Models

name: bug

# What does this PR do?

The `local_rank` wasn't being properly set when using the `PartialState`, causing failures on the nightlies. This PR fixes it. 

Fixes # (issue)

Failing nightly tests

## 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.

@sgugger 


Comments:
_The documentation is not available anymore as the PR was closed or merged._
Fix to removing ESM special tokens
Author: Rocketknight1
Pull request: yes

This is a followup to PR #22770 - I forgot that because of the way the ESM tokenizer is structured that the EOS token would come back after it was saved and reloaded. By making the special tokens arguments to the tokenizer, we can set them using `init_kwargs` and ensure that they stay changed permanently. Sorry for overlooking this in the last PR!

Comments:
that took like 45 seconds how do you see your notifications that quickly

Ah ah, I was on my GitHub already that's all.

I'm intimidated nonetheless!

_The documentation is not available anymore as the PR was closed or merged._
Update modeling_opt.py
Author: sushmanthreddy
Pull request: yes

# What does this PR do?

As suggested in the [#22561 ](https://github.com/huggingface/transformers/issues/22561) ,moved labels to the same device as logits for OTP , codegen  and gptj

@sgugger  can u pls review this once??

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

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_22868). All of your documentation changes will be reflected on that endpoint.
Add Segment Anything Model (SAM)
Author: ArthurZucker
Pull request: yes

# What does this PR do?

Original repo: https://github.com/facebookresearch/segment-anything

Segment Anything Model (SAM) is a recent model from Meta AI that makes it possible to predict image segmentation masks given an image and various inputs such as bounding boxes, 2D points or previous masks.
It is also mentioned in the original paper that the model can take textual input, but this feature has not been released yet in the original repository.
The release came with 3 weights, namely:

- `sam_vit_b`
- `sam_vit_h`
- `sam_vit_l`

Their main difference is about the vision encoder size, the prompt encoder, and mask decoder should stay the same.

According to the paper, for each input, the model predicts 3 binary masks, corresponding to the region where the "object of interest" lives in the image.

cc @sgugger @amyeroberts 


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

I have few comments for the reviewers before starting the review, IMO we should not expose `SamPromptEncoder` and `SamMaskDecoder` inside the main init (contrary to other models such as Blip where we used to expose the text module and the vision module) mainly because these modules cannot be used as a standalone module. The MaskDecoder needs the image embeddings and the points/bounding box/masks embeddings to predict the masks, and the Prompt Encoder is a super small module (just two embedding layers). But both the image encoder and prompt encoder can be called through `get_xxxx_embeddings` method from `SamForMaskGeneration`.
One last point regarding the PromptEmbedding module, in the paper they mention that this module should also accept textual inputs. However according to the authors this has not been released yet.

cc @sgugger @amyeroberts just FYI

Might be a few things here and there, but new pairs of eyes will help us fix fast. Pinging @amyeroberts and @sgugger for a review!

Merging as I need it to update the pipeline based on reviews. Will adresse remaining comments in a follow up PR
moved labels to the same device as logits for opt, GPTJ , codeine and pix2struct models
Author: sushmanthreddy
Pull request: yes

# What does this PR do?

As suggested in the [#22561 ](https://github.com/huggingface/transformers/issues/22561) , moved labels to the same device as logits for OPT, GPTj, codegen and pix2struct models.





I am new to open source pls give me suggestions for keeping better pr.




Comments:
### there are some things to be changed i will keep pr after that changes

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_22871). All of your documentation changes will be reflected on that endpoint.
XGLM: Fix left-padding (PT and TF)
Author: gante
Pull request: yes

# What does this PR do?

Fixes left-padding for XGLM, on PT and TF. It is the usual problem: `position_ids` was not being used/passed around, and the code assumed that position ids = past length + input length (which is not true when left-padding is present).

Fixes #22707 

While touching XGLM, other issues were sorted:
1. on PT, docs were duplicated (and one of the copies was wrong)
2. XGLM generate with sampling integration test was pinned on CPU (as always, GPU gives different results, which was making our slow CI report an error)
3. TF XLA was failing because of this (left padding support), so now we have TF XLA XGLM 🙌 

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

@amyeroberts now it is ready :)

Adding a new keyword argument is not considered breaking, so that's fine!
Transformers and BERT: dealing with possessives and apostrophes when encode
Author: al-yakubovich
Pull request: no
Labels: name: wontfix

Let's consider two sentences:

    "why isn't Alex's text tokenizing? The house on the left is the Smiths' house"

Now let's tokenize and decode:

    from transformers import BertTokenizer
    tokenizer = BertTokenizer.from_pretrained('bert-base-uncased', do_lower_case=True)
    tokenizer.decode(tokenizer.convert_tokens_to_ids(tokenizer.tokenize("why isn't Alex's text tokenizing? The house on the left is the Smiths' house")))

We get:

    "why isn't alex's text tokenizing? the house on the left is the smiths'house"



**My question is how dealing with missing space in some possessives like *smiths'house*?**


For me, it seems that the process of tokenization in Transformers is done not right. Let's consider output of

    tokenizer.tokenize("why isn't Alex's text tokenizing? The house on the left is the Smiths' house")

we get:

    ['why', 'isn', "'", 't', 'alex', "'", 's', 'text', 'token', '##izing', '?', 'the', 'house', 'on', 'the', 'left', 'is', 'the', 'smith', '##s', "'", 'house']

So in this step, we already have lost important information about the last apostrophe. It would be much better if tokenization was done in the another way:

    ['why', 'isn', "##'", '##t', 'alex', "##'", '##s', 'text', 'token', '##izing', '?', 'the', 'house', 'on', 'the', 'left', 'is', 'the', 'smith', '##s', "##'", 'house']

In this way, tokenization keeps all information about apostrophes, and we will not have problems with possessives.

Comments:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


Any idea about how to solve this apostrophe seperating word token with different word id in bert rokenizer ?
Small typo in `conversation.py` docstring.
Author: oscar-defelice
Pull request: no

## Reproduction

There is a small typographical error in the docstring for Conversation class in conversational.py.

```python

class Conversation:
    """
    Utility class containing a conversation and its history. This class is meant to be used as an input to the
    [`ConversationalPipeline`]. The conversation contains a number of utility function to manage the addition of new
    user input and generated model responses. A conversation needs to contain an unprocessed user input before being
    passed to the [`ConversationalPipeline`]. This user input is either created when the class is instantiated, or by
    calling `conversational_pipeline.append_response("input")` after a conversation turn.

...
```

## Proposed Solution
This could be fixed by just rewriting this to:

```python
class Conversation:
    """
    Utility class containing a conversation and its history. This class is meant to be used as an input to the
    [`ConversationalPipeline`]. The conversation contains several utility functions to manage the addition of new
    user inputs and generated model responses. A conversation needs to contain an unprocessed user input before being
    passed to the [`ConversationalPipeline`]. This user input is either created when the class is instantiated, or by
    calling `conversational_pipeline.append_response("input")` after a conversation turn.

...
```



Comments:
@oscar-defelice Good spot! Would you like to open a PR with the amendment? 

Yes, I can open a PR and add a fixing commit.

thank you very much!
fix: Correct small typo in docstring
Author: oscar-defelice
Pull request: yes

# What does this PR do?

Fixes #22855 

<!--
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 -->


## 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?

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.

@amyeroberts 

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker and @younesbelkada
- vision models: @amyeroberts
- speech models: @sanchit-gandhi
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @gante
- pipelines: @Narsil
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @sgugger

Integrations:

- deepspeed: HF Trainer: @stas00, Accelerate: @pacman100
- ray/raytune: @richardliaw, @amogkam

Documentation: @sgugger, @stevhliu and @MKhalusova

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: @sgugger
- TensorFlow: @Rocketknight1

 -->


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

> Thanks for the fix and quick PR! 🚀
> 
> For the quality checks, you'll need to run `make fixup` locally and push and changes.

@amyeroberts Thank you very much!
Done!
Generation: only search for eos_token if set
Author: xloem
Pull request: yes

In generation, the current check for `unfinished_sequences.max()`, which is to find sequences that have ended early via `eos_token_id`, creates a synchronization point even when there is no `eos_token`, which slows inference down.

This pull request moves that calculation to inside the condition checking for an `eos_token`, so that such slowdown may be removed by disabling this token.

On my old system with `iommu=soft`, this change is saving me 6 seconds per token on a large model by setting `model.config.eos_token_id = None`.

@gante 

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

> 1. If you rebase with `main`, you'll see that there is another decoding method with the same pattern. Would you be able to rebase and push the change there as well?

Done. I have not reviewed the rest of the function for synchronization points but observe the impact of this change will be much smaller with the nested assistant loop. Nice new feature.

> 2. How's the system where you noticed the big speed change? How are you running `.generate()`? I'd be interested in knowing more about it :)

This is an old Asus KGPE-D16 motherboard. They've been popular with some independent developers as higher-end hackable hardware. It has two old K80s in it and is running the Dasharo third-party bios firmware, which adds support for newer PCI cards.

The firmware is not fully polished and I was observing corruption when transferring data between cards; the solution from nvidia's forums was to pass `iommu=soft` to the kernel. This fixes the issue in a pinch but makes data transfer very slow, and points where data is transferred became the biggest bottlenecks.

The generation call I'm presently using is roughly ````model.generate(input_ids,
            do_sample=False,
            min_length=10,
            max_length=50,
            top_p=0.95,
            temperature=0.0)```` from the cuda branch of the gptq llama repository.

The model approaches 40GB in size and is spread across all 4 logical cards using huggingface accelerate `device_map="auto"`. (I've made another small patch to transformers, not submitted yet (it would affect every model, hard to test), to additionally reduce the need to transfer data between the cards inside the model layer loop, when running with accelerate. The attention mask and position ids are not properly moved off the first card to prepare for layers on other cards, with the vanilla code.)

@xloem thank you for the explanation! 💛
Change schedule CI time
Author: ydshieh
Pull request: yes

# What does this PR do?

**This PR changes the test CI to be scheduled 2 hours after the image build CI.**

Despite #22859 changed to use `accelerate@main`, the last DeepSpeed CI docker image still has `accelerate==0.18.0`. This is because that image build takes ~1h30m to finish, but the test CI is scheduled (only) 1 hour after the image build CI.

Although from the next run, the schedule test CI will start  to use `accelerate@main`, there will be a gap - i.e. it will use the `accelerate@main` **one day before** the current `main`.





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

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_22884). All of your documentation changes will be reflected on that endpoint.
[`SAM`] Change to `facebook/sam-vit-base`
Author: younesbelkada
Pull request: yes

# What does this PR do?

Changes the checkpoint name to `sam-vit-base` instead of `sam-vit-big` which was slightly confusing for users. The checkpoints are sorted with their size

- `sam-vit-base`: 350MB
- `sam-vit-large`: 1GB
- `sam-vit-huge`: 2GB

Which now makes more sense. The repo on the Hub have been updated accordingly

cc @amyeroberts @ArthurZucker @sgugger 

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