Merge remote-tracking branch 'origin/master' into vision

.github/workflows/issues.yml

@@ -0,0 +1,54 @@
+name: Issues Bot
+
+on:
+  # Uncomment this PR trigger for testing.
+  # pull_request:
+  #   branches:
+  #   - master
+  schedule:
+    - cron: '5 16 * * 1,2,3,4,5'  # runs at 16:05 UTC Monday - Friday
+
+jobs:
+  close_stale_issues:
+    name: Close Stale Issues
+    if: github.repository == 'allenai/allennlp'
+    runs-on: ubuntu-latest
+    env:
+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+    - uses: actions/checkout@v2
+
+    - name: Setup Python
+      uses: actions/setup-python@v1
+      with:
+        python-version: 3.7
+
+    - name: Install requirements
+      run: |
+        pip install PyGithub
+
+    - name: Close stale issues
+      run: |
+        python scripts/close_stale_issues.py
+
+  ping_assignees:
+    name: Ping Inactive Assignees
+    if: github.repository == 'allenai/allennlp'
+    runs-on: ubuntu-latest
+    env:
+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+    - uses: actions/checkout@v2
+
+    - name: Setup Python
+      uses: actions/setup-python@v1
+      with:
+        python-version: 3.7
+
+    - name: Install requirements
+      run: |
+        pip install PyGithub
+
+    - name: Ping inactive assignees
+      run: |
+        python scripts/ping_issue_assignees.py

CHANGELOG.md

@@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+
+- Added a workflow to GitHub Actions that will automatically close unassigned stale issues and
+  ping the assignees of assigned stale issues.
+
+## [v1.1.0rc3](https://github.com/allenai/allennlp/releases/tag/v1.1.0rc3) - 2020-08-12
+
+### Fixed
+
+- Fixed how truncation was handled with `PretrainedTransformerTokenizer`.
+  Previously, if `max_length` was set to `None`, the tokenizer would still do truncation if the
+  transformer model had a default max length in its config.
+  Also, when `max_length` was set to a non-`None` value, several warnings would appear
+  for certain transformer models around the use of the `truncation` parameter.
+- Fixed evaluation of all metrics when using distributed training.
+- Added a `py.typed` marker. Fixed type annotations in `allennlp.training.util`.
+- Fixed problem with automatically detecting whether tokenization is necessary.
+  This affected primarily the Roberta SST model.
+
+
 ## [v1.1.0rc2](https://github.com/allenai/allennlp/releases/tag/v1.1.0rc2) - 2020-07-31
 
 ### Changed
@@ -20,11 +40,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Removed unnecessary warning about deadlocks in `DataLoader`.
 - Fixed testing models that only return a loss when they are in training mode.
 - Fixed a bug in `FromParams` that caused silent failure in case of the parameter type being `Optional[Union[...]]`.
+- Fixed a bug where the program crashes if `evaluation_data_loader` is a `AllennlpLazyDataset`.
 
 ### Added
 
 - Added the option to specify `requires_grad: false` within an optimizer's parameter groups.
 - Added the `file-friendly-logging` flag back to the `train` command. Also added this flag to the `predict`, `evaluate`, and `find-learning-rate` commands.
+- Added an `EpochCallback` to track current epoch as a model class member. 
+- Added the option to enable or disable gradient checkpointing for transformer token embedders via boolean parameter `gradient_checkpointing`.
 
 ### Removed
 

allennlp/commands/train.py

@@ -506,7 +506,7 @@ def finish(self, metrics: Dict[str, Any]):
 
             for key, value in test_metrics.items():
                 metrics["test_" + key] = value
