RasaHQ / rasa

💬 Open source machine learning framework to automate text- and voice-based conversations: NLU, dialogue management, connect to Slack, Facebook, and more - Create chatbots and voice assistants
https://rasa.com/docs/rasa/
Apache License 2.0
18.91k stars 4.63k forks source link

Backport fixes to `rasa.core.test._get_e2e_entity_evaluation_result` to `2.8.x` #10743

Closed samsucik closed 2 years ago

samsucik commented 2 years ago

There's a customer-affecting bug in 2.8.x due to which, when one runs rasa test core, the trained Core model is also tested on extracting entities from user turns for which the actual user text isn't present, such as this test story:

- story: happy path
  steps:
  - intent: greet
    entities:
      - name
  - action: utter_greet

This bug is fixed in 3.0 and we should back-port the fix.

How to reproduce the error

With the latest 2.8.x, create a rasa init project and add an entity to a training story by adding

entities:
  - name

to the domain, adding

    - hey [sara](name)
    - hello [rasa](name)

to the NLU examples for intent greet, and changing the first training story to:

- story: happy path
  steps:
  - intent: greet
    entities:
      - name
  - action: utter_greet

Now, rasa train and rasa test core --stories ./data. You get this error:

  File "/Users/sams/.miniconda/envs/rasa2.8/bin/rasa", line 5, in <module>
    main()
  File "/Users/sams/rasa2.8/rasa/__main__.py", line 118, in main
    cmdline_arguments.func(cmdline_arguments)
  File "/Users/sams/rasa2.8/rasa/cli/test.py", line 132, in run_core_test
    test_core(
  File "/Users/sams/rasa2.8/rasa/model_testing.py", line 182, in test_core
    rasa.utils.common.run_in_loop(
  File "/Users/sams/rasa2.8/rasa/utils/common.py", line 296, in run_in_loop
    result = loop.run_until_complete(f)
  File "uvloop/loop.pyx", line 1456, in uvloop.loop.Loop.run_until_complete
  File "/Users/sams/rasa2.8/rasa/core/test.py", line 1104, in test
    evaluate_entities(
  File "/Users/sams/rasa2.8/rasa/nlu/test.py", line 864, in evaluate_entities
    aligned_predictions = align_all_entity_predictions(entity_results, extractors)
  File "/Users/sams/rasa2.8/rasa/nlu/test.py", line 1224, in align_all_entity_predictions
    aligned_predictions.append(align_entity_predictions(result, extractors))
  File "/Users/sams/rasa2.8/rasa/nlu/test.py", line 1127, in align_entity_predictions
    for t in result.tokens:
TypeError: 'NoneType' object is not iterable

What's the culprit and how it's fixed in 3.0

The issue is that the Core testing code still expects the trained Core model to be able to extract entities even those even if the corresponding user message is not present, i.e. its text is None. In Rasa 3.0, there's a fixed version where the Core model is not expected to extract entities if the text of the user message isn't present. It's hard to tell if this fix was put into 3.0 because of a bug report or if it was just Enable engineers who noticed this "imperfection" and improved the code while porting it from 2.8 to 3.0. In any case, this should be back-ported to 2.8 to unblock the customer.

_Note: This issue does not occur if one trains the Core model only (rasa train core). However, this should be considered a matter of luck. Why? It just happens that when you run rasa train core, the resulting model will end up with a simple rasa.shared.nlu.interpreter.RegexInterpreter whose featurize_message will return None over here and so the Core model won't be expected to predict the entities for the given UserUttered event. rasa train, on the other hand, produces a proper rasa.core.interpreter.RasaNLUInterpreter which on the above line returns an object like Message(data={'text': None}, output_properties={'text'}, time=None, features=), which then breaks things further down the line (as per the above-pasted error message) because this message doesn't have any tokens and entity extraction shouldn't be carried out on it._

Definition of done

samsucik commented 2 years ago

I should say that I noticed also this line in 3.0 which looks like a fix to an issue that might still be present in 2.8. If this is right, we might want to pack-port that fix as well?

ancalita commented 2 years ago

Released in v2.8.23

raoulvm commented 2 years ago

Thanks for fixing this!