nyu-mll / jiant-v1-legacy

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

[CLOSED] Updating to Transformers v2.6.0 #1059

Closed jeswan closed 4 years ago

jeswan commented 4 years ago

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


Performance comparison on a set of representative tasks.

Task albert-xxlarge-v2 (#990) roberta-large (#990) albert-xxlarge-v2 (v2.6.0) roberta-large (v2.6.0)
wsc 0.76 0.692 0.769 0.692
rte 0.888 0.866 0.892 0.848
wic 0.74 0.724 0.741 0.721
sst 0.955 0.961 0.956 0.956
cosmosqa 0.847 0.814 0.846 0.822
qa-srl 0.641 0.664 0.641 0.666

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

jeswan commented 4 years ago

Comment by sleepinyourhat Friday Apr 10, 2020 at 23:12 GMT


Looks good so far! Thanks!

jeswan commented 4 years ago

Comment by sleepinyourhat Wednesday Apr 15, 2020 at 00:33 GMT


Since you asked, this still looks good to me. If the testing infrastructure is all ready to go, though, it couldn't hurt to kick off tests with 2.8.0 now, too.

jeswan commented 4 years ago

Comment by zphang Sunday Apr 19, 2020 at 20:12 GMT


Updated the full table. I think we should be good to merge.

Given the upcoming deadlines, I recommend waiting till after EMNLP to update transformers again.

jeswan commented 4 years ago

Comment by sleepinyourhat Monday Apr 20, 2020 at 13:15 GMT


@zphang Why delay the merge? If it we've vetted it to our usual degree, than we should get some additional speedups/options out of this PR.

Of course, it's not great to make major changes after a round of experiments has already started, but the solution to that would just be to maintain a separate branch for each major experiment, which is a good idea in any case.

jeswan commented 4 years ago

Comment by zphang Monday Apr 20, 2020 at 18:37 GMT


@pyeres I've removed the commit concerting the XLMRoBERTaTokenizer. This PR should only update the requirements (transformers, and tokenizers).

@sleepinyourhat To clarify, I support merging in the update to v2.6.0 now, and putting off the update to v2.8.0.

jeswan commented 4 years ago

Comment by zphang Monday Apr 20, 2020 at 18:53 GMT


Unrelated: test_dynamic_masking failed on the first pass, and passed on rerunning.