Repository: RasaHQ/rasa
Pull request: Fix logging of conversation level core metrics.
Fix logging of conversation level core metrics to only compute and log conversation level accuracy. Formerly F1-Score and precision were being printed to console but upon investigation, these metrics were not very meaningful (precision was always 1.0 if any story was completely correct and 0 otherwise). Additionally, in-data fraction was being printed twice. The new output of rasa test looks like this:


```
2021-02-23 15:54:43 INFO     rasa.core.test  - Finished collecting predictions.
2021-02-23 15:54:43 INFO     rasa.core.test  - Evaluation Results on END-TO-END level:
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Correct:          6 / 7
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Accuracy:         0.857
2021-02-23 15:54:43 INFO     rasa.core.test  - Stories report saved to results/story_report.json.
2021-02-23 15:54:43 INFO     rasa.core.test  - Evaluation Results on ACTION level:
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Correct:          33 / 35
2021-02-23 15:54:43 INFO     rasa.core.test  - 	F1-Score:         0.960
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Precision:        0.970
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Accuracy:         0.961
2021-02-23 15:54:43 INFO     rasa.core.test  - 	In-data fraction: 0.886
```
Formerly the output looked like this:

```
2021-02-23 15:54:43 INFO     rasa.core.test  - Finished collecting predictions.
2021-02-23 15:54:43 INFO     rasa.core.test  - Evaluation Results on END-TO-END level:
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Correct:          6 / 7
2021-02-23 15:54:43 INFO     rasa.core.test  - 	F1-Score:         0.923   # <-- harmonic mean of 1 and recall 
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Precision:        1.0     # <-- this is always 1 if correct > 0
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Accuracy:         0.857
2021-02-23 15:54:43 INFO     rasa.core.test  - 	In-data fraction: 0.886   # <-- printed again below in ACTION level results
2021-02-23 15:54:43 INFO     rasa.core.test  - Stories report saved to results/story_report.json.
2021-02-23 15:54:43 INFO     rasa.core.test  - Evaluation Results on ACTION level:
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Correct:          33 / 35
2021-02-23 15:54:43 INFO     rasa.core.test  - 	F1-Score:         0.960
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Precision:        0.970
2021-02-23 15:54:43 INFO     rasa.core.test  - 	Accuracy:         0.961
2021-02-23 15:54:43 INFO     rasa.core.test  - 	In-data fraction: 0.886
```

Additionally, conversation level accuracy is logged to file (i.e., `results/story_report.json`) as an additional field in a JSON dictionary:

```
{
...
 "conversation_accuracy": {"accuracy": 0.857, "correct": 6, "total": 7}
...
}
```


