-
Notifications
You must be signed in to change notification settings - Fork 27
Allennlp2 #13
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.
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
Open
UrszulaCzerwinska
wants to merge
8
commits into
allenai:master
Choose a base branch
from
UrszulaCzerwinska:allennlp2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Allennlp2 #13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
best params bert sep
shuffle before train
Contributor
|
Thanks for the PR! You mentioned:
I wonder how much worse are the new results? I would be happy to merge this if the results are close. |
Author
|
I got these best results with roberta sep I will report sci-bert results if it is of interest Params |
Author
|
@armancohan RESULTS PARAMS I guess with more fine tuning this results can get slightly. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Updated code to allennlp 2.0
Changed "[SEP]" mention to a param #8 and updated the dataset reader to deal with special tokens : solved TODO in your code
Changed the way to deal with sequence sentences too long: before the last sentence(s) was(were) cut out and the number of labels and sentences were not matching, in this version all "sub-sentences" of sequence sentencre are shortened till the total < 512: solved TODO in your code
Modified code to deal with only one sentence in sequential sentences (case found in my data)
Added dealing with RoBERTa special tokens (selectin the first one); side-effect, the batch must be set to 1
Why?
To be able to run your code with RoBERTa model or other models from hugging face.
To apply models with different special tokens, some slight adaptation may be necessary.
Code was tested with roberta-base and camembert-base, for bert with scibert-uncased and bert-uncased.
How?
Modified config, datareader and model files to work with allennlp 2.0 and roberta model
Anything Else?
I didn't manage to reproduce your results with this version (using the same model). It seems that in general the model needs to be trained longer (20 epochs) and still the results are not as good as with the older version. It is quite difficult to define why as many elements changed, such as attention layer, the tokenization, optimizer. Maybe you can have a look at the params and see where the problems come from. In the last commit the params are set for the best model I manage to obtain.
I will test it on my data soon, I can share the performances with you.