Notebook test for Gensen Local. #179
Conversation
|
Check out this pull request on ReviewNB: https://app.reviewnb.com/microsoft/nlp/pull/179 You'll be able to see visual diffs and write comments on notebook cells. Powered by ReviewNB. |
|
hey @AbhiramE how long does it take to compute this notebook with 1 epoch? |
It takes around 6 minutes. (But this is not factoring in the time to download the word vectors) |
How much is the word vectors? |
With word vectors included the test runs for 9m 45s. Around 10 minutes. |
| parameters=dict( | ||
| max_epoch=1, | ||
| config_filepath="../../scenarios/sentence_similarity/gensen_config.json", |
There was a problem hiding this comment.
of the configuration that you have here https://github.com/microsoft/nlp/blob/staging/scenarios/sentence_similarity/gensen_config.json, would it be possible to add some of this parameters as default values in the code and remove the config json?
There was a problem hiding this comment.
Getting rid of config requires some major changes to the code. @catherine667 had mentioned this in her PR comments here,#78 (comment)
It would be difficult to restructure the code in next week we have left in the project.
1. Added compute_correlation_coeff method to utils and separated it from the predict method to ensure single responsbility. 2. Added tests accordingly. 3. In the notebook added a scrap to track preditions and assert it in tests. 4. Also added extra documentation to explain what the predict method is doing. 5. Minor fix to stop train at max_epoch.
|
Pushed an update with the following,
|
Description
gensen_local.ipynb. This test ensures the notebook runs quickly (timed at 6 minutes) end to end by limiting the number of epochs to 1.gensen_trainfile.The change addresses missing tests for gensen_local notebook.
Related Issues
#25
Checklist: