Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CLOSED] Updating to Transformers v2.6.0 #1059

Closed
jeswan opened this issue Sep 17, 2020 · 6 comments
Closed

[CLOSED] Updating to Transformers v2.6.0 #1059

jeswan opened this issue Sep 17, 2020 · 6 comments

Comments

@jeswan
Copy link
Contributor

jeswan commented Sep 17, 2020

Issue by zphang
Friday Apr 10, 2020 at 21:37 GMT
Originally opened as nyu-mll/jiant#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
Copy link
Contributor Author

jeswan commented Sep 17, 2020

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

Looks good so far! Thanks!

@jeswan
Copy link
Contributor Author

jeswan commented Sep 17, 2020

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
Copy link
Contributor Author

jeswan commented Sep 17, 2020

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
Copy link
Contributor Author

jeswan commented Sep 17, 2020

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
Copy link
Contributor Author

jeswan commented Sep 17, 2020

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
Copy link
Contributor Author

jeswan commented Sep 17, 2020

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

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

@jeswan jeswan mentioned this issue Sep 17, 2020
Open
@jeswan jeswan closed this as completed Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@jeswan