search

nyu-mll / jiant-v1-legacy

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

[CLOSED] AllenNLP update/support for whole-word-masking BERT [WIP] #765

Closed jeswan closed 4 years ago

jeswan commented 4 years ago

Issue by sleepinyourhat Wednesday Jun 26, 2019 at 22:28 GMT Originally opened as https://github.com/nyu-mll/jiant/pull/765

Addresses half of #730.

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

jeswan commented 4 years ago

Comment by sleepinyourhat Wednesday Jun 26, 2019 at 22:31 GMT

Also resolves #709.

jeswan commented 4 years ago

Comment by sleepinyourhat Thursday Jun 27, 2019 at 17:46 GMT

Relevant: https://github.com/huggingface/pytorch-pretrained-BERT/issues/659#issuecomment-506210983

jeswan commented 4 years ago

Comment by pep8speaks Saturday Jun 29, 2019 at 23:13 GMT

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

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 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 2019-07-06 18:26:05 UTC
jeswan commented 4 years ago

Comment by sleepinyourhat Saturday Jun 29, 2019 at 23:16 GMT

@W4ngatang, I may need your help on this one. It looks like upgrading AllenNLP 0.8.1 => 0.8.4 breaks our patched version of MultiLabelField, and I don't think I understand the use case for the patch well enough to fix it.

Mind taking a look? We can wait until after 1.0 if necessary, but it'd be nice to be up to date from launch.

jeswan commented 4 years ago

Comment by W4ngatang Monday Jul 01, 2019 at 16:19 GMT

Can take a look

jeswan commented 4 years ago

Comment by pruksmhc Thursday Jul 04, 2019 at 19:59 GMT

What's the status of this @sleepinyourhat ?

jeswan commented 4 years ago

Comment by sleepinyourhat Thursday Jul 04, 2019 at 20:02 GMT

See last couple of comments—it's in @W4ngatang's hands.

jeswan commented 4 years ago

Comment by sleepinyourhat Thursday Jul 04, 2019 at 20:02 GMT

Sooner is better, but doesn't have to be in 1.0.

jeswan commented 4 years ago

Comment by W4ngatang Friday Jul 05, 2019 at 19:33 GMT

@sleepinyourhat do you have a MWE for the break with multilabel field? I'm not seeing it running demo.

jeswan commented 4 years ago

Comment by W4ngatang Friday Jul 05, 2019 at 19:38 GMT

Oh, looks like WSC uses it. NVM.

jeswan commented 4 years ago

Comment by W4ngatang Friday Jul 05, 2019 at 19:41 GMT

I'm still not getting the error re: MultiLabelField using WSC (winograd-coreference) to test

jeswan commented 4 years ago

Comment by sleepinyourhat Friday Jul 05, 2019 at 19:42 GMT

Hrm—it was in a slightly odd setup. If you can't reproduce it and WWM BERT works, feel free to merge...

On Fri, Jul 5, 2019 at 3:41 PM Alex Wang notifications@github.com wrote:

I'm still not getting the error re: MultiLabelField using WSC ( winograd-coreference) to test

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_nyu-2Dmll_jiant_pull_765-3Femail-5Fsource-3Dnotifications-26email-5Ftoken-3DAAJZSWMQ6NOVNN4HVQZAH3TP56PXFA5CNFSM4H3WR3BKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZKGL6Q-23issuecomment-2D508847610&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=sCzLyHdE8zgQwk2-sKwA1w&m=sCxxlW72xN22P4bCwqxIaCrzgFs6gBkdhqM0-QS6bJI&s=Y1JZjnBjZKJ8u73pNv2zdGK5KrUocY9iMxQxEOJLfJM&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAJZSWORUQ7QUI5GONTOEWLP56PXFANCNFSM4H3WR3BA&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=sCzLyHdE8zgQwk2-sKwA1w&m=sCxxlW72xN22P4bCwqxIaCrzgFs6gBkdhqM0-QS6bJI&s=6KOZTl47HezkZhyGD9kkP4ufPXUjCnILWzBV7qJw-8k&e= .

jeswan commented 4 years ago

Comment by W4ngatang Friday Jul 05, 2019 at 19:49 GMT

Caveat: PPB on pip doesn't have the whole word masking models, so users that want to use them will have to install PPB through the git repo for a while.

jeswan commented 4 years ago

Comment by W4ngatang Friday Jul 05, 2019 at 19:52 GMT

Oh you said that haha, I just can't read.