Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UnexpecTEDIntentPolicy predictions fix #9079

Merged
merged 20 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions rasa/core/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
from rasa.shared.core.training_data.story_writer.yaml_story_writer import (
YAMLStoryWriter,
)
from rasa.shared.core.training_data.structures import (
StoryStep,
)
from rasa.shared.core.domain import Domain
from rasa.nlu.constants import (
RESPONSE_SELECTOR_DEFAULT_INTENT,
Expand Down Expand Up @@ -680,9 +683,7 @@ def _collect_action_executed_predictions(
action_predictions=[predicted_action], action_targets=[expected_action]
)

if action_executed_eval_store.has_prediction_target_mismatch() or (
predicted_action_unlikely_intent and predicted_action != expected_action
):
if action_executed_eval_store.has_prediction_target_mismatch():
partial_tracker.update(
WronglyPredictedAction(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have an predict action_unlikely_intent but it's not in the story, and the following action is correct, won't be now end up with two WronglyPredictedAction in the tracker?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we predict action_unlikely_intent then we ask for another prediction, and only add this WronglyPredictedAction if there is a mismatch in expected/predicted. This is the only case when two WronglyPredictedActions will be added but that's perfectly fine because we filter out action_unlikely_intents from logs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok but if predicted_action_unlikely_intent and predicted_action != expected_action on line 584 is False, then predicted_action is not changed because we don't re-run so we no predicted_action_unlikely_intent and predicted_action != expected_action on line 609 will also always be False. And if line 584 is True then we know the next action can't be another action_unlikely_intents right? So can we change 609 to just if action_executed_eval_store.has_prediction_target_mismatch()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Changed that.

expected_action_name,
Expand All @@ -698,7 +699,7 @@ def _collect_action_executed_predictions(
if (
fail_on_prediction_errors
and predicted_action != ACTION_UNLIKELY_INTENT_NAME
and expected_action != ACTION_UNLIKELY_INTENT_NAME
and predicted_action != expected_action
):
story_dump = YAMLStoryWriter().dumps(partial_tracker.as_story().story_steps)
error_msg = (
Expand Down Expand Up @@ -972,6 +973,20 @@ async def _collect_story_predictions(
)


def _filter_step_events(step: StoryStep) -> StoryStep:
events = []
for event in step.events:
if (
type(event) == WronglyPredictedAction
and event.action_name == event.action_name_prediction == ACTION_UNLIKELY_INTENT_NAME
):
continue
events.append(event)
updated_step = step.create_copy(use_new_id=False)
updated_step.events = events
return updated_step


def _log_stories(
trackers: List[DialogueStateTracker], file_path: Text, message_if_no_trackers: Text
) -> None:
Expand All @@ -981,7 +996,7 @@ def _log_stories(
f.write(f"# {message_if_no_trackers}")
else:
stories = [tracker.as_story(include_source=True) for tracker in trackers]
steps = [step for story in stories for step in story.story_steps]
steps = [_filter_step_events(step) for story in stories for step in story.story_steps]
f.write(YAMLStoryWriter().dumps(steps))


Expand Down
43 changes: 42 additions & 1 deletion tests/core/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,54 @@ async def test_action_unlikely_intent_correctly_predicted(
)

result = await rasa.core.test.test(
str(file_name), agent, out_directory=str(tmp_path),
str(file_name), agent, out_directory=str(tmp_path), fail_on_prediction_errors = True
dakshvar22 marked this conversation as resolved.
Show resolved Hide resolved
)
assert "report" in result.keys()
assert result["report"]["conversation_accuracy"]["correct"] == 1
assert result["report"]["conversation_accuracy"]["with_warnings"] == 0


async def test_action_unlikely_intent_fail_on_prediction_errors(
alwx marked this conversation as resolved.
Show resolved Hide resolved
monkeypatch: MonkeyPatch, tmp_path: Path, moodbot_domain: Domain
):
monkeypatch.setattr(
SimplePolicyEnsemble,
"probabilities_using_best_policy",
_probabilities_with_action_unlikely_intent_for(["mood_unhappy"]),
)
dakshvar22 marked this conversation as resolved.
Show resolved Hide resolved

file_name = tmp_path / "test_action_unlikely_intent_2.yml"
file_name.write_text(
"""
version: "2.0"
stories:
- story: unlikely path (with action_unlikely_intent)
steps:
- user: |
very terrible
intent: mood_unhappy
- action: utter_cheer_up
- action: action_unlikely_intent
- action: utter_did_that_help
- intent: affirm
- action: utter_happy
"""
)

# We train on the above story so that RulePolicy can memorize
# it and we don't have to worry about other actions being
# predicted correctly.
agent = await _train_rule_based_agent(
moodbot_domain, file_name, monkeypatch, ignore_action_unlikely_intent=False
)

with pytest.raises(rasa.core.test.WrongPredictionException):
result = await rasa.core.test.test(
alwx marked this conversation as resolved.
Show resolved Hide resolved
str(file_name), agent, out_directory=str(tmp_path), fail_on_prediction_errors=True
)



async def test_wrong_action_after_action_unlikely_intent(
monkeypatch: MonkeyPatch, tmp_path: Path, moodbot_domain: Domain
):
Expand Down