`RepetitionPenaltyLogitsProcessor` and `EncoderRepetitionPenaltyLogitsProcessor` contains incorrect and unclear docstrings
Author: larekrow
Pull request: no

1. The class docstring of `RepetitionPenaltyLogitsProcessor` says "tokens with higher scores are _less_ likely to be selected". However, according to the [paper](https://arxiv.org/pdf/1909.05858.pdf) which states that "this penalized sampling works by _discounting_ the scores of previously generated tokens" and the code which lowers the score when penalizing tokens (e.g. by multiplying a negative score with a 1.2 penalty, 1.2 being a value the paper highlighted), the docstring should be corrected to say that "tokens with higher scores are _more_ likely to be selected".

https://github.com/huggingface/transformers/blob/d8e13b3e04da9e61c6f16df43815656f59688abd/src/transformers/generation/logits_process.py#L314-L317

2. `EncoderRepetitionPenaltyLogitsProcessor` requires an additional `encoder_input_ids` arg which docstring says "the encoder_input_ids that _should not_ be repeated within the decoder ids". However, according to the [class docstring](https://huggingface.co/docs/transformers/main/en/internal/generation_utils#transformers.EncoderRepetitionPenaltyLogitsProcessor), https://github.com/huggingface/transformers/issues/18354#issuecomment-1219118151, and the code with increases the score of tokens found within the original input ids (e.g. by multiplying a negative score with a 1 / 2 = 0.5 penalty, where `hallucination_penalty = 2` and is a value the PR author used), these are the ids that _should_ be repeated within the decoder ids.

https://github.com/huggingface/transformers/blob/d8e13b3e04da9e61c6f16df43815656f59688abd/src/transformers/generation/logits_process.py#L338-L346

3. Both `RepetitionPenaltyLogitsProcessor` and `EncoderRepetitionPenaltyLogitsProcessor` require a `penalty` input, which is enforced as a positive float. However, this input only works as expected when `penalty > 1`. If `0 < penalty < 1` is given, the "penalty" becomes a "reward". The docstring does not mention this in any way.

`RepetitionPenaltyLogitsProcessor`
https://github.com/huggingface/transformers/blob/d8e13b3e04da9e61c6f16df43815656f59688abd/src/transformers/generation/logits_process.py#L307-L308

`EncoderRepetitionPenaltyLogitsProcessor`
https://github.com/huggingface/transformers/blob/d8e13b3e04da9e61c6f16df43815656f59688abd/src/transformers/generation/logits_process.py#L335-L336

@gante 

Before delving deeper into the source code and other resources, I was truly confused by the contradicting messages. I hope this will be rectified for other users.

Comments:
@larekrow yes, I agree, there are some inconsistencies and some lack of documentation. 

Issues 1 and 2 get partially resolved with your PR (#25971). On 2, there is clearly an example missing, but I'm reviewing examples ATM.

Regarding 3: it's a shame that `penalty` has the opposite meaning in on both processors, but changing it would be a breaking change. The best we can do is to clarify the docstring, including documenting the reward case! Would you like to open a PR to clarify this one? :) 

Thanks for the feedback @gante. I have attempted to clarify the reward cases in PR #26129 but as alluded to, I feel that the docstrings for `EncoderRepetitionPenaltyLogitsProcessor` will require some adjustments.

Both the class name `EncoderRepetitionPenaltyLogitsProcessor` and the original description (which I have left untouched) are misleading, because the class is not actually penalizing the encoder ids but only rewarding it when an intended `hallucination_penalty` value of >1 is given. In fact, it does not penalize any ids in that case.

The docstring I wrote for this class became somewhat convoluted because of this complication. A class name that would be more accurate would be `EncoderRepetitionRewardLogitsProcessor`, but this would be a breaking change as you pointed out.

Any suggestions as to how we should move forward?

@larekrow I agree with your sentiment, the original implementation should be better (and I, as a reviewer, should have paid more attention to the implications) 🤗 Our north star here at `transformers` is to preserve backward compatibility, even if the original design is sub-optimal. We may lose in terms of clarity, but production users are reassured that we don't make sudden changes!

As such, documenting what's going on (like you did) is the best compromise solution 🤗 Thank you for iterating with me 💛 

No worries, we all appreciate the important work Hugging Face is doing (and there is a lot of work to be done). It's really cool how this huge project is driven by both the staff and the community. Happy to be a part of it 🤗

I've updated the docstrings according to your remarks in #26129. Please take a look whenever you can!

our amzing @gante is off for a few weeks, feel free to ping me once this is ready! 😉 

@ArthurZucker yep this is ready! Please take a look when you can. 

PR merged!