[BUG] ExponentialDecayLengthPenalty decreases negative scores

[pokjay]

System Info

• transformers version: 4.31.0
• Platform: Linux-5.15.109+-x86_64-with-glibc2.35
• Python version: 3.10.12
• Huggingface_hub version: 0.16.4
• Safetensors version: 0.3.2
• Accelerate version: not installed
• Accelerate config: not found
• PyTorch version (GPU?): 2.0.1+cu118 (False)
• Tensorflow version (GPU?): 2.12.0 (False)
• Flax version (CPU?/GPU?/TPU?): 0.7.1 (cpu)
• Jax version: 0.4.14
• JaxLib version: 0.4.14
• Using GPU in script?: No
• Using distributed or parallel set-up in script?: No

Who can help?

@gante

Information

• The official example scripts
• My own modified scripts

Tasks

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

Reproduction

https://colab.research.google.com/drive/1_Ur0sQhrUan68OXDlFK0SGQfDhwlb8Pc?usp=sharing

Expected behavior

Issue Explanation

ExponentialDecayLengthPenalty is intended to exponentially increase the score of the eos_token_id after start_index has been reached, allowing generating shorter sequences without having a hard cutoff. (Original PR: #15245 )

When working with shorter sequences it doesn't necessarily cut the sequence, no matter how large the decay factor is.
In the line below, the processor attempts to increase the score of EOS. However when EOS score is negative, this actually decreases the score, as the exponent will be positive.
As I understand, giving a negative decay factor won't work as well due to the power. Due to this it will only succeed if EOS becomes positive.

transformers/src/transformers/generation/logits_process.py

Line 982 in f1732e1

scores[:, i] = scores[:, i] * pow(self.regulation_factor, cur_len - self.regulation_start)

Proposed solution

In the attached Colab notebook, I added a proposed solution. If the score is negative, we can compute the expected penalty added to EOS if the score was positive, and add that to the original negative score.

If this is acceptable, I'd like to create a PR to fix the issue!

[sgugger]

cc @gante

[gante]

Hey @pokjay! It is indeed a bug, and one that is not caught in its test (test_exponential_decay_length_penalty) because the test uses positive logits.

I am in favor of the fix, and I'd also like to ask to add a case with negative logits to the test :)

[pokjay]

@gante Great, I'll fix it and add the test case! I'll add to the same PR the documentation examples from #24783

[github-actions]

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 are likely to be ignored.