Changes in Pytorch 1.6 multinomial could break backward compatibility
Author: andifunke
Pull request: no
Labels: name: wontfix

## Environment info
     
- `transformers` version: 3.0.2
- Platform: Linux-5.4.0-42-generic-x86_64-with-debian-bullseye-sid
- Python version: 3.6.10
- PyTorch version (GPU?): 1.6.0 (True)
- Tensorflow version (GPU?): not installed (NA)
- Using GPU in script?: True
- Using distributed or parallel set-up in script?: False

### Who can help

@TevenLeScao 

## Information

Model I am using:

Gpt2, but relevant to all language models using sampling.

This is not really a bug report on behalf of transformers, but rather a suggestion to handle a breaking change in the behavior of `torch.multinomial()` that was introduced with v1.6. I've noticed, that the sampling in `GenerationMixin._generate_no_beam_search()` has changed between PT1.5.1 and PT1.6, even when given the same inputs and random state. This is due to a new implementation path of `torch.multinomial()` with `replacement=False`. This new implementation breaks determinism compared to older PT versions. However, there is an easy fix to return to the previous behavior in order to maintain backward compatibility. Since `num_samples` is set to 1, the actual `replacement` value is irrelevant. But setting `replacement` to `True` will use the old sampling implementation and return deterministic results equal to those of earlier versions.

I would therefore recommend to change the call to `multinomial()` in `_generate_no_beam_search()` like this:

```next_token = torch.multinomial(probs, num_samples=1, replacement=True).squeeze(1)```

Best regards,
Andreas

Comments:
Hey @andifunke, 

Thanks a lot for your issue! Could you link the different implementation of `torch.multinomial` between PT v1.5.1 and PT v1.6.0 ? 
I understand your argument, but I think setting `replacement=True` is logically false...

Hi @patrickvonplaten ,

thanks for your reply!

> Could you link the different implementation of torch.multinomial between PT v1.5.1 and PT v1.6.0 ?

Sure. The PR for the implementation is here: https://github.com/pytorch/pytorch/pull/39742 and the merge commit here: https://github.com/pytorch/pytorch/commit/97dfdaaad89c2082c90aebfa9180293847cffd60

> I understand your argument, but I think setting replacement=True is logically false...

I agree, it feels a bit hacky, but let me give you an example, why I think this workaround is justified:

The following code will behave differently in PT1.5.1 vs 1.6:

```python
import torch

torch.manual_seed(0)
t = torch.rand(10, 10)

torch.manual_seed(0)
a = torch.multinomial(t, num_samples=1, replacement=False)

torch.manual_seed(0)
b = torch.multinomial(t, num_samples=1, replacement=True)

torch.__version__, a, b, all(a == b)
```

Pytorch 1.5.1:

```
Out[1]: 
('1.5.1',
 tensor([[9],
         [7],
         [3],
         [9],
         [7],
         [6],
         [1],
         [3],
         [5],
         [1]]),
 tensor([[9],
         [7],
         [3],
         [9],
         [7],
         [6],
         [1],
         [3],
         [5],
         [1]]),
 True)
```

Pytorch 1.6:

```
('1.6.0',
 tensor([[7],
         [7],
         [6],
         [1],
         [6],
         [1],
         [9],
         [5],
         [1],
         [2]]),
 tensor([[9],
         [7],
         [3],
         [9],
         [7],
         [6],
         [1],
         [3],
         [5],
         [1]]),
 False)
```

This of course breaks reproducibility between versions when generating text.

Oh, and here is another option, if `replacement=True` feels irritating:

You could use `torch.distributions.categorical.Categorical` instead, which uses the same sampling approach.

example:
```python
import torch

torch.manual_seed(0)
t = torch.rand(10, 10)

torch.manual_seed(0)
a = torch.distributions.categorical.Categorical(t).sample()

torch.manual_seed(0)
b = torch.multinomial(t, num_samples=1, replacement=True)

torch.__version__, a, b, all(a == b.reshape(10))
```

```
Out[1]:
('1.6.0',
 tensor([9, 7, 3, 9, 7, 6, 1, 3, 5, 1]),
 tensor([[9],
         [7],
         [3],
         [9],
         [7],
         [6],
         [1],
         [3],
         [5],
         [1]]),
 True)
```


Hey @andifunke, 

Thanks for your detailed comments - this is great! So it seems like the change was made to speed up the `torch.multinomial(do_replacement=False)` function. This is not really of interest to us though as it will never be the bottleneck in the `.generate()` function. 

I agree with you that we want to keep backward compatibility here. I think the best option in this case in to use `torch.distributions.categorical.Categorical(t).sample()` in this case.

Will open a PR about it :-) 

Great, thanks!

Actually, I just noticed that `torch.distributions.categorical.Categorical(...)` uses `torch.multinomial` under the hood with `do_replacement=True` - so that this is not a better option. 

I'm not 100% sure how to proceed here now. @LysandreJik, @sgugger - what is your opinion on that? 

The problem is the following: Because of a change in PyTorch's `torch.multinomial` function for 1.6, our generation method with `do_sample=True` yields different results when setting `torch.manual_seed(0)` between torch > 1.6 and 1.6.

As @andifunke pointed out, a simple fix would be to set `do_replacement=True`, which is logically not correct IMO, but it does not make a difference for sampling with `num_beams = 1`. For sampling with `num_beams > 1`.

Do you guys think we should go for the simple fix of `do_replacement=True` to keep backward compatibility when using `torch.manual_seed(0)` ? 
It seems like backwards compatibility for `num_beams > 1` is broken either way since it would be false to set `do_replacement=True` there. 

Can we copy the old implementation somewhere and just use that or is it hidden in C/CUDA?

Did we also reach out to the PyTorch team and make sure they are aware of this BC?

Looks like this is hidden in C/CUDA: https://github.com/pytorch/pytorch/pull/39742/files .
Not sure whether the PyTorch is aware of it...

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.