**Status (please check what you already did)**:
- [X] added some tests for the functionality
- [ ] updated the documentation
- [X] updated the changelog (please check [changelog](https://github.com/RasaHQ/rasa/tree/main/changelog) for instructions)
- [X] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa#code-style) for instructions)


Problem statement:
Clean up and log to file Conversation level performance measures.
**Description of Problem**:
When running rasa test, performance measures for the core model are printed to the console and/or logged to `results/story_report.json`. What is printed to the console is split into two main result blocks, CONVERSATION (or E2E when evaluating E2E data) and ACTION level performance. 

```
2021-02-19 14:07:43 INFO     rasa.core.test  - Evaluation Results on CONVERSATION level:     #<-- CONVERSATION LEVEL BLOCK
2021-02-19 14:07:43 INFO     rasa.core.test  - 	Correct:          42 / 43
2021-02-19 14:07:43 INFO     rasa.core.test  - 	F1-Score:         0.988
2021-02-19 14:07:43 INFO     rasa.core.test  - 	Precision:        1.000
2021-02-19 14:07:43 INFO     rasa.core.test  - 	Accuracy:         0.977
2021-02-19 14:07:43 INFO     rasa.core.test  - 	In-data fraction: 0.863
2021-02-19 14:07:43 INFO     rasa.core.test  - Stories report saved to results/story_report.json.
2021-02-19 14:07:43 INFO     rasa.core.test  - Evaluation Results on ACTION level:  #<-- ACTION LEVEL BLOCK
2021-02-19 14:07:43 INFO     rasa.core.test  -  Correct:          247 / 249
2021-02-19 14:07:43 INFO     rasa.core.test  -  F1-Score:         0.996
2021-02-19 14:07:43 INFO     rasa.core.test  -  Precision:        1.000
2021-02-19 14:07:43 INFO     rasa.core.test  -  Accuracy:         0.992
2021-02-19 14:07:43 INFO     rasa.core.test  -  In-data fraction: 0.863
```

Digging into the CONVERSATION level measures, it seems that what is being computed is not very informative or useful. Because of how these metrics are computed, precision is always 1.0 (unless no stories are correct, in which case it is 0), F1-Score is just the harmonic mean of 1 and the recall (recall is not printed to the console), and recall = accuracy. Additionally, in-data fraction is always the same in the CONVERSATION block as it is in ACTION block. The only thing that would currently be helpful to display at the CONVERSATION level block would be accuracy (number of correct stories / total stories).

**Overview of the Solution**:
I propose only printing Correct and Accuracy at the CONVERSATION level.

```
2021-02-18 10:55:04 INFO     rasa.core.test  - Evaluation Results on CONVERSATION level:
2021-02-18 10:55:04 INFO     rasa.core.test  -  Correct:          42 / 43
2021-02-18 10:55:04 INFO     rasa.core.test  -  Accuracy:         0.977
2021-02-18 10:55:04 INFO     rasa.core.test  - Stories report saved to results/story_report.json.
2021-02-18 10:55:04 INFO     rasa.core.test  - Evaluation Results on ACTION level:
2021-02-18 10:55:04 INFO     rasa.core.test  -  Correct:          247 / 249
2021-02-18 10:55:04 INFO     rasa.core.test  -  F1-Score:         0.996
2021-02-18 10:55:04 INFO     rasa.core.test  -  Precision:        1.000
2021-02-18 10:55:04 INFO     rasa.core.test  -  Accuracy:         0.992
2021-02-18 10:55:04 INFO     rasa.core.test  -  In-data fraction: 0.863
```

Additionally, I propose to include in the `results/story_report.json` an additional field for conversation level accuracy:
```
{
...
 "conversation_accuracy": {"accuracy": 0.977, "correct": 42, "total": 43}
...
}
```

Hints from issue discussion:
Thanks for submitting this feature request 🚀 @joejuzl will get back to you about it soon! ✨

Commit message: Fix logging of conversation level core metrics to only show conversation level accuracy. Add conversation level accuracy to story_report.json

Patch to review:
diff --git a/rasa/core/test.py b/rasa/core/test.py
index 503cb211f4c7..71605667fd43 100644
--- a/rasa/core/test.py
+++ b/rasa/core/test.py
@@ -702,26 +702,18 @@ async def _collect_story_predictions(
             success.append(predicted_tracker)
 
     logger.info("Finished collecting predictions.")
-    with warnings.catch_warnings():
-        from sklearn.exceptions import UndefinedMetricWarning
-
-        warnings.simplefilter("ignore", UndefinedMetricWarning)
-        report, precision, f1, accuracy = get_evaluation_metrics(
-            [1] * len(completed_trackers), correct_dialogues
-        )
 
     in_training_data_fraction = _in_training_data_fraction(action_list)
 
-    _log_evaluation_table(
-        [1] * len(completed_trackers),
-        "END-TO-END" if use_e2e else "CONVERSATION",
-        report,
-        precision,
-        f1,
-        accuracy,
-        in_training_data_fraction,
-        include_report=False,
+    num_convs = len(correct_dialogues)
+    num_correct = sum(correct_dialogues)
+    accuracy = num_correct / num_convs if num_convs else 0.0
+
+    logger.info(
+        f"Evaluation Results on {'END-TO-END' if use_e2e else 'CONVERSATION'} level:"
     )
+    logger.info(f"\tCorrect:          {num_correct} / {num_convs}")
+    logger.info(f"\tAccuracy:         {accuracy:.3f}")
 
     return (
         StoryEvaluation(
@@ -799,6 +791,16 @@ async def test(
                 targets, predictions, output_dict=True
             )
 
+            # Add conversation level accuracy to story report.
+            num_failed = len(story_evaluation.failed_stories)
+            num_correct = len(story_evaluation.successful_stories)
+            num_convs = num_failed + num_correct
+            conv_acc = num_correct / num_correct if num_correct else 0.0
+            report["conversation_accuracy"] = {
+                "accuracy": conv_acc,
+                "correct": num_correct,
+                "total": num_convs,
+            }
             report_filename = os.path.join(out_directory, REPORT_STORIES_FILE)
             rasa.shared.utils.io.dump_obj_as_json_to_file(report_filename, report)
             logger.info(f"Stories report saved to {report_filename}.")


Code review comments:
diff_hunk: @@ -702,26 +702,18 @@ async def _collect_story_predictions(
             success.append(predicted_tracker)
 
     logger.info("Finished collecting predictions.")
-    with warnings.catch_warnings():
-        from sklearn.exceptions import UndefinedMetricWarning
-
-        warnings.simplefilter("ignore", UndefinedMetricWarning)
-        report, precision, f1, accuracy = get_evaluation_metrics(
-            [1] * len(completed_trackers), correct_dialogues
-        )
 
     in_training_data_fraction = _in_training_data_fraction(action_list)
 
-    _log_evaluation_table(
-        [1] * len(completed_trackers),
-        "END-TO-END" if use_e2e else "CONVERSATION",
-        report,
-        precision,
-        f1,
-        accuracy,
-        in_training_data_fraction,
-        include_report=False,
+    num_convs = len(correct_dialogues)
+    num_correct = sum(correct_dialogues)
+    accuracy = num_correct / num_convs if num_convs else 0.0
+
+    logger.info(
path: rasa/core/test.py
text: @user1:
Could we make `_log_evaluation_table` handle `None` (and skip those) so we can reuse that method?

@author:
Yeah that sounds good! Will do.

diff_hunk: @@ -799,6 +791,16 @@ async def test(
                 targets, predictions, output_dict=True
             )
 
+            # Add conversation level accuracy to story report.
+            num_failed = len(story_evaluation.failed_stories)
+            num_correct = len(story_evaluation.successful_stories)
+            num_convs = num_failed + num_correct
+            conv_acc = num_correct / num_correct if num_correct else 0.0
path: rasa/core/test.py
text: @user1:
This seems like a typo? num_correct/num_correct == 1

@user1:
Also GH won't let me comment on the particular line because it's too far out of scope, but while you're at it would you remove the unused import on line 670? (`from rasa.test import get_evaluation_metrics`)

@author:
Oof that's embarrassing! Thank you for catching! Will fix!

diff_hunk: @@ -799,6 +791,16 @@ async def test(
                 targets, predictions, output_dict=True
             )
 
+            # Add conversation level accuracy to story report.
+            num_failed = len(story_evaluation.failed_stories)
+            num_correct = len(story_evaluation.successful_stories)
+            num_convs = num_failed + num_correct
+            conv_acc = num_correct / num_correct if num_correct else 0.0
path: rasa/core/test.py
text: @user1:
Does this differ from the `accuracy` above?

@author:
Yeah the accuracy above it is the percentage of correctly predicted next actions. Should I modify it to be action_accuracy to make it more clear?

@author:
I went ahead and renamed the previous accuracy to action_accuracy to make the distinction clear.