-        elif self.evaluation_data_loader:
+        elif self.evaluation_data_loader is not None:
             logger.info(
                 "To evaluate on the test set after training, pass the "
                 "'evaluate_on_test' flag, or use the 'allennlp evaluate' command."

allennlp/common/checks.py

@@ -2,11 +2,10 @@
 Functions and exceptions for checking that
 AllenNLP and its models are configured correctly.
 """
-from typing import Union, List
-
 import logging
 import re
 import subprocess
+from typing import List, Union
 
 import torch
 from torch import cuda
@@ -20,7 +19,7 @@ class ConfigurationError(Exception):
     (e.g. missing properties, invalid properties, unknown properties).
     """
 
-    def __init__(self, message):
+    def __init__(self, message: str):
         super().__init__()
         self.message = message
 

allennlp/common/file_utils.py

@@ -40,7 +40,6 @@
 
 # Warn if the user is still using the deprecated cache directory.
 if os.path.exists(DEPRECATED_CACHE_DIRECTORY):
-    logger = logging.getLogger(__name__)
     logger.warning(
         f"Deprecated cache directory found ({DEPRECATED_CACHE_DIRECTORY}).  "
         f"Please remove this directory from your system to free up space."

allennlp/common/params.py

@@ -1,12 +1,12 @@
-from os import PathLike
-from typing import Any, Dict, List, Union
-from collections.abc import MutableMapping
-from collections import OrderedDict
 import copy
 import json
 import logging
 import os
 import zlib
+from collections import OrderedDict
+from collections.abc import MutableMapping
+from os import PathLike
+from typing import Any, Dict, List, Union
 
 from overrides import overrides
 
@@ -392,7 +392,7 @@ def log_recursively(parameters, history):
         log_recursively(self.params, self.history)
         return params_as_dict
 
-    def as_flat_dict(self):
+    def as_flat_dict(self) -> Dict[str, Any]:
         """
         Returns the parameters of a flat dictionary from keys to values.
         Nested structure is collapsed with periods.

allennlp/common/testing/__init__.py

@@ -1,11 +1,16 @@
 """
 Utilities and helpers for writing tests.
 """
+from typing import Dict, Any, Optional, Union, Tuple, List
 import torch
+from torch.testing import assert_allclose
 import pytest
 
 from allennlp.common.testing.test_case import AllenNlpTestCase
 from allennlp.common.testing.model_test_case import ModelTestCase
+from allennlp.common.testing.distributed_test import run_distributed_test
+
+from allennlp.training.metrics import Metric
 
 
 _available_devices = ["cpu"] + (["cuda"] if torch.cuda.is_available() else [])
@@ -45,3 +50,53 @@ def cpu_or_gpu(test_method):
     Decorator to indicate that a test should run on both CPU and GPU
     """
     return pytest.mark.gpu(test_method)
+
+
+# Helpers for testing distributed metrics
+
+
+def assert_metrics_values(
+    metrics: Dict[str, Any],
+    desired_values: Dict[str, Any],
+    rtol: float = 0.0001,
+    atol: float = 1e-05,
+):
+    for key in metrics:
+        assert_allclose(metrics[key], desired_values[key], rtol=rtol, atol=atol)
+
+
+def global_distributed_metric(
+    global_rank: int,
+    world_size: int,
+    gpu_id: Union[int, torch.device],
+    metric: Metric,
+    metric_kwargs: Dict[str, List[Any]],
+    desired_values: Dict[str, Any],
+    exact: Union[bool, Tuple[float, float]] = True,
+):
+    kwargs = {}
+
+    # Use the arguments meant for the process with rank `global_rank`.
+    for argname in metric_kwargs:
+        kwargs[argname] = metric_kwargs[argname][global_rank]
+
+    metric(**kwargs)
+
+    metrics = metric.get_metric(False)
+    if not isinstance(metrics, Dict) and not isinstance(desired_values, Dict):
+        metrics = {"metric_value": metrics}
+        desired_values = {"metric_value": desired_values}
+
+    # Call `assertion_metrics_values` to check if the metrics have the desired values.
+    if isinstance(exact, bool):
+        if exact:
+            rtol = 0.0
+            atol = 0.0
+        else:
+            rtol = 0.0001
+            atol = 1e-05
+    else:
+        rtol = exact[0]
+        atol = exact[1]
+
+    assert_metrics_values(metrics, desired_values, rtol, atol)  # type: ignore

allennlp/common/testing/distributed_test.py

@@ -0,0 +1,70 @@
+import datetime
+from typing import List, Dict, Any, Tuple, Callable
+import torch
+import torch.distributed as dist
+import torch.multiprocessing as mp
+
+from allennlp.common.checks import check_for_gpu
+
+
+def init_process(
+    process_rank: int,
+    distributed_device_ids: List[int] = None,
+    world_size: int = 1,
+    func: Callable = None,
+    func_args: Tuple = None,
+    func_kwargs: Dict[str, Any] = None,
+    master_addr: str = "127.0.0.1",
+    master_port: int = 29500,
+):
+    assert world_size > 1
+
+    global_rank = process_rank
+
+    gpu_id = distributed_device_ids[process_rank]  # type: ignore
+
+    if gpu_id >= 0:
+        torch.cuda.set_device(int(gpu_id))
+        dist.init_process_group(
+            backend="nccl",
+            init_method=f"tcp://{master_addr}:{master_port}",
+            world_size=world_size,
+            rank=global_rank,
+        )
+    else:
+        dist.init_process_group(
+            backend="gloo",
+            init_method=f"tcp://{master_addr}:{master_port}",
+            world_size=world_size,
+            rank=global_rank,
+            timeout=datetime.timedelta(seconds=120),
+        )
+
+    func(global_rank, world_size, gpu_id, *func_args, **func_kwargs)
+
+    dist.barrier()
+
+
+def run_distributed_test(
+    device_ids: List[int] = [-1, -1], func: Callable = None, *args, **kwargs,
+):
+    """
+    This runs the `func` in a simulated distributed environment.
+
+    # Parameters
+
+    device_ids: `List[int]`
+        List of devices. There need to be at least 2 devices. Default is [-1, -1].
+
+    func: `Callable`
+        `func` needs to be global for spawning the processes, so that it can be pickled.
+    """
+
+    check_for_gpu(device_ids)
+    nprocs = world_size = len(device_ids)
+    mp.start_processes(
+        init_process,
+        args=(device_ids, world_size, func, args, kwargs),
+        nprocs=nprocs,
+        start_method="fork",
+    )

allennlp/common/tqdm.py

@@ -3,6 +3,7 @@
 global defaults for certain tqdm parameters.
 """
 import logging
+from allennlp.common import logging as common_logging
 import sys
 from time import time
 from typing import Optional
@@ -17,8 +18,6 @@
 else:
     from tqdm import tqdm as _tqdm
 
-from allennlp.common import logging as common_logging
-
 
 # This is necessary to stop tqdm from hanging
 # when exceptions are raised inside iterators.

allennlp/data/tokenizers/pretrained_transformer_tokenizer.py

@@ -236,7 +236,7 @@ def tokenize(self, text: str) -> List[Token]:
             add_special_tokens=False,
             max_length=self._max_length,
             stride=self._stride,
-            truncation_strategy=self._truncation_strategy,
+            truncation=self._truncation_strategy if self._max_length is not None else False,
             return_tensors=None,
             return_offsets_mapping=self.tokenizer.is_fast,
             return_attention_mask=False,

allennlp/data/tokenizers/token.py

@@ -81,6 +81,9 @@ def __init__(
         text_id: int = None,
         type_id: int = None,
     ) -> None:
+        assert text is None or isinstance(
+            text, str
+        )  # Some very hard to debug errors happen when this is not true.
         self.text = text
         self.idx = idx
         self.idx_end = idx_end

allennlp/models/simple_tagger.py

@@ -213,7 +213,7 @@ def get_metrics(self, reset: bool = False) -> Dict[str, float]:
         }
 
         if self.calculate_span_f1:
-            f1_dict = self._f1_metric.get_metric(reset=reset)
+            f1_dict = self._f1_metric.get_metric(reset)
             if self._verbose_metrics:
                 metrics_to_return.update(f1_dict)
             else:

allennlp/modules/token_embedders/pretrained_transformer_embedder.py

@@ -40,6 +40,8 @@ class PretrainedTransformerEmbedder(TokenEmbedder):
         When `True` (the default), only the final layer of the pretrained transformer is taken
         for the embeddings. But if set to `False`, a scalar mix of all of the layers
         is used.
+    gradient_checkpointing: `bool`, optional (default = `None`)
+        Enable or disable gradient checkpointing.
     """
 
     def __init__(
@@ -51,14 +53,19 @@ def __init__(
         train_parameters: bool = True,
         last_layer_only: bool = True,
         override_weights_file: Optional[str] = None,
-        override_weights_strip_prefix: Optional[str] = None
+        override_weights_strip_prefix: Optional[str] = None,
+        gradient_checkpointing: Optional[bool] = None,
     ) -> None:
         super().__init__()
         from allennlp.common import cached_transformers
 
         self.transformer_model = cached_transformers.get(
             model_name, True, override_weights_file, override_weights_strip_prefix
         )
+
+        if gradient_checkpointing is not None:
+            self.transformer_model.config.update({"gradient_checkpointing": gradient_checkpointing})
+
         self.config = self.transformer_model.config
         if sub_module:
             assert hasattr(self.transformer_model, sub_module)