use functional interface for softmax in attention
Author: t-vi
Pull request: yes

There are several instances of (ab)using the PyTorch modular interface to compute softmax where it would be more natural to use the functional interface. This patch changes the occurrences I found.



Comments:
Probably it's an attention thing. :) 

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.

Closing this as stale.

Out of curiosity, why didn't you want to fix the last two instances and closed your PR instead?

@sgugger To be honest, I'd have preferred to not comment on this further. But as you asked:

So from my side, this is what happened
- I use transformers as a reference for some custom implementation (i.e. I don't plan on using transformers itself) and notice a dubious code pattern of instantiating Softmax and immediately using it.
- Out of courtesy, I submit a PR. To make the review worthwhile, I look for other instances of the identical pattern and fix these, too.
At this point, I submitted a patch that looked "very low risk (because I never say obviously correct), easy to review" to me.
It probably doesn't fix all dubious patterns in transformers, but it takes out 28 instances spread across as many files.

- The patch is promptly approved by you and stas. Thank you!

Then instead of merging:
- You point to two other directories that you say contain more of it (I still don't know what you mean, there are uses of the modular Softmax, but in a perfectly OK way to me, I didn't want to audit your entire codebase for dubious patterns).
- Nothing happens for a month.
- Now your bot says the PR is stale and places the burden of moving it along on me.

I  agree with the bot that the PR isn't moving with the expected speed and but I don't want to spend more time with it. I thought that searching for the same patterns across your codebase and fixing the other 27 places would have been a reasonable trade-off between "don't submit 28 identical one-liners" and "don't make it more complicated than it needs to be", obviously, you did not agree. That is OK, but I don't want to do the extra work that would be needed to make this patch acceptable to you.

So you work at scale and process hundreds of patches any given day and I am not saying your process isn't adequate. It is just not for me.


There seems to have been a misunderstanding here and I just wish you had either told us that the remaining instances you had found were fine in your book, or that you didn't want to work further on this PR. It was just a suggestion on my side and I never said the PR would not be merged if you didn't include those last two instances. I apologize if my comment upset you.

If you want to reopen your PR, we'll be happy to merge it.

No worries, there is nothing wrong with it nor with your comment, it's just that I don't want to change this patch anymore. (And maybe I wasn't in the mood for your bot comment, but hey.)
If it's still useful, I'm happy to have it merged, if not, it's OK too.

The bot is there to remind us when we forget PRs for a long time like this one, sorry about it :-)
Thanks again for your contribution!

Thank you! You're awesome!