Argument 'filter_value' for the _get_logits_warper function
Author: jbdel
Pull request: no

# 🚀 Feature request

In generation_utils, when sampling (i.e. `is_sample_gen_mode = True`), add the argument `"filter_value"` to `_get_logits_warper` function call.

[https://github.com/huggingface/transformers/blob/3187228206cce052c5df0a8643fe85d2fd50e6a0/src/transformers/generation_utils.py#L1004](https://github.com/huggingface/transformers/blob/3187228206cce052c5df0a8643fe85d2fd50e6a0/src/transformers/generation_utils.py#L1004
)
## Motivation

In all the logits processors, default `filter_value` is `-float("Inf")`. This is very confusing when working with masks, since `-float("Inf") * 0` is NaN, this would results to a loss of Nan. 

Use case: 

Sampling for RL and calling:
```
out = decoder.generate(
            input_ids=torch.ones((batch_size, 1), dtype=torch.long).cuda() * self.bos_token_id,
            max_length=100,
            num_beams=1,
            num_return_sequences=1,
            output_scores=True,
            return_dict_in_generate=True,
            do_sample=True,
        )
```
to return scores (logits) and the sampled ids.

but sample() overrides `next_tokens` with `pad_token_id` once `eos_token_id` is encountered. 
[https://github.com/huggingface/transformers/blob/3187228206cce052c5df0a8643fe85d2fd50e6a0/src/transformers/generation_utils.py#L1333](https://github.com/huggingface/transformers/blob/3187228206cce052c5df0a8643fe85d2fd50e6a0/src/transformers/generation_utils.py#L1333)

Therefore, if you `torch.gather` the logits according the sampled ids (to get the sampled logits), you might gather a `-float('inf')` (if the logit processor did put `pad_token_id` to `-float('inf')`).

```
samples_ids = out.sequences[:, 1:]
logits = torch.stack(out.scores, dim=1)
sampled_logits = logits.gather(2, samples_ids.unsqueeze(-1))
```

Even if you mask your sampled_logits, you still get the NaN.

 






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/master/CONTRIBUTING.md) are likely to be ignored.

@jbdel - I'm not sure our `generate()` method would work well with RL to be honest. Also, do you have an idea what we could improve to fix the problem? Could you maybe provide a minimum reproducible code snippet that I could run to better understand the problem?

hey @patrickvonplaten, could you expand on why you think generate() would not work well (provided one removes the @torch.no_grad() decorator) ?

To my understanding, If you do sampling and then get the logits of the sampled ids then you can use this logits in a loss and back propagate the gradient (i'm referring to this [paper](https://arxiv.org/abs/1612.00563)).

Should I open a new thread for this question ?

I don't really have a minimal code because this only occurs in special condition being:
- you use `generate()` using `top_k` argument
- `TopKLogitsWarper` puts the logprobs `-float("Inf")` for the non top-k before multinomial sampling
- For a finished sequence in the batch, `generate()` does override the sampled input_id with pad_token_id
- If `TopKLogitsWarper` assigned the logprob `-float(inf)` to `pad_token_id` in these situation, then when you get the logits of your sampled sentences, you can gather some  `-float(inf)`, that breaks the loss computation.

My idea would be to make it possible to change the "filter_value" of  TopKLogitsWarper
[https://github.com/huggingface/transformers/blob/master/src/transformers/generation_logits_process.py#L229](https://github.com/huggingface/transformers/blob/master/src/transformers/generation_logits_process.py#L229)

Hey @jbdel,

Thanks a lot for the clarification! So in order for your code to work you would just need a way to costumize the `"filter_value"` ? So if we would add a `"top_k_filter_value"` argument to `generate()` your code would work? I think in this case we can find definitely some kind of general solution.

Its tricky.

Using a `filter_value` of  -inf is the right way to proceed (as it is right now) because then you are sure non-topk will not be sampled by the multinoulli. So that shouldn't be messed with.

The filtering should rather be done on the logits of the sampled sequences, making sure they do not contain a -inf logprob (as it sometimes happens my scenario). I think replacing -inf with 0 might be a good idea...

Sorry to bounce back also on my question, but is there any specific reason you think `generate` should not be used for RL or is it just a general feeling.

JB



Hey @jbdel,

We can definitely add a new logits processor class that replaces `-inf` with `0`. It should actually be enough to adapt this class https://github.com/huggingface/transformers/blob/c1125dc2ba9f3c383bf860ac9fcd67268385ad8d/src/transformers/generation_logits_process.py#L608 so that the user could decide to replace `inf` with 0 (or -inf) instead. 

Feel free to play around with that to see whether it would help your case.

The reason why I said `generate()` "should" not be used for RL was simply that we've never tested it with RL, but if it works for you even better!

Hey Patrick,

This is actually great. I was indeed doing :
`logits[logits == -float("Inf")] = 0.`
afterward, so if it can be embedded in a processor then its great :) 

I didn't fully validate my experiments but just ran one epoch of SCST training optimizing the ROUGE score using `generate()` and did observe an increase of 6 points. So the function should be usable for RL provided you remove the `no_grad` decorator.

 JB

Awesome - great to hear