search

nyu-mll / jiant-v1-legacy

The jiant toolkit for general-purpose text understanding models
MIT License
21 stars 9 forks source link

[CLOSED] Adding Sentence Order Prediction [WIP] #1058

Closed jeswan closed 4 years ago

jeswan commented 4 years ago

Issue by pruksmhc Friday Apr 10, 2020 at 21:22 GMT Originally opened as https://github.com/nyu-mll/jiant/pull/1058

Adding Sentence Order Prediction Task for ALBERT What this version of MLM supports: ALBERT embedder. Below are the runs for MLM + SOP + intermediate task (which is how we intend to use SOP).

Task Task performance (best ALBERT finetuned on task) MLM perplexity SOP performance
CCG 0.944 5.123 0.978
QQP 0.8594 4.303 0.8428

pruksmhc included the following code: https://github.com/nyu-mll/jiant/pull/1058/commits

jeswan commented 4 years ago

Comment by pep8speaks Friday Apr 10, 2020 at 21:22 GMT

Hello @pruksmhc! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 177:101: E501 line too long (103 > 100 characters) Line 708:17: W291 trailing whitespace Line 1227:80: W291 trailing whitespace Line 1228:41: W291 trailing whitespace

Line 190:101: E501 line too long (103 > 100 characters)

You can repair most issues by installing black and running: black -l 100 ./*. If you contribute often, have a look at the 'Contributing' section of the README for instructions on doing this automatically.

Comment last updated at 2020-04-11 01:21:21 UTC
jeswan commented 4 years ago

Comment by sleepinyourhat Friday Apr 10, 2020 at 22:33 GMT

That was fast! Taking a look shortly...

jeswan commented 4 years ago

Comment by sleepinyourhat Friday Apr 10, 2020 at 22:39 GMT

General sanity check before we run anything big/expensive:

jeswan commented 4 years ago

Comment by pruksmhc Saturday Apr 11, 2020 at 01:25 GMT

I'm reworking this task entirely (basically not making it inherit from Pairclassificationtask) to make the code most cleaner/clearer, so the game plan is to incorporate comments from here. I'm going to start another PR, since most of these comments will be resolved with this change.