Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Thanks @arnaujc91 . Can you please fix the test failure shown here in CI? It's saying ModuleAttributeError: 'AWD_LSTM' object has no attribute 'encoder_dp'. Also, could you add to the notebook some basic docs and a test or example for EmbeddingDropout so that I can understand the new code you've added?

Ok, I have created a notebook where I tried to explain as good as possible why I am doing this PR and what are the possible solutions that I see to the problem.

See the following notebook: EmbeddingDropout

P.S. I updated again the notebook. Let me know what you think. Thanks a lot!

Thanks @arnaujc91 - I'm still seeing the test failure however. Would you be able to fix that?

I modified the PR to pass the previous test, but now is failing in another test:

      1 enc = nn.Embedding(10, 7, padding_idx=1)
----> 2 enc_dp = EmbeddingDropout(enc, 0.5)
      3 tst_inp = torch.randint(0,10,(8,))
      4 tst_out = enc_dp(tst_inp)
      5 for i in range(8):

This is because the class EmbeddingDropout expects that you pass it an instance of nn.Embedding to the constructor but precisely the point of my PR is that the new EmbeddingDropout does not need an instance of nn.Embedding. So either I can change this test or the tests are too rigid for this PR to make sense.

As I explained in the previous Notebook there are two possibilities to fix the ActivationStats problem:

1. Modify flatten_model
2. Modify EmbeddingDropout

I decided to modify EmbeddingDropout. The reason was twofold: it simplified the code and it solved the issue with the hooks. My current modification just affects EmbeddingDropout; AWD_LST stays the same. Do you really need that EmbeddingDropout requires an instance of nn.Embedding in other parts of the library?

If the tests can't be modified then I will need to close this PR and maybe check if i can modify flatten_model instead.

Can you or me modify the tests for the layer EmbeddingDropout?

Thanks a lot Jeremy.

Please before accepting the modifications of this PR check the Notebook I created explaining how to make full sense of this PR. Right now everything works, but the code can be further cleaned.

Here is the notebook: encoder and encoder_dp are redundant

Thanks a lot!