Skip to content

Conversation

jessijzhao
Copy link
Contributor

* changed number of embedding dimensions from 50 to 100
* retrained model in models/imdb-model-cnn.pt
* ran IMDB_TorchText_Interpret.ipynb with new model and current library versions
* updated img/sentiment_analysis.png with image of results
Copy link
Contributor

@NarineK NarineK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this PR, @jessijzhao!
A question: It looks like the size of the model decreased by ~2x. Is that because we store 2x less parameters or did the old version store more than the parameters ?

@jessijzhao
Copy link
Contributor Author

Thanks for the comment @NarineK! The model I trained has 2,620,801 parameters (matching the original training script by bentrevett) and the old model has 5,159,701. I followed the original script by keeping
MAX_VOCAB_SIZE = 25_000 whereas it looks like the old model was using all 101982 words - should I retrain the model with the full vocabulary?

@NarineK
Copy link
Contributor

NarineK commented Jan 28, 2021

Thanks for the comment @NarineK! The model I trained has 2,620,801 parameters (matching the original training script by bentrevett) and the old model has 5,159,701. I followed the original script by keeping
MAX_VOCAB_SIZE = 25_000 whereas it looks like the old model was using all 101982 words - should I retrain the model with the full vocabulary?

Thank you for verifying it, @jessijzhao! If it is not too time consuming, let's try full vocab size. We might obtain better performance results.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NarineK has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@NarineK merged this pull request in 3c47914.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants