Skip to content

Commit

Permalink
Merge branch 'master' into fix-RedisTrackerStore
Browse files Browse the repository at this point in the history
  • Loading branch information
ducminh-phan authored Nov 28, 2020
2 parents 2304766 + a099316 commit 4366223
Show file tree
Hide file tree
Showing 9 changed files with 394 additions and 52 deletions.
21 changes: 21 additions & 0 deletions CHANGELOG.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,27 @@ https://github.com/RasaHQ/rasa/tree/master/changelog/ . -->
- [#6470](https://github.com/rasahq/rasa/issues/6470), [#7015](https://github.com/rasahq/rasa/issues/7015), [#7090](https://github.com/rasahq/rasa/issues/7090)


## [2.0.8] - 2020-11-26


### Bugfixes
- [#7235](https://github.com/rasahq/rasa/issues/7235): Slots that use `initial_value` won't cause rule contradiction errors when `conversation_start: true` is used. Previously, two rules that differed only in their use of `conversation_start` would be flagged as contradicting when a slot used `initial_value`.

In checking for incomplete rules, an action will be required to have set _only_ those slots that the same action has set in another rule. Previously, an action was expected to have set also slots which, despite being present after this action in another rule, were not actually set by this action.


## [2.0.7] - 2020-11-24


### Bugfixes
- [#5974](https://github.com/rasahq/rasa/issues/5974): `ActionRestart` will now trigger `ActionSessionStart` as a followup action.
- [#7317](https://github.com/rasahq/rasa/issues/7317): Fixed Rasa Open Source not being able to fetch models from certain URLs.

This addresses an issue introduced in 2.0.3 where `rasa-production` could not use the models from `rasa-x` in Rasa X server mode.
- [#7316](https://github.com/rasahq/rasa/issues/7316): `SingleStateFeaturizer` checks whether it was trained with `RegexInterpreter` as
NLU interpreter. If that is the case, `RegexInterpreter` is used during prediction.


## [2.0.6] - 2020-11-10


Expand Down
27 changes: 25 additions & 2 deletions rasa/core/featurizers/single_state_featurizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import rasa.shared.utils.io
from rasa.shared.core.domain import SubState, State, Domain
from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter
from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter, RegexInterpreter
from rasa.shared.core.constants import PREVIOUS_ACTION, ACTIVE_LOOP, USER, SLOTS
from rasa.shared.constants import DOCS_URL_MIGRATION_GUIDE
from rasa.shared.core.trackers import is_prev_action_listen_in_state
Expand Down Expand Up @@ -34,15 +34,29 @@ class SingleStateFeaturizer:
"""

def __init__(self) -> None:
"""Initialize the single state featurizer."""
# rasa core can be trained separately, therefore interpreter during training
# will be `RegexInterpreter`. If the model is combined with a rasa nlu model
# during prediction the interpreter might be different.
# If that is the case, we need to make sure to "reset" the interpreter.
self._use_regex_interpreter = False
self._default_feature_states = {}
self.action_texts = []

def prepare_from_domain(self, domain: Domain) -> None:
def prepare_for_training(
self, domain: Domain, interpreter: NaturalLanguageInterpreter
) -> None:
"""Gets necessary information for featurization from domain.
Args:
domain: An instance of :class:`rasa.shared.core.domain.Domain`.
interpreter: The interpreter used to encode the state
"""
if isinstance(interpreter, RegexInterpreter):
# this method is called during training,
# RegexInterpreter means that core was trained separately
self._use_regex_interpreter = True

# store feature states for each attribute in order to create binary features
def convert_to_dict(feature_states: List[Text]) -> Dict[Text, int]:
return {
Expand Down Expand Up @@ -156,6 +170,15 @@ def _extract_state_features(
interpreter: NaturalLanguageInterpreter,
sparse: bool = False,
) -> Dict[Text, List["Features"]]:
# this method is called during both prediction and training,
# `self._use_regex_interpreter == True` means that core was trained
# separately, therefore substitute interpreter based on some trained
# nlu model with default RegexInterpreter to make sure
# that prediction and train time features are the same
if self._use_regex_interpreter and not isinstance(
interpreter, RegexInterpreter
):
interpreter = RegexInterpreter()

message = Message(data=sub_state)
# remove entities from possible attributes
Expand Down
2 changes: 1 addition & 1 deletion rasa/core/featurizers/tracker_featurizers.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def featurize_trackers(
f"to get numerical features for trackers."
)

self.state_featurizer.prepare_from_domain(domain)
self.state_featurizer.prepare_for_training(domain, interpreter)

trackers_as_states, trackers_as_actions = self.training_states_and_actions(
trackers, domain
Expand Down
84 changes: 52 additions & 32 deletions rasa/core/policies/rule_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ def __init__(
enable_fallback_prediction: If `True` `core_fallback_action_name` is
predicted in case no rule matched.
"""

self._core_fallback_threshold = core_fallback_threshold
self._fallback_action_name = core_fallback_action_name
self._enable_fallback_prediction = enable_fallback_prediction
Expand Down Expand Up @@ -194,7 +193,6 @@ def _states_for_unhappy_loop_predictions(states: List[State]) -> List[State]:
Returns:
modified states
"""

# leave only last 2 dialogue turns to
# - capture previous meaningful action before action_listen
# - ignore previous intent
Expand Down Expand Up @@ -226,7 +224,6 @@ def _create_loop_unhappy_lookup_from_states(
Returns:
lookup dictionary
"""

lookup = {}
for states, actions in zip(trackers_as_states, trackers_as_actions):
action = actions[0]
Expand Down Expand Up @@ -281,16 +278,13 @@ def _check_rule_restriction(
)

@staticmethod
def _check_slots_fingerprint(
def _expected_but_missing_slots(
fingerprint: Dict[Text, List[Text]], state: State
) -> Set[Text]:
expected_slots = set(fingerprint.get(SLOTS, {}))
current_slots = set(state.get(SLOTS, {}).keys())
if expected_slots == current_slots:
# all expected slots are satisfied
return set()

return expected_slots
# report all slots that are expected but aren't set in current slots
return expected_slots.difference(current_slots)

@staticmethod
def _check_active_loops_fingerprint(
Expand All @@ -309,17 +303,17 @@ def _check_active_loops_fingerprint(
@staticmethod
def _error_messages_from_fingerprints(
action_name: Text,
fingerprint_slots: Set[Text],
missing_fingerprint_slots: Set[Text],
fingerprint_active_loops: Set[Text],
rule_name: Text,
) -> List[Text]:
error_messages = []
if action_name and fingerprint_slots:
if action_name and missing_fingerprint_slots:
error_messages.append(
f"- the action '{action_name}' in rule '{rule_name}' does not set all "
f"the slots, that it sets in other rules: "
f"'{', '.join(fingerprint_slots)}'. Please update the rule with "
f"an appropriate slot or if it is the last action "
f"- the action '{action_name}' in rule '{rule_name}' does not set some "
f"of the slots that it sets in other rules. Slots not set in rule "
f"'{rule_name}': '{', '.join(missing_fingerprint_slots)}'. Please "
f"update the rule with an appropriate slot or if it is the last action "
f"add 'wait_for_user_input: false' after this action."
)
if action_name and fingerprint_active_loops:
Expand Down Expand Up @@ -375,14 +369,16 @@ def _check_for_incomplete_rules(
# for a previous action if current action is rule snippet action
continue

expected_slots = self._check_slots_fingerprint(fingerprint, state)
missing_expected_slots = self._expected_but_missing_slots(
fingerprint, state
)
expected_active_loops = self._check_active_loops_fingerprint(
fingerprint, state
)
error_messages.extend(
self._error_messages_from_fingerprints(
previous_action_name,
expected_slots,
missing_expected_slots,
expected_active_loops,
tracker.sender_id,
)
Expand Down Expand Up @@ -429,10 +425,12 @@ def _check_prediction(
) -> Optional[Text]:

predicted_action_name = self._predict_next_action(tracker, domain, interpreter)
# if there is an active_loop,
# RulePolicy will always predict active_loop first,
# but inside loop unhappy path there might be another action
if (
predicted_action_name != gold_action_name
tracker.active_loop_name
and predicted_action_name != gold_action_name
and predicted_action_name == tracker.active_loop_name
):
rasa.core.test.emulate_loop_rejection(tracker)
Expand Down Expand Up @@ -658,24 +656,46 @@ def _rule_key_to_state(rule_key: Text) -> List[State]:
def _is_rule_applicable(
self, rule_key: Text, turn_index: int, conversation_state: State
) -> bool:
"""Check if rule is satisfied with current state at turn."""
"""Check if rule is satisfied with current state at turn.
Args:
rule_key: the textual representation of learned rule
turn_index: index of a current dialogue turn
conversation_state: the state that corresponds to turn_index
Returns:
a boolean that says whether the rule is applicable to current state
"""
# turn_index goes back in time
reversed_rule_states = list(reversed(self._rule_key_to_state(rule_key)))

return bool(
# rule is shorter than current turn index
turn_index >= len(reversed_rule_states)
# current rule and state turns are empty
or (not reversed_rule_states[turn_index] and not conversation_state)
# check that current rule turn features are present in current state turn
or (
reversed_rule_states[turn_index]
and conversation_state
and self._does_rule_match_state(
reversed_rule_states[turn_index], conversation_state
)
)
# the rule must be applicable because we got (without any applicability issues)
# further in the conversation history than the rule's length
if turn_index >= len(reversed_rule_states):
return True

# a state has previous action if and only if it is not a conversation start
# state
current_previous_action = conversation_state.get(PREVIOUS_ACTION)
rule_previous_action = reversed_rule_states[turn_index].get(PREVIOUS_ACTION)

# current conversation state and rule state are conversation starters.
# any slots with initial_value set will necessarily be in both states and don't
# need to be checked.
if not rule_previous_action and not current_previous_action:
return True

# current rule state is a conversation starter (due to conversation_start: true)
# but current conversation state is not.
# or
# current conversation state is a starter
# but current rule state is not.
if not rule_previous_action or not current_previous_action:
return False

# check: current rule state features are present in current conversation state
return self._does_rule_match_state(
reversed_rule_states[turn_index], conversation_state
)

def _get_possible_keys(
Expand Down
3 changes: 2 additions & 1 deletion rasa/core/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ async def load_agent_on_start(
)
except Exception as e:
rasa.shared.utils.io.raise_warning(
f"The model at '{model_path}' could not be loaded. " f"Error: {e}"
f"The model at '{model_path}' could not be loaded. "
f"Error: {type(e)}: {e}"
)
app.agent = None

Expand Down
27 changes: 13 additions & 14 deletions rasa/nlu/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import os
import re
from typing import Any, Optional, Text
from pathlib import Path

Expand Down Expand Up @@ -49,23 +48,23 @@ def is_model_dir(model_dir: Text) -> bool:
def is_url(resource_name: Text) -> bool:
"""Check whether the url specified is a well formed one.
Regex adapted from https://stackoverflow.com/a/7160778/3001665
Args:
resource_name: Remote URL to validate
Returns: `True` if valid, otherwise `False`.
Returns:
`True` if valid, otherwise `False`.
"""
URL_REGEX = re.compile(
r"^(?:http|ftp|file)s?://" # http:// or https:// or file://
r"(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|" # domain
r"localhost|" # localhost
r"\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})" # or ip
r"(?::\d+)?" # optional port
r"(?:/?|[/?]\S+)$",
re.IGNORECASE,
)
return URL_REGEX.match(resource_name) is not None
from urllib import parse

try:
result = parse.urlparse(resource_name)
except Exception:
return False

if result.scheme == "file":
return bool(result.path)

return bool(result.scheme in ["http", "https", "ftp", "ftps"] and result.netloc)


def remove_model(model_dir: Text) -> bool:
Expand Down
Loading

0 comments on commit 4366223

Please sign in to comment.