-
Notifications
You must be signed in to change notification settings - Fork 10
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
AMR: Getting back our 75 f-score #45
Comments
I can easily change the NE tagset if you tell me what we need. (Will it make a difference at all? My impression is that the identity of the NE tags doesn't matter anywhere in am-tools. Can we talk about training time briefly once again? It seems strange to me that the NE tags are only available at dev+test time, but the parser is never trained to actually take them into account at training time. Where are NE tags predicted in training? |
No, it won't make a difference, except if we want to run an old model on new data. ToAMConll creates the train.amconll file and uses the NE tagger. |
The dev_fixed.amconll file you sent me has some strange asymmetries that I don't understand:
Obviously column 4 is for the lemmas, but what is colum 3 for? And is it okay that a lot of sentences only have a non-null entry in column 3 for their first tokens, but then not for the subsequent ones? Are our classes |
Ah, okay.
Got it. |
Collecting weirnesses about dev_fixed.amconll: Some "sentences" actually consist of multiple sentences. This seems like a tokenizer bug from when the graphbank was created, but is it at least consistently wrong in the companion data? I guess if the POS tags and lemmas were taken from the companion data, that means it is?
|
Column 3 is the "replacement" column and is mostly intended for "name" in AMR. I populate it with tokens from sentences.txt that differ from their respective token in literals.txt. Since sentences.txt is lower-cased such strange things happen but don't affect performance because the NN doesn't read this column and the replacement column is only a backup option in the relabeling. It's the same as in our ACL experiments.
Yes. Let's do the renaming after the deadline. There's a brief documentation here: https://docs.google.com/spreadsheets/d/1oNNFy6vuDr8dcKNjsHrogqjhsCTCOQU5_WQN6-XFDJU/edit#gid=0 |
The lemmas, POS tags and NE tags in the dev data all seem fine to me. In particular, there is no mismatch that would be explained by a token sequence and a tag sequence differing in length. The NE tag |
I moved the documentation of AM-CoNLL to the wiki: https://github.com/coli-saar/am-parser/wiki/AM-CoNLL-file-format Let's put further updates there instead of the GD file. |
Similar mis-tokenized sentence to my dev example above also occur in the training data:
|
The mistokenizations are from the MRP companion data, e.g. /proj/irtg/sempardata/mrp/LDC2019E45/2019/companion/amr/bolt.conllu |
This is what the sentence looked like in the ConceptNet + CoreNLP version:
|
One difference between decomposable devset and dev set is how well we recognize named entities, which affects whether tokens are joined; this has of course an impact on the AM dependency tree. Prediction on decomposable dev set (= with gold named entities, thus "joining" is gold):
Prediction on dev set (different model though):
|
I indeed messed something up in the relabeling with the properties. The fix (pushed) gives us an improvement of 1 point (see spreadsheet for results with bug):
I parsed the new dev set with an old model (https://www.comet.ml/namednil/mrp/93b5d12f46b5462bbe562ab8e16b9865) and got the following results:
The property score is so low here because I used the new version of am-tools where the property handling expects the input to look different and thus doesn't recognize named entities -- so nothing to worry about.
Also to keep in mind about these scores:
The text was updated successfully, but these errors were encountered: