Skip to content
This repository was archived by the owner on Aug 5, 2025. It is now read-only.

Update Bert support#78

Merged
danielhers merged 25 commits intodanielhers:masterfrom
OfirArviv:master
Aug 4, 2019
Merged

Update Bert support#78
danielhers merged 25 commits intodanielhers:masterfrom
OfirArviv:master

Conversation

@OfirArviv
Copy link
Collaborator

  1. Update readme
  2. Prepare for release
  3. Filter passages for bert
  4. minor fixes

README.md Outdated
To download and extract [a multilingual model](https://github.com/huji-nlp/tupa/releases/download/v1.3.10/ucca-bilstm-1.3.10.tar.gz), run:

curl -LO https://github.com/huji-nlp/tupa/releases/download/v1.4.0/bert_multilingual_layers_4_layers_pooling_weighted_align_sum.tar.gz
tar xvzf ucca-bilstm-1.3.10.tar.gz
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the model named 1.3.10 if it's under the 1.4.0 release?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy-paste mistake

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, in case you are wondering - the url doesn't exists yet. I just concluded what it will be once I make the release based on others uploads.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielhers Pay attention. As the url in the readme is not created yet, you should approve this PR only if you ok with merging it to huji/tupa and creating a new release.

README.md Outdated
Run the parser using the model:

python -m tupa example.txt --lang [example lang] -m models/bert_multilingual_layers_4_layers_pooling_weighed_align_sum --use-bert --bert-model bert-base-multilingual-cased --bert-layers -1 -2 -3 -4 --bert-layers-pooling weighed --bert-token-align-by sum --bert-multilingual 0
python -m tupa example.txt --lang [example lang] -m bert_multilingual_layers_4_layers_pooling_weighed_align_sum --use-bert --bert-model bert-base-multilingual-cased --bert-layers -1 -2 -3 -4 --bert-layers-pooling weighed --bert-token-align-by sum --bert-multilingual 0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, shouldn't the bert-specific paramters be saved to the model .json and loaded in model.py load()?
Also, [example lang] is not a language; should be explained.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning on doing this in the "final version" (with xlm, leno, etc.), but it's probably better to so now, as people who are not familiar with the code might use it. I'll work on it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will it work if the params will be loaded from model.json but the user also pass the argument in the command?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSON will have to override the user since we can't distinguish whether Argparser's attributes are from the command line or from the default values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding new commit.
Tested on:

  1. Creating a new mode without bert and reloading it.
  2. Creating a bert model and reloading it
  3. reloading a model created from the vanilla tupa code

@danielhers danielhers merged commit ae4c421 into danielhers:master Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants