Repository: RasaHQ/rasa
Pull request: Fix overlapping entities
**Proposed changes**:
- fixes #1562

**Status (please check what you already did)**:
- [x] made PR ready for code review
- [x] added some tests for the functionality
- [ ] updated the documentation
- [ ] updated the changelog


Problem statement:
Fix overlapping entities in evaluate script
Right now the evaluate script fails whenever there's any overlapping entity labels. It should only fail for overlapping entities *within* an entity classifier. In fact, I think it only actually makes sense for the `ner_crf`, given `ner_duckling_http` could identify both a number and a date in one which is fine.
PS. this causes evaluation on the platform to fail

Hints from issue discussion:
@akelad do we have an example of the offending data and/or error logs? 
The website demo data causes this, and the error includes "overlapping entities". The error is thrown here: https://github.com/RasaHQ/rasa_nlu/blob/master/rasa_nlu/evaluate.py#L476 and the function is defined here: https://github.com/RasaHQ/rasa_nlu/blob/master/rasa_nlu/evaluate.py#L407
Basically, just add a check there whether the entity recogniser is `ner_crf`, if yes check for overlapping entities, if not then don't

Commit message: Merge branch 'master' into fix-overlapping-entities

Patch to review:
diff --git a/rasa_nlu/evaluate.py b/rasa_nlu/evaluate.py
index 2f9098c71c37..b22b2c0c0e8e 100644
--- a/rasa_nlu/evaluate.py
+++ b/rasa_nlu/evaluate.py
@@ -41,8 +41,8 @@
 def create_argument_parser():
     import argparse
     parser = argparse.ArgumentParser(
-            description='evaluate a Rasa NLU pipeline with cross '
-                        'validation or on external data')
+        description='evaluate a Rasa NLU pipeline with cross '
+                    'validation or on external data')
 
     parser.add_argument('-d', '--data', required=True,
                         help="file containing training/evaluation data")
@@ -204,7 +204,7 @@ def drop_intents_below_freq(td, cutoff=5):
     """Remove intent groups with less than cutoff instances."""
 
     logger.debug(
-            "Raw data intent examples: {}".format(len(td.intent_examples)))
+        "Raw data intent examples: {}".format(len(td.intent_examples)))
     keep_examples = [ex
                      for ex in td.intent_examples
                      if td.examples_per_intent[ex.get("intent")] >= cutoff]
@@ -369,10 +369,10 @@ def evaluate_entities(targets,
     for extractor in extractors:
         merged_predictions = merge_labels(aligned_predictions, extractor)
         merged_predictions = substitute_labels(
-                merged_predictions, "O", "no_entity")
+            merged_predictions, "O", "no_entity")
         logger.info("Evaluation for entity extractor: {} ".format(extractor))
         report, precision, f1, accuracy = get_evaluation_metrics(
-                merged_targets, merged_predictions)
+            merged_targets, merged_predictions)
         log_evaluation_table(report, precision, f1, accuracy)
         result[extractor] = {
             "report": report,
@@ -462,18 +462,19 @@ def pick_best_entity_fit(token, candidates):
         return candidates[best_fit]["entity"]
 
 
-def determine_token_labels(token, entities):
+def determine_token_labels(token, entities, extractors):
     """Determines the token label given entities that do not overlap.
 
     :param token: a single token
     :param entities: entities found by a single extractor
+    :param extractors: list of extractors
     :return: entity type
     """
 
     if len(entities) == 0:
         return "O"
-
-    if do_entities_overlap(entities):
+    if extractors is not None and "ner_crf" in extractors and \
+            do_entities_overlap(entities):
         raise ValueError("The possible entities should not overlap")
 
     candidates = find_intersecting_entites(token, entities)
@@ -501,9 +502,9 @@ def align_entity_predictions(targets, predictions, tokens, extractors):
         entities_by_extractors[p["extractor"]].append(p)
     extractor_labels = defaultdict(list)
     for t in tokens:
-        true_token_labels.append(determine_token_labels(t, targets))
+        true_token_labels.append(determine_token_labels(t, targets, extractors))
         for extractor, entities in entities_by_extractors.items():
-            extracted = determine_token_labels(t, entities)
+            extracted = determine_token_labels(t, entities, extractor)
             extractor_labels[extractor].append(extracted)
 
     return {"target_labels": true_token_labels,
@@ -569,10 +570,10 @@ def get_intent_predictions(targets, interpreter,
     for e, target in zip(test_data.training_examples, targets):
         res = interpreter.parse(e.text, only_output_properties=False)
         intent_results.append(IntentEvaluationResult(
-                target,
-                extract_intent(res),
-                extract_message(res),
-                extract_confidence(res)))
+            target,
+            extract_intent(res),
+            extract_message(res),
+            extract_confidence(res)))
 
     return intent_results
 
@@ -692,7 +693,7 @@ def run_evaluation(data_path, model,
     if is_intent_classifier_present(interpreter):
         intent_targets = get_intent_targets(test_data)
         intent_results = get_intent_predictions(
-                intent_targets, interpreter, test_data)
+            intent_targets, interpreter, test_data)
 
         logger.info("Intent evaluation results:")
         result['intent_evaluation'] = evaluate_intents(intent_results,
@@ -894,7 +895,7 @@ def main():
         data = training_data.load_data(cmdline_args.data)
         data = drop_intents_below_freq(data, cutoff=5)
         results, entity_results = run_cv_evaluation(
-                data, int(cmdline_args.folds), nlu_config)
+            data, int(cmdline_args.folds), nlu_config)
         logger.info("CV evaluation (n={})".format(cmdline_args.folds))
 
         if any(results):
diff --git a/src/pip-delete-this-directory.txt b/src/pip-delete-this-directory.txt
new file mode 100644
index 000000000000..c8883ea99f50
--- /dev/null
+++ b/src/pip-delete-this-directory.txt
@@ -0,0 +1,5 @@
+This file is placed here by pip to indicate the source was put
+here by pip.
+
+Once this package is successfully installed this source code will be
+deleted (unless you remove this file).
diff --git a/src/sphinx-rtd-theme b/src/sphinx-rtd-theme
new file mode 160000
index 000000000000..6b89a7c13bba
--- /dev/null
+++ b/src/sphinx-rtd-theme
@@ -0,0 +1 @@
+Subproject commit 6b89a7c13bba2a7100d7093ed5007b2abf67f277
diff --git a/tests/base/test_evaluation.py b/tests/base/test_evaluation.py
index 5092b7b9c987..ff123f7243cd 100644
--- a/tests/base/test_evaluation.py
+++ b/tests/base/test_evaluation.py
@@ -18,6 +18,7 @@
 from rasa_nlu.evaluate import does_token_cross_borders
 from rasa_nlu.evaluate import align_entity_predictions
 from rasa_nlu.evaluate import determine_intersection
+from rasa_nlu.evaluate import determine_token_labels
 from rasa_nlu.config import RasaNLUModelConfig
 from rasa_nlu.tokenizers import Token
 from rasa_nlu import training_data, config
@@ -30,10 +31,10 @@
 def duckling_interpreter(component_builder, tmpdir_factory):
     conf = RasaNLUModelConfig({"pipeline": [{"name": "ner_duckling"}]})
     return utilities.interpreter_for(
-            component_builder,
-            data="./data/examples/rasa/demo-rasa.json",
-            path=tmpdir_factory.mktemp("projects").strpath,
-            config=conf)
+        component_builder,
+        data="./data/examples/rasa/demo-rasa.json",
+        path=tmpdir_factory.mktemp("projects").strpath,
+        config=conf)
 
 
 # Chinese Example
@@ -165,6 +166,23 @@ def test_entity_overlap():
     assert not do_entities_overlap(EN_targets)
 
 
+def test_determine_token_labels_throws_error():
+    with pytest.raises(ValueError):
+        determine_token_labels(CH_correct_segmentation,
+                               [CH_correct_entity,
+                                CH_wrong_entity], ["ner_crf"])
+
+
+def test_determine_token_labels_no_extractors():
+    determine_token_labels(CH_correct_segmentation[0],
+                           [CH_correct_entity, CH_wrong_entity], None)
+
+
+def test_determine_token_labels_with_extractors():
+    determine_token_labels(CH_correct_segmentation[0],
+                           [CH_correct_entity, CH_wrong_entity], ["A", "B"])
+
+
 def test_label_merging():
     aligned_predictions = [
         {"target_labels": ["O", "O"], "extractor_labels":
@@ -260,6 +278,7 @@ def test_evaluate_entities():
     mock_extractors = ["A", "B"]
     result = align_entity_predictions(EN_targets, EN_predicted,
                                       EN_tokens, mock_extractors)
+
     assert result == {
         "target_labels": ["O", "O", "O", "O", "O", "O", "O", "O", "food",
                           "location", "location", "datetime"],


Code review comments:
diff_hunk: @@ -462,18 +462,19 @@ def pick_best_entity_fit(token, candidates):
         return candidates[best_fit]["entity"]
 
 
-def determine_token_labels(token, entities):
+def determine_token_labels(token, entities, extractors):
     """Determines the token label given entities that do not overlap.
 
     :param token: a single token
     :param entities: entities found by a single extractor
+    :param extractors: list of extractors
     :return: entity type
     """
 
     if len(entities) == 0:
         return "O"
-
-    if do_entities_overlap(entities):
+    if extractors is not None and "ner_crf" in extractors and \
path: rasa_nlu/evaluate.py
text: @user1:
please put the condition into a separate function, e.g.

```python
def do_extractors_support_overlap(extractors):
    """...""""
    return extractors is not None and CRFEntityExtractor.name in extractors
```

diff_hunk: @@ -0,0 +1,5 @@
+This file is placed here by pip to indicate the source was put
path: src/pip-delete-this-directory.txt
text: @user1:
please remove the files under `src/` from source control