Fixes the BERT SRL model (allenai#124)

* Fixes the BERT SRL model * Productivity through formatting * changelog * Put the vocab into tags * Productivity through formatting * Formatting * Fix random import * Use newly trained model * Updates test Co-authored-by: Matt Gardner <mattg@allenai.org>
matt-gardner · Sep 3, 2020 · 2c2e4e4 · 2c2e4e4
1 parent 44a3ca5
commit 2c2e4e4
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fixed evaluation of metrics when using distributed setting.
+- Fixed a bug introduced in 1.0 where the SRL model did not reproduce the original result.
 
 ## [v1.1.0rc4](https://github.com/allenai/allennlp-models/releases/tag/v1.1.0rc4) - 2020-08-21
 

diff --git a/allennlp_models/lm/models/language_model.py b/allennlp_models/lm/models/language_model.py
@@ -21,7 +21,7 @@ class LanguageModel(Model):
     `Seq2SeqEncoder` to uncontextualized embeddings, using a `SoftmaxLoss`
     module (defined above) to compute the language modeling loss.
 
-    If bidirectional is True,  the language model is trained to predict the next and
+    If bidirectional is True, the language model is trained to predict the next and
     previous tokens for each token in the input. In this case, the contextualizer must
     be bidirectional. If bidirectional is False, the language model is trained to only
     predict the next token for each token in the input; the contextualizer should also

diff --git a/allennlp_models/lm/modules/token_embedders/language_model.py b/allennlp_models/lm/modules/token_embedders/language_model.py
@@ -15,7 +15,7 @@
 
 # Importing at runtime results in a circular dependency.
 if TYPE_CHECKING:
-    from allennlp.models.language_model import LanguageModel
+    from allennlp_models.lm.models.language_model import LanguageModel
 
 
 @TokenEmbedder.register("language_model_token_embedder")

diff --git a/allennlp_models/modelcards/structured-prediction-srl-bert.json b/allennlp_models/modelcards/structured-prediction-srl-bert.json
@@ -2,5 +2,5 @@
     "id": "structured-prediction-srl-bert",
     "registered_model_name": "srl_bert",
     "display_name": "SRL BERT",
-    "archive_file": "bert-base-srl-2020.07.14.tar.gz"
+    "archive_file": "bert-base-srl-2020.09.03.tar.gz"
 }
diff --git a/allennlp_models/structured_prediction/dataset_readers/srl.py b/allennlp_models/structured_prediction/dataset_readers/srl.py
@@ -133,7 +133,14 @@ def __init__(
         **kwargs,
     ) -> None:
         super().__init__(**kwargs)
-        self._token_indexers = token_indexers or {"tokens": SingleIdTokenIndexer()}
+        if token_indexers is not None:
+            self._token_indexers = token_indexers
+        elif bert_model_name is not None:
+            from allennlp.data.token_indexers import PretrainedTransformerIndexer
+
+            self._token_indexers = {"tokens": PretrainedTransformerIndexer(bert_model_name)}
+        else:
+            self._token_indexers = {"tokens": SingleIdTokenIndexer()}
         self._domain_identifier = domain_identifier
 
         if bert_model_name is not None:

diff --git a/tests/pretrained_test.py b/tests/pretrained_test.py
@@ -83,9 +83,7 @@ def test_semantic_role_labeling(self):
             },
             {
                 "verb": "playing",
-                "description": "If you liked [ARG1: the music] [ARG0: we] were [V: playing] "
-                "[ARGM-TMP: last night] , you will absolutely love what we "
-                "'re playing tomorrow !",
+                "description": "If you liked [ARG1: the music] [ARG0: we] were [V: playing] [ARGM-TMP: last night] , you will absolutely love what we 're playing tomorrow !",
                 "tags": [
                     "O",
                     "O",
@@ -112,30 +110,28 @@ def test_semantic_role_labeling(self):
             },
             {
                 "verb": "will",
-                "description": "[ARGM-ADV: If you liked the music we were playing last "
-                "night] , [ARG0: you] [V: will] [ARG1: absolutely love what "
-                "we 're playing tomorrow] !",
+                "description": "If you liked the music we were playing last night , you [V: will] absolutely love what we 're playing tomorrow !",
                 "tags": [
-                    "B-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
-                    "I-ARGM-ADV",
                     "O",
-                    "B-ARG0",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
                     "B-V",
-                    "B-ARG1",
-                    "I-ARG1",
-                    "I-ARG1",
-                    "I-ARG1",
-                    "I-ARG1",
-                    "I-ARG1",
-                    "I-ARG1",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
+                    "O",
                     "O",
                 ],
             },

diff --git a/training_config/structured_prediction/bert_base_srl.jsonnet b/training_config/structured_prediction/bert_base_srl.jsonnet
@@ -1,7 +1,9 @@
+local bert_model = "bert-base-uncased";
+
 {
     "dataset_reader": {
       "type": "srl",
-      "bert_model_name": "bert-base-uncased",
+      "bert_model_name": bert_model,
     },
 
     "data_loader": {
@@ -19,7 +21,7 @@
     "model": {
         "type": "srl_bert",
         "embedding_dropout": 0.1,
-        "bert_model": "bert-base-uncased",
+        "bert_model": bert_model,
     },
 
     "trainer